From 12776354e399f4863ea1bc3af973312d39707925 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 08:36:52 -0800 Subject: [PATCH 01/13] fix broken go live email notification --- app/celery/provider_tasks.py | 16 +++++++++++----- app/service/sender.py | 13 +++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 011b00d98..9165fe37b 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -19,7 +19,7 @@ from app.delivery import send_to_providers from app.enums import NotificationStatus from app.exceptions import NotificationTechnicalFailureException -from app.utils import utc_now +from app.utils import hilite, utc_now # This is the amount of time to wait after sending an sms message before we check the aws logs and look for delivery # receipts @@ -170,7 +170,7 @@ def deliver_sms(self, notification_id): @notify_celery.task( - bind=True, name="deliver_email", max_retries=48, default_retry_delay=300 + bind=True, name="deliver_email", max_retries=48, default_retry_delay=30 ) def deliver_email(self, notification_id): try: @@ -182,9 +182,15 @@ def deliver_email(self, notification_id): if not notification: raise NoResultFound() personalisation = redis_store.get(f"email-personalisation-{notification_id}") - - notification.personalisation = json.loads(personalisation) - send_to_providers.send_email_to_provider(notification) + recipient = redis_store.get(f"email-recipient-{notification_id}") + if personalisation: + notification.personalisation = json.loads(personalisation) + if recipient: + notification.recipient = json.loads(recipient) + + print(hilite(f"HERE IS THE NOTIFICATION {notification.serialize_for_csv()}")) + if recipient: + send_to_providers.send_email_to_provider(notification) except EmailClientNonRetryableException: current_app.logger.exception(f"Email notification {notification_id} failed") update_notification_status_by_id(notification_id, "technical-failure") diff --git a/app/service/sender.py b/app/service/sender.py index 4b954f60b..4661dbb9b 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -1,5 +1,8 @@ +import json + from flask import current_app +from app import redis_store from app.config import QueueNames from app.dao.services_dao import ( dao_fetch_active_users_for_service, @@ -40,7 +43,17 @@ def send_notification_to_service_users( key_type=KeyType.NORMAL, reply_to_text=notify_service.get_default_reply_to_email_address(), ) + redis_store.set( + f"email-personalisation-{notification.id}", + json.dumps(personalisation), + ex=24 * 60 * 60, + ) + redis_store.set( + f"email-recipient-{notification.id}", notification.to, ex=24 * 60 * 60 + ) + send_notification_to_queue(notification, queue=QueueNames.NOTIFY) + return notification def _add_user_fields(user, personalisation, fields): From 639d2e23354c0c0ba00d31c7886b99fd224f4764 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 08:50:12 -0800 Subject: [PATCH 02/13] fix broken go live email notification --- app/celery/provider_tasks.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 9165fe37b..b0c6a4c9b 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -19,7 +19,7 @@ from app.delivery import send_to_providers from app.enums import NotificationStatus from app.exceptions import NotificationTechnicalFailureException -from app.utils import hilite, utc_now +from app.utils import utc_now # This is the amount of time to wait after sending an sms message before we check the aws logs and look for delivery # receipts @@ -188,9 +188,7 @@ def deliver_email(self, notification_id): if recipient: notification.recipient = json.loads(recipient) - print(hilite(f"HERE IS THE NOTIFICATION {notification.serialize_for_csv()}")) - if recipient: - send_to_providers.send_email_to_provider(notification) + send_to_providers.send_email_to_provider(notification) except EmailClientNonRetryableException: current_app.logger.exception(f"Email notification {notification_id} failed") update_notification_status_by_id(notification_id, "technical-failure") From a1dea3c6122fcc87121df0fcd555db27a1fb0b2f Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 09:02:34 -0800 Subject: [PATCH 03/13] fix broken go live email notification --- tests/app/service/test_sender.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index 4b9c10ee1..d85ac3eea 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -58,6 +58,7 @@ def test_send_notification_to_service_users_includes_user_fields_in_personalisat ): persist_mock = mocker.patch("app.service.sender.persist_notification") mocker.patch("app.service.sender.send_notification_to_queue") + mocker.patch("app.service.sender.redis_store") user = sample_service.users[0] From d23fece060789c85a9e0450daef63381a2b420da Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 09:16:20 -0800 Subject: [PATCH 04/13] fix broken go live email notification --- tests/app/service/test_sender.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index d85ac3eea..d974f1694 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -83,6 +83,7 @@ def test_send_notification_to_service_users_sends_to_active_users_only( notify_service, mocker ): mocker.patch("app.service.sender.send_notification_to_queue") + mocker.patch("app.service.sender.redis_store") first_active_user = create_user(email="foo@bar.com", state="active") second_active_user = create_user(email="foo1@bar.com", state="active") From 9b392af576fe7d7d8cd7b91db86cde4244b2d4ee Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 10:05:41 -0800 Subject: [PATCH 05/13] fix broken go live email notification --- app/service/sender.py | 3 +++ tests/app/service/test_sender.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/service/sender.py b/app/service/sender.py index 4661dbb9b..2a8a7775e 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -14,6 +14,7 @@ persist_notification, send_notification_to_queue, ) +from app.utils import hilite def send_notification_to_service_users( @@ -28,6 +29,7 @@ def send_notification_to_service_users( for user in active_users: personalisation = _add_user_fields(user, personalisation, include_user_fields) + print(hilite(f"PERSONALISATION IS {personalisation}")) notification = persist_notification( template_id=template.id, template_version=template.version, @@ -43,6 +45,7 @@ def send_notification_to_service_users( key_type=KeyType.NORMAL, reply_to_text=notify_service.get_default_reply_to_email_address(), ) + print(hilite(f"NOTIFICATION IS {notification.serialize_for_csv()}")) redis_store.set( f"email-personalisation-{notification.id}", json.dumps(personalisation), diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index d974f1694..ff1749d2a 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -83,7 +83,9 @@ def test_send_notification_to_service_users_sends_to_active_users_only( notify_service, mocker ): mocker.patch("app.service.sender.send_notification_to_queue") - mocker.patch("app.service.sender.redis_store") + mocker.patch( + "app.service.sender.redis_store", + ) first_active_user = create_user(email="foo@bar.com", state="active") second_active_user = create_user(email="foo1@bar.com", state="active") From 6c0cbd2d34304e8d034d6575bebf0f81fe1d0471 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 10:19:09 -0800 Subject: [PATCH 06/13] fix broken go live email notification --- app/service/sender.py | 1 + tests/app/service/test_sender.py | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/service/sender.py b/app/service/sender.py index 2a8a7775e..8e03d5964 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -25,6 +25,7 @@ def send_notification_to_service_users( template = dao_get_template_by_id(template_id) service = dao_fetch_service_by_id(service_id) active_users = dao_fetch_active_users_for_service(service.id) + print(hilite(f"ACTIVE USERS ARE {active_users}")) notify_service = dao_fetch_service_by_id(current_app.config["NOTIFY_SERVICE_ID"]) for user in active_users: diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index ff1749d2a..a66f1e462 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -1,9 +1,10 @@ import pytest from flask import current_app +from app.utils import hilite from sqlalchemy import func, select from app import db -from app.dao.services_dao import dao_add_user_to_service +from app.dao.services_dao import dao_add_user_to_service, dao_fetch_active_users_for_service from app.enums import NotificationType, TemplateType from app.models import Notification from app.service.sender import send_notification_to_service_users @@ -91,8 +92,15 @@ def test_send_notification_to_service_users_sends_to_active_users_only( second_active_user = create_user(email="foo1@bar.com", state="active") pending_user = create_user(email="foo2@bar.com", state="pending") service = create_service(user=first_active_user) + print(hilite(f"CREATED THE SERVICE {service} with user {first_active_user}")) dao_add_user_to_service(service, second_active_user) + print(hilite(f"ADDED user {second_active_user}")) + dao_add_user_to_service(service, pending_user) + print(hilite(f"ADDED PENDING USER {pending_user}")) + + active_users = dao_fetch_active_users_for_service(service.id) + print(hilite(f"ACTIVE USERS IN THE TEST {active_users}")) template = create_template(service, template_type=TemplateType.EMAIL) send_notification_to_service_users(service_id=service.id, template_id=template.id) From 4a31b2ece6db0b2f0fa9b5fad32e1b2d6dda7b09 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 10:30:03 -0800 Subject: [PATCH 07/13] fix broken go live email notification --- tests/app/service/test_sender.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index a66f1e462..b37e5468c 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -1,13 +1,16 @@ import pytest from flask import current_app -from app.utils import hilite from sqlalchemy import func, select from app import db -from app.dao.services_dao import dao_add_user_to_service, dao_fetch_active_users_for_service +from app.dao.services_dao import ( + dao_add_user_to_service, + dao_fetch_active_users_for_service, +) from app.enums import NotificationType, TemplateType from app.models import Notification from app.service.sender import send_notification_to_service_users +from app.utils import hilite from tests.app.db import create_service, create_template, create_user From 61b994827098441a520fa08fce0558d2988de839 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 10:46:49 -0800 Subject: [PATCH 08/13] fix broken go live email notification --- app/service/sender.py | 2 ++ tests/app/service/test_sender.py | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/service/sender.py b/app/service/sender.py index 8e03d5964..484126d0f 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -52,9 +52,11 @@ def send_notification_to_service_users( json.dumps(personalisation), ex=24 * 60 * 60, ) + print(hilite("GOT PAST FIRST SET")) redis_store.set( f"email-recipient-{notification.id}", notification.to, ex=24 * 60 * 60 ) + print(hilite("GOT PAST SECOND SET")) send_notification_to_queue(notification, queue=QueueNames.NOTIFY) return notification diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index b37e5468c..3cf60588d 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -87,9 +87,7 @@ def test_send_notification_to_service_users_sends_to_active_users_only( notify_service, mocker ): mocker.patch("app.service.sender.send_notification_to_queue") - mocker.patch( - "app.service.sender.redis_store", - ) + mocker.patch("app.service.sender.redis_store", autospec=True) first_active_user = create_user(email="foo@bar.com", state="active") second_active_user = create_user(email="foo1@bar.com", state="active") From 43eba01bb46b4a86940eb96c517084b8771671fb Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 10:56:13 -0800 Subject: [PATCH 09/13] fix broken go live email notification --- app/service/sender.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/service/sender.py b/app/service/sender.py index 484126d0f..1a3b1400a 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -59,7 +59,7 @@ def send_notification_to_service_users( print(hilite("GOT PAST SECOND SET")) send_notification_to_queue(notification, queue=QueueNames.NOTIFY) - return notification + def _add_user_fields(user, personalisation, fields): From a1155dff7ea755c1a0a85953fe72a91ac436de93 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 11:03:01 -0800 Subject: [PATCH 10/13] fix broken go live email notification --- app/service/sender.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/service/sender.py b/app/service/sender.py index 1a3b1400a..9370db0a9 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -61,7 +61,6 @@ def send_notification_to_service_users( send_notification_to_queue(notification, queue=QueueNames.NOTIFY) - def _add_user_fields(user, personalisation, fields): for field in fields: personalisation[field] = getattr(user, field) From a5b83f5eff673b66c733901e23863dc961c7122a Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 27 Nov 2024 11:12:21 -0800 Subject: [PATCH 11/13] fix broken go live email notification --- app/service/sender.py | 6 ------ tests/app/service/test_sender.py | 11 +---------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/app/service/sender.py b/app/service/sender.py index 9370db0a9..a769dc4d9 100644 --- a/app/service/sender.py +++ b/app/service/sender.py @@ -14,7 +14,6 @@ persist_notification, send_notification_to_queue, ) -from app.utils import hilite def send_notification_to_service_users( @@ -25,12 +24,10 @@ def send_notification_to_service_users( template = dao_get_template_by_id(template_id) service = dao_fetch_service_by_id(service_id) active_users = dao_fetch_active_users_for_service(service.id) - print(hilite(f"ACTIVE USERS ARE {active_users}")) notify_service = dao_fetch_service_by_id(current_app.config["NOTIFY_SERVICE_ID"]) for user in active_users: personalisation = _add_user_fields(user, personalisation, include_user_fields) - print(hilite(f"PERSONALISATION IS {personalisation}")) notification = persist_notification( template_id=template.id, template_version=template.version, @@ -46,17 +43,14 @@ def send_notification_to_service_users( key_type=KeyType.NORMAL, reply_to_text=notify_service.get_default_reply_to_email_address(), ) - print(hilite(f"NOTIFICATION IS {notification.serialize_for_csv()}")) redis_store.set( f"email-personalisation-{notification.id}", json.dumps(personalisation), ex=24 * 60 * 60, ) - print(hilite("GOT PAST FIRST SET")) redis_store.set( f"email-recipient-{notification.id}", notification.to, ex=24 * 60 * 60 ) - print(hilite("GOT PAST SECOND SET")) send_notification_to_queue(notification, queue=QueueNames.NOTIFY) diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index 3cf60588d..d35eb2edc 100644 --- a/tests/app/service/test_sender.py +++ b/tests/app/service/test_sender.py @@ -3,14 +3,10 @@ from sqlalchemy import func, select from app import db -from app.dao.services_dao import ( - dao_add_user_to_service, - dao_fetch_active_users_for_service, -) +from app.dao.services_dao import dao_add_user_to_service from app.enums import NotificationType, TemplateType from app.models import Notification from app.service.sender import send_notification_to_service_users -from app.utils import hilite from tests.app.db import create_service, create_template, create_user @@ -93,15 +89,10 @@ def test_send_notification_to_service_users_sends_to_active_users_only( second_active_user = create_user(email="foo1@bar.com", state="active") pending_user = create_user(email="foo2@bar.com", state="pending") service = create_service(user=first_active_user) - print(hilite(f"CREATED THE SERVICE {service} with user {first_active_user}")) dao_add_user_to_service(service, second_active_user) - print(hilite(f"ADDED user {second_active_user}")) dao_add_user_to_service(service, pending_user) - print(hilite(f"ADDED PENDING USER {pending_user}")) - active_users = dao_fetch_active_users_for_service(service.id) - print(hilite(f"ACTIVE USERS IN THE TEST {active_users}")) template = create_template(service, template_type=TemplateType.EMAIL) send_notification_to_service_users(service_id=service.id, template_id=template.id) From 791d18b4ecc44cfe01a1b8123d3c4bf2672020ab Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 2 Dec 2024 11:48:26 -0800 Subject: [PATCH 12/13] fix logic --- app/celery/tasks.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index c8ad8cc6d..63d4fdc09 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -347,14 +347,13 @@ def save_api_email_or_sms(self, encrypted_notification): document_download_count=notification["document_download_count"], ) - provider_task.apply_async([notification["id"]], queue=q) - current_app.logger.debug( - f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue." - ) except IntegrityError: current_app.logger.info( f"{notification['notification_type']} {notification['id']} already exists." ) + # If we don't have the return statement here, we will fall through and end + # up retrying because IntegrityError is a subclass of SQLAlchemyError + return except SQLAlchemyError: try: @@ -363,6 +362,10 @@ def save_api_email_or_sms(self, encrypted_notification): current_app.logger.exception( f"Max retry failed Failed to persist notification {notification['id']}", ) + provider_task.apply_async([notification["id"]], queue=q) + current_app.logger.debug( + f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue." + ) def handle_exception(task, notification, notification_id, exc): From 6124fbc6a4be2b1bb47609b7eb000061c8e71006 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 2 Dec 2024 12:15:32 -0800 Subject: [PATCH 13/13] fix logic --- app/celery/tasks.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 63d4fdc09..0794ad4da 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -346,9 +346,14 @@ def save_api_email_or_sms(self, encrypted_notification): status=notification["status"], document_download_count=notification["document_download_count"], ) + # Only get here if save to the db was successful (i.e. first time) + provider_task.apply_async([notification["id"]], queue=q) + current_app.logger.debug( + f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue." + ) except IntegrityError: - current_app.logger.info( + current_app.logger.warning( f"{notification['notification_type']} {notification['id']} already exists." ) # If we don't have the return statement here, we will fall through and end @@ -362,10 +367,6 @@ def save_api_email_or_sms(self, encrypted_notification): current_app.logger.exception( f"Max retry failed Failed to persist notification {notification['id']}", ) - provider_task.apply_async([notification["id"]], queue=q) - current_app.logger.debug( - f"{notification['notification_type']} {notification['id']} has been persisted and sent to delivery queue." - ) def handle_exception(task, notification, notification_id, exc):