mail: Refactor as background task

[jpmckinney]

The association between email functions, templates and message types could be simplified by using the message type for all 3 (in cases where there is an appropriate message type). If there's no relevant message type, use the same string, at least.

Since #375 is reported, I won't start the following changes (same with #359). The patch under details has already done (2) and (3).

1. Rename template files to read: type.es.html, where type is a lowercase MessageType enum value

2. Rename mail.send_* functions to match a MessageType enum value (where one is available, a str if not)

3. Change mail.send_* calls and Message creation to: (or, maybe call the send_* message directly, if not saving)

   mail.send(session, client.ses, models.MessageType.MY_TYPE, application, keyword=argument, save=False)

   and define a send function like:

   message_id = getattr(sys.modules[__name__], f"send_{message_type.lower()}")(ses, application, **send_kwargs)
   if save:
       models.Message.create(
           session, application=application, type=message_type, external_message_id=message_id, **message_kwargs
       )

4. Now that sending the email and creating the Message is one function call, we can easily make email a BackgroundTask (the typical example). This would mean moving the (inconsistently applied) ClientError handling.

Draft commit messages:

feat: Stop saving a Message for overdue applications to lenders (the associated application was random, and there are already Messages for the overdue applications to OCP).
chore: Send mail and create message as background task.

Related: #367

---

Note: If multiple background tasks are added: https://www.starlette.io/background/

The tasks are executed in order. In case one of the tasks raises an exception, the following tasks will not get the opportunity to be executed.

Details

diff --git a/app/commands.py b/app/commands.py
index bf4913c..ccdfad2 100644
--- a/app/commands.py
+++ b/app/commands.py
@@ -93,13 +93,7 @@ def _create_complete_application(
             expired_at=datetime.utcnow() + timedelta(days=app_settings.application_expiration_days),
         )
 
-        message_id = mail.send_invitation_email(aws.ses_client, application)
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.BORROWER_INVITATION,
-            external_message_id=message_id,
-        )
+        mail.send(session, aws.ses_client, models.MessageType.BORROWER_INVITATION, application)
 
         session.commit()
 
@@ -194,12 +188,8 @@ def send_reminders() -> None:
             logger.info("No new intro reminder to be sent")
         else:
             for application in applications_to_send_intro_reminder:
-                message_id = mail.send_mail_intro_reminder(aws.ses_client, application)
-                models.Message.create(
-                    session,
-                    application=application,
-                    type=models.MessageType.BORROWER_PENDING_APPLICATION_REMINDER,
-                    external_message_id=message_id,
+                mail.send(
+                    session, aws.ses_client, models.MessageType.BORROWER_PENDING_APPLICATION_REMINDER, application
                 )
 
                 logger.info("Mail sent and status updated")
@@ -221,13 +211,7 @@ def send_reminders() -> None:
             logger.info("No new submit reminder to be sent")
         else:
             for application in applications_to_send_submit_reminder:
-                message_id = mail.send_mail_submit_reminder(aws.ses_client, application)
-                models.Message.create(
-                    session,
-                    application=application,
-                    type=models.MessageType.BORROWER_PENDING_SUBMIT_REMINDER,
-                    external_message_id=message_id,
-                )
+                mail.send(session, aws.ses_client, models.MessageType.BORROWER_PENDING_SUBMIT_REMINDER, application)
 
                 logger.info("Mail sent and status updated")
 
@@ -336,26 +320,17 @@ def sla_overdue_applications() -> None:
                     if days_passed > application.lender.sla_days:
                         application.overdued_at = datetime.now(application.created_at.tzinfo)
 
-                        message_id = mail.send_overdue_application_email_to_ocp(aws.ses_client, application)
-                        models.Message.create(
-                            session,
-                            application=application,
-                            type=models.MessageType.OVERDUE_APPLICATION,
-                            external_message_id=message_id,
-                        )
+                        mail.send(session, aws.ses_client, models.MessageType.OVERDUE_APPLICATION, application)
 
                         session.commit()
 
         for lender_id, lender_data in overdue_lenders.items():
-            message_id = mail.send_overdue_application_email_to_lender(
-                aws.ses_client, lender_data["name"], lender_data["count"], lender_data["email"]
-            )
-            models.Message.create(
-                session,
-                # NOTE: A random application that might not even be to the lender, but application is not nullable.
-                application=application,
-                type=models.MessageType.OVERDUE_APPLICATION,
-                external_message_id=message_id,
+            mail.send_overdue_application_to_lender(
+                aws.ses_client,
+                lender_name=lender_data["name"],
+                lender_email=lender_data["count"],
+                amount=lender_data["email"],
             )
 
             session.commit()
diff --git a/app/mail.py b/app/mail.py
index 54ee9e9..01f5ae5 100644
--- a/app/mail.py
+++ b/app/mail.py
@@ -1,14 +1,16 @@
 import json
 import logging
 import os
+import sys
 from pathlib import Path
 from typing import Any
 from urllib.parse import quote
 
 from mypy_boto3_ses.client import SESClient
+from sqlalchemy.orm import Session
 
 from app.i18n import _
-from app.models import Application
+from app.models import Application, Message
 from app.settings import app_settings
 
 logger = logging.getLogger(__name__)
@@ -50,6 +52,23 @@ def get_template_data(template_name: str, subject: str, parameters: dict[str, An
     }
 
 
+def send(
+    session: Session,
+    ses: SESClient,
+    message_type: str,
+    application: Application | None,
+    *,
+    message_kwargs: dict[str, Any] | None = None,
+    **send_kwargs: Any,
+) -> None:
+    message_id = getattr(sys.modules[__name__], f"send_{message_type.lower()}")(ses, application, **send_kwargs)
+    if message_kwargs is None:
+        message_kwargs = {}
+    Message.create(
+        session, application=application, type=message_type, external_message_id=message_id, **message_kwargs
+    )
+
+
 def send_email(ses: SESClient, email: str, data: dict[str, str], *, to_borrower: bool = True) -> str:
     if app_settings.environment == "production" or not to_borrower:
         to_address = email
@@ -66,7 +85,7 @@ def send_email(ses: SESClient, email: str, data: dict[str, str], *, to_borrower:
     )["MessageId"]
 
 
-def send_application_approved_email(ses: SESClient, application: Application) -> str:
+def send_approved_application(ses: SESClient, application: Application) -> str:
     """
     Sends an email notification when an application has been approved.
 
@@ -100,7 +119,7 @@ def send_application_approved_email(ses: SESClient, application: Application) ->
     )
 
 
-def send_application_submission_completed(ses: SESClient, application: Application) -> str:
+def send_submission_completed(ses: SESClient, application: Application) -> str:
     """
     Sends an email notification when an application is submitted.
 
@@ -121,7 +140,7 @@ def send_application_submission_completed(ses: SESClient, application: Applicati
     )
 
 
-def send_application_credit_disbursed(ses: SESClient, application: Application) -> str:
+def send_credit_disbursed(ses: SESClient, application: Application) -> str:
     """
     Sends an email notification when an application has the credit dibursed.
 
@@ -143,7 +162,7 @@ def send_application_credit_disbursed(ses: SESClient, application: Application)
     )
 
 
-def send_mail_to_new_user(ses: SESClient, name: str, username: str, temporary_password: str) -> str:
+def send_new_user(ses: SESClient, *, name: str, username: str, temporary_password: str) -> str:
     """
     Sends an email to a new user with a link to set their password.
 
@@ -174,7 +193,7 @@ def send_mail_to_new_user(ses: SESClient, name: str, username: str, temporary_pa
     )
 
 
-def send_upload_contract_notification_to_lender(ses: SESClient, application: Application) -> str:
+def send_contract_upload_confirmation_to_fi(ses: SESClient, application: Application) -> str:
     """
     Sends an email to the lender to notify them of a new contract submission.
 
@@ -197,7 +216,7 @@ def send_upload_contract_notification_to_lender(ses: SESClient, application: App
     )
 
 
-def send_upload_contract_confirmation(ses: SESClient, application: Application) -> str:
+def send_contract_upload_confirmation(ses: SESClient, application: Application) -> str:
     """
     Sends an email to the borrower confirming the successful upload of the contract.
 
@@ -219,8 +238,8 @@ def send_upload_contract_confirmation(ses: SESClient, application: Application)
     )
 
 
-def send_new_email_confirmation(
-    ses: SESClient, application: Application, new_email: str, confirmation_email_token: str
+def send_email_change_confirmation(
+    ses: SESClient, application: Application, *, new_email: str, confirmation_email_token: str
 ) -> str:
     """
     Sends an email to confirm the new primary email for the borrower.
@@ -249,7 +268,7 @@ def send_new_email_confirmation(
     return send_email(ses, new_email, data)
 
 
-def send_mail_to_reset_password(ses: SESClient, username: str, temporary_password: str) -> str:
+def send_reset_password(ses: SESClient, *, username: str, temporary_password: str) -> str:
     """
     Sends an email to a user with instructions to reset their password.
 
@@ -292,7 +311,7 @@ def get_invitation_email_parameters(application: Application) -> dict[str, str]:
     }
 
 
-def send_invitation_email(ses: SESClient, application: Application) -> str:
+def send_borrower_invitation(ses: SESClient, application: Application) -> str:
     """
     Sends an invitation email to the provided email address.
 
@@ -310,7 +329,7 @@ def send_invitation_email(ses: SESClient, application: Application) -> str:
     )
 
 
-def send_mail_intro_reminder(ses: SESClient, application: Application) -> str:
+def send_borrower_pending_application_reminder(ses: SESClient, application: Application) -> str:
     """
     Sends an introductory reminder email to the provided email address.
 
@@ -328,7 +347,7 @@ def send_mail_intro_reminder(ses: SESClient, application: Application) -> str:
     )
 
 
-def send_mail_submit_reminder(ses: SESClient, application: Application) -> str:
+def send_borrower_pending_submit_reminder(ses: SESClient, application: Application) -> str:
     """
     Sends a submission reminder email to the provided email address.
 
@@ -354,7 +373,7 @@ def send_mail_submit_reminder(ses: SESClient, application: Application) -> str:
     )
 
 
-def send_notification_new_app_to_lender(ses: SESClient, application: Application) -> str:
+def send_new_application_fi(ses: SESClient, application: Application) -> str:
     """
     Sends a notification email about a new application to a lender's email group.
 
@@ -375,7 +394,7 @@ def send_notification_new_app_to_lender(ses: SESClient, application: Application
     )
 
 
-def send_notification_new_app_to_ocp(ses: SESClient, application: Application) -> str:
+def send_new_application_ocp(ses: SESClient, application: Application) -> str:
     """
     Sends a notification email about a new application to the Open Contracting Partnership's (OCP) email group.
     """
@@ -395,11 +414,11 @@ def send_notification_new_app_to_ocp(ses: SESClient, application: Application) -
     )
 
 
-def send_mail_request_to_borrower(ses: SESClient, application: Application, email_message: str) -> str:
+def send_fi_message(ses: SESClient, application: Application, *, message: str) -> str:
     """
     Sends an email request to the borrower for additional data.
 
-    :param email_message: Message content from the lender to be included in the email.
+    :param message: Message content from the lender to be included in the email.
     """
     return send_email(
         ses,
@@ -409,7 +428,7 @@ def send_mail_request_to_borrower(ses: SESClient, application: Application, emai
             _("New message from a financial institution"),
             {
                 "LENDER_NAME": application.lender.name,
-                "LENDER_MESSAGE": email_message,
+                "LENDER_MESSAGE": message,
                 "LOGIN_DOCUMENTS_URL": f"{app_settings.frontend_url}/application/{quote(application.uuid)}/documents",
                 "LOGIN_IMAGE_LINK": f"{LOCALIZED_IMAGES_BASE_URL}/uploadDocument.png",
             },
@@ -417,7 +436,7 @@ def send_mail_request_to_borrower(ses: SESClient, application: Application, emai
     )
 
 
-def send_overdue_application_email_to_lender(ses: SESClient, lender_name: str, lender_email: str, amount: int) -> str:
+def send_overdue_application_to_lender(ses: SESClient, *, lender_name: str, lender_email: str, amount: int) -> str:
     """
     Sends an email notification to the lender about overdue applications.
 
@@ -442,7 +461,7 @@ def send_overdue_application_email_to_lender(ses: SESClient, lender_name: str, l
     )
 
 
-def send_overdue_application_email_to_ocp(ses: SESClient, application: Application) -> str:
+def send_overdue_application(ses: SESClient, application: Application) -> str:
     """
     Sends an email notification to the Open Contracting Partnership (OCP) about overdue applications.
     """
@@ -463,33 +482,28 @@ def send_overdue_application_email_to_ocp(ses: SESClient, application: Applicati
     )
 
 
-def send_rejected_application_email(ses: SESClient, application: Application) -> str:
+def send_rejected_application(ses: SESClient, application: Application, *, options: bool) -> str:
     """
     Sends an email notification to the applicant when an application has been rejected.
     """
-    return send_email(
-        ses,
-        application.primary_email,
-        get_template_data(
-            "Application_declined",
-            _("Your credit application has been declined"),
-            {
-                "LENDER_NAME": application.lender.name,
-                "AWARD_SUPPLIER_NAME": application.borrower.legal_name,
-                "FIND_ALTENATIVE_URL": (
-                    f"{app_settings.frontend_url}/application/{quote(application.uuid)}/find-alternative-credit"
-                ),
-                "FIND_ALTERNATIVE_IMAGE_LINK": f"{LOCALIZED_IMAGES_BASE_URL}/findAlternative.png",
-            },
-        ),
-    )
+    if options:
+        return send_email(
+            ses,
+            application.primary_email,
+            get_template_data(
+                "Application_declined",
+                _("Your credit application has been declined"),
+                {
+                    "LENDER_NAME": application.lender.name,
+                    "AWARD_SUPPLIER_NAME": application.borrower.legal_name,
+                    "FIND_ALTENATIVE_URL": (
+                        f"{app_settings.frontend_url}/application/{quote(application.uuid)}/find-alternative-credit"
+                    ),
+                    "FIND_ALTERNATIVE_IMAGE_LINK": f"{LOCALIZED_IMAGES_BASE_URL}/findAlternative.png",
+                },
+            ),
+        )
 
-
-def send_rejected_application_email_without_alternatives(ses: SESClient, application: Application) -> str:
-    """
-    Sends an email notification to the applicant when an application has been rejected,
-    and no alternatives are available.
-    """
     return send_email(
         ses,
         application.primary_email,
@@ -504,7 +518,7 @@ def send_rejected_application_email_without_alternatives(ses: SESClient, applica
     )
 
 
-def send_copied_application_notification_to_borrower(ses: SESClient, application: Application) -> str:
+def send_application_copied(ses: SESClient, application: Application) -> str:
     """
     Sends an email notification to the borrower when an application
     has been copied, allowing them to continue with the application process.
@@ -524,7 +538,7 @@ def send_copied_application_notification_to_borrower(ses: SESClient, application
     )
 
 
-def send_upload_documents_notifications_to_lender(ses: SESClient, application: Application) -> str:
+def send_borrower_document_updated(ses: SESClient, application: Application) -> str:
     """
     Sends an email notification to the lender to notify them that new
     documents have been uploaded and are ready for their review.
diff --git a/app/routers/applications.py b/app/routers/applications.py
index 2067ac1..d14f950 100644
--- a/app/routers/applications.py
+++ b/app/routers/applications.py
@@ -48,7 +48,7 @@ async def reject_application(
         payload_dict = jsonable_encoder(payload, exclude_unset=True)
         application.stage_as_rejected(payload_dict)
 
-        options = (
+        options = session.query(
             session.query(models.CreditProduct)
             .join(models.Lender)
             .options(joinedload(models.CreditProduct.lender))
@@ -58,8 +58,8 @@ async def reject_application(
                 col(models.CreditProduct.lower_limit) <= application.amount_requested,
                 col(models.CreditProduct.upper_limit) >= application.amount_requested,
             )
-            .all()
-        )
+            .exists()
+        ).scalar()
 
         models.ApplicationAction.create(
             session,
@@ -69,16 +69,7 @@ async def reject_application(
             user_id=user.id,
         )
 
-        if options:
-            message_id = mail.send_rejected_application_email(client.ses, application)
-        else:
-            message_id = mail.send_rejected_application_email_without_alternatives(client.ses, application)
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.REJECTED_APPLICATION,
-            external_message_id=message_id,
-        )
+        mail.send(session, client.ses, models.MessageType.REJECTED_APPLICATION, application, options=options)
 
         session.commit()
         return application
@@ -121,13 +112,7 @@ async def complete_application(
             user_id=user.id,
         )
 
-        message_id = mail.send_application_credit_disbursed(client.ses, application)
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.CREDIT_DISBURSED,
-            external_message_id=message_id,
-        )
+        mail.send(session, client.ses, models.MessageType.CREDIT_DISBURSED, application)
 
         session.commit()
         return application
@@ -199,13 +184,7 @@ async def approve_application(
             user_id=user.id,
         )
 
-        message_id = mail.send_application_approved_email(client.ses, application)
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.APPROVED_APPLICATION,
-            external_message_id=message_id,
-        )
+        mail.send(session, client.ses, models.MessageType.APPROVED_APPLICATION, application)
 
         session.commit()
         return application
@@ -570,21 +549,20 @@ async def email_borrower(
         )
 
-         try:
-            message_id = mail.send_mail_request_to_borrower(client.ses, application, payload.message)
+        mail.send(
+            session,
+            client.ses,
+            models.MessageType.FI_MESSAGE,
+            application,
+            message=payload.message,
+            message_kwargs={"body": payload.message, "lender_id": application.lender.id},
+        )
-         except ClientError as e:
-             logger.exception(e)
-             return HTTPException(
-                 status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-                 detail="There was an error",
-             )
-        models.Message.create(
-            session,
-            application_id=application.id,
-            body=payload.message,
-            lender_id=application.lender.id,
-            type=models.MessageType.FI_MESSAGE,
-            external_message_id=message_id,
-        )
 
         session.commit()
         return application
diff --git a/app/routers/guest/applications.py b/app/routers/guest/applications.py
index 9d9633b..c078501 100644
--- a/app/routers/guest/applications.py
+++ b/app/routers/guest/applications.py
@@ -536,22 +536,15 @@ async def update_apps_send_notifications(
         application.pending_documents = False
 
-         try:
-            mail.send_notification_new_app_to_lender(client.ses, application)
-            mail.send_notification_new_app_to_ocp(client.ses, application)
-
-            message_id = mail.send_application_submission_completed(client.ses, application)
+        mail.send_new_application_ocp(client.ses, application)
+        mail.send_new_application_fi(client.ses, application)
+        mail.send(session, client.ses, models.MessageType.SUBMISSION_COMPLETED, application)
-         except ClientError as e:
-             logger.exception(e)
-             raise HTTPException(
-                 status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-                 detail="There was an error submitting the application",
-             )
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.SUBMISSION_COMPLETED,
-            external_message_id=message_id,
-        )
 
         session.commit()
         return serializers.ApplicationResponse(
@@ -647,13 +640,7 @@ async def complete_information_request(
             application_id=application.id,
         )
 
-        message_id = mail.send_upload_documents_notifications_to_lender(client.ses, application)
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.BORROWER_DOCUMENT_UPDATED,
-            external_message_id=message_id,
-        )
+        mail.send(session, client.ses, models.MessageType.BORROWER_DOCUMENT_UPDATED, application)
 
         session.commit()
         return serializers.ApplicationResponse(
@@ -733,22 +720,8 @@ async def confirm_upload_contract(
             application_id=application.id,
         )
 
-        lender_message_id, borrower_message_id = (
-            mail.send_upload_contract_notification_to_lender(client.ses, application),
-            mail.send_upload_contract_confirmation(client.ses, application),
-        )
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.CONTRACT_UPLOAD_CONFIRMATION_TO_FI,
-            external_message_id=lender_message_id,
-        )
-        models.Message.create(
-            session,
-            application=application,
-            type=models.MessageType.CONTRACT_UPLOAD_CONFIRMATION,
-            external_message_id=borrower_message_id,
-        )
+        mail.send(session, client.ses, models.MessageType.CONTRACT_UPLOAD_CONFIRMATION_TO_FI, application)
+        mail.send(session, client.ses, models.MessageType.CONTRACT_UPLOAD_CONFIRMATION, application)
 
         session.commit()
         return serializers.ApplicationResponse(
@@ -835,13 +808,7 @@ async def find_alternative_credit_option(
             application_id=new_application.id,
         )
 
-        message_id = mail.send_copied_application_notification_to_borrower(client.ses, new_application)
-        models.Message.create(
-            session,
-            application=new_application,
-            type=models.MessageType.APPLICATION_COPIED,
-            external_message_id=message_id,
-        )
+        mail.send(session, client.ses, models.MessageType.APPLICATION_COPIED, new_application)
 
         session.commit()
         return serializers.ApplicationResponse(
diff --git a/app/routers/guest/emails.py b/app/routers/guest/emails.py
index 35e50c8..9002298 100644
--- a/app/routers/guest/emails.py
+++ b/app/routers/guest/emails.py
@@ -44,14 +44,13 @@ async def change_email(
             application_id=application.id,
         )
 
-        message_id = mail.send_new_email_confirmation(
-            client.ses, application, payload.new_email, confirmation_email_token
-        )
-        models.Message.create(
+        mail.send(
             session,
-            application=application,
-            type=models.MessageType.EMAIL_CHANGE_CONFIRMATION,
-            external_message_id=message_id,
+            client.ses,
+            models.MessageType.EMAIL_CHANGE_CONFIRMATION,
+            application,
+            new_email=payload.new_email,
+            confirmation_email_token=confirmation_email_token,
         )
 
         session.commit()
diff --git a/app/routers/users.py b/app/routers/users.py
index b6d2b70..b6ad65e 100644
--- a/app/routers/users.py
+++ b/app/routers/users.py
@@ -51,7 +51,9 @@ async def create_user(
 
             session.commit()
 
-            mail.send_mail_to_new_user(client.ses, payload.name, payload.email, temporary_password)
+            mail.send_new_user(
+                client.ses, name=payload.name, username=payload.email, temporary_password=temporary_password
+            )
 
             return user
         except (client.cognito.exceptions.UsernameExistsException, IntegrityError):
@@ -278,7 +280,7 @@ def forgot_password(
             Permanent=False,
         )
 
-        mail.send_mail_to_reset_password(client.ses, payload.username, temporary_password)
+        mail.send_reset_password(client.ses, username=payload.username, temporary_password=temporary_password)
     except Exception:
         logger.exception("Error resetting password")
 

[jpmckinney]

Once #411 is merged, all that's left is:

make email a BackgroundTask (the typical example)