Refactored notifications_dao

- Introduce a `_raise` flag for `get_notification_by_id` so that sql alchemy will raise the NoResults error rather than the app
- Refactor `dao_set_created_live_letter_api_notifications_to_pending` to use a join for getting services that don't have `letters_as_pdf` as marginally faster.

app/celery/letters_pdf_tasks.py

@@ -5,7 +5,6 @@ from requests import (
 )
 
 from botocore.exceptions import ClientError as BotoClientError
-from sqlalchemy.orm.exc import NoResultFound
 
 from app import notify_celery
 from app.aws import s3
@@ -18,9 +17,7 @@ from app.statsd_decorators import statsd
 @statsd(namespace="tasks")
 def create_letters_pdf(self, notification_id):
     try:
-        notification = get_notification_by_id(notification_id)
+        notification = get_notification_by_id(notification_id, _raise=True)
-        if not notification:
-            raise NoResultFound()
 
         pdf_data = get_letters_pdf(
             notification.template,

app/dao/notifications_dao.py

@@ -226,8 +226,11 @@ def get_notification_with_personalisation(service_id, notification_id, key_type)
 
 
 @statsd(namespace="dao")
-def get_notification_by_id(notification_id):
+def get_notification_by_id(notification_id, _raise=False):
-    return Notification.query.filter_by(id=notification_id).first()
+    if _raise:
+        return Notification.query.filter_by(id=notification_id).one()
+    else:
+        return Notification.query.filter_by(id=notification_id).first()
 
 
 def get_notifications(filter_dict=None):
@@ -547,24 +550,19 @@ def dao_set_created_live_letter_api_notifications_to_pending():
     Note - do not process services that have letters_as_pdf permission as they
            will get processed when the letters PDF zip task is created
     """
-
-    # Ignore services that have letters_as_pdf permission
-    services_without_letters_as_pdf = [
-        s.id for s in Service.query.filter(
-            ~Service.permissions.any(
-                ServicePermission.permission == 'letters_as_pdf'
-            )
-        ).all()
-    ]
-
     notifications = db.session.query(
         Notification
+    ).join(
+        Service
     ).filter(
         Notification.notification_type == LETTER_TYPE,
         Notification.status == NOTIFICATION_CREATED,
         Notification.key_type == KEY_TYPE_NORMAL,
         Notification.api_key != None,  # noqa
-        Notification.service_id.in_(services_without_letters_as_pdf)
+        # Ignore services that have letters_as_pdf permission
+        ~Service.permissions.any(
+            ServicePermission.permission == 'letters_as_pdf'
+        )
     ).with_for_update(
     ).all()
 

tests/app/dao/notification_dao/test_letter_api_notification_dao.py

@@ -25,9 +25,7 @@ def test_should_only_get_letter_notifications(
     assert ret == [sample_letter_notification]
 
 
-def test_should_ignore_letters_as_pdf(
+def test_should_ignore_letters_as_pdf(sample_letter_notification):
-    sample_letter_notification,
-):
     service = create_service(service_permissions=[LETTER_TYPE, 'letters_as_pdf'])
     template = create_template(service, template_type=LETTER_TYPE)
     create_notification(template)