From 033bcb65d2c3db782177ccef9d7e6f47068031fe Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Dec 2019 16:17:00 +0000 Subject: [PATCH] Update the `dao_get_notification_outcomes_for_job` to return the stats from either the notification table or the ft_notification_status table. Currently if you visit the job page and the job is older than the data retention the totals on the page are all wrong because this query gets the counts from the notification table. With this change the data should always be correct. It also eliminates the need for looking at data retention. If the job is new and nothing has been created yet (i.e. the job hasn't started yet) then the page should show the correctly because the outcomes are empty (as expected), once the notifications for the jobs are created the numbers will start going up. --- app/dao/jobs_dao.py | 18 ++++++++++++++---- tests/app/job/test_rest.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index ad447f484..baff3bbb5 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -27,15 +27,15 @@ from app.models import ( ServiceDataRetention, NOTIFICATION_CREATED, NOTIFICATION_CANCELLED, - JOB_STATUS_CANCELLED + JOB_STATUS_CANCELLED, + FactNotificationStatus ) @statsd(namespace="dao") def dao_get_notification_outcomes_for_job(service_id, job_id): - return db.session.query( - func.count(Notification.status).label('count'), - Notification.status + notification_statuses = db.session.query( + func.count(Notification.status).label('count'), Notification.status ).filter( Notification.service_id == service_id, Notification.job_id == job_id @@ -43,6 +43,16 @@ def dao_get_notification_outcomes_for_job(service_id, job_id): Notification.status ).all() + if not notification_statuses: + notification_statuses = db.session.query( + FactNotificationStatus.notification_count.label('count'), + FactNotificationStatus.notification_status.label('status') + ).filter( + FactNotificationStatus.service_id == service_id, + FactNotificationStatus.job_id == job_id + ).all() + return notification_statuses + def dao_get_job_by_service_id_and_job_id(service_id, job_id): return Job.query.filter_by(service_id=service_id, id=job_id).one() diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 998c576d5..79645049d 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -622,6 +622,37 @@ def test_get_job_by_id_should_return_summed_statistics(admin_request, sample_job assert resp_json['data']['created_by']['name'] == 'Test User' +def test_get_job_by_id_with_stats_for_old_job_where_notifications_have_been_purged(admin_request, sample_template): + old_job = create_job(sample_template, notification_count=10, created_at=datetime.utcnow() - timedelta(days=9), + job_status='finished') + + def __create_ft_status(job, status, count): + create_ft_notification_status(bst_date=job.created_at.date(), + notification_type='sms', + service=job.service, + job=job, + template=job.template, + key_type='normal', + notification_status=status, + count=count) + + __create_ft_status(old_job, 'created', 3) + __create_ft_status(old_job, 'sending', 1) + __create_ft_status(old_job, 'failed', 3) + __create_ft_status(old_job, 'technical-failure', 1) + __create_ft_status(old_job, 'temporary-failure', 2) + + resp_json = admin_request.get('job.get_job_by_service_and_job_id', service_id=old_job.service_id, job_id=old_job.id) + + assert resp_json['data']['id'] == str(old_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' + + def test_get_jobs(admin_request, sample_template): _setup_jobs(sample_template)