diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 1390201e8..8925c81f0 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/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() diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 5eaf71457..e22c1c709 100644 --- a/app/dao/jobs_dao.py +++ b/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( - func.count(NotificationHistory.status).label('count'), - NotificationHistory.status - ) - - return query.filter( - NotificationHistory.service_id == service_id + return db.session.query( + func.count(Notification.status).label('count'), + Notification.status ).filter( - NotificationHistory.job_id == job_id + Notification.service_id == service_id, + Notification.job_id == job_id ).group_by( - NotificationHistory.status - ).order_by( - asc(NotificationHistory.status) + Notification.status ).all() diff --git a/app/job/rest.py b/app/job/rest.py index 6e64e045c..61665ec45 100644 --- a/app/job/rest.py +++ b/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']) - job_data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics] + created_at = dateutil.parser.parse(job_data['created_at']).replace(tzinfo=None) + 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, diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 9dcb975eb..db90c6d61 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/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 + } diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 535808d3b..524d17f8e 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -19,13 +19,54 @@ from app.models import ( Job, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE ) -from tests.app.db import create_job, create_service, create_template +from tests.app.db import create_job, create_service, create_template, create_notification 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_count_of_statuses_for_notifications_associated_with_job(sample_template, sample_job): + create_notification(sample_template, job=sample_job, status='created') + create_notification(sample_template, job=sample_job, status='created') + create_notification(sample_template, job=sample_job, status='created') + create_notification(sample_template, job=sample_job, status='sending') + create_notification(sample_template, job=sample_job, status='delivered') + + results = dao_get_notification_outcomes_for_job(sample_template.service_id, sample_job.id) + assert {row.status: row.count for row in results} == { + 'created': 3, + 'sending': 1, + 'delivered': 1, + } + + +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(sample_template): + job_1 = create_job(sample_template) + job_2 = create_job(sample_template) + + create_notification(sample_template, job=job_1, status='created') + create_notification(sample_template, job=job_2, status='sent') + + results = dao_get_notification_outcomes_for_job(sample_template.service_id, job_1.id) + assert {row.status: row.count for row in results} == {'created': 1} + + +def test_should_return_notifications_only_for_this_service(sample_notification_with_job): + other_service = create_service(service_name='one') + other_template = create_template(service=other_service) + other_job = create_job(other_template) + + create_notification(other_template, job=other_job) + + assert len(dao_get_notification_outcomes_for_job(sample_notification_with_job.service_id, other_job.id)) == 0 + assert len(dao_get_notification_outcomes_for_job(other_service.id, sample_notification_with_job.id)) == 0 + + def test_create_sample_job(sample_template): assert Job.query.count() == 0 diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index e1797233a..9adb6e8ac 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -1,6 +1,6 @@ import json import uuid -from datetime import datetime, timedelta +from datetime import datetime, timedelta, date from freezegun import freeze_time import pytest @@ -12,7 +12,7 @@ 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.db import create_job, create_notification +from tests.app.db import create_ft_notification_status, create_job, create_notification def test_get_job_with_invalid_service_id_returns404(client, sample_service): @@ -690,3 +690,32 @@ def test_get_all_notifications_for_job_returns_csv_format(admin_request, sample_ '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}]