Merge pull request #2258 from alphagov/dashboard-jobs-speedup

stop dashboard reading notification_history

app/dao/fact_notification_status_dao.py

@@ -186,3 +186,14 @@ def fetch_notification_status_totals_for_all_services(start_date, end_date):
     else:
         query = stats
     return query.all()
+
+
+def fetch_notification_statuses_for_job(job_id):
+    return db.session.query(
+        FactNotificationStatus.notification_status.label('status'),
+        func.sum(FactNotificationStatus.notification_count).label('count'),
+    ).filter(
+        FactNotificationStatus.job_id == job_id,
+    ).group_by(
+        FactNotificationStatus.notification_status
+    ).all()

app/dao/jobs_dao.py

@@ -16,7 +16,7 @@ from app.models import (
     JOB_STATUS_PENDING,
     JOB_STATUS_SCHEDULED,
     LETTER_TYPE,
-    NotificationHistory,
+    Notification,
     Template,
     ServiceDataRetention
 )
@@ -25,19 +25,14 @@ from app.variables import LETTER_TEST_API_FILENAME
 
 @statsd(namespace="dao")
 def dao_get_notification_outcomes_for_job(service_id, job_id):
-    query = db.session.query(
+    return db.session.query(
-        func.count(NotificationHistory.status).label('count'),
+        func.count(Notification.status).label('count'),
-        NotificationHistory.status
+        Notification.status
-    )
-
-    return query.filter(
-        NotificationHistory.service_id == service_id
     ).filter(
-        NotificationHistory.job_id == job_id
+        Notification.service_id == service_id,
+        Notification.job_id == job_id
     ).group_by(
-        NotificationHistory.status
+        Notification.status
-    ).order_by(
-        asc(NotificationHistory.status)
     ).all()
 
 

app/job/rest.py

@@ -1,3 +1,4 @@
+import dateutil
 from flask import (
     Blueprint,
     jsonify,
@@ -13,6 +14,7 @@ from app.dao.jobs_dao import (
     dao_get_jobs_by_service_id,
     dao_get_future_scheduled_job_by_id_and_service_id,
     dao_get_notification_outcomes_for_job)
+from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job
 from app.dao.services_dao import dao_fetch_service_by_id
 from app.dao.templates_dao import dao_get_template_by_id
 from app.dao.notifications_dao import get_notifications_for_job
@@ -24,7 +26,7 @@ from app.schemas import (
 )
 from app.celery.tasks import process_job
 from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANCELLED, LETTER_TYPE
-from app.utils import pagination_links
+from app.utils import pagination_links, midnight_n_days_ago
 from app.config import QueueNames
 from app.errors import (
     register_errors,
@@ -171,8 +173,14 @@ def get_paginated_jobs(service_id, limit_days, statuses, page):
     )
     data = job_schema.dump(pagination.items, many=True).data
     for job_data in data:
-        statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id'])
+        created_at = dateutil.parser.parse(job_data['created_at']).replace(tzinfo=None)
-        job_data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics]
+        if created_at < midnight_n_days_ago(3):
+            # ft_notification_status table
+            statistics = fetch_notification_statuses_for_job(job_data['id'])
+        else:
+            # notifications table
+            statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id'])
+        job_data['statistics'] = [{'status': statistic.status, 'count': statistic.count} for statistic in statistics]
 
     return {
         'data': data,

tests/app/conftest.py

@@ -1101,7 +1101,9 @@ def restore_provider_details(notify_db, notify_db_session):
 
 @pytest.fixture
 def admin_request(client):
+
     class AdminRequest:
+        app = client.application
 
         @staticmethod
         def get(endpoint, _expected_status=200, **endpoint_kwargs):

tests/app/dao/test_fact_notification_status_dao.py

@@ -9,11 +9,12 @@ from app.dao.fact_notification_status_dao import (
     fetch_notification_status_for_service_by_month,
     fetch_notification_status_for_service_for_day,
     fetch_notification_status_for_service_for_today_and_7_previous_days,
-    fetch_notification_status_totals_for_all_services
+    fetch_notification_status_totals_for_all_services,
+    fetch_notification_statuses_for_job,
 )
 from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
 from freezegun import freeze_time
-from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status
+from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job
 
 
 def test_update_fact_notification_status(notify_db_session):
@@ -288,3 +289,18 @@ def set_up_data():
     create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))
     create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered')
     create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered')
+
+
+def test_fetch_notification_statuses_for_job(sample_template):
+    j1 = create_job(sample_template)
+    j2 = create_job(sample_template)
+
+    create_ft_notification_status(date(2018, 10, 1), job=j1, notification_status='created', count=1)
+    create_ft_notification_status(date(2018, 10, 1), job=j1, notification_status='delivered', count=2)
+    create_ft_notification_status(date(2018, 10, 2), job=j1, notification_status='created', count=4)
+    create_ft_notification_status(date(2018, 10, 1), job=j2, notification_status='created', count=8)
+
+    assert {x.status: x.count for x in fetch_notification_statuses_for_job(j1.id)} == {
+        'created': 5,
+        'delivered': 2
+    }

tests/app/dao/test_jobs_dao.py

@@ -19,106 +19,55 @@ from app.models import (
     Job,
     EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
 )
-from tests.app.conftest import sample_job as create_job
+from tests.app.db import create_job, create_service, create_template, create_notification
-from tests.app.conftest import sample_notification as create_notification
-from tests.app.conftest import sample_service as create_service
-from tests.app.conftest import sample_template as create_template
-from tests.app.db import (
-    create_user
-)
 
 
 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(
+def test_should_count_of_statuses_for_notifications_associated_with_job(sample_template, sample_job):
-        notify_db,
+    create_notification(sample_template, job=sample_job, status='created')
-        notify_db_session,
+    create_notification(sample_template, job=sample_job, status='created')
-        sample_service,
+    create_notification(sample_template, job=sample_job, status='created')
-        sample_job
+    create_notification(sample_template, job=sample_job, status='sending')
-):
+    create_notification(sample_template, job=sample_job, status='delivered')
-    notification = partial(create_notification, notify_db, notify_db_session, service=sample_service, job=sample_job)
-    notification(status='created')
-    notification(status='sending')
-    notification(status='delivered')
-    notification(status='pending')
-    notification(status='failed')
-    notification(status='technical-failure')
-    notification(status='temporary-failure')
-    notification(status='permanent-failure')
-    notification(status='sent')
 
-    results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id)
+    results = dao_get_notification_outcomes_for_job(sample_template.service_id, sample_job.id)
-    assert set([(row.count, row.status) for row in results]) == set([
+    assert {row.status: row.count for row in results} == {
-        (1, 'created'),
+        'created': 3,
-        (1, 'sending'),
+        'sending': 1,
-        (1, 'delivered'),
+        'delivered': 1,
-        (1, 'pending'),
+    }
-        (1, 'failed'),
-        (1, 'technical-failure'),
-        (1, 'temporary-failure'),
-        (1, 'permanent-failure'),
-        (1, 'sent')
-    ])
-
-
-def test_should_count_of_statuses_for_notifications_associated_with_job(
-        notify_db,
-        notify_db_session,
-        sample_service,
-        sample_job
-):
-    notification = partial(create_notification, notify_db, notify_db_session, service=sample_service, job=sample_job)
-    notification(status='created')
-    notification(status='created')
-
-    notification(status='sending')
-    notification(status='sending')
-    notification(status='sending')
-    notification(status='sending')
-    notification(status='delivered')
-    notification(status='delivered')
-
-    results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id)
-    assert set([(row.count, row.status) for row in results]) == set([
-        (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):
+def test_should_return_notifications_only_for_this_job(sample_template):
-    job_1 = create_job(notify_db, notify_db_session, service=sample_service)
+    job_1 = create_job(sample_template)
-    job_2 = create_job(notify_db, notify_db_session, service=sample_service)
+    job_2 = create_job(sample_template)
 
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created')
+    create_notification(sample_template, job=job_1, status='created')
-    create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='created')
+    create_notification(sample_template, job=job_2, status='sent')
 
-    results = dao_get_notification_outcomes_for_job(sample_service.id, job_1.id)
+    results = dao_get_notification_outcomes_for_job(sample_template.service_id, job_1.id)
-    assert [(row.count, row.status) for row in results] == [
+    assert {row.status: row.count for row in results} == {'created': 1}
-        (1, 'created')
-    ]
 
 
-def test_should_return_notifications_only_for_this_service(notify_db, notify_db_session):
+def test_should_return_notifications_only_for_this_service(sample_notification_with_job):
-    service_1 = create_service(notify_db, notify_db_session, service_name="one", email_from="one")
+    other_service = create_service(service_name='one')
-    service_2 = create_service(notify_db, notify_db_session, service_name="two", email_from="two")
+    other_template = create_template(service=other_service)
+    other_job = create_job(other_template)
 
-    job_1 = create_job(notify_db, notify_db_session, service=service_1)
+    create_notification(other_template, job=other_job)
-    job_2 = create_job(notify_db, notify_db_session, service=service_2)
 
-    create_notification(notify_db, notify_db_session, service=service_1, job=job_1, status='created')
+    assert len(dao_get_notification_outcomes_for_job(sample_notification_with_job.service_id, other_job.id)) == 0
-    create_notification(notify_db, notify_db_session, service=service_2, job=job_2, status='created')
+    assert len(dao_get_notification_outcomes_for_job(other_service.id, sample_notification_with_job.id)) == 0
-
-    assert len(dao_get_notification_outcomes_for_job(service_1.id, job_2.id)) == 0
 
 
-def test_create_job(sample_template):
+def test_create_sample_job(sample_template):
     assert Job.query.count() == 0
 
     job_id = uuid.uuid4()
@@ -147,14 +96,12 @@ def test_get_job_by_id(sample_job):
     assert sample_job == job_from_db
 
 
-def test_get_jobs_for_service(notify_db, notify_db_session, sample_template):
+def test_get_jobs_for_service(sample_template):
-    one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template)
+    one_job = create_job(sample_template)
 
-    other_user = create_user(email="test@digital.cabinet-office.gov.uk")
+    other_service = create_service(service_name="other service")
-    other_service = create_service(notify_db, notify_db_session, user=other_user, service_name="other service",
+    other_template = create_template(service=other_service)
-                                   email_from='other.service')
+    other_job = create_job(other_template)
-    other_template = create_template(notify_db, notify_db_session, service=other_service)
-    other_job = create_job(notify_db, notify_db_session, service=other_service, template=other_template)
 
     one_job_from_db = dao_get_jobs_by_service_id(one_job.service_id).items
     other_job_from_db = dao_get_jobs_by_service_id(other_job.service_id).items
@@ -168,10 +115,9 @@ def test_get_jobs_for_service(notify_db, notify_db_session, sample_template):
     assert one_job_from_db != other_job_from_db
 
 
-def test_get_jobs_for_service_with_limit_days_param(notify_db, notify_db_session, sample_template):
+def test_get_jobs_for_service_with_limit_days_param(sample_template):
-    one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template)
+    one_job = create_job(sample_template)
-    old_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template,
+    old_job = create_job(sample_template, created_at=datetime.now() - timedelta(days=8))
-                         created_at=datetime.now() - timedelta(days=8))
 
     jobs = dao_get_jobs_by_service_id(one_job.service_id).items
 
@@ -185,34 +131,27 @@ def test_get_jobs_for_service_with_limit_days_param(notify_db, notify_db_session
     assert old_job not in jobs_limit_days
 
 
-def test_get_jobs_for_service_with_limit_days_edge_case(notify_db, notify_db_session, sample_template):
+@freeze_time('2017-06-10')
-    one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template)
+def test_get_jobs_for_service_with_limit_days_edge_case(sample_template):
-    job_two = create_job(notify_db, notify_db_session, sample_template.service, sample_template,
+    one_job = create_job(sample_template)
-                         created_at=(datetime.now() - timedelta(days=7)).date())
+    just_after_midnight_job = create_job(sample_template, created_at=datetime(2017, 6, 2, 23, 0, 1))
-    one_second_after_midnight = datetime.combine((datetime.now() - timedelta(days=7)).date(),
+    just_before_midnight_job = create_job(sample_template, created_at=datetime(2017, 6, 2, 22, 59, 0))
-                                                 datetime.strptime("000001", "%H%M%S").time())
-    just_after_midnight_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template,
-                                         created_at=one_second_after_midnight)
-    job_eight_days_old = create_job(notify_db, notify_db_session, sample_template.service, sample_template,
-                                    created_at=datetime.now() - timedelta(days=8))
 
     jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7).items
-    assert len(jobs_limit_days) == 3
+    assert len(jobs_limit_days) == 2
     assert one_job in jobs_limit_days
-    assert job_two in jobs_limit_days
     assert just_after_midnight_job in jobs_limit_days
-    assert job_eight_days_old not in jobs_limit_days
+    assert just_before_midnight_job not in jobs_limit_days
 
 
 def test_get_jobs_for_service_in_processed_at_then_created_at_order(notify_db, notify_db_session, sample_template):
-    _create_job = partial(create_job, notify_db, notify_db_session, sample_template.service, sample_template)
     from_hour = partial(datetime, 2001, 1, 1)
 
     created_jobs = [
-        _create_job(created_at=from_hour(2), processing_started=None),
+        create_job(sample_template, created_at=from_hour(2), processing_started=None),
-        _create_job(created_at=from_hour(1), processing_started=None),
+        create_job(sample_template, created_at=from_hour(1), processing_started=None),
-        _create_job(created_at=from_hour(1), processing_started=from_hour(4)),
+        create_job(sample_template, created_at=from_hour(1), processing_started=from_hour(4)),
-        _create_job(created_at=from_hour(2), processing_started=from_hour(3)),
+        create_job(sample_template, created_at=from_hour(2), processing_started=from_hour(3)),
     ]
 
     jobs = dao_get_jobs_by_service_id(sample_template.service.id).items
@@ -235,21 +174,20 @@ def test_update_job(sample_job):
     assert job_from_db.job_status == 'in progress'
 
 
-def test_set_scheduled_jobs_to_pending_gets_all_jobs_in_scheduled_state_before_now(notify_db, notify_db_session):
+def test_set_scheduled_jobs_to_pending_gets_all_jobs_in_scheduled_state_before_now(sample_template):
     one_minute_ago = datetime.utcnow() - timedelta(minutes=1)
     one_hour_ago = datetime.utcnow() - timedelta(minutes=60)
-    job_new = create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled')
+    job_new = create_job(sample_template, scheduled_for=one_minute_ago, job_status='scheduled')
-    job_old = create_job(notify_db, notify_db_session, scheduled_for=one_hour_ago, job_status='scheduled')
+    job_old = create_job(sample_template, scheduled_for=one_hour_ago, job_status='scheduled')
     jobs = dao_set_scheduled_jobs_to_pending()
     assert len(jobs) == 2
     assert jobs[0].id == job_old.id
     assert jobs[1].id == job_new.id
 
 
-def test_set_scheduled_jobs_to_pending_gets_ignores_jobs_not_scheduled(notify_db, notify_db_session):
+def test_set_scheduled_jobs_to_pending_gets_ignores_jobs_not_scheduled(sample_template, sample_job):
     one_minute_ago = datetime.utcnow() - timedelta(minutes=1)
-    create_job(notify_db, notify_db_session)
+    job_scheduled = create_job(sample_template, scheduled_for=one_minute_ago, job_status='scheduled')
-    job_scheduled = create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled')
     jobs = dao_set_scheduled_jobs_to_pending()
     assert len(jobs) == 1
     assert jobs[0].id == job_scheduled.id
@@ -260,11 +198,11 @@ def test_set_scheduled_jobs_to_pending_gets_ignores_jobs_scheduled_in_the_future
     assert len(jobs) == 0
 
 
-def test_set_scheduled_jobs_to_pending_updates_rows(notify_db, notify_db_session):
+def test_set_scheduled_jobs_to_pending_updates_rows(sample_template):
     one_minute_ago = datetime.utcnow() - timedelta(minutes=1)
     one_hour_ago = datetime.utcnow() - timedelta(minutes=60)
-    create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled')
+    create_job(sample_template, scheduled_for=one_minute_ago, job_status='scheduled')
-    create_job(notify_db, notify_db_session, scheduled_for=one_hour_ago, job_status='scheduled')
+    create_job(sample_template, scheduled_for=one_hour_ago, job_status='scheduled')
     jobs = dao_set_scheduled_jobs_to_pending()
     assert len(jobs) == 2
     assert jobs[0].job_status == 'pending'
@@ -277,7 +215,7 @@ def test_get_future_scheduled_job_gets_a_job_yet_to_send(sample_scheduled_job):
 
 
 @freeze_time('2016-10-31 10:00:00')
-def test_should_get_jobs_seven_days_old(notify_db, notify_db_session, sample_template):
+def test_should_get_jobs_seven_days_old(sample_template):
     """
     Jobs older than seven days are deleted, but only two day's worth (two-day window)
     """
@@ -289,12 +227,11 @@ def test_should_get_jobs_seven_days_old(notify_db, notify_db_session, sample_tem
     nine_days_ago = eight_days_ago - timedelta(days=2)
     nine_days_one_second_ago = nine_days_ago - timedelta(seconds=1)
 
-    job = partial(create_job, notify_db, notify_db_session)
+    create_job(sample_template, created_at=seven_days_ago)
-    job(created_at=seven_days_ago)
+    create_job(sample_template, created_at=within_seven_days)
-    job(created_at=within_seven_days)
+    job_to_delete = create_job(sample_template, created_at=eight_days_ago)
-    job_to_delete = job(created_at=eight_days_ago)
+    create_job(sample_template, created_at=nine_days_ago, archived=True)
-    job(created_at=nine_days_ago, archived=True)
+    create_job(sample_template, created_at=nine_days_one_second_ago, archived=True)
-    job(created_at=nine_days_one_second_ago, archived=True)
 
     jobs = dao_get_jobs_older_than_data_retention(notification_types=[sample_template.template_type])
 
@@ -306,7 +243,7 @@ def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_
     with freeze_time('2015-01-01T00:00:00') as the_time:
         for _ in range(10):
             the_time.tick(timedelta(hours=1))
-            create_job(notify_db, notify_db_session, sample_service, sample_template)
+            create_job(sample_template)
 
     res = dao_get_jobs_by_service_id(sample_service.id, page=1, page_size=2)
 
@@ -328,19 +265,11 @@ def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_
     'Report',
 ])
 def test_get_jobs_for_service_doesnt_return_test_messages(
-        notify_db,
-        notify_db_session,
         sample_template,
         sample_job,
         file_name,
 ):
-    create_job(
+    create_job(sample_template, original_file_name=file_name,)
-        notify_db,
-        notify_db_session,
-        sample_template.service,
-        sample_template,
-        original_file_name=file_name,
-    )
 
     jobs = dao_get_jobs_by_service_id(sample_job.service_id).items
 
@@ -348,16 +277,15 @@ def test_get_jobs_for_service_doesnt_return_test_messages(
 
 
 @freeze_time('2016-10-31 10:00:00')
-def test_should_get_jobs_seven_days_old_filters_type(notify_db, notify_db_session):
+def test_should_get_jobs_seven_days_old_filters_type(sample_service):
     eight_days_ago = datetime.utcnow() - timedelta(days=8)
-    letter_template = create_template(notify_db, notify_db_session, template_type=LETTER_TYPE)
+    letter_template = create_template(sample_service, template_type=LETTER_TYPE)
-    sms_template = create_template(notify_db, notify_db_session, template_type=SMS_TYPE)
+    sms_template = create_template(sample_service, template_type=SMS_TYPE)
-    email_template = create_template(notify_db, notify_db_session, template_type=EMAIL_TYPE)
+    email_template = create_template(sample_service, template_type=EMAIL_TYPE)
 
-    job = partial(create_job, notify_db, notify_db_session, created_at=eight_days_ago)
+    job_to_remain = create_job(letter_template, created_at=eight_days_ago)
-    job_to_remain = job(template=letter_template)
+    create_job(sms_template, created_at=eight_days_ago)
-    job(template=sms_template)
+    create_job(email_template, created_at=eight_days_ago)
-    job(template=email_template)
 
     jobs = dao_get_jobs_older_than_data_retention(
         notification_types=[EMAIL_TYPE, SMS_TYPE]

tests/app/db.py

@@ -164,7 +164,7 @@ def create_template(
 
 
 def create_notification(
-        template,
+        template=None,
         job=None,
         job_row_number=None,
         to_field=None,
@@ -190,6 +190,10 @@ def create_notification(
         created_by_id=None,
         postage=None
 ):
+    assert job or template
+    if job:
+        template = job.template
+
     if created_at is None:
         created_at = datetime.utcnow()
 
@@ -557,6 +561,8 @@ def create_ft_notification_status(
     notification_status='delivered',
     count=1
 ):
+    if job:
+        template = job.template
     if template:
         service = template.service
         notification_type = template.template_type

tests/app/job/test_rest.py

@@ -1,21 +1,18 @@
 import json
 import uuid
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, date
-from functools import partial
 
 from freezegun import freeze_time
 import pytest
 import pytz
+
 import app.celery.tasks
+from app.dao.templates_dao import dao_update_template
+from app.models import JOB_STATUS_TYPES, JOB_STATUS_PENDING
 
 from tests import create_authorization_header
 from tests.conftest import set_config
-from tests.app.conftest import (
+from tests.app.db import create_ft_notification_status, create_job, create_notification
-    sample_job as create_job,
-    sample_notification as create_notification
-)
-from app.dao.templates_dao import dao_update_template
-from app.models import NOTIFICATION_STATUS_TYPES, JOB_STATUS_TYPES, JOB_STATUS_PENDING
 
 
 def test_get_job_with_invalid_service_id_returns404(client, sample_service):
@@ -444,54 +441,26 @@ def test_create_job_returns_400_if_archived_template(client, sample_template, mo
     assert 'Template has been deleted' in resp_json['message']['template']
 
 
-def _setup_jobs(notify_db, notify_db_session, template, number_of_jobs=5):
+def _setup_jobs(template, number_of_jobs=5):
     for i in range(number_of_jobs):
-        create_job(
+        create_job(template=template)
-            notify_db,
-            notify_db_session,
-            service=template.service,
-            template=template)
 
 
-def test_get_all_notifications_for_job_in_order_of_job_number(
+def test_get_all_notifications_for_job_in_order_of_job_number(admin_request, sample_template):
-    client, notify_db, notify_db_session, sample_service
+    main_job = create_job(sample_template)
-):
+    another_job = create_job(sample_template)
-    main_job = create_job(notify_db, notify_db_session, service=sample_service)
-    another_job = create_job(notify_db, notify_db_session, service=sample_service)
 
-    notification_1 = create_notification(
+    notification_1 = create_notification(job=main_job, to_field="1", job_row_number=1)
-        notify_db,
+    notification_2 = create_notification(job=main_job, to_field="2", job_row_number=2)
-        notify_db_session,
+    notification_3 = create_notification(job=main_job, to_field="3", job_row_number=3)
-        job=main_job,
+    create_notification(job=another_job)
-        to_field="1",
+
-        created_at=datetime.utcnow(),
+    resp = admin_request.get(
-        job_row_number=1
+        'job.get_all_notifications_for_service_job',
+        service_id=main_job.service_id,
+        job_id=main_job.id
     )
-    notification_2 = create_notification(
-        notify_db,
-        notify_db_session,
-        job=main_job,
-        to_field="2",
-        created_at=datetime.utcnow(),
-        job_row_number=2
-    )
-    notification_3 = create_notification(
-        notify_db,
-        notify_db_session,
-        job=main_job,
-        to_field="3",
-        created_at=datetime.utcnow(),
-        job_row_number=3
-    )
-    create_notification(notify_db, notify_db_session, job=another_job)
 
-    auth_header = create_authorization_header()
-
-    response = client.get(
-        path='/service/{}/job/{}/notifications'.format(sample_service.id, main_job.id),
-        headers=[auth_header])
-
-    resp = json.loads(response.get_data(as_text=True))
     assert len(resp['notifications']) == 3
     assert resp['notifications'][0]['to'] == notification_1.to
     assert resp['notifications'][0]['job_row_number'] == notification_1.job_row_number
@@ -499,133 +468,76 @@ def test_get_all_notifications_for_job_in_order_of_job_number(
     assert resp['notifications'][1]['job_row_number'] == notification_2.job_row_number
     assert resp['notifications'][2]['to'] == notification_3.to
     assert resp['notifications'][2]['job_row_number'] == notification_3.job_row_number
-    assert response.status_code == 200
 
 
 @pytest.mark.parametrize(
     "expected_notification_count, status_args",
     [
-        (1, '?status={}'.format(NOTIFICATION_STATUS_TYPES[0])),
+        (1, ['created']),
-        (0, '?status={}'.format(NOTIFICATION_STATUS_TYPES[1])),
+        (0, ['sending']),
-        (1, '?status={}&status={}&status={}'.format(*NOTIFICATION_STATUS_TYPES[0:3])),
+        (1, ['created', 'sending']),
-        (0, '?status={}&status={}&status={}'.format(*NOTIFICATION_STATUS_TYPES[3:6])),
+        (0, ['sending', 'delivered']),
     ]
 )
 def test_get_all_notifications_for_job_filtered_by_status(
-        client,
+        admin_request,
-        notify_db,
+        sample_job,
-        notify_db_session,
-        sample_service,
         expected_notification_count,
         status_args
 ):
-    job = create_job(notify_db, notify_db_session, service=sample_service)
+    create_notification(job=sample_job, to_field="1", status='created')
 
-    create_notification(
+    resp = admin_request.get(
-        notify_db,
+        'job.get_all_notifications_for_service_job',
-        notify_db_session,
+        service_id=sample_job.service_id,
-        job=job,
+        job_id=sample_job.id,
-        to_field="1",
+        status=status_args
-        created_at=datetime.utcnow(),
-        status=NOTIFICATION_STATUS_TYPES[0],
-        job_row_number=1
     )
-
-    response = client.get(
-        path='/service/{}/job/{}/notifications{}'.format(sample_service.id, job.id, status_args),
-        headers=[create_authorization_header()]
-    )
-    resp = json.loads(response.get_data(as_text=True))
     assert len(resp['notifications']) == expected_notification_count
-    assert response.status_code == 200
 
 
 def test_get_all_notifications_for_job_returns_correct_format(
-    client,
+    admin_request,
     sample_notification_with_job
 ):
     service_id = sample_notification_with_job.service_id
     job_id = sample_notification_with_job.job_id
-    response = client.get(
+
-        path='/service/{}/job/{}/notifications'.format(service_id, job_id),
+    resp = admin_request.get('job.get_all_notifications_for_service_job', service_id=service_id, job_id=job_id)
-        headers=[create_authorization_header()]
+
-    )
-    assert response.status_code == 200
-    resp = json.loads(response.get_data(as_text=True))
     assert len(resp['notifications']) == 1
     assert resp['notifications'][0]['id'] == str(sample_notification_with_job.id)
     assert resp['notifications'][0]['status'] == sample_notification_with_job.status
 
 
-def test_get_job_by_id(notify_api, sample_job):
+def test_get_job_by_id(admin_request, 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()
-            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'
 
+    resp_json = admin_request.get('job.get_job_by_service_and_job_id', service_id=service_id, job_id=job_id)
 
-def test_get_job_by_id_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_job):
-    job_id = str(sample_job.id)
-    service_id = sample_job.service.id
-    partial_notification = partial(
-        create_notification, notify_db, notify_db_session, service=sample_job.service, job=sample_job
-    )
-    partial_notification(status='created')
-    partial_notification(status='sending')
-    partial_notification(status='delivered')
-    partial_notification(status='pending')
-    partial_notification(status='failed')
-    partial_notification(status='technical-failure')  # noqa
-    partial_notification(status='temporary-failure')  # noqa
-    partial_notification(status='permanent-failure')  # noqa
-
-    path = '/service/{}/job/{}'.format(service_id, job_id)
-    auth_header = create_authorization_header()
-    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 {'status': 'created', 'count': 1} in resp_json['data']['statistics']
+    assert 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(client, notify_db, notify_db_session, notify_api, sample_job):
+def test_get_job_by_id_should_return_summed_statistics(admin_request, sample_job):
     job_id = str(sample_job.id)
     service_id = sample_job.service.id
-    partial_notification = partial(
-        create_notification, notify_db, notify_db_session, service=sample_job.service, job=sample_job
-    )
-    partial_notification(status='created')
-    partial_notification(status='created')
-    partial_notification(status='created')
-    partial_notification(status='sending')
-    partial_notification(status='failed')
-    partial_notification(status='failed')
-    partial_notification(status='failed')
-    partial_notification(status='technical-failure')
-    partial_notification(status='temporary-failure')
-    partial_notification(status='temporary-failure')
 
-    path = '/service/{}/job/{}'.format(service_id, job_id)
+    create_notification(job=sample_job, status='created')
-    auth_header = create_authorization_header()
+    create_notification(job=sample_job, status='created')
-    response = client.get(path, headers=[auth_header])
+    create_notification(job=sample_job, status='created')
-    assert response.status_code == 200
+    create_notification(job=sample_job, status='sending')
-    resp_json = json.loads(response.get_data(as_text=True))
+    create_notification(job=sample_job, status='failed')
+    create_notification(job=sample_job, status='failed')
+    create_notification(job=sample_job, status='failed')
+    create_notification(job=sample_job, status='technical-failure')
+    create_notification(job=sample_job, status='temporary-failure')
+    create_notification(job=sample_job, status='temporary-failure')
+
+    resp_json = admin_request.get('job.get_job_by_service_and_job_id', service_id=service_id, job_id=job_id)
+
     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']
@@ -635,32 +547,23 @@ def test_get_job_by_id_should_return_summed_statistics(client, notify_db, notify
     assert resp_json['data']['created_by']['name'] == 'Test User'
 
 
-def test_get_jobs(client, notify_db, notify_db_session, sample_template):
+def test_get_jobs(admin_request, sample_template):
-    _setup_jobs(notify_db, notify_db_session, sample_template)
+    _setup_jobs(sample_template)
 
     service_id = sample_template.service.id
 
-    path = '/service/{}/job'.format(service_id)
+    resp_json = admin_request.get('job.get_jobs_by_service', service_id=service_id)
-    auth_header = create_authorization_header()
-    response = client.get(path, headers=[auth_header])
-    assert response.status_code == 200
-    resp_json = json.loads(response.get_data(as_text=True))
     assert len(resp_json['data']) == 5
 
 
-def test_get_jobs_with_limit_days(admin_request, notify_db, notify_db_session, sample_template):
+def test_get_jobs_with_limit_days(admin_request, sample_template):
     for time in [
         'Sunday 1st July 2018 22:59',
         'Sunday 2nd July 2018 23:00',  # beginning of monday morning
         'Monday 3rd July 2018 12:00'
     ]:
         with freeze_time(time):
-            create_job(
+            create_job(template=sample_template)
-                notify_db,
-                notify_db_session,
-                service=sample_template.service,
-                template=sample_template,
-            )
 
     with freeze_time('Monday 9th July 2018 12:00'):
         resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id, limit_days=7)
@@ -668,51 +571,35 @@ def test_get_jobs_with_limit_days(admin_request, notify_db, notify_db_session, s
     assert len(resp_json['data']) == 2
 
 
-def test_get_jobs_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_service):
+def test_get_jobs_should_return_statistics(admin_request, sample_template):
     now = datetime.utcnow()
     earlier = datetime.utcnow() - timedelta(days=1)
-    job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier)
+    job_1 = create_job(sample_template, created_at=earlier)
-    job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now)
+    job_2 = create_job(sample_template, created_at=now)
-    partial_notification = partial(create_notification, notify_db, notify_db_session, service=sample_service)
+    create_notification(job=job_1, status='created')
-    partial_notification(job=job_1, status='created')
+    create_notification(job=job_1, status='created')
-    partial_notification(job=job_1, status='created')
+    create_notification(job=job_1, status='created')
-    partial_notification(job=job_1, status='created')
+    create_notification(job=job_2, status='sending')
-    partial_notification(job=job_2, status='sending')
+    create_notification(job=job_2, status='sending')
-    partial_notification(job=job_2, status='sending')
+    create_notification(job=job_2, status='sending')
-    partial_notification(job=job_2, status='sending')
 
-    with notify_api.test_request_context():
+    resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id)
-        with notify_api.test_client() as client:
+
-            path = '/service/{}/job'.format(sample_service.id)
+    assert len(resp_json['data']) == 2
-            auth_header = create_authorization_header()
+    assert resp_json['data'][0]['id'] == str(job_2.id)
-            response = client.get(path, headers=[auth_header])
+    assert {'status': 'sending', 'count': 3} in resp_json['data'][0]['statistics']
-            assert response.status_code == 200
+    assert resp_json['data'][1]['id'] == str(job_1.id)
-            resp_json = json.loads(response.get_data(as_text=True))
+    assert {'status': 'created', 'count': 3} in resp_json['data'][1]['statistics']
-            assert len(resp_json['data']) == 2
-            assert resp_json['data'][0]['id'] == str(job_2.id)
-            assert {'status': 'sending', 'count': 3} in resp_json['data'][0]['statistics']
-            assert resp_json['data'][1]['id'] == str(job_1.id)
-            assert {'status': 'created', 'count': 3} in resp_json['data'][1]['statistics']
 
 
-def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(
+def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_request, sample_template):
-    client,
-    notify_db,
-    notify_db_session,
-    notify_api,
-    sample_service,
-):
-
     now = datetime.utcnow()
     earlier = datetime.utcnow() - timedelta(days=1)
-    job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier)
+    job_1 = create_job(sample_template, created_at=earlier)
-    job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now)
+    job_2 = create_job(sample_template, created_at=now)
+
+    resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id)
 
-    path = '/service/{}/job'.format(sample_service.id)
-    auth_header = create_authorization_header()
-    response = client.get(path, headers=[auth_header])
-    assert response.status_code == 200
-    resp_json = json.loads(response.get_data(as_text=True))
     assert len(resp_json['data']) == 2
     assert resp_json['data'][0]['id'] == str(job_2.id)
     assert resp_json['data'][0]['statistics'] == []
@@ -720,23 +607,12 @@ def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(
     assert resp_json['data'][1]['statistics'] == []
 
 
-def test_get_jobs_should_paginate(
+def test_get_jobs_should_paginate(admin_request, sample_template):
-    notify_db,
+    create_10_jobs(sample_template)
-    notify_db_session,
-    client,
-    sample_template
-):
-    create_10_jobs(notify_db, notify_db_session, sample_template.service, sample_template)
 
-    path = '/service/{}/job'.format(sample_template.service_id)
+    with set_config(admin_request.app, 'PAGE_SIZE', 2):
-    auth_header = create_authorization_header()
+        resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id)
 
-    with set_config(client.application, 'PAGE_SIZE', 2):
-        response = client.get(path, headers=[auth_header])
-
-    assert response.status_code == 200
-    resp_json = json.loads(response.get_data(as_text=True))
-    assert len(resp_json['data']) == 2
     assert resp_json['data'][0]['created_at'] == '2015-01-01T10:00:00+00:00'
     assert resp_json['data'][1]['created_at'] == '2015-01-01T09:00:00+00:00'
     assert resp_json['page_size'] == 2
@@ -745,23 +621,12 @@ def test_get_jobs_should_paginate(
     assert set(resp_json['links'].keys()) == {'next', 'last'}
 
 
-def test_get_jobs_accepts_page_parameter(
+def test_get_jobs_accepts_page_parameter(admin_request, sample_template):
-    notify_db,
+    create_10_jobs(sample_template)
-    notify_db_session,
-    client,
-    sample_template
-):
-    create_10_jobs(notify_db, notify_db_session, sample_template.service, sample_template)
 
-    path = '/service/{}/job'.format(sample_template.service_id)
+    with set_config(admin_request.app, 'PAGE_SIZE', 2):
-    auth_header = create_authorization_header()
+        resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id, page=2)
 
-    with set_config(client.application, 'PAGE_SIZE', 2):
-        response = client.get(path, headers=[auth_header], query_string={'page': 2})
-
-    assert response.status_code == 200
-    resp_json = json.loads(response.get_data(as_text=True))
-    assert len(resp_json['data']) == 2
     assert resp_json['data'][0]['created_at'] == '2015-01-01T08:00:00+00:00'
     assert resp_json['data'][1]['created_at'] == '2015-01-01T07:00:00+00:00'
     assert resp_json['page_size'] == 2
@@ -778,71 +643,79 @@ def test_get_jobs_accepts_page_parameter(
     # bad statuses are accepted, just return no data
     ('foo', [])
 ])
-def test_get_jobs_can_filter_on_statuses(
+def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuses_filter, expected_statuses):
-    notify_db,
+    create_job(sample_template, job_status='pending')
-    notify_db_session,
+    create_job(sample_template, job_status='in progress')
-    client,
+    create_job(sample_template, job_status='finished')
-    sample_service,
+    create_job(sample_template, job_status='sending limits exceeded')
-    statuses_filter,
+    create_job(sample_template, job_status='scheduled')
-    expected_statuses
+    create_job(sample_template, job_status='cancelled')
-):
+    create_job(sample_template, job_status='ready to send')
-    create_job(notify_db, notify_db_session, job_status='pending')
+    create_job(sample_template, job_status='sent to dvla')
-    create_job(notify_db, notify_db_session, job_status='in progress')
+    create_job(sample_template, job_status='error')
-    create_job(notify_db, notify_db_session, job_status='finished')
-    create_job(notify_db, notify_db_session, job_status='sending limits exceeded')
-    create_job(notify_db, notify_db_session, job_status='scheduled')
-    create_job(notify_db, notify_db_session, job_status='cancelled')
-    create_job(notify_db, notify_db_session, job_status='ready to send')
-    create_job(notify_db, notify_db_session, job_status='sent to dvla')
-    create_job(notify_db, notify_db_session, job_status='error')
 
-    path = '/service/{}/job'.format(sample_service.id)
+    resp_json = admin_request.get(
-    response = client.get(
+        'job.get_jobs_by_service',
-        path,
+        service_id=sample_template.service_id,
-        headers=[create_authorization_header()],
+        statuses=statuses_filter
-        query_string={'statuses': statuses_filter}
     )
 
-    assert response.status_code == 200
-    resp_json = json.loads(response.get_data(as_text=True))
-    from pprint import pprint
-    pprint(resp_json)
     assert {x['job_status'] for x in resp_json['data']} == set(expected_statuses)
 
 
-def create_10_jobs(db, session, service, template):
+def create_10_jobs(template):
     with freeze_time('2015-01-01T00:00:00') as the_time:
         for _ in range(10):
             the_time.tick(timedelta(hours=1))
-            create_job(db, session, service, template)
+            create_job(template)
 
 
-def test_get_all_notifications_for_job_returns_csv_format(
+def test_get_all_notifications_for_job_returns_csv_format(admin_request, sample_notification_with_job):
-    client,
+    resp = admin_request.get(
-    notify_db,
+        'job.get_all_notifications_for_service_job',
-    notify_db_session,
+        service_id=sample_notification_with_job.service_id,
-):
+        job_id=sample_notification_with_job.job_id,
-    job = create_job(notify_db, notify_db_session)
+        format_for_csv=True
-    notification = create_notification(
-        notify_db,
-        notify_db_session,
-        job=job,
-        job_row_number=1,
-        created_at=datetime.utcnow(),
     )
 
-    path = '/service/{}/job/{}/notifications'.format(notification.service.id, job.id)
-
-    response = client.get(
-        path=path,
-        headers=[create_authorization_header()],
-        query_string={'format_for_csv': True}
-    )
-    assert response.status_code == 200
-
-    resp = json.loads(response.get_data(as_text=True))
     assert len(resp['notifications']) == 1
-    notification = resp['notifications'][0]
+    assert set(resp['notifications'][0].keys()) == {
-    assert set(notification.keys()) == \
+        'created_at',
-        set(['created_at', 'created_by_name', 'created_by_email_address', 'template_type',
+        'created_by_name',
-             'template_name', 'job_name', 'status', 'row_number', 'recipient'])
+        'created_by_email_address',
+        'template_type',
+        'template_name',
+        'job_name',
+        'status',
+        'row_number',
+        'recipient'
+    }
+
+
+@freeze_time('2017-06-10 12:00')
+def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template):
+    # it's the 10th today, so 3 days should include all of 7th, 8th, 9th, and some of 10th.
+    just_three_days_ago = datetime(2017, 6, 6, 22, 59, 59)
+    not_quite_three_days_ago = just_three_days_ago + timedelta(seconds=1)
+
+    job_1 = create_job(sample_template, created_at=just_three_days_ago)
+    job_2 = create_job(sample_template, created_at=not_quite_three_days_ago)
+
+    # some notifications created more than three days ago, some created after the midnight cutoff
+    create_ft_notification_status(date(2017, 6, 6), job=job_1, notification_status='delivered', count=2)
+    create_ft_notification_status(date(2017, 6, 7), job=job_1, notification_status='delivered', count=4)
+    # job2's new enough
+    create_notification(job=job_2, status='created', created_at=not_quite_three_days_ago)
+
+    # this isn't picked up because the job is too new
+    create_ft_notification_status(date(2017, 6, 7), job=job_2, notification_status='delivered', count=8)
+
+    # this isn't picked up because we're using the ft status table for job_1 as it's old
+    create_notification(job=job_1, status='created', created_at=not_quite_three_days_ago)
+
+    resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id)
+
+    assert resp_json['data'][0]['id'] == str(job_2.id)
+    assert resp_json['data'][0]['statistics'] == [{'status': 'created', 'count': 1}]
+    assert resp_json['data'][1]['id'] == str(job_1.id)
+    assert resp_json['data'][1]['statistics'] == [{'status': 'delivered', 'count': 6}]