From 2fcde009ac16d1b5326ed9eb19380b28ab8d9f0e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 28 Sep 2020 09:57:32 +0100 Subject: [PATCH 1/2] Add an endpoint for stats about scheduled jobs At the moment we display the count of scheduled jobs on the dashboard by sending all the scheduled jobs to the admin app and letting it work out the stats. This is inefficient and, because the get jobs response has a page size of 50, becomes incorrect if a service schedules more than 50 jobs. This commit adds a separate endpoint which gives the admin app the stats it needs directly and correctly. --- app/dao/jobs_dao.py | 12 ++++++++++++ app/job/rest.py | 14 ++++++++++++++ tests/app/job/test_rest.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index cee3d16b1..ebf2f0197 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -84,6 +84,18 @@ def dao_get_jobs_by_service_id( .paginate(page=page, per_page=page_size) +def dao_get_scheduled_job_stats( + service_id, +): + return db.session.query( + func.count(Job.id), + func.min(Job.scheduled_for), + ).filter( + Job.service_id == service_id, + Job.job_status == JOB_STATUS_SCHEDULED, + ).one() + + def dao_get_job_by_id(job_id): return Job.query.filter_by(id=job_id).one() diff --git a/app/job/rest.py b/app/job/rest.py index 2831f9da9..11cc6fc5a 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -1,4 +1,5 @@ import dateutil +import pytz from flask import ( Blueprint, jsonify, @@ -14,6 +15,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, + dao_get_scheduled_job_stats, dao_cancel_letter_job, can_letter_job_be_cancelled ) @@ -188,6 +190,18 @@ def create_job(service_id): return jsonify(data=job_json), 201 +@job_blueprint.route('/scheduled-job-stats', methods=['GET']) +def get_scheduled_job_stats(service_id): + count, soonest_scheduled_for = dao_get_scheduled_job_stats(service_id) + return jsonify( + count=count, + soonest_scheduled_for=( + soonest_scheduled_for.replace(tzinfo=pytz.UTC).isoformat() + if soonest_scheduled_for else None + ), + ), 200 + + def get_paginated_jobs( service_id, *, diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 0654ed6ff..9144cf68c 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -910,3 +910,36 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin assert resp_json['data'][1]['statistics'] == [{'status': 'created', 'count': 1}] assert resp_json['data'][2]['id'] == str(job_1.id) assert resp_json['data'][2]['statistics'] == [{'status': 'delivered', 'count': 6}] + + +@freeze_time('2017-07-17 07:17') +def test_get_scheduled_job_stats_when_no_scheduled_jobs(admin_request, sample_template): + + # This sets up a bunch of regular, non-scheduled jobs + _setup_jobs(sample_template) + + service_id = sample_template.service.id + + resp_json = admin_request.get('job.get_scheduled_job_stats', service_id=service_id) + assert resp_json == { + 'count': 0, + 'soonest_scheduled_for': None, + } + + +@freeze_time('2017-07-17 07:17') +def test_get_scheduled_job_stats(admin_request, sample_template): + create_job(sample_template, job_status='scheduled', scheduled_for='2017-07-17 09:00') + create_job(sample_template, job_status='scheduled', scheduled_for='2017-07-17 10:00') + create_job(sample_template, job_status='scheduled', scheduled_for='2017-07-17 11:00') + + service_id = sample_template.service.id + + resp_json = admin_request.get( + 'job.get_scheduled_job_stats', + service_id=service_id, + ) + assert resp_json == { + 'count': 3, + 'soonest_scheduled_for': '2017-07-17T09:00:00+00:00', + } From 4cee47da68b20b57a0b876eaddade016c9ad79cc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 28 Sep 2020 13:15:39 +0100 Subject: [PATCH 2/2] Add more tests for scheduled job stats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensures we don’t count jobs that are in a different status, or belong to another service. --- tests/app/job/test_rest.py | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 9144cf68c..af6fd0524 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -17,7 +17,9 @@ from tests.app.db import ( create_ft_notification_status, create_job, create_notification, - create_service_contact_list + create_service, + create_service_contact_list, + create_template, ) @@ -928,18 +930,37 @@ def test_get_scheduled_job_stats_when_no_scheduled_jobs(admin_request, sample_te @freeze_time('2017-07-17 07:17') -def test_get_scheduled_job_stats(admin_request, sample_template): - create_job(sample_template, job_status='scheduled', scheduled_for='2017-07-17 09:00') - create_job(sample_template, job_status='scheduled', scheduled_for='2017-07-17 10:00') - create_job(sample_template, job_status='scheduled', scheduled_for='2017-07-17 11:00') +def test_get_scheduled_job_stats(admin_request): - service_id = sample_template.service.id + service_1 = create_service(service_name='service 1') + service_1_template = create_template(service=service_1) + service_2 = create_service(service_name='service 2') + service_2_template = create_template(service=service_2) - resp_json = admin_request.get( + # Shouldn’t be counted – wrong status + create_job(service_1_template, job_status='finished', scheduled_for='2017-07-17 07:00') + create_job(service_1_template, job_status='in progress', scheduled_for='2017-07-17 08:00') + + # Should be counted – service 1 + create_job(service_1_template, job_status='scheduled', scheduled_for='2017-07-17 09:00') + create_job(service_1_template, job_status='scheduled', scheduled_for='2017-07-17 10:00') + create_job(service_1_template, job_status='scheduled', scheduled_for='2017-07-17 11:00') + + # Should be counted – service 2 + create_job(service_2_template, job_status='scheduled', scheduled_for='2017-07-17 11:00') + + assert admin_request.get( 'job.get_scheduled_job_stats', - service_id=service_id, - ) - assert resp_json == { + service_id=service_1.id, + ) == { 'count': 3, 'soonest_scheduled_for': '2017-07-17T09:00:00+00:00', } + + assert admin_request.get( + 'job.get_scheduled_job_stats', + service_id=service_2.id, + ) == { + 'count': 1, + 'soonest_scheduled_for': '2017-07-17T11:00:00+00:00', + }