diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 011b00d98..b0c6a4c9b 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -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,8 +182,12 @@ def deliver_email(self, notification_id): if not notification: raise NoResultFound() personalisation = redis_store.get(f"email-personalisation-{notification_id}") + recipient = redis_store.get(f"email-recipient-{notification_id}") + if personalisation: + notification.personalisation = json.loads(personalisation) + if recipient: + notification.recipient = json.loads(recipient) - notification.personalisation = json.loads(personalisation) send_to_providers.send_email_to_provider(notification) except EmailClientNonRetryableException: current_app.logger.exception(f"Email notification {notification_id} failed") diff --git a/app/celery/tasks.py b/app/celery/tasks.py index c8ad8cc6d..0794ad4da 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -346,15 +346,19 @@ 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 + # up retrying because IntegrityError is a subclass of SQLAlchemyError + return except SQLAlchemyError: try: diff --git a/app/service/sender.py b/app/service/sender.py index 4b954f60b..a769dc4d9 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,6 +43,15 @@ 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) diff --git a/tests/app/service/test_sender.py b/tests/app/service/test_sender.py index 4b9c10ee1..d35eb2edc 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] @@ -82,13 +83,16 @@ 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", autospec=True) first_active_user = create_user(email="foo@bar.com", state="active") 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) dao_add_user_to_service(service, second_active_user) + dao_add_user_to_service(service, pending_user) + template = create_template(service, template_type=TemplateType.EMAIL) send_notification_to_service_users(service_id=service.id, template_id=template.id)