Merge pull request #767 from alphagov/fix-client-reference

Fix: Retrieve notifications by reference

app/dao/notifications_dao.py

@@ -248,15 +248,6 @@ def get_notification_by_id(notification_id):
     return Notification.query.filter_by(id=notification_id).first()
 
 
-@statsd(namespace="dao")
-def get_notification_by_reference(service_id, reference, key_type):
-    filter_dict = {'service_id': service_id, 'client_reference': reference}
-    if key_type:
-        filter_dict['key_type'] = key_type
-
-    return Notification.query.filter_by(**filter_dict).options(joinedload('template_history')).one()
-
-
 def get_notifications(filter_dict=None):
     return _filter_query(Notification.query, filter_dict=filter_dict)
 
@@ -272,7 +263,8 @@ def get_notifications_for_service(
     personalisation=False,
     include_jobs=False,
     include_from_test_key=False,
-    older_than=None
+    older_than=None,
+    client_reference=None
 ):
     if page_size is None:
         page_size = current_app.config['PAGE_SIZE']
@@ -296,6 +288,9 @@ def get_notifications_for_service(
     elif not include_from_test_key:
         filters.append(Notification.key_type != KEY_TYPE_TEST)
 
+    if client_reference is not None:
+        filters.append(Notification.client_reference == client_reference)
+
     query = Notification.query.filter(*filters)
     query = _filter_query(query, filter_dict)
     if personalisation:

app/v2/notifications/get_notifications.py

@@ -24,23 +24,18 @@ def get_notifications():
         _data['older_than'] = _data['older_than'][0]
 
     # and client reference
-    if 'client_reference' in _data:
+    if 'reference' in _data:
-        _data['client_reference'] = _data['client_reference'][0]
+        _data['reference'] = _data['reference'][0]
 
     data = validate(_data, get_notifications_request)
 
-    if data.get('client_reference'):
-        notification = notifications_dao.get_notification_by_reference(
-            str(api_user.service_id), data.get('client_reference'), key_type=None
-        )
-        return jsonify(notification.serialize()), 200
-
     paginated_notifications = notifications_dao.get_notifications_for_service(
         str(api_user.service_id),
         filter_dict=data,
         key_type=api_user.key_type,
         personalisation=True,
-        older_than=data.get('older_than')
+        older_than=data.get('older_than'),
+        client_reference=data.get('reference')
     )
 
     def _build_links(notifications):

app/v2/notifications/notification_schemas.py

@@ -78,7 +78,7 @@ get_notifications_request = {
     "description": "schema for query parameters allowed when getting list of notifications",
     "type": "object",
     "properties": {
-        "client_reference": {"type": "string"},
+        "reference": {"type": "string"},
         "status": {
             "type": "array",
             "items": {

tests/app/dao/test_notification_dao.py

@@ -30,7 +30,6 @@ from app.dao.notifications_dao import (
     dao_update_notification,
     delete_notifications_created_more_than_a_week_ago,
     get_notification_by_id,
-    get_notification_by_reference,
     get_notification_for_job,
     get_notification_billable_unit_count_per_month,
     get_notification_with_personalisation,
@@ -626,14 +625,14 @@ def test_get_notification_by_id(sample_notification):
     assert sample_notification == notification_from_db
 
 
-def test_get_notification_by_reference(notify_db, notify_db_session):
+def test_get_notifications_by_reference(notify_db, notify_db_session, sample_service):
-    notification = sample_notification(notify_db, notify_db_session, client_reference="some-client-ref")
+    client_reference = 'some-client-ref'
-    notification_from_db = get_notification_by_reference(
+    assert len(Notification.query.all()) == 0
-        notification.service.id,
+    sample_notification(notify_db, notify_db_session, client_reference=client_reference)
-        notification.client_reference,
+    sample_notification(notify_db, notify_db_session, client_reference=client_reference)
-        key_type=None
+    sample_notification(notify_db, notify_db_session, client_reference='other-ref')
-    )
+    all_notifications = get_notifications_for_service(sample_service.id, client_reference=client_reference).items
-    assert notification == notification_from_db
+    assert len(all_notifications) == 2
 
 
 def test_save_notification_no_job_id(sample_template, mmg_provider):

tests/app/v2/notifications/test_get_notifications.py

@@ -66,32 +66,30 @@ def test_get_notification_by_reference_returns_200(client, notify_db, notify_db_
 
     auth_header = create_authorization_header(service_id=sample_notification_with_reference.service_id)
     response = client.get(
-        path='/v2/notifications?client_reference={}'.format(sample_notification_with_reference.client_reference),
+        path='/v2/notifications?reference={}'.format(sample_notification_with_reference.client_reference),
         headers=[('Content-Type', 'application/json'), auth_header])
 
     assert response.status_code == 200
     assert response.headers['Content-type'] == 'application/json'
 
+    json_response = json.loads(response.get_data(as_text=True))
+    assert len(json_response['notifications']) == 1
 
-def test_get_notification_by_reference_nonexistent_reference_returns_400(client, sample_notification):
+    assert json_response['notifications'][0]['id'] == str(sample_notification_with_reference.id)
-    auth_header = create_authorization_header(service_id=sample_notification.service_id)
+    assert json_response['notifications'][0]['reference'] == "some-client-reference"
+
+
+def test_get_notification_by_reference_nonexistent_reference_returns_no_notifications(client, sample_service):
+    auth_header = create_authorization_header(service_id=sample_service.id)
     response = client.get(
-        path='/v2/notifications?client_reference={}'.format(sample_notification.client_reference),
+        path='/v2/notifications?reference={}'.format('nonexistent-reference'),
         headers=[('Content-Type', 'application/json'), auth_header])
 
-    assert response.status_code == 404
-    assert response.headers['Content-type'] == 'application/json'
-
     json_response = json.loads(response.get_data(as_text=True))
-    assert json_response == {
+
-        "errors": [
+    assert response.status_code == 200
-            {
+    assert response.headers['Content-type'] == "application/json"
-                "error": "NoResultFound",
+    assert len(json_response['notifications']) == 0
-                "message": "No result found"
-            }
-        ],
-        "status_code": 404
-    }
 
 
 def test_get_notification_by_id_nonexistent_id(client, sample_notification):