From 1e26511dc6a17e26da6003809524a46160f19fe9 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 31 Jul 2024 13:54:03 -0700 Subject: [PATCH 1/3] raise exception if phone number doesn't match --- app/delivery/send_to_providers.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index c0d32bf86..c5b8fe0ee 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -10,6 +10,7 @@ from app.dao.email_branding_dao import dao_get_email_branding_by_id from app.dao.notifications_dao import dao_update_notification from app.dao.provider_details_dao import get_provider_details_by_notification_type +from app.dao.service_sms_sender_dao import dao_get_sms_senders_by_service_id from app.enums import BrandType, KeyType, NotificationStatus, NotificationType from app.exceptions import NotificationTechnicalFailureException from app.serialised_models import SerialisedService, SerialisedTemplate @@ -101,6 +102,18 @@ def send_sms_to_provider(notification): raise Exception( f"The recipient for (Service ID: {si}; Job ID: {ji}; Job Row Number {jrn} was not found." ) + + possible_senders = dao_get_sms_senders_by_service_id( + notification.service_id + ) + sender_numbers = [] + for possible_sender in possible_senders: + sender_numbers.append(possible_sender.sms_sender) + if notification.reply_to_text not in sender_numbers: + raise ValueError( + f"{notification.reply_to_text} not in {sender_numbers} #notify-admin-1701" + ) + send_sms_kwargs = { "to": recipient, "content": str(template), From e244278ccf4827fe0e4cc535ff3df5a3398de842 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 31 Jul 2024 14:21:30 -0700 Subject: [PATCH 2/3] fix tests --- app/delivery/send_to_providers.py | 15 +++++++++------ tests/app/delivery/test_send_to_providers.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index c5b8fe0ee..4f811de22 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -103,12 +103,7 @@ def send_sms_to_provider(notification): f"The recipient for (Service ID: {si}; Job ID: {ji}; Job Row Number {jrn} was not found." ) - possible_senders = dao_get_sms_senders_by_service_id( - notification.service_id - ) - sender_numbers = [] - for possible_sender in possible_senders: - sender_numbers.append(possible_sender.sms_sender) + sender_numbers = get_sender_numbers(notification) if notification.reply_to_text not in sender_numbers: raise ValueError( f"{notification.reply_to_text} not in {sender_numbers} #notify-admin-1701" @@ -143,6 +138,14 @@ def send_sms_to_provider(notification): return message_id +def get_sender_numbers(notification): + possible_senders = dao_get_sms_senders_by_service_id(notification.service_id) + sender_numbers = [] + for possible_sender in possible_senders: + sender_numbers.append(possible_sender.sms_sender) + return sender_numbers + + def send_email_to_provider(notification): # Someone needs an email, possibly new registration recipient = redis_store.get(f"email-address-{notification.id}") diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index af65dd766..ade2563cb 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -318,6 +318,9 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): # é, o, and u are in GSM. # ī, grapes, tabs, zero width space and ellipsis are not # ó isn't in GSM, but it is in the welsh alphabet so will still be sent + mocker.patch( + "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] + ) msg = "a é ī o u 🍇 foo\tbar\u200bbaz((misc))…" placeholder = "∆∆∆abc" gsm_message = "?ódz Housing Service: a é i o u ? foo barbaz???abc..." @@ -609,6 +612,10 @@ def __update_notification(notification_to_update, research_mode, expected_status def test_should_update_billable_units_and_status_according_to_research_mode_and_key_type( sample_template, mocker, research_mode, key_type, billable_units, expected_status ): + + mocker.patch( + "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] + ) notification = create_notification( template=sample_template, billable_units=0, @@ -771,6 +778,10 @@ def test_send_email_to_provider_uses_reply_to_from_notification( def test_send_sms_to_provider_should_use_normalised_to(mocker, client, sample_template): + + mocker.patch( + "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] + ) send_mock = mocker.patch("app.aws_sns_client.send_sms") notification = create_notification( template=sample_template, to_field="+12028675309", normalised_to="2028675309" @@ -826,6 +837,10 @@ def test_send_email_to_provider_should_user_normalised_to( def test_send_sms_to_provider_should_return_template_if_found_in_redis( mocker, client, sample_template ): + + mocker.patch( + "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] + ) from app.schemas import service_schema, template_schema service_dict = service_schema.dump(sample_template.service) From 635c17008e19d8442e6f7c0633f63d2382b14cba Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 31 Jul 2024 14:47:10 -0700 Subject: [PATCH 3/3] fix tests --- tests/app/delivery/test_send_to_providers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index ade2563cb..7f16a799a 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -330,6 +330,7 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): template=template, ) db_notification.personalisation = {"misc": placeholder} + db_notification.reply_to_text = 'testing' mocker.patch("app.aws_sns_client.send_sms") @@ -621,6 +622,7 @@ def test_should_update_billable_units_and_status_according_to_research_mode_and_ billable_units=0, status=NotificationStatus.CREATED, key_type=key_type, + reply_to_text='testing', ) mocker.patch("app.aws_sns_client.send_sms") mocker.patch( @@ -784,7 +786,7 @@ def test_send_sms_to_provider_should_use_normalised_to(mocker, client, sample_te ) send_mock = mocker.patch("app.aws_sns_client.send_sms") notification = create_notification( - template=sample_template, to_field="+12028675309", normalised_to="2028675309" + template=sample_template, to_field="+12028675309", normalised_to="2028675309", reply_to_text='testing' ) mock_s3 = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3") @@ -860,7 +862,7 @@ def test_send_sms_to_provider_should_return_template_if_found_in_redis( send_mock = mocker.patch("app.aws_sns_client.send_sms") notification = create_notification( - template=sample_template, to_field="+447700900855", normalised_to="447700900855" + template=sample_template, to_field="+447700900855", normalised_to="447700900855", reply_to_text='testing' ) mock_s3 = mocker.patch("app.delivery.send_to_providers.get_phone_number_from_s3")