Add one_off filter when getting all notifications for a service

Added the option to filter by one_off messages to the DAO function
`get_notifications_for_service`. Previously, one-off notifications
were not returned - this has changed so that the default is for
one-off notifications to be returned. Also simplified the `include_jobs`
filter for this function.

The DAO function gets used in 3 places - for the V1 and V2 API endpoints,
which will now start to return one-off messages. It also gets used by
the admin app which needs to pass in `include_one_off=False` to the
`get_all_notifications_for_service` where we don't want one-off
notifications to show, such as the API message log page.

app/dao/notifications_dao.py

@@ -31,7 +31,6 @@ from app.models import (
     Service,
     Template,
     TemplateHistory,
-    KEY_TYPE_NORMAL,
     KEY_TYPE_TEST,
     LETTER_TYPE,
     NOTIFICATION_CREATED,
@@ -245,7 +244,8 @@ def get_notifications_for_service(
     include_jobs=False,
     include_from_test_key=False,
     older_than=None,
-    client_reference=None
+    client_reference=None,
+    include_one_off=True
 ):
     if page_size is None:
         page_size = current_app.config['PAGE_SIZE']
@@ -260,9 +260,11 @@ def get_notifications_for_service(
             Notification.created_at).filter(Notification.id == older_than).as_scalar()
         filters.append(Notification.created_at < older_than_created_at)
 
-    if not include_jobs or (key_type and key_type != KEY_TYPE_NORMAL):
+    if not include_jobs:
-        # we can't say "job_id == None" here, because letters sent via the API still have a job_id :(
+        filters.append(Notification.job_id == None)  # noqa
-        filters.append(Notification.api_key_id != None)  # noqa
+
+    if not include_one_off:
+        filters.append(Notification.created_by_id == None)  # noqa
 
     if key_type is not None:
         filters.append(Notification.key_type == key_type)

app/schemas.py

@@ -593,6 +593,7 @@ class NotificationsFilterSchema(ma.Schema):
     older_than = fields.UUID(required=False)
     format_for_csv = fields.String()
     to = fields.String()
+    include_one_off = fields.Boolean(required=False)
 
     @pre_load
     def handle_multidict(self, in_data):

app/service/rest.py

@@ -335,6 +335,7 @@ def get_all_notifications_for_service(service_id):
     limit_days = data.get('limit_days')
     include_jobs = data.get('include_jobs', True)
     include_from_test_key = data.get('include_from_test_key', False)
+    include_one_off = data.get('include_one_off', True)
 
     pagination = notifications_dao.get_notifications_for_service(
         service_id,
@@ -343,7 +344,8 @@ def get_all_notifications_for_service(service_id):
         page_size=page_size,
         limit_days=limit_days,
         include_jobs=include_jobs,
-        include_from_test_key=include_from_test_key
+        include_from_test_key=include_from_test_key,
+        include_one_off=include_one_off
     )
     kwargs = request.args.to_dict()
     kwargs['service_id'] = service_id

tests/app/dao/notification_dao/test_notification_dao.py

@@ -957,6 +957,21 @@ def test_should_return_notifications_excluding_jobs_by_default(sample_template,
     assert exclude_jobs_manually[0].id == without_job.id
 
 
+def test_should_return_notifications_including_one_offs_by_default(sample_user, sample_template):
+    create_notification(sample_template, one_off=True, created_by_id=sample_user.id)
+    not_one_off = create_notification(sample_template)
+
+    exclude_one_offs = get_notifications_for_service(sample_template.service_id, include_one_off=False).items
+    assert len(exclude_one_offs) == 1
+    assert exclude_one_offs[0].id == not_one_off.id
+
+    include_one_offs_manually = get_notifications_for_service(sample_template.service_id, include_one_off=True).items
+    assert len(include_one_offs_manually) == 2
+
+    include_one_offs_by_default = get_notifications_for_service(sample_template.service_id).items
+    assert len(include_one_offs_by_default) == 2
+
+
 def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excluding_test_key_notifications(
         notify_db,
         notify_db_session,

tests/app/db.py

@@ -176,7 +176,8 @@ def create_notification(
         normalised_to=None,
         one_off=False,
         sms_sender_id=None,
-        reply_to_text=None
+        reply_to_text=None,
+        created_by_id=None
 ):
     if created_at is None:
         created_at = datetime.utcnow()
@@ -221,7 +222,8 @@ def create_notification(
         'international': international,
         'phone_prefix': phone_prefix,
         'normalised_to': normalised_to,
-        'reply_to_text': reply_to_text
+        'reply_to_text': reply_to_text,
+        'created_by_id': created_by_id
     }
     notification = Notification(**data)
     dao_create_notification(notification)

tests/app/service/test_rest.py

@@ -1384,15 +1384,21 @@ def test_get_all_notifications_for_service_including_ones_made_by_jobs(
 def test_get_only_api_created_notifications_for_service(
     admin_request,
     sample_job,
-    sample_template
+    sample_template,
+    sample_user,
 ):
+    # notification sent as a job
     create_notification(sample_template, job=sample_job)
+    # notification sent as a one-off
+    create_notification(sample_template, one_off=True, created_by_id=sample_user.id)
+    # notification sent via API
     without_job = create_notification(sample_template)
 
     resp = admin_request.get(
         'service.get_all_notifications_for_service',
         service_id=sample_template.service_id,
-        include_jobs=False
+        include_jobs=False,
+        include_one_off=False
     )
     assert len(resp['notifications']) == 1
     assert resp['notifications'][0]['id'] == str(without_job.id)