Adding get_notifcation_by_job_and_job_row_number and using it to filter exceptions

Job Id and job row number are unique in the notifications table so they can be used
to see if a notification has been added to the db already even if the notification
has been given a different notification id.

Handle error is the logic for all types of notification, but I only tested one save function
.

Author: whpearson

app/celery/tasks.py

@@ -39,6 +39,7 @@
     dao_get_unknown_references,
     dao_update_notifications_by_reference,
     get_notification_by_id,
+    get_notification_by_job_and_job_row_number,
 )
 from app.dao.report_requests_dao import dao_get_report_request_by_id, dao_update_report_request
 from app.dao.returned_letters_dao import _get_notification_ids_for_references, insert_returned_letters
@@ -513,12 +514,16 @@ def save_letter(
 
 
 def handle_exception(task, notification, notification_id, exc):
-    if not get_notification_by_id(notification_id):
+    job_id = notification.get("job", None)
+    job_row_number = notification.get("row_number", None)
+    if not get_notification_by_id(notification_id) and not get_notification_by_job_and_job_row_number(
+        job_id, job_row_number
+    ):
         extra = {
             "notification_id": notification_id,
             "celery_task": task.name,
-            "job_id": notification.get("job", None),
-            "job_row_number": notification.get("row_number", None),
+            "job_id": job_id,
+            "job_row_number": job_row_number,
         }
         base_msg = (
             "task %(celery_task)s notification for job %(job_id)s row number %(job_row_number)s "

app/dao/notifications_dao.py

@@ -272,6 +272,12 @@ def get_notification_by_id(notification_id, service_id=None, _raise=False):
     return query.one() if _raise else query.first()
 
 
+def get_notification_by_job_and_job_row_number(job_id, job_row_number):
+    filters = [Notification.job_id == job_id, Notification.job_row_number == job_row_number]
+    query = Notification.query.filter(*filters)
+    return query.first()
+
+
 def dao_get_notification_or_history_by_id(notification_id):
     if notification := Notification.query.get(notification_id):
         return notification

tests/app/celery/test_tasks.py

@@ -1716,6 +1716,29 @@ def test_save_email_does_not_send_duplicate_and_does_not_put_in_retry_queue(
     assert not retry.called
 
 
+def test_save_email_does_not_send_duplicate_and_does_not_put_in_retry_queue_based_on_job_info(
+    sample_email_notification, mocker, mock_celery_task
+):
+    json = _notification_json(
+        sample_email_notification.template,
+        sample_email_notification.to,
+        job_id=sample_email_notification.job_id,
+        row_number=sample_email_notification.job_row_number,
+    )
+    mock_task = mock_celery_task(provider_tasks.deliver_email)
+    retry = mocker.patch("app.celery.tasks.save_email.retry", side_effect=Exception())
+
+    save_email(
+        sample_email_notification.service_id,
+        uuid.uuid4(),
+        signing.encode(json),
+    )
+
+    assert Notification.query.count() == 1
+    assert not mock_task.called
+    assert not retry.called
+
+
 def test_save_sms_does_not_send_duplicate_and_does_not_put_in_retry_queue(
     sample_notification, mocker, mock_celery_task
 ):

tests/app/dao/notification_dao/test_notification_dao.py

@@ -39,6 +39,7 @@
     dao_update_notification,
     dao_update_notifications_by_reference,
     get_notification_by_id,
+    get_notification_by_job_and_job_row_number,
     get_notification_with_personalisation,
     get_notifications_for_job,
     get_notifications_for_service,
@@ -404,6 +405,20 @@ def test_get_notification_by_id_when_notification_does_not_exist(notify_db_sessi
     assert notification_from_db is None
 
 
+def test_get_notification_by_job_and_row_number_when_notification_exists(sample_notification):
+    notification_from_db = get_notification_by_job_and_job_row_number(
+        sample_notification.job_id, sample_notification.job_row_number
+    )
+
+    assert sample_notification == notification_from_db
+
+
+def test_get_notification_by_job_and_row_number_when_job_id_notification_does_not_exist(notify_db_session, fake_uuid):
+    notification_from_db = get_notification_by_job_and_job_row_number(fake_uuid, 1)
+
+    assert notification_from_db is None
+
+
 def test_get_notification_by_id_when_notification_exists_for_different_service(sample_notification):
     another_service = create_service(service_name="Another service")