diff --git a/app/clients/sms/aws_sns.py b/app/clients/sms/aws_sns.py index e1c872665..c351ec179 100644 --- a/app/clients/sms/aws_sns.py +++ b/app/clients/sms/aws_sns.py @@ -48,7 +48,6 @@ class AwsSnsClient(SmsClient): def send_sms(self, to, content, reference, sender=None, international=False): matched = False - for match in phonenumbers.PhoneNumberMatcher(to, "US"): matched = True to = phonenumbers.format_number( diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 4f811de22..19e132e4b 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -1,4 +1,5 @@ import json +from contextlib import suppress from urllib import parse from cachetools import TTLCache, cached @@ -81,27 +82,15 @@ def send_sms_to_provider(notification): # We start by trying to get the phone number from a job in s3. If we fail, we assume # the phone number is for the verification code on login, which is not a job. recipient = None - try: + # It is our 2facode, maybe + recipient = _get_verify_code(notification) + + if recipient is None: recipient = get_phone_number_from_s3( notification.service_id, notification.job_id, notification.job_row_number, ) - except Exception: - # It is our 2facode, maybe - key = f"2facode-{notification.id}".replace(" ", "") - recipient = redis_store.get(key) - - if recipient: - recipient = recipient.decode("utf-8") - - if recipient is None: - si = notification.service_id - ji = notification.job_id - jrn = notification.job_row_number - raise Exception( - f"The recipient for (Service ID: {si}; Job ID: {ji}; Job Row Number {jrn} was not found." - ) sender_numbers = get_sender_numbers(notification) if notification.reply_to_text not in sender_numbers: @@ -138,6 +127,14 @@ def send_sms_to_provider(notification): return message_id +def _get_verify_code(notification): + key = f"2facode-{notification.id}".replace(" ", "") + recipient = redis_store.get(key) + with suppress(AttributeError): + recipient = recipient.decode("utf-8") + return recipient + + def get_sender_numbers(notification): possible_senders = dao_get_sms_senders_by_service_id(notification.service_id) sender_numbers = [] diff --git a/app/user/rest.py b/app/user/rest.py index faaca4664..a789ee128 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -308,7 +308,6 @@ def send_user_2fa_code(user_id, code_type): def send_user_sms_code(user_to_send_to, data): recipient = data.get("to") or user_to_send_to.mobile_number - secret_code = create_secret_code() personalisation = {"verify_code": secret_code} diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 63ab31ec7..1c8b9a111 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -75,6 +75,8 @@ def test_provider_to_use_raises_if_no_active_providers( def test_should_send_personalised_template_to_correct_sms_provider_and_persist( sample_sms_template_with_html, mocker ): + + mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None) db_notification = create_notification( template=sample_sms_template_with_html, personalisation={}, @@ -114,6 +116,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( def test_should_send_personalised_template_to_correct_email_provider_and_persist( sample_email_template_with_html, mocker ): + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") utf8_encoded_email = "jo.smith@example.com".encode("utf-8") mock_redis.get.return_value = utf8_encoded_email @@ -213,6 +216,8 @@ def test_should_not_send_sms_message_when_service_is_inactive_notification_is_in def test_send_sms_should_use_template_version_from_notification_not_latest( sample_template, mocker ): + + mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None) db_notification = create_notification( template=sample_template, to_field="2028675309", @@ -318,6 +323,8 @@ 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.redis_store", return_value=None) mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] ) @@ -352,6 +359,8 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): def test_send_sms_should_use_service_sms_sender( sample_service, sample_template, mocker ): + + mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) mocker.patch("app.aws_sns_client.send_sms") sms_sender = create_service_sms_sender( @@ -614,6 +623,7 @@ def test_should_update_billable_units_and_status_according_to_research_mode_and_ sample_template, mocker, research_mode, key_type, billable_units, expected_status ): + mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] ) @@ -676,6 +686,8 @@ def test_should_set_notification_billable_units_and_reduces_provider_priority_if def test_should_send_sms_to_international_providers( sample_template, sample_user, mocker ): + + mocker.patch("app.delivery.send_to_providers._get_verify_code", return_value=None) mocker.patch("app.aws_sns_client.send_sms") notification_international = create_notification( @@ -725,6 +737,8 @@ def test_should_send_sms_to_international_providers( def test_should_handle_sms_sender_and_prefix_message( mocker, sms_sender, prefix_sms, expected_sender, expected_content, notify_db_session ): + + mocker.patch("app.delivery.send_to_providers.redis_store", return_value=None) mocker.patch("app.aws_sns_client.send_sms") service = create_service_with_defined_sms_sender( sms_sender_value=sms_sender, prefix_sms=prefix_sms @@ -781,6 +795,7 @@ 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_verify_code", return_value=None) mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] ) @@ -843,6 +858,7 @@ 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_verify_code", return_value=None) mocker.patch( "app.delivery.send_to_providers.get_sender_numbers", return_value=["testing"] )