From f9f1013f5bd61bf92bce132ba6263ade4b374fdb Mon Sep 17 00:00:00 2001
From: Kenneth Kehl <@kkehl@flexion.us>
Date: Tue, 16 Jan 2024 11:21:24 -0800
Subject: [PATCH 1/6] notify-api-742 don't write phone numbers to db

---
 app/dao/notifications_dao.py                  |  7 +-
 app/user/rest.py                              |  4 -
 tests/app/celery/test_tasks.py                | 14 +--
 .../notification_dao/test_notification_dao.py | 88 +++++++++++--------
 tests/app/dao/test_services_dao.py            |  3 +
 tests/app/delivery/test_send_to_providers.py  |  6 +-
 .../test_process_notification.py              |  8 +-
 tests/app/notifications/test_rest.py          |  2 +-
 tests/app/organization/test_rest.py           |  2 +-
 .../public_contracts/test_GET_notification.py |  4 +
 .../test_send_notification.py                 |  4 +-
 tests/app/service/test_rest.py                | 35 +++++++-
 tests/app/service/test_sender.py              |  9 +-
 tests/app/user/test_rest_verify.py            |  6 +-
 .../notifications/test_get_notifications.py   |  6 +-
 .../notifications/test_post_notifications.py  |  2 +-
 16 files changed, 124 insertions(+), 76 deletions(-)

diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py
index ec9ea5053..23905944c 100644
--- a/app/dao/notifications_dao.py
+++ b/app/dao/notifications_dao.py
@@ -72,7 +72,9 @@ def dao_create_notification(notification):
         notification.id = create_uuid()
     if not notification.status:
         notification.status = NOTIFICATION_CREATED
-
+    # notify-api-742 remove phone numbers from db
+    notification.to = "1"
+    notification.normalised_to = "1"
     db.session.add(notification)
 
 
@@ -179,6 +181,9 @@ def update_notification_status_by_reference(reference, status):
 @autocommit
 def dao_update_notification(notification):
     notification.updated_at = datetime.utcnow()
+    # notify-api-742 remove phone numbers from db
+    notification.to = "1"
+    notification.normalised_to = "1"
     db.session.add(notification)
 
 
diff --git a/app/user/rest.py b/app/user/rest.py
index 303c1a39c..c7fa8055a 100644
--- a/app/user/rest.py
+++ b/app/user/rest.py
@@ -401,10 +401,6 @@ def send_new_user_email_verification(user_id):
 
     # when registering, we verify all users' email addresses using this function
     user_to_send_to = get_user_by_id(user_id=user_id)
-    current_app.logger.info("user_to_send_to is {}".format(user_to_send_to))
-    current_app.logger.info(
-        "user_to_send_to.email_address is {}".format(user_to_send_to.email_address)
-    )
 
     template = dao_get_template_by_id(
         current_app.config["NEW_USER_EMAIL_VERIFICATION_TEMPLATE_ID"]
diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py
index 80ba897a5..41c613563 100644
--- a/tests/app/celery/test_tasks.py
+++ b/tests/app/celery/test_tasks.py
@@ -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
diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py
index a0d19ecd1..13056105a 100644
--- a/tests/app/dao/notification_dao/test_notification_dao.py
+++ b/tests/app/dao/notification_dao/test_notification_dao.py
@@ -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(
diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py
index a7d630db3..41d003ec7 100644
--- a/tests/app/dao/test_services_dao.py
+++ b/tests/app/dao/test_services_dao.py
@@ -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)
diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py
index 4dfb336a0..dbae3014b 100644
--- a/tests/app/delivery/test_send_to_providers.py
+++ b/tests/app/delivery/test_send_to_providers.py
@@ -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,
diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py
index b68e4503d..87c8da988 100644
--- a/tests/app/notifications/test_process_notification.py
+++ b/tests/app/notifications/test_process_notification.py
@@ -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):
diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py
index a3b31beb5..c12132c58 100644
--- a/tests/app/notifications/test_rest.py
+++ b/tests/app/notifications/test_rest.py
@@ -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
     )
diff --git a/tests/app/organization/test_rest.py b/tests/app/organization/test_rest.py
index e239dfd3d..25f25ec43 100644
--- a/tests/app/organization/test_rest.py
+++ b/tests/app/organization/test_rest.py
@@ -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)
diff --git a/tests/app/public_contracts/test_GET_notification.py b/tests/app/public_contracts/test_GET_notification.py
index d36704083..ff23762f0 100644
--- a/tests/app/public_contracts/test_GET_notification.py
+++ b/tests/app/public_contracts/test_GET_notification.py
@@ -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(
diff --git a/tests/app/service/send_notification/test_send_notification.py b/tests/app/service/send_notification/test_send_notification.py
index e4477fa25..c65614e22 100644
--- a/tests/app/service/send_notification/test_send_notification.py
+++ b/tests/app/service/send_notification/test_send_notification.py
@@ -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(
diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py
index ce6afc065..219bb853b 100644
--- a/tests/app/service/test_rest.py
+++ b/tests/app/service/test_rest.py
@@ -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
 ):
diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py
index 9056be8a9..fbc8b784b 100644
--- a/tests/app/service/test_sender.py
+++ b/tests/app/service/test_sender.py
@@ -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
diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py
index bd89b9fd8..0a4185416 100644
--- a/tests/app/user/test_rest_verify.py
+++ b/tests/app/user/test_rest_verify.py
@@ -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)
diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py
index 4229b2cc2..82b589e4c 100644
--- a/tests/app/v2/notifications/test_get_notifications.py
+++ b/tests/app/v2/notifications/test_get_notifications.py
@@ -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"
 
 
diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py
index cb3957450..f461531bc 100644
--- a/tests/app/v2/notifications/test_post_notifications.py
+++ b/tests/app/v2/notifications/test_post_notifications.py
@@ -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
 

From c13ed73d231d375dc6115127e8ccb70f12426264 Mon Sep 17 00:00:00 2001
From: Kenneth Kehl <@kkehl@flexion.us>
Date: Wed, 17 Jan 2024 09:04:04 -0800
Subject: [PATCH 2/6] substitute phone numbers back in when sending data to
 reports

---
 app/job/rest.py                | 12 +++++++++++-
 app/service/rest.py            | 11 +++++++++++
 tests/app/job/test_rest.py     | 20 ++++++++++++++++----
 tests/app/service/test_rest.py |  4 ++++
 4 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/app/job/rest.py b/app/job/rest.py
index 5852d7f63..1aab2ca60 100644
--- a/app/job/rest.py
+++ b/app/job/rest.py
@@ -2,7 +2,7 @@ import dateutil
 import pytz
 from flask import Blueprint, current_app, jsonify, request
 
-from app.aws.s3 import get_job_metadata_from_s3
+from app.aws.s3 import get_job_metadata_from_s3, get_phone_number_from_s3
 from app.celery.tasks import process_job
 from app.config import QueueNames
 from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job
@@ -76,6 +76,16 @@ def get_all_notifications_for_service_job(service_id, job_id):
     kwargs["service_id"] = service_id
     kwargs["job_id"] = job_id
 
+    for notification in paginated_notifications.items:
+        if notification.job_id is not None:
+            recipient = get_phone_number_from_s3(
+                notification.service_id,
+                notification.job_id,
+                notification.job_row_number,
+            )
+            notification.to = recipient
+            notification.normalised_to = recipient
+
     notifications = None
     if data.get("format_for_csv"):
         notifications = [
diff --git a/app/service/rest.py b/app/service/rest.py
index 99d2b4c97..cef10ef1a 100644
--- a/app/service/rest.py
+++ b/app/service/rest.py
@@ -6,6 +6,7 @@ from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm.exc import NoResultFound
 from werkzeug.datastructures import MultiDict
 
+from app.aws.s3 import get_phone_number_from_s3
 from app.config import QueueNames
 from app.dao import fact_notification_status_dao, notifications_dao
 from app.dao.annual_billing_dao import set_default_free_allowance_for_service
@@ -425,6 +426,16 @@ def get_all_notifications_for_service(service_id):
         include_one_off=include_one_off,
     )
 
+    for notification in pagination.items:
+        if notification.job_id is not None:
+            recipient = get_phone_number_from_s3(
+                notification.service_id,
+                notification.job_id,
+                notification.job_row_number,
+            )
+            notification.to = recipient
+            notification.normalised_to = recipient
+
     kwargs = request.args.to_dict()
     kwargs["service_id"] = service_id
 
diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py
index 56f2461b1..c48ef89d8 100644
--- a/tests/app/job/test_rest.py
+++ b/tests/app/job/test_rest.py
@@ -448,8 +448,11 @@ def _setup_jobs(template, number_of_jobs=5):
 
 
 def test_get_all_notifications_for_job_in_order_of_job_number(
-    admin_request, sample_template
+    admin_request, sample_template, mocker
 ):
+    mock_s3 = mocker.patch("app.job.rest.get_phone_number_from_s3")
+    mock_s3.return_value = "15555555555"
+
     main_job = create_job(sample_template)
     another_job = create_job(sample_template)
 
@@ -483,8 +486,11 @@ def test_get_all_notifications_for_job_in_order_of_job_number(
     ],
 )
 def test_get_all_notifications_for_job_filtered_by_status(
-    admin_request, sample_job, expected_notification_count, status_args
+    admin_request, sample_job, expected_notification_count, status_args, mocker
 ):
+    mock_s3 = mocker.patch("app.job.rest.get_phone_number_from_s3")
+    mock_s3.return_value = "15555555555"
+
     create_notification(job=sample_job, to_field="1", status="created")
 
     resp = admin_request.get(
@@ -497,8 +503,11 @@ def test_get_all_notifications_for_job_filtered_by_status(
 
 
 def test_get_all_notifications_for_job_returns_correct_format(
-    admin_request, sample_notification_with_job
+    admin_request, sample_notification_with_job, mocker
 ):
+    mock_s3 = mocker.patch("app.job.rest.get_phone_number_from_s3")
+    mock_s3.return_value = "15555555555"
+
     service_id = sample_notification_with_job.service_id
     job_id = sample_notification_with_job.job_id
 
@@ -813,8 +822,11 @@ def create_10_jobs(template):
 
 
 def test_get_all_notifications_for_job_returns_csv_format(
-    admin_request, sample_notification_with_job
+    admin_request, sample_notification_with_job, mocker
 ):
+    mock_s3 = mocker.patch("app.job.rest.get_phone_number_from_s3")
+    mock_s3.return_value = "15555555555"
+
     resp = admin_request.get(
         "job.get_all_notifications_for_service_job",
         service_id=sample_notification_with_job.service_id,
diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py
index 219bb853b..7a5a92222 100644
--- a/tests/app/service/test_rest.py
+++ b/tests/app/service/test_rest.py
@@ -1846,7 +1846,11 @@ def test_get_all_notifications_for_service_including_ones_made_by_jobs(
     sample_notification,
     sample_notification_with_job,
     sample_template,
+    mocker,
 ):
+    mock_s3 = mocker.patch("app.service.rest.get_phone_number_from_s3")
+    mock_s3.return_value = "1"
+
     # notification from_test_api_key
     create_notification(sample_template, key_type=KEY_TYPE_TEST)
 

From 4efeb19d022a71424198fe474dce46dd3a10ec71 Mon Sep 17 00:00:00 2001
From: Carlo Costino <carlo.costino@gsa.gov>
Date: Wed, 17 Jan 2024 22:57:04 -0500
Subject: [PATCH 3/6] Update try/except block to catch Exception

This changeset updates a try/except block to catch Exception instead of BaseException.  Using BaseException is an anti-pattern and is not intended for user-defined code; it can also lead to other unintended side effects if not handled properly.

This is enough of a concern that the new release of flake8-bugbear now issues warnings when it sees BaseException being caught without an immediate re-raise.

Signed-off-by: Carlo Costino <carlo.costino@gsa.gov>
---
 app/delivery/send_to_providers.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py
index 6db858eed..9590f5bd6 100644
--- a/app/delivery/send_to_providers.py
+++ b/app/delivery/send_to_providers.py
@@ -76,7 +76,7 @@ def send_sms_to_provider(notification):
                         notification.job_id,
                         notification.job_row_number,
                     )
-                except BaseException:
+                except Exception:
                     # It is our 2facode, maybe
                     key = f"2facode-{notification.id}".replace(" ", "")
                     recipient = redis_store.raw_get(key)

From 4b98106037459d2e31ce0c75038da9c2330d81a6 Mon Sep 17 00:00:00 2001
From: Kenneth Kehl <@kkehl@flexion.us>
Date: Thu, 18 Jan 2024 10:16:14 -0800
Subject: [PATCH 4/6] debug

---
 app/aws/s3.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/app/aws/s3.py b/app/aws/s3.py
index 86676f3fc..e5311207c 100644
--- a/app/aws/s3.py
+++ b/app/aws/s3.py
@@ -101,8 +101,12 @@ def get_phone_number_from_s3(service_id, job_id, job_row_number):
     # At the same time we don't want to store it in redis or the db
     # So this is a little recycling mechanism to reduce the number of downloads.
     job = JOBS.get(job_id)
+    # TODO REMOVE
+    current_app.logger.info(f"HERE IS THE JOB FROM CACHE {job}")
     if job is None:
         job = get_job_from_s3(service_id, job_id)
+        # TODO REMOVE
+        current_app.logger.info(f"HERE IS THE JOB FROM S3 {job}")
         JOBS[job_id] = job
         incr_jobs_cache_misses()
     else:
@@ -117,9 +121,17 @@ def get_phone_number_from_s3(service_id, job_id, job_row_number):
         if item == "phone number":
             break
         phone_index = phone_index + 1
+
     correct_row = job[job_row_number]
     correct_row = correct_row.split(",")
+    # TODO REMOVE
+    current_app.logger.info(
+        f"HERE IS THE CORRECT ROW AND PHONE INDEX {correct_row} {phone_index}"
+    )
 
+    # This could happen if an old job cannot be retrieved from s3
+    if len(correct_row) <= phone_index:
+        return "Unknown Phone"
     my_phone = correct_row[phone_index]
     my_phone = re.sub(r"[\+\s\(\)\-\.]*", "", my_phone)
     return my_phone

From 7997d887c6f56533e8461c3a28894e66cf3272b8 Mon Sep 17 00:00:00 2001
From: Kenneth Kehl <@kkehl@flexion.us>
Date: Thu, 18 Jan 2024 10:53:20 -0800
Subject: [PATCH 5/6] fix case sensitivity looking up phone number

---
 app/aws/s3.py            | 10 +---------
 tests/app/aws/test_s3.py |  6 ++++++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/app/aws/s3.py b/app/aws/s3.py
index e5311207c..acc89ebc0 100644
--- a/app/aws/s3.py
+++ b/app/aws/s3.py
@@ -101,12 +101,8 @@ def get_phone_number_from_s3(service_id, job_id, job_row_number):
     # At the same time we don't want to store it in redis or the db
     # So this is a little recycling mechanism to reduce the number of downloads.
     job = JOBS.get(job_id)
-    # TODO REMOVE
-    current_app.logger.info(f"HERE IS THE JOB FROM CACHE {job}")
     if job is None:
         job = get_job_from_s3(service_id, job_id)
-        # TODO REMOVE
-        current_app.logger.info(f"HERE IS THE JOB FROM S3 {job}")
         JOBS[job_id] = job
         incr_jobs_cache_misses()
     else:
@@ -118,16 +114,12 @@ def get_phone_number_from_s3(service_id, job_id, job_row_number):
     first_row = first_row.split(",")
     phone_index = 0
     for item in first_row:
-        if item == "phone number":
+        if item.lower() == "phone number":
             break
         phone_index = phone_index + 1
 
     correct_row = job[job_row_number]
     correct_row = correct_row.split(",")
-    # TODO REMOVE
-    current_app.logger.info(
-        f"HERE IS THE CORRECT ROW AND PHONE INDEX {correct_row} {phone_index}"
-    )
 
     # This could happen if an old job cannot be retrieved from s3
     if len(correct_row) <= phone_index:
diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py
index 342f6d7df..ad01a00c5 100644
--- a/tests/app/aws/test_s3.py
+++ b/tests/app/aws/test_s3.py
@@ -61,6 +61,12 @@ def test_get_s3_file_makes_correct_call(notify_api, mocker):
             0,
             "15551111111",
         ),
+        (
+            "Phone number,name,date,time,address,English,Spanish\r\n15553333333,Tim,10/16,2:00 PM,5678 Tom St.,no,yes",
+            "ddd",
+            0,
+            "15553333333",
+        ),
     ],
 )
 def test_get_phone_number_from_s3(

From df97d42c7d1ba0d4bae4948da005dd7a65518359 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Thu, 18 Jan 2024 22:47:06 +0000
Subject: [PATCH 6/6] Bump flake8-bugbear from 23.12.2 to 24.1.17

Bumps [flake8-bugbear](https://github.com/PyCQA/flake8-bugbear) from 23.12.2 to 24.1.17.
- [Release notes](https://github.com/PyCQA/flake8-bugbear/releases)
- [Commits](https://github.com/PyCQA/flake8-bugbear/compare/23.12.2...24.1.17)

---
updated-dependencies:
- dependency-name: flake8-bugbear
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
---
 poetry.lock    | 9 +++++----
 pyproject.toml | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/poetry.lock b/poetry.lock
index aa72aa73b..406d20bed 100644
--- a/poetry.lock
+++ b/poetry.lock
@@ -1263,13 +1263,13 @@ pyflakes = ">=3.1.0,<3.2.0"
 
 [[package]]
 name = "flake8-bugbear"
-version = "23.12.2"
+version = "24.1.17"
 description = "A plugin for flake8 finding likely bugs and design problems in your program. Contains warnings that don't belong in pyflakes and pycodestyle."
 optional = false
 python-versions = ">=3.8.1"
 files = [
-    {file = "flake8-bugbear-23.12.2.tar.gz", hash = "sha256:32b2903e22331ae04885dae25756a32a8c666c85142e933f43512a70f342052a"},
-    {file = "flake8_bugbear-23.12.2-py3-none-any.whl", hash = "sha256:83324bad4d90fee4bf64dd69c61aff94debf8073fbd807c8b6a36eec7a2f0719"},
+    {file = "flake8-bugbear-24.1.17.tar.gz", hash = "sha256:bcb388a4f3b516258749b1e690ee394c082eff742f44595e3754cf5c7781c2c7"},
+    {file = "flake8_bugbear-24.1.17-py3-none-any.whl", hash = "sha256:46cc840ddaed26507cd0ada530d1526418b717ee76c9b5dfdbd238b5eab34139"},
 ]
 
 [package.dependencies]
@@ -3443,6 +3443,7 @@ files = [
     {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"},
     {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"},
     {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"},
+    {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"},
     {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"},
     {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"},
     {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"},
@@ -4719,4 +4720,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p
 [metadata]
 lock-version = "2.0"
 python-versions = ">=3.9,<3.12"
-content-hash = "e915371224cb1a76603cf5b3e57c14ddce536d933d05ac9520da8e79b4b69665"
+content-hash = "3389aa4ce5477dd99ab467168569e9920b942777f37e671bceb8e362ca362f76"
diff --git a/pyproject.toml b/pyproject.toml
index 55a605f11..bec3af860 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -58,7 +58,7 @@ black = "^23.12.1"
 cloudfoundry-client = "*"
 exceptiongroup = "==1.2.0"
 flake8 = "^6.1.0"
-flake8-bugbear = "^23.12.2"
+flake8-bugbear = "^24.1.17"
 freezegun = "^1.4.0"
 honcho = "*"
 isort = "^5.13.2"