From c3cb60f3b098e9d719525214ed83c250181a099e Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Thu, 11 Jan 2024 11:11:12 -0800 Subject: [PATCH 1/2] fix registration so email gets sent --- app/delivery/send_to_providers.py | 24 ++++++++++++++---------- app/user/rest.py | 8 +++++++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index e370a1111..d874106d2 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -69,29 +69,30 @@ 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. - my_phone = None + recipient = None try: - my_phone = get_phone_number_from_s3( + recipient = get_phone_number_from_s3( notification.service_id, notification.job_id, notification.job_row_number, ) except BaseException: + # It is our 2facode, maybe key = f"2facode-{notification.id}".replace(" ", "") - my_phone = redis_store.raw_get(key) + recipient = redis_store.raw_get(key) - if my_phone: - my_phone = my_phone.decode("utf-8") + if recipient: + recipient = recipient.decode("utf-8") - if my_phone is None: + if recipient is None: si = notification.service_id ji = notification.job_id jrn = notification.job_row_number raise Exception( - f"The phone number for (Service ID: {si}; Job ID: {ji}; Job Row Number {jrn} was not found." + f"The recipient for (Service ID: {si}; Job ID: {ji}; Job Row Number {jrn} was not found." ) send_sms_kwargs = { - "to": my_phone, + "to": recipient, "content": str(template), "reference": str(notification.id), "sender": notification.reply_to_text, @@ -132,10 +133,13 @@ def send_email_to_provider(notification): plain_text_email = PlainTextEmailTemplate( template_dict, values=notification.personalisation ) + # Someone needs an email, possibly new registration + recipient = redis_store.get(f"email-address-{notification.id}").decode("utf-8") + if notification.key_type == KEY_TYPE_TEST: notification.reference = str(create_uuid()) update_notification_to_sending(notification, provider) - send_email_response(notification.reference, notification.to) + send_email_response(notification.reference, recipient) else: from_address = '"{}" <{}@{}>'.format( service.name, @@ -145,7 +149,7 @@ def send_email_to_provider(notification): reference = provider.send_email( from_address, - notification.normalised_to, + recipient, plain_text_email.subject, body=str(plain_text_email), html_body=str(html_email), diff --git a/app/user/rest.py b/app/user/rest.py index e98151f1e..303c1a39c 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -353,7 +353,7 @@ def create_2fa_code( key = f"2facode-{saved_notification.id}".replace(" ", "") recipient = str(recipient) - redis_store.raw_set(key, recipient) + redis_store.raw_set(key, recipient, ex=60 * 60) # Assume that we never want to observe the Notify service's research mode # setting for this notification - we still need to be able to log into the @@ -431,6 +431,12 @@ def send_new_user_email_verification(user_id): key_type=KEY_TYPE_NORMAL, reply_to_text=service.get_default_reply_to_email_address(), ) + + redis_store.set( + f"email-address-{saved_notification.id}", + str(user_to_send_to.email_address), + ex=60 * 60, + ) current_app.logger.info("Sending notification to queue") send_notification_to_queue(saved_notification, queue=QueueNames.NOTIFY) From ab0066589c7439eda03118e4f07220248e1072ed Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Thu, 11 Jan 2024 11:51:00 -0800 Subject: [PATCH 2/2] fix registration and mock tests for redis --- app/delivery/send_to_providers.py | 5 ++-- tests/app/delivery/test_send_to_providers.py | 29 +++++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index d874106d2..ac844f540 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -112,7 +112,6 @@ def send_sms_to_provider(notification): def send_email_to_provider(notification): service = SerialisedService.from_id(notification.service_id) - if not service.active: technical_failure(notification=notification) return @@ -134,8 +133,8 @@ def send_email_to_provider(notification): template_dict, values=notification.personalisation ) # Someone needs an email, possibly new registration - recipient = redis_store.get(f"email-address-{notification.id}").decode("utf-8") - + recipient = redis_store.get(f"email-address-{notification.id}") + recipient = recipient.decode("utf-8") if notification.key_type == KEY_TYPE_TEST: notification.reference = str(create_uuid()) update_notification_to_sending(notification, provider) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 4cbf22b9a..4dfb336a0 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -85,11 +85,9 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist( ): db_notification = create_notification( template=sample_sms_template_with_html, - to_field="2028675309", personalisation={"name": "Jo"}, status="created", reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), - normalised_to="2028675309", ) mocker.patch("app.aws_sns_client.send_sms") @@ -119,11 +117,12 @@ 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") + mock_redis.get.return_value = "jo.smith@example.com".encode("utf-8") + db_notification = create_notification( template=sample_email_template_with_html, - to_field="jo.smith@example.com", personalisation={"name": "Jo"}, - normalised_to="jo.smith@example.com", ) mocker.patch("app.aws_ses_client.send_email", return_value="reference") @@ -313,6 +312,9 @@ def test_send_sms_should_use_service_sms_sender( def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created( sample_email_template, mocker ): + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "test@example.com".encode("utf-8") + notification = create_notification(template=sample_email_template, status="sending") mocker.patch("app.aws_ses_client.send_email") mocker.patch("app.delivery.send_to_providers.send_email_response") @@ -327,6 +329,9 @@ def test_send_email_should_use_service_reply_to_email( ): mocker.patch("app.aws_ses_client.send_email", return_value="reference") + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "test@example.com".encode("utf-8") + db_notification = create_notification( template=sample_email_template, reply_to_text="foo@bar.com" ) @@ -622,6 +627,9 @@ def test_should_handle_sms_sender_and_prefix_message( def test_send_email_to_provider_uses_reply_to_from_notification( sample_email_template, mocker ): + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "test@example.com".encode("utf-8") + mocker.patch("app.aws_ses_client.send_email", return_value="reference") db_notification = create_notification( @@ -661,14 +669,14 @@ def test_send_email_to_provider_should_user_normalised_to( send_mock = mocker.patch("app.aws_ses_client.send_email", return_value="reference") notification = create_notification( template=sample_email_template, - to_field="TEST@example.com", - normalised_to="test@example.com", ) + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "test@example.com".encode("utf-8") send_to_providers.send_email_to_provider(notification) send_mock.assert_called_once_with( ANY, - notification.normalised_to, + "test@example.com", ANY, body=ANY, html_body=ANY, @@ -721,6 +729,9 @@ def test_send_email_to_provider_should_return_template_if_found_in_redis( ): from app.schemas import service_schema, template_schema + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "test@example.com".encode("utf-8") + service_dict = service_schema.dump(sample_email_template.service) template_dict = template_schema.dump(sample_email_template) @@ -738,8 +749,6 @@ def test_send_email_to_provider_should_return_template_if_found_in_redis( send_mock = mocker.patch("app.aws_ses_client.send_email", return_value="reference") notification = create_notification( template=sample_email_template, - to_field="TEST@example.com", - normalised_to="test@example.com", ) send_to_providers.send_email_to_provider(notification) @@ -747,7 +756,7 @@ def test_send_email_to_provider_should_return_template_if_found_in_redis( assert mock_get_service.called is False send_mock.assert_called_once_with( ANY, - notification.normalised_to, + "test@example.com", ANY, body=ANY, html_body=ANY,