Merge pull request #627 from alphagov/update-job-page-query-2

Returns the outcome statistics for the job on the API call.
2026-05-07 09:38:23 -04:00 · 2016-08-25 12:07:20 +01:00
parent 020d998baa 09cb94081b
commit 415f2416cf
5 changed files with 197 additions and 22 deletions
--- a/app/dao/jobs_dao.py
+++ b/app/dao/jobs_dao.py
@@ -1,9 +1,24 @@
-from datetime import date, timedelta
-
 from sqlalchemy import desc, cast, Date as sql_date
 from app import db
 from app.dao import days_ago
-from app.models import Job
+from app.models import Job, NotificationHistory
+from app.statsd_decorators import statsd
+from sqlalchemy import func, asc
+
+
+@statsd(namespace="dao")
+def dao_get_notification_outcomes_for_job(service_id, job_id):
+    query = db.session.query(
+        func.count(NotificationHistory.status).label('count'),
+        NotificationHistory.status.label('status')
+    )
+
+    return query \
+        .filter(NotificationHistory.service_id == service_id) \
+        .filter(NotificationHistory.job_id == job_id)\
+        .group_by(NotificationHistory.status) \
+        .order_by(asc(NotificationHistory.status)) \
+        .all()


 def dao_get_job_by_service_id_and_job_id(service_id, job_id):
--- a/app/job/rest.py
+++ b/app/job/rest.py
@@ -8,7 +8,8 @@ from flask import (
 from app.dao.jobs_dao import (
    dao_create_job,
    dao_get_job_by_service_id_and_job_id,
-    dao_get_jobs_by_service_id
+    dao_get_jobs_by_service_id,
+    dao_get_notification_outcomes_for_job
 )

 from app.dao.services_dao import (
@@ -42,7 +43,11 @@ register_errors(job)
@job.route('/<job_id>', methods=['GET'])
 def get_job_by_service_and_job_id(service_id, job_id):
    job = dao_get_job_by_service_id_and_job_id(service_id, job_id)
+    statistics = dao_get_notification_outcomes_for_job(service_id, job_id)
    data = job_schema.dump(job).data
+
+    data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics]
+
    return jsonify(data=data)


--- a/app/schemas.py
+++ b/app/schemas.py
@@ -210,7 +210,11 @@ class JobSchema(BaseSchema):

    class Meta:
        model = models.Job
-        exclude = ('notifications',)
+        exclude = (
+            'notifications',
+            'notifications_sent',
+            'notifications_delivered',
+            'notifications_failed')
        strict = True


--- a/tests/app/dao/test_jobs_dao.py
+++ b/tests/app/dao/test_jobs_dao.py
@@ -5,10 +5,95 @@ from app.dao.jobs_dao import (
    dao_get_job_by_service_id_and_job_id,
    dao_create_job,
    dao_update_job,
-    dao_get_jobs_by_service_id
-)
+    dao_get_jobs_by_service_id,
+    dao_get_notification_outcomes_for_job)

 from app.models import Job
+from tests.app.conftest import sample_notification, sample_job, sample_service
+
+
+def test_should_have_decorated_notifications_dao_functions():
+    assert dao_get_notification_outcomes_for_job.__wrapped__.__name__ == 'dao_get_notification_outcomes_for_job'  # noqa
+
+
+def test_should_get_all_statuses_for_notifications_associated_with_job(
+        notify_db,
+        notify_db_session,
+        sample_service,
+        sample_job):
+
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='pending')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='failed')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='technical-failure')  # noqa
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='temporary-failure')  # noqa
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='permanent-failure')  # noqa
+
+    results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id)
+    assert [(row.count, row.status) for row in results] == [
+        (1, 'created'),
+        (1, 'sending'),
+        (1, 'delivered'),
+        (1, 'pending'),
+        (1, 'failed'),
+        (1, 'technical-failure'),
+        (1, 'temporary-failure'),
+        (1, 'permanent-failure')
+    ]
+
+
+def test_should_count_of_statuses_for_notifications_associated_with_job(
+        notify_db,
+        notify_db_session,
+        sample_service,
+        sample_job):
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=sample_job, status='delivered')
+
+    results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id)
+    assert [(row.count, row.status) for row in results] == [
+        (2, 'created'),
+        (4, 'sending'),
+        (2, 'delivered')
+    ]
+
+
+def test_should_return_zero_length_array_if_no_notifications_for_job(sample_service, sample_job):
+    assert len(dao_get_notification_outcomes_for_job(sample_job.id, sample_service.id)) == 0
+
+
+def test_should_return_notifications_only_for_this_job(notify_db, notify_db_session, sample_service):
+    job_1 = sample_job(notify_db, notify_db_session, service=sample_service)
+    job_2 = sample_job(notify_db, notify_db_session, service=sample_service)
+
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='created')
+
+    results = dao_get_notification_outcomes_for_job(sample_service.id, job_1.id)
+    assert [(row.count, row.status) for row in results] == [
+        (1, 'created')
+    ]
+
+
+def test_should_return_notifications_only_for_this_service(notify_db, notify_db_session):
+    service_1 = sample_service(notify_db, notify_db_session, service_name="one", email_from="one")
+    service_2 = sample_service(notify_db, notify_db_session, service_name="two", email_from="two")
+
+    job_1 = sample_job(notify_db, notify_db_session, service=service_1)
+    job_2 = sample_job(notify_db, notify_db_session, service=service_2)
+
+    sample_notification(notify_db, notify_db_session, service=service_1, job=job_1, status='created')
+    sample_notification(notify_db, notify_db_session, service=service_2, job=job_2, status='created')
+
+    assert len(dao_get_notification_outcomes_for_job(service_1.id, job_2.id)) == 0


 def test_create_job(sample_template):
--- a/tests/app/job/test_rest.py
+++ b/tests/app/job/test_rest.py
@@ -9,7 +9,7 @@ import app.celery.tasks
 from tests import create_authorization_header
 from tests.app.conftest import (
    sample_job as create_job,
-    sample_notification as create_sample_notification)
+    sample_notification as create_sample_notification, sample_notification)
 from app.dao.templates_dao import dao_update_template
 from app.models import NOTIFICATION_STATUS_TYPES

@@ -94,20 +94,6 @@ def test_get_job_with_unknown_id_returns404(notify_api, sample_template, fake_uu
            }


-def test_get_job_by_id(notify_api, sample_job):
-    job_id = str(sample_job.id)
-    service_id = sample_job.service.id
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            path = '/service/{}/job/{}'.format(service_id, job_id)
-            auth_header = create_authorization_header(service_id=sample_job.service.id)
-            response = client.get(path, headers=[auth_header])
-            assert response.status_code == 200
-            resp_json = json.loads(response.get_data(as_text=True))
-            assert resp_json['data']['id'] == job_id
-            assert resp_json['data']['created_by']['name'] == 'Test User'
-
-
 def test_create_job(notify_api, sample_template, mocker, fake_uuid):
    with notify_api.test_request_context():
        with notify_api.test_client() as client:
@@ -350,3 +336,83 @@ def test_get_all_notifications_for_job_filtered_by_status(
        resp = json.loads(response.get_data(as_text=True))
        assert len(resp['notifications']) == expected_notification_count
        assert response.status_code == 200
+
+
+def test_get_job_by_id(notify_api, sample_job):
+    job_id = str(sample_job.id)
+    service_id = sample_job.service.id
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            path = '/service/{}/job/{}'.format(service_id, job_id)
+            auth_header = create_authorization_header(service_id=sample_job.service.id)
+            response = client.get(path, headers=[auth_header])
+            assert response.status_code == 200
+            resp_json = json.loads(response.get_data(as_text=True))
+            assert resp_json['data']['id'] == job_id
+            assert resp_json['data']['statistics'] == []
+            assert resp_json['data']['created_by']['name'] == 'Test User'
+
+
+def test_get_job_by_id_should_return_statistics(notify_db, notify_db_session, notify_api, sample_job):
+    job_id = str(sample_job.id)
+    service_id = sample_job.service.id
+
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='delivered')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='pending')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure')  # noqa
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure')  # noqa
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='permanent-failure')  # noqa
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            path = '/service/{}/job/{}'.format(service_id, job_id)
+            auth_header = create_authorization_header(service_id=sample_job.service.id)
+            response = client.get(path, headers=[auth_header])
+            assert response.status_code == 200
+            resp_json = json.loads(response.get_data(as_text=True))
+            print(resp_json)
+            assert resp_json['data']['id'] == job_id
+            assert {'status': 'created', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'sending', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'delivered', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'pending', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'failed', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'technical-failure', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'temporary-failure', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'permanent-failure', 'count': 1} in resp_json['data']['statistics']
+            assert resp_json['data']['created_by']['name'] == 'Test User'
+
+
+def test_get_job_by_id_should_return_summed_statistics(notify_db, notify_db_session, notify_api, sample_job):
+    job_id = str(sample_job.id)
+    service_id = sample_job.service.id
+
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='created')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='sending')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='failed')
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='technical-failure')  # noqa
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure')  # noqa
+    sample_notification(notify_db, notify_db_session, service=sample_job.service, job=sample_job, status='temporary-failure')  # noqa
+
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            path = '/service/{}/job/{}'.format(service_id, job_id)
+            auth_header = create_authorization_header(service_id=sample_job.service.id)
+            response = client.get(path, headers=[auth_header])
+            assert response.status_code == 200
+            resp_json = json.loads(response.get_data(as_text=True))
+            print(resp_json)
+            assert resp_json['data']['id'] == job_id
+            assert {'status': 'created', 'count': 3} in resp_json['data']['statistics']
+            assert {'status': 'sending', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'failed', 'count': 3} in resp_json['data']['statistics']
+            assert {'status': 'technical-failure', 'count': 1} in resp_json['data']['statistics']
+            assert {'status': 'temporary-failure', 'count': 2} in resp_json['data']['statistics']
+            assert resp_json['data']['created_by']['name'] == 'Test User'