From b5a05620773f26f0ab54c2f4e8c60e5132914d67 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 24 Jan 2024 07:55:14 -0800 Subject: [PATCH] fix tests --- app/delivery/send_to_providers.py | 28 ++++++---- app/notifications/process_notifications.py | 10 ++-- app/organization/invite_rest.py | 8 +++ app/service_invite/rest.py | 8 +++ app/user/rest.py | 2 +- tests/app/delivery/test_send_to_providers.py | 54 ++++++++++++++++++-- 6 files changed, 94 insertions(+), 16 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index a3c986ead..8a72a66bd 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -1,3 +1,4 @@ +import json from datetime import datetime from urllib import parse @@ -32,12 +33,15 @@ from app.serialised_models import SerialisedService, SerialisedTemplate def send_sms_to_provider(notification): # we no longer store the personalisation in the db, # need to retrieve from s3 before generating content - personalisation = get_personalisation_from_s3( - notification.service_id, - notification.job_id, - notification.job_row_number, - ) - notification.personalisation = personalisation + # However, we are still sending the initial verify code through personalisation + # so if there is some value there, don't overwrite it + if not notification.personalisation: + personalisation = get_personalisation_from_s3( + notification.service_id, + notification.job_id, + notification.job_row_number, + ) + notification.personalisation = personalisation service = SerialisedService.from_id(notification.service_id) message_id = None @@ -123,6 +127,14 @@ def send_sms_to_provider(notification): def send_email_to_provider(notification): + # Someone needs an email, possibly new registration + recipient = redis_store.get(f"email-address-{notification.id}") + recipient = recipient.decode("utf-8") + personalisation = redis_store.get(f"email-personalisation-{notification.id}") + if personalisation: + personalisation = personalisation.decode("utf-8") + notification.personalisation = json.loads(personalisation) + service = SerialisedService.from_id(notification.service_id) if not service.active: technical_failure(notification=notification) @@ -144,9 +156,7 @@ 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}") - 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/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 0b46c8b7c..28c7ed8a8 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -9,6 +9,7 @@ from notifications_utils.recipients import ( ) from notifications_utils.template import PlainTextEmailTemplate, SMSMessageTemplate +from app import redis_store from app.celery import provider_tasks from app.config import QueueNames from app.dao.notifications_dao import ( @@ -81,8 +82,6 @@ def persist_notification( document_download_count=None, updated_at=None, ): - current_app.logger.info("Persisting notification") - notification_created_at = created_at or datetime.utcnow() if not notification_id: notification_id = uuid.uuid4() @@ -123,7 +122,12 @@ def persist_notification( notification.rate_multiplier = recipient_info.billable_units elif notification_type == EMAIL_TYPE: current_app.logger.info(f"Persisting notification with type: {EMAIL_TYPE}") - notification.normalised_to = format_email_address(notification.to) + # This is typically for something like inviting a user or the 90 day email check + redis_store.set( + f"email-address-{notification.id}", + format_email_address(notification.to), + ex=1800, + ) # if simulated create a Notification model to return but do not persist the Notification to the dB if not simulated: diff --git a/app/organization/invite_rest.py b/app/organization/invite_rest.py index 3e9191ddf..d4e052136 100644 --- a/app/organization/invite_rest.py +++ b/app/organization/invite_rest.py @@ -1,7 +1,10 @@ +import json + from flask import Blueprint, current_app, jsonify, request from itsdangerous import BadData, SignatureExpired from notifications_utils.url_safe_token import check_token, generate_token +from app import redis_store from app.config import QueueNames from app.dao.invited_org_user_dao import ( get_invited_org_user as dao_get_invited_org_user, @@ -70,6 +73,11 @@ def invite_user_to_org(organization_id): key_type=KEY_TYPE_NORMAL, reply_to_text=invited_org_user.invited_by.email_address, ) + redis_store.set( + f"email-personalisation-{saved_notification.id}", + json.dumps(personalisation), + ex=1800, + ) saved_notification.personalisation = personalisation send_notification_to_queue(saved_notification, queue=QueueNames.NOTIFY) diff --git a/app/service_invite/rest.py b/app/service_invite/rest.py index aee6516fa..8133161b2 100644 --- a/app/service_invite/rest.py +++ b/app/service_invite/rest.py @@ -1,9 +1,11 @@ +import json from datetime import datetime from flask import Blueprint, current_app, jsonify, request from itsdangerous import BadData, SignatureExpired from notifications_utils.url_safe_token import check_token, generate_token +from app import redis_store from app.config import QueueNames from app.dao.invited_user_dao import ( get_expired_invite_by_service_and_id, @@ -38,6 +40,7 @@ def _create_service_invite(invited_user, invite_link_host): "service_name": invited_user.service.name, "url": invited_user_url(invited_user.id, invite_link_host), } + saved_notification = persist_notification( template_id=template.id, template_version=template.version, @@ -50,6 +53,11 @@ def _create_service_invite(invited_user, invite_link_host): reply_to_text=invited_user.from_user.email_address, ) saved_notification.personalisation = personalisation + redis_store.set( + f"email-personalisation-{saved_notification.id}", + json.dumps(personalisation), + ex=1800, + ) send_notification_to_queue(saved_notification, queue=QueueNames.NOTIFY) diff --git a/app/user/rest.py b/app/user/rest.py index f9aaf05f9..4de2ed8e7 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -351,7 +351,7 @@ def create_2fa_code( key_type=KEY_TYPE_NORMAL, reply_to_text=reply_to, ) - + saved_notification.personalisation = personalisation key = f"2facode-{saved_notification.id}".replace(" ", "") recipient = str(recipient) redis_store.raw_set(key, recipient, ex=60 * 60) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 75bcdf1d9..3b6b5ba22 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -124,6 +124,13 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist ): mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") mock_redis.get.return_value = "jo.smith@example.com".encode("utf-8") + email = "jo.smith@example.com".encode("utf-8") + personalisation = { + "name": "Jo", + } + personalisation = json.dumps(personalisation) + personalisation = personalisation.encode("utf-8") + mock_redis.get.side_effect = [email, personalisation] db_notification = create_notification( template=sample_email_template_with_html, @@ -169,6 +176,17 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i ) mock_personalisation.return_value = {"name": "Jo"} + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "jo.smith@example.com".encode("utf-8") + email = "jo.smith@example.com".encode("utf-8") + personalisation = { + "name": "Jo", + } + + personalisation = json.dumps(personalisation) + personalisation = personalisation.encode("utf-8") + mock_redis.get.side_effect = [email, personalisation] + with pytest.raises(NotificationTechnicalFailureException) as e: send_to_providers.send_email_to_provider(sample_notification) assert str(sample_notification.id) in str(e.value) @@ -390,6 +408,14 @@ def test_send_email_should_use_service_reply_to_email( mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") mock_redis.get.return_value = "test@example.com".encode("utf-8") + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + email = "foo@bar.com".encode("utf-8") + personalisation = {} + + personalisation = json.dumps(personalisation) + personalisation = personalisation.encode("utf-8") + mock_redis.get.side_effect = [email, personalisation] + db_notification = create_notification( template=sample_email_template, reply_to_text="foo@bar.com" ) @@ -709,7 +735,10 @@ 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") + mock_redis.get.side_effect = [ + "test@example.com".encode("utf-8"), + json.dumps({}).encode("utf-8"), + ] mocker.patch("app.aws_ses_client.send_email", return_value="reference") @@ -759,6 +788,15 @@ def test_send_email_to_provider_should_user_normalised_to( mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") mock_redis.get.return_value = "test@example.com".encode("utf-8") + mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + mock_redis.get.return_value = "jo.smith@example.com".encode("utf-8") + email = "test@example.com".encode("utf-8") + personalisation = {} + + personalisation = json.dumps(personalisation) + personalisation = personalisation.encode("utf-8") + mock_redis.get.side_effect = [email, personalisation] + send_to_providers.send_email_to_provider(notification) send_mock.assert_called_once_with( ANY, @@ -820,8 +858,16 @@ 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") + # mock_redis = mocker.patch("app.delivery.send_to_providers.redis_store") + # mock_redis.get.return_value = "jo.smith@example.com".encode("utf-8") + email = "test@example.com".encode("utf-8") + personalisation = { + "name": "Jo", + } + + personalisation = json.dumps(personalisation) + personalisation = personalisation.encode("utf-8") + # mock_redis.get.side_effect = [email, personalisation] service_dict = service_schema.dump(sample_email_template.service) template_dict = template_schema.dump(sample_email_template) @@ -829,6 +875,8 @@ def test_send_email_to_provider_should_return_template_if_found_in_redis( mocker.patch( "app.redis_store.get", side_effect=[ + email, + personalisation, json.dumps({"data": service_dict}).encode("utf-8"), json.dumps({"data": template_dict}).encode("utf-8"), ],