Merge pull request #1451 from GSA/main

12/2/24 - 12/3/24 Production Deploy - DB insert retry and go live email fixes

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")

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:

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)
 
 

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)