notify-api-742 don't write phone numbers to db

This commit is contained in:
Kenneth Kehl
2024-01-16 11:21:24 -08:00
parent 5af876a963
commit f9f1013f5b
16 changed files with 124 additions and 76 deletions
+7 -7
View File
@@ -417,7 +417,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "+447234123123"
assert persisted_notification.to == "1"
assert persisted_notification.template_id == sample_template_with_placeholders.id
assert (
persisted_notification.template_version
@@ -456,7 +456,7 @@ def test_should_save_sms_if_restricted_service_and_valid_number(
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "+12028675309"
assert persisted_notification.to == "1"
assert persisted_notification.template_id == template.id
assert persisted_notification.template_version == template.version
assert persisted_notification.status == "created"
@@ -566,7 +566,7 @@ def test_should_save_sms_template_to_and_persist_with_job_id(sample_job, mocker)
encryption.encrypt(notification),
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "+447234123123"
assert persisted_notification.to == "1"
assert persisted_notification.job_id == sample_job.id
assert persisted_notification.template_id == sample_job.template.id
assert persisted_notification.status == "created"
@@ -631,7 +631,7 @@ def test_should_use_email_template_and_persist(
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "my_email@my_email.com"
assert persisted_notification.to == "1"
assert (
persisted_notification.template_id == sample_email_template_with_placeholders.id
)
@@ -678,7 +678,7 @@ def test_save_email_should_use_template_version_from_job_not_latest(
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "my_email@my_email.com"
assert persisted_notification.to == "1"
assert persisted_notification.template_id == sample_email_template.id
assert persisted_notification.template_version == version_on_notification
assert persisted_notification.created_at >= now
@@ -707,7 +707,7 @@ def test_should_use_email_template_subject_placeholders(
encryption.encrypt(notification),
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "my_email@my_email.com"
assert persisted_notification.to == "1"
assert (
persisted_notification.template_id == sample_email_template_with_placeholders.id
)
@@ -786,7 +786,7 @@ def test_should_use_email_template_and_persist_without_personalisation(
encryption.encrypt(notification),
)
persisted_notification = Notification.query.one()
assert persisted_notification.to == "my_email@my_email.com"
assert persisted_notification.to == "1"
assert persisted_notification.template_id == sample_email_template.id
assert persisted_notification.created_at >= now
assert not persisted_notification.sent_at
@@ -25,7 +25,6 @@ from app.dao.notifications_dao import (
get_notifications_for_service,
get_service_ids_with_notifications_on_date,
notifications_not_yet_sent,
sanitize_successful_notification_by_id,
update_notification_status_by_id,
update_notification_status_by_reference,
)
@@ -92,36 +91,6 @@ def test_should_by_able_to_update_status_by_id(
assert notification.status == "delivered"
def test_should_be_able_to_sanitize_successful_notification(
sample_template, sample_job, sns_provider
):
with freeze_time("2000-01-01 12:00:00"):
data = _notification_json(
sample_template, job_id=sample_job.id, status="sending"
)
notification = Notification(**data)
notification.to = "15555555555"
notification.normalised_to = "15555555555"
dao_create_notification(notification)
assert notification.status == "sending"
assert notification.normalised_to == "15555555555"
assert notification.to == "15555555555"
assert Notification.query.get(notification.id).status == "sending"
with freeze_time("2000-01-02 12:00:00"):
sanitize_successful_notification_by_id(
notification.id, carrier="ATT", provider_response="Don't know what happened"
)
assert Notification.query.get(notification.id).status == "delivered"
assert Notification.query.get(notification.id).normalised_to == "1"
assert Notification.query.get(notification.id).to == "1"
assert (
Notification.query.get(notification.id).provider_response
== "Don't know what happened"
)
def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(
sample_job,
):
@@ -341,7 +310,7 @@ def test_save_notification_creates_sms(sample_template, sample_job):
assert Notification.query.count() == 1
notification_from_db = Notification.query.all()[0]
assert notification_from_db.id
assert data["to"] == notification_from_db.to
assert "1" == notification_from_db.to
assert data["job_id"] == notification_from_db.job_id
assert data["service"] == notification_from_db.service
assert data["template_id"] == notification_from_db.template_id
@@ -361,7 +330,7 @@ def test_save_notification_and_create_email(sample_email_template, sample_job):
assert Notification.query.count() == 1
notification_from_db = Notification.query.all()[0]
assert notification_from_db.id
assert data["to"] == notification_from_db.to
assert "1" == notification_from_db.to
assert data["job_id"] == notification_from_db.job_id
assert data["service"] == notification_from_db.service
assert data["template_id"] == notification_from_db.template_id
@@ -438,7 +407,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, sns_pr
assert Notification.query.count() == 1
notification_from_db = Notification.query.all()[0]
assert notification_from_db.id
assert data["to"] == notification_from_db.to
assert "1" == notification_from_db.to
assert data["job_id"] == notification_from_db.job_id
assert data["service"] == notification_from_db.service
assert data["template_id"] == notification_from_db.template_id
@@ -464,7 +433,7 @@ def test_save_notification_and_increment_correct_job(sample_template, sns_provid
assert Notification.query.count() == 1
notification_from_db = Notification.query.all()[0]
assert notification_from_db.id
assert data["to"] == notification_from_db.to
assert "1" == notification_from_db.to
assert data["job_id"] == notification_from_db.job_id
assert data["service"] == notification_from_db.service
assert data["template_id"] == notification_from_db.template_id
@@ -484,7 +453,7 @@ def test_save_notification_with_no_job(sample_template, sns_provider):
assert Notification.query.count() == 1
notification_from_db = Notification.query.all()[0]
assert notification_from_db.id
assert data["to"] == notification_from_db.to
assert "1" == notification_from_db.to
assert data["service"] == notification_from_db.service
assert data["template_id"] == notification_from_db.template_id
assert data["template_version"] == notification_from_db.template_version
@@ -545,7 +514,7 @@ def test_save_notification_no_job_id(sample_template):
assert Notification.query.count() == 1
notification_from_db = Notification.query.all()[0]
assert notification_from_db.id
assert data["to"] == notification_from_db.to
assert "1" == notification_from_db.to
assert data["service"] == notification_from_db.service
assert data["template_id"] == notification_from_db.template_id
assert data["template_version"] == notification_from_db.template_version
@@ -1024,6 +993,9 @@ def test_should_exclude_test_key_notifications_by_default(
assert len(all_notifications) == 1
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_recipient(sample_template):
recipient_to_search_for = {
"to_field": "+447700900855",
@@ -1057,6 +1029,9 @@ def test_dao_get_notifications_by_recipient(sample_template):
assert notification1.id == results.items[0].id
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_recipient_is_limited_to_50_results(sample_template):
for _ in range(100):
create_notification(
@@ -1075,6 +1050,9 @@ def test_dao_get_notifications_by_recipient_is_limited_to_50_results(sample_temp
assert len(results.items) == 50
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@pytest.mark.parametrize("search_term", ["JACK", "JACK@gmail.com", "jack@gmail.com"])
def test_dao_get_notifications_by_recipient_is_not_case_sensitive(
sample_email_template, search_term
@@ -1093,6 +1071,9 @@ def test_dao_get_notifications_by_recipient_is_not_case_sensitive(
assert notification.id in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_recipient_matches_partial_emails(
sample_email_template,
):
@@ -1116,6 +1097,9 @@ def test_dao_get_notifications_by_recipient_matches_partial_emails(
assert notification_2.id not in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@pytest.mark.parametrize(
"search_term, expected_result_count",
[
@@ -1165,6 +1149,9 @@ def test_dao_get_notifications_by_recipient_escapes(
)
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@pytest.mark.parametrize(
"search_term, expected_result_count",
[
@@ -1215,6 +1202,9 @@ def test_dao_get_notifications_by_reference_escapes_special_character(
)
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@pytest.mark.parametrize(
"search_term",
[
@@ -1252,6 +1242,9 @@ def test_dao_get_notifications_by_recipient_matches_partial_phone_numbers(
assert notification_2.id not in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@pytest.mark.parametrize("to", ["not@email", "123"])
def test_dao_get_notifications_by_recipient_accepts_invalid_phone_numbers_and_email_addresses(
sample_template,
@@ -1268,6 +1261,9 @@ def test_dao_get_notifications_by_recipient_accepts_invalid_phone_numbers_and_em
assert len(results.items) == 0
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_recipient_ignores_spaces(sample_template):
notification1 = create_notification(
template=sample_template, to_field="+447700900855", normalised_to="447700900855"
@@ -1299,6 +1295,9 @@ def test_dao_get_notifications_by_recipient_ignores_spaces(sample_template):
assert notification3.id in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@pytest.mark.parametrize("phone_search", ("202", "7-5", "+1 (202) 867-5309"))
@pytest.mark.parametrize(
"email_search",
@@ -1342,6 +1341,9 @@ def test_dao_get_notifications_by_recipient_searches_across_notification_types(
assert results.items[1].id == sms.id
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_reference(notify_db_session):
service = create_service()
sms_template = create_template(service=service)
@@ -1416,6 +1418,9 @@ def test_dao_get_notifications_by_reference(notify_db_session):
assert len(results.items) == 0
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_to_field_filters_status(sample_template):
notification = create_notification(
template=sample_template,
@@ -1441,6 +1446,9 @@ def test_dao_get_notifications_by_to_field_filters_status(sample_template):
assert notification.id == notifications.items[0].id
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_to_field_filters_multiple_statuses(sample_template):
notification1 = create_notification(
template=sample_template,
@@ -1468,6 +1476,9 @@ def test_dao_get_notifications_by_to_field_filters_multiple_statuses(sample_temp
assert notification2.id in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_dao_get_notifications_by_to_field_returns_all_if_no_status_filter(
sample_template,
):
@@ -1494,6 +1505,9 @@ def test_dao_get_notifications_by_to_field_returns_all_if_no_status_filter(
assert notification2.id in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@freeze_time("2016-01-01 11:10:00")
def test_dao_get_notifications_by_to_field_orders_by_created_at_desc(sample_template):
notification = partial(
+3
View File
@@ -1363,6 +1363,9 @@ def _assert_service_permissions(service_permissions, expected):
assert set(expected) == set(p.permission for p in service_permissions)
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
@freeze_time("2019-12-02 12:00:00.000000")
def test_dao_find_services_sending_to_tv_numbers(notify_db_session, fake_uuid):
service_1 = create_service(service_name="Service 1", service_id=fake_uuid)
+3 -3
View File
@@ -652,10 +652,10 @@ def test_send_sms_to_provider_should_use_normalised_to(mocker, client, sample_te
)
mock_s3 = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3")
mock_s3.return_value = "2028675309"
mock_s3.return_value = "12028675309"
send_to_providers.send_sms_to_provider(notification)
send_mock.assert_called_once_with(
to=notification.normalised_to,
to="12028675309",
content=ANY,
reference=str(notification.id),
sender=notification.reply_to_text,
@@ -716,7 +716,7 @@ def test_send_sms_to_provider_should_return_template_if_found_in_redis(
assert mock_get_template.called is False
assert mock_get_service.called is False
send_mock.assert_called_once_with(
to=notification.normalised_to,
to="447700900855",
content=ANY,
reference=str(notification.id),
sender=notification.reply_to_text,
@@ -377,8 +377,8 @@ def test_persist_sms_notification_stores_normalised_number(
)
persisted_notification = Notification.query.all()[0]
assert persisted_notification.to == recipient
assert persisted_notification.normalised_to == expected_recipient_normalised
assert persisted_notification.to == "1"
assert persisted_notification.normalised_to == "1"
@pytest.mark.parametrize(
@@ -401,8 +401,8 @@ def test_persist_email_notification_stores_normalised_email(
)
persisted_notification = Notification.query.all()[0]
assert persisted_notification.to == recipient
assert persisted_notification.normalised_to == expected_recipient_normalised
assert persisted_notification.to == "1"
assert persisted_notification.normalised_to == "1"
def test_persist_notification_with_billable_units_stores_correct_info(mocker):
+1 -1
View File
@@ -159,7 +159,7 @@ def test_get_all_notifications(client, sample_notification):
"version": 1,
}
assert notifications["notifications"][0]["to"] == "+447700900855"
assert notifications["notifications"][0]["to"] == "1"
assert notifications["notifications"][0]["service"] == str(
sample_notification.service_id
)
+1 -1
View File
@@ -551,7 +551,7 @@ def test_post_update_organization_set_mou_emails_signed_by(
)
notifications = [x[0][0] for x in queue_mock.call_args_list]
assert {n.template.name: n.to for n in notifications} == templates_and_recipients
# assert {n.template.name: n.to for n in notifications} == templates_and_recipients
for n in notifications:
# we pass in the same personalisation for all templates (though some templates don't use all fields)
@@ -1,3 +1,5 @@
import pytest
from app.dao.api_key_dao import save_model_api_key
from app.models import KEY_TYPE_NORMAL, ApiKey
from app.v2.notifications.notification_schemas import (
@@ -70,6 +72,7 @@ def test_get_api_sms_contract(client, sample_notification):
validate_v0(response_json, "GET_notification_return_sms.json")
@pytest.mark.skip(reason="Update to fetch email from s3")
def test_get_api_email_contract(client, sample_email_notification):
response_json = return_json_from_response(
_get_notification(
@@ -92,6 +95,7 @@ def test_get_job_sms_contract(client, sample_notification):
validate_v0(response_json, "GET_notification_return_sms.json")
@pytest.mark.skip(reason="Update to fetch email from s3")
def test_get_job_email_contract(client, sample_email_notification):
response_json = return_json_from_response(
_get_notification(
@@ -791,7 +791,7 @@ def test_should_persist_notification(
assert response.status_code == 201
notification = notifications_dao.get_notification_by_id(fake_uuid)
assert notification.to == to
assert notification.to == "1"
assert notification.template_id == template.id
assert notification.notification_type == template_type
@@ -1202,7 +1202,7 @@ def test_should_allow_store_original_number_on_sms_notification(
assert notification_id
notifications = Notification.query.all()
assert len(notifications) == 1
assert "(202) 867-5309" == notifications[0].to
assert "1" == notifications[0].to
def test_should_not_allow_sending_to_international_number_without_international_permission(
+34 -1
View File
@@ -1685,6 +1685,9 @@ def test_get_all_notifications_for_service_in_order_with_post_request(
assert response.status_code == 200
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_get_all_notifications_for_service_filters_notifications_when_using_post_request(
client, notify_db_session
):
@@ -1725,7 +1728,7 @@ def test_get_all_notifications_for_service_filters_notifications_when_using_post
resp = json.loads(response.get_data(as_text=True))
assert len(resp["notifications"]) == 1
assert resp["notifications"][0]["to"] == returned_notification.to
assert resp["notifications"][0]["to"] == "1"
assert resp["notifications"][0]["status"] == returned_notification.status
assert response.status_code == 200
@@ -2256,6 +2259,9 @@ def test_get_detailed_services_for_date_range(
}
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field(
client, sample_template, sample_email_template
):
@@ -2281,6 +2287,9 @@ def test_search_for_notification_by_to_field(
assert str(notification2.id) == notifications[0]["id"]
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_return_empty_list_if_there_is_no_match(
client, sample_template, sample_email_template
):
@@ -2299,6 +2308,9 @@ def test_search_for_notification_by_to_field_return_empty_list_if_there_is_no_ma
assert len(notifications) == 0
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_return_multiple_matches(
client, sample_template, sample_email_template
):
@@ -2333,6 +2345,9 @@ def test_search_for_notification_by_to_field_return_multiple_matches(
assert str(notification4.id) not in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_returns_next_link_if_more_than_50(
client, sample_template
):
@@ -2355,6 +2370,9 @@ def test_search_for_notification_by_to_field_returns_next_link_if_more_than_50(
assert "page=2" in response_json["links"]["next"]
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_returns_no_next_link_if_50_or_less(
client, sample_template
):
@@ -2446,6 +2464,9 @@ def test_update_service_does_not_call_send_notification_when_restricted_not_chan
assert not send_notification_mock.called
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_filters_by_status(client, sample_template):
notification1 = create_notification(
sample_template,
@@ -2474,6 +2495,9 @@ def test_search_for_notification_by_to_field_filters_by_status(client, sample_te
assert str(notification1.id) in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_filters_by_statuses(
client, sample_template
):
@@ -2505,6 +2529,9 @@ def test_search_for_notification_by_to_field_filters_by_statuses(
assert str(notification2.id) in notification_ids
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_returns_content(
client, sample_template_with_placeholders
):
@@ -2608,6 +2635,9 @@ def test_get_all_notifications_for_service_includes_template_redacted(
# assert resp['notifications'][1]['template']['is_precompiled_letter'] is False
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_returns_personlisation(
client, sample_template_with_placeholders
):
@@ -2632,6 +2662,9 @@ def test_search_for_notification_by_to_field_returns_personlisation(
assert notifications[0]["personalisation"]["name"] == "Foo"
@pytest.mark.skip(
reason="We can't search on recipient if recipient is not kept in the db"
)
def test_search_for_notification_by_to_field_returns_notifications_by_type(
client, sample_template, sample_email_template
):
+1 -8
View File
@@ -13,17 +13,15 @@ def test_send_notification_to_service_users_persists_notifications_correctly(
):
mocker.patch("app.service.sender.send_notification_to_queue")
user = sample_service.users[0]
template = create_template(sample_service, template_type=notification_type)
send_notification_to_service_users(
service_id=sample_service.id, template_id=template.id
)
to = user.email_address if notification_type == EMAIL_TYPE else user.mobile_number
notification = Notification.query.one()
assert Notification.query.count() == 1
assert notification.to == to
assert notification.to == "1"
assert str(notification.service_id) == current_app.config["NOTIFY_SERVICE_ID"]
assert notification.template.id == template.id
assert notification.template.template_type == notification_type
@@ -87,10 +85,5 @@ def test_send_notification_to_service_users_sends_to_active_users_only(
template = create_template(service, template_type=EMAIL_TYPE)
send_notification_to_service_users(service_id=service.id, template_id=template.id)
notifications = Notification.query.all()
notifications_recipients = [notification.to for notification in notifications]
assert Notification.query.count() == 2
assert pending_user.email_address not in notifications_recipients
assert first_active_user.email_address in notifications_recipients
assert second_active_user.email_address in notifications_recipients
+3 -3
View File
@@ -226,7 +226,7 @@ def test_send_user_sms_code(client, sample_user, sms_code_template, mocker):
notification = Notification.query.one()
assert notification.personalisation == {"verify_code": "11111"}
assert notification.to == sample_user.mobile_number
assert notification.to == "1"
assert str(notification.service_id) == current_app.config["NOTIFY_SERVICE_ID"]
assert notification.reply_to_text == notify_service.get_default_sms_sender()
@@ -261,7 +261,7 @@ def test_send_user_code_for_sms_with_optional_to_field(
assert resp.status_code == 204
assert mocked.call_count == 1
notification = Notification.query.first()
assert notification.to == to_number
assert notification.to == "1"
app.celery.provider_tasks.deliver_sms.apply_async.assert_called_once_with(
([str(notification.id)]), queue="notify-internal-tasks"
)
@@ -479,7 +479,7 @@ def test_send_user_email_code(
noti.reply_to_text
== email_2fa_code_template.service.get_default_reply_to_email_address()
)
assert noti.to == sample_user.email_address
assert noti.to == "1"
assert str(noti.template_id) == current_app.config["EMAIL_2FA_TEMPLATE_ID"]
assert noti.personalisation["name"] == "Test User"
assert noti.personalisation["url"].startswith(expected_auth_url)
@@ -289,7 +289,7 @@ def test_get_all_notifications_except_job_notifications_returns_200(
"uri": notification.template.get_link(),
"version": 1,
}
assert json_response["notifications"][0]["phone_number"] == "+447700900855"
assert json_response["notifications"][0]["phone_number"] == "1"
assert json_response["notifications"][0]["type"] == "sms"
assert not json_response["notifications"][0]["scheduled_for"]
@@ -322,7 +322,7 @@ def test_get_all_notifications_with_include_jobs_arg_returns_200(
assert json_response["notifications"][0]["id"] == str(notification.id)
assert json_response["notifications"][0]["status"] == notification.status
assert json_response["notifications"][0]["phone_number"] == notification.to
assert "1" == notification.to
assert (
json_response["notifications"][0]["type"] == notification.template.template_type
)
@@ -381,7 +381,7 @@ def test_get_all_notifications_filter_by_template_type(client, sample_service):
"uri": notification.template.get_link(),
"version": 1,
}
assert json_response["notifications"][0]["email_address"] == "don.draper@scdp.biz"
assert json_response["notifications"][0]["email_address"] == "1"
assert json_response["notifications"][0]["type"] == "email"
@@ -838,7 +838,7 @@ def test_post_sms_should_persist_supplied_sms_number(
notifications = Notification.query.all()
assert len(notifications) == 1
notification_id = notifications[0].id
assert "+(44) 77009-00855" == notifications[0].to
assert "1" == notifications[0].to
assert resp_json["id"] == str(notification_id)
assert mocked.called