From c7707873e4877093a10964338aa5a9a4b35bcdf1 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 12 Jun 2017 14:25:17 +0100 Subject: [PATCH 1/5] Queries to get job statistics by job_id or by service_id. --- app/dao/jobs_dao.py | 35 +++++++++- tests/app/dao/test_jobs_dao.py | 117 ++++++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 2 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 86e565619..d6770943f 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -8,7 +8,7 @@ from app.dao import days_ago from app.models import ( Job, JobStatistics, Notification, NotificationHistory, Template, JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, - EMAIL_TYPE, SMS_TYPE, LETTER_TYPE + LETTER_TYPE ) from app.statsd_decorators import statsd @@ -142,3 +142,36 @@ def dao_get_all_letter_jobs(): return db.session.query(Job).join(Job.template).filter( Template.template_type == LETTER_TYPE ).order_by(desc(Job.created_at)).all() + + +@statsd(namespace="dao") +def dao_get_job_statistics_for_job(job_id): + query = db.session.query( + JobStatistics.job_id, + Job.original_file_name, + Job.created_at, + JobStatistics.sent, + JobStatistics.delivered, + JobStatistics.failed + ).filter(JobStatistics.job_id == job_id).filter(Job.id == JobStatistics.job_id) + return query.one() + + +@statsd(namespace="dao") +def dao_get_job_stats_for_service(service_id, page=1, page_size=50, limit_days=None): + query = Job.query.join( + JobStatistics, Job.id == JobStatistics.job_id + ).filter( + Job.service_id == service_id + ).add_columns( + JobStatistics.job_id, + Job.original_file_name, + Job.created_at, + JobStatistics.sent, + JobStatistics.delivered, + JobStatistics.failed + ) + if limit_days: + query = query.filter(Job.created_at >= days_ago(limit_days)) + query = query.order_by(Job.created_at.desc()) + return query.paginate(page=page, per_page=page_size).items diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 42f93d230..476e6b1de 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -16,7 +16,10 @@ from app.dao.jobs_dao import ( all_notifications_are_created_for_job, dao_update_job_status, dao_get_all_notifications_for_job, - dao_get_jobs_older_than_limited_by) + dao_get_jobs_older_than_limited_by, + dao_get_job_statistics_for_job, + dao_get_job_stats_for_service) +from app.dao.statistics_dao import create_or_update_job_sending_statistics, update_job_stats_outcome_count from app.models import ( Job, JobStatistics, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE @@ -411,3 +414,115 @@ def test_should_get_jobs_seven_days_old_filters_type(notify_db, notify_db_sessio assert len(jobs) == 2 assert job_to_remain.id not in [job.id for job in jobs] + + +def test_dao_get_job_statistics_for_job(notify_db, notify_db_session, sample_job): + notification = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=sample_job) + notification_delivered = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, + job=sample_job, status='delivered') + notification_failed = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=sample_job, + status='permanent-failure') + + create_or_update_job_sending_statistics(notification) + create_or_update_job_sending_statistics(notification_delivered) + create_or_update_job_sending_statistics(notification_failed) + update_job_stats_outcome_count(notification_delivered) + update_job_stats_outcome_count(notification_failed) + results = dao_get_job_statistics_for_job(sample_job.id) + assert results.job_id == sample_job.id + assert results.sent == 3 + assert results.delivered == 1 + assert results.failed == 1 + + +def test_dao_get_job_statistics_for_job(notify_db, notify_db_session, sample_service): + job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) + results = dao_get_job_statistics_for_job(job_1.id) + assert_job_stat(job_1, results, 2, 1, 0) + + results_2 = dao_get_job_statistics_for_job(job_2.id) + assert_job_stat(job_2, results_2, 1, 0, 1) + + +def test_dao_get_job_stats_for_service(notify_db, notify_db_session, sample_service): + job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) + + results = dao_get_job_stats_for_service(sample_service.id) + assert len(results) == 2 + assert_job_stat(job_2, results[0], 1, 0, 1) + assert_job_stat(job_1, results[1], 2, 1, 0) + + +def test_dao_get_job_stats_for_service_only_returns_stats_for_service(notify_db, notify_db_session, sample_service): + job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) + another_service = create_service(notify_db=notify_db, notify_db_session=notify_db_session, + service_name='Another Service') + job_3, job_4 = stats_set_up(notify_db, notify_db_session, service=another_service) + + results = dao_get_job_stats_for_service(sample_service.id) + assert len(results) == 2 + assert_job_stat(job_2, results[0], 1, 0, 1) + assert_job_stat(job_1, results[1], 2, 1, 0) + + results = dao_get_job_stats_for_service(another_service.id) + assert len(results) == 2 + assert_job_stat(job_4, results[0], 1, 0, 1) + assert_job_stat(job_3, results[1], 2, 1, 0) + + +def test_dao_get_job_stats_for_service_only_returns_jobs_created_within_limited_days( + notify_db, notify_db_session, sample_service): + job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) + + results = dao_get_job_stats_for_service(sample_service.id, limit_days=1) + assert len(results) == 1 + assert_job_stat(job_2, results[0], 1, 0, 1) + + +def test_dao_get_job_stats_for_service_only_returns_jobs_created_within_limited_days_inclusive( + notify_db, notify_db_session, sample_service): + job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) + + results = dao_get_job_stats_for_service(sample_service.id, limit_days=2) + assert len(results) == 2 + assert_job_stat(job_2, results[0], 1, 0, 1) + assert_job_stat(job_1, results[1], 2, 1, 0) + + +def test_dao_get_job_stats_paginates_results( + notify_db, notify_db_session, sample_service): + job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) + + results = dao_get_job_stats_for_service(sample_service.id, page=1, page_size=1) + assert len(results) == 1 + assert_job_stat(job_2, results[0], 1, 0, 1) + results_2 = dao_get_job_stats_for_service(sample_service.id, page=2, page_size=1) + assert len(results_2) == 1 + assert_job_stat(job_1, results_2[0], 2, 1, 0) + + +def assert_job_stat(job, result, sent, delivered, failed): + assert result.job_id == job.id + assert result.original_file_name == job.original_file_name + assert result.created_at == job.created_at + assert result.sent == sent + assert result.delivered == delivered + assert result.failed == failed + + +def stats_set_up(notify_db, notify_db_session, service): + job_1 = create_job(notify_db=notify_db, notify_db_session=notify_db_session, + service=service, created_at=datetime.utcnow() - timedelta(days=2)) + job_2 = create_job(notify_db=notify_db, notify_db_session=notify_db_session, + service=service, original_file_name='Another job') + notification = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=job_1) + notification_delivered = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, + job=job_1, status='delivered') + notification_failed = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=job_2, + status='permanent-failure') + create_or_update_job_sending_statistics(notification) + create_or_update_job_sending_statistics(notification_delivered) + create_or_update_job_sending_statistics(notification_failed) + update_job_stats_outcome_count(notification_delivered) + update_job_stats_outcome_count(notification_failed) + return job_1, job_2 From cc04b5eb1ddb3d568183ee5f7e50f67384ea25c1 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 12 Jun 2017 17:15:32 +0100 Subject: [PATCH 2/5] Added new endpoint for job statistics. The structure has been flattened, so I need to create a new endpoint, start using that endpoint, then change the name back. Added template_id and version to the get job stats by id. --- app/dao/jobs_dao.py | 25 +++++++++++++---- app/job/rest.py | 50 ++++++++++++++++++++++++++++++++-- tests/app/dao/test_jobs_dao.py | 49 ++++++++++++++++++++------------- tests/app/job/test_rest.py | 43 +++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 26 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index d6770943f..9d791b8ad 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -145,20 +145,31 @@ def dao_get_all_letter_jobs(): @statsd(namespace="dao") -def dao_get_job_statistics_for_job(job_id): - query = db.session.query( +def dao_get_job_statistics_for_job(service_id, job_id): + query = Job.query.join( + JobStatistics, Job.id == JobStatistics.job_id + ).filter( + Job.id == job_id + ).join( + Template, Template.id == Job.template_id and Template.version == Job.template_version + ).add_columns( JobStatistics.job_id, Job.original_file_name, Job.created_at, + Job.scheduled_for, + Job.template_id, + Job.template_version, JobStatistics.sent, JobStatistics.delivered, JobStatistics.failed - ).filter(JobStatistics.job_id == job_id).filter(Job.id == JobStatistics.job_id) + ).filter( + Job.service_id == service_id + ) return query.one() @statsd(namespace="dao") -def dao_get_job_stats_for_service(service_id, page=1, page_size=50, limit_days=None): +def dao_get_job_stats_for_service(service_id, page=1, page_size=50, limit_days=None, statuses=None): query = Job.query.join( JobStatistics, Job.id == JobStatistics.job_id ).filter( @@ -167,11 +178,15 @@ def dao_get_job_stats_for_service(service_id, page=1, page_size=50, limit_days=N JobStatistics.job_id, Job.original_file_name, Job.created_at, + Job.scheduled_for, JobStatistics.sent, JobStatistics.delivered, JobStatistics.failed ) if limit_days: query = query.filter(Job.created_at >= days_ago(limit_days)) + if statuses is not None and statuses != ['']: + query = query.filter(Job.job_status.in_(statuses)) + query = query.order_by(Job.created_at.desc()) - return query.paginate(page=page, per_page=page_size).items + return query.paginate(page=page, per_page=page_size) diff --git a/app/job/rest.py b/app/job/rest.py index b4882945f..5e5bc33c1 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -11,8 +11,8 @@ from app.dao.jobs_dao import ( dao_get_job_by_service_id_and_job_id, dao_get_jobs_by_service_id, dao_get_future_scheduled_job_by_id_and_service_id, - dao_get_notification_outcomes_for_job -) + dao_get_notification_outcomes_for_job, + dao_get_job_stats_for_service) from app.dao.services_dao import ( dao_fetch_service_by_id @@ -117,6 +117,52 @@ def get_jobs_by_service(service_id): return jsonify(**get_paginated_jobs(service_id, limit_days, statuses, page)) +@job_blueprint.route('/job-stats', methods=['GET']) +def get_jobs_for_service(service_id): + if request.args.get('limit_days'): + try: + limit_days = int(request.args['limit_days']) + except ValueError: + errors = {'limit_days': ['{} is not an integer'.format(request.args['limit_days'])]} + raise InvalidRequest(errors, status_code=400) + else: + limit_days = None + statuses = _parse_statuses(request.args.get('statuses', '')) + page = int(request.args.get('page', 1)) + + pagination = dao_get_job_stats_for_service(service_id=service_id, + page=page, + page_size=current_app.config['PAGE_SIZE'], + limit_days=limit_days, + statuses=statuses) + return jsonify({ + 'data': [_serialize_job_stats(x) for x in pagination.items], + 'page_size': pagination.per_page, + 'total': pagination.total, + 'links': pagination_links( + pagination, + '.get_jobs_by_service', + service_id=service_id + ) + }) + + +def _parse_statuses(statuses): + return [x.strip() for x in statuses.split(',')] + + +def _serialize_job_stats(stat): + return { + "job_id": stat.job_id, + "original_file_name": stat.original_file_name, + "created_at": stat.created_at, + "scheduled_for": stat.scheduled_for, + "sent": stat.sent, + "delivered": stat.delivered, + "failed": stat.failed + } + + @job_blueprint.route('', methods=['POST']) def create_job(service_id): service = dao_fetch_service_by_id(service_id) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 476e6b1de..32508c628 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -428,26 +428,23 @@ def test_dao_get_job_statistics_for_job(notify_db, notify_db_session, sample_job create_or_update_job_sending_statistics(notification_failed) update_job_stats_outcome_count(notification_delivered) update_job_stats_outcome_count(notification_failed) - results = dao_get_job_statistics_for_job(sample_job.id) - assert results.job_id == sample_job.id - assert results.sent == 3 - assert results.delivered == 1 - assert results.failed == 1 + result = dao_get_job_statistics_for_job(sample_job.service_id, sample_job.id) + assert_job_stat(job=sample_job, result=result, sent=3, delivered=1, failed=1, with_template=True) def test_dao_get_job_statistics_for_job(notify_db, notify_db_session, sample_service): job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) - results = dao_get_job_statistics_for_job(job_1.id) - assert_job_stat(job_1, results, 2, 1, 0) + result = dao_get_job_statistics_for_job(sample_service.id, job_1.id) + assert_job_stat(job=job_1, result=result, sent=2, delivered=1, failed=0, with_template=True) - results_2 = dao_get_job_statistics_for_job(job_2.id) - assert_job_stat(job_2, results_2, 1, 0, 1) + result_2 = dao_get_job_statistics_for_job(sample_service.id, job_2.id) + assert_job_stat(job=job_2, result=result_2, sent=1, delivered=0, failed=1, with_template=True) def test_dao_get_job_stats_for_service(notify_db, notify_db_session, sample_service): job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) - results = dao_get_job_stats_for_service(sample_service.id) + results = dao_get_job_stats_for_service(sample_service.id).items assert len(results) == 2 assert_job_stat(job_2, results[0], 1, 0, 1) assert_job_stat(job_1, results[1], 2, 1, 0) @@ -459,12 +456,12 @@ def test_dao_get_job_stats_for_service_only_returns_stats_for_service(notify_db, service_name='Another Service') job_3, job_4 = stats_set_up(notify_db, notify_db_session, service=another_service) - results = dao_get_job_stats_for_service(sample_service.id) + results = dao_get_job_stats_for_service(sample_service.id).items assert len(results) == 2 assert_job_stat(job_2, results[0], 1, 0, 1) assert_job_stat(job_1, results[1], 2, 1, 0) - results = dao_get_job_stats_for_service(another_service.id) + results = dao_get_job_stats_for_service(another_service.id).items assert len(results) == 2 assert_job_stat(job_4, results[0], 1, 0, 1) assert_job_stat(job_3, results[1], 2, 1, 0) @@ -475,15 +472,15 @@ def test_dao_get_job_stats_for_service_only_returns_jobs_created_within_limited_ job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) results = dao_get_job_stats_for_service(sample_service.id, limit_days=1) - assert len(results) == 1 - assert_job_stat(job_2, results[0], 1, 0, 1) + assert results.total == 1 + assert_job_stat(job_2, results.items[0], 1, 0, 1) def test_dao_get_job_stats_for_service_only_returns_jobs_created_within_limited_days_inclusive( notify_db, notify_db_session, sample_service): job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) - results = dao_get_job_stats_for_service(sample_service.id, limit_days=2) + results = dao_get_job_stats_for_service(sample_service.id, limit_days=2).items assert len(results) == 2 assert_job_stat(job_2, results[0], 1, 0, 1) assert_job_stat(job_1, results[1], 2, 1, 0) @@ -493,28 +490,42 @@ def test_dao_get_job_stats_paginates_results( notify_db, notify_db_session, sample_service): job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) - results = dao_get_job_stats_for_service(sample_service.id, page=1, page_size=1) + results = dao_get_job_stats_for_service(sample_service.id, page=1, page_size=1).items assert len(results) == 1 assert_job_stat(job_2, results[0], 1, 0, 1) - results_2 = dao_get_job_stats_for_service(sample_service.id, page=2, page_size=1) + results_2 = dao_get_job_stats_for_service(sample_service.id, page=2, page_size=1).items assert len(results_2) == 1 assert_job_stat(job_1, results_2[0], 2, 1, 0) -def assert_job_stat(job, result, sent, delivered, failed): +def test_dao_get_job_returns_jobs_for_status( + notify_db, notify_db_session, sample_service): + stats_set_up(notify_db, notify_db_session, sample_service) + + results = dao_get_job_stats_for_service(sample_service.id, statuses=['pending']) + assert results.total == 1 + results_2 = dao_get_job_stats_for_service(sample_service.id, statuses=['pending', 'finished']) + assert results_2.total == 2 + + +def assert_job_stat(job, result, sent, delivered, failed, with_template=False): assert result.job_id == job.id assert result.original_file_name == job.original_file_name assert result.created_at == job.created_at + assert result.scheduled_for == job.scheduled_for assert result.sent == sent assert result.delivered == delivered assert result.failed == failed + if with_template: + assert result.template_id == job.template_id + assert result.template_version == job.template_version def stats_set_up(notify_db, notify_db_session, service): job_1 = create_job(notify_db=notify_db, notify_db_session=notify_db_session, service=service, created_at=datetime.utcnow() - timedelta(days=2)) job_2 = create_job(notify_db=notify_db, notify_db_session=notify_db_session, - service=service, original_file_name='Another job') + service=service, original_file_name='Another job', job_status='finished') notification = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=job_1) notification_delivered = create_notification(notify_db=notify_db, notify_db_session=notify_db_session, job=job_1, status='delivered') diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 6ebcb2e89..37fbe02dd 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -762,3 +762,46 @@ def test_get_all_notifications_for_job_returns_csv_format( notification = resp['notifications'][0] assert set(notification.keys()) == \ set(['created_at', 'template_type', 'template_name', 'job_name', 'status', 'row_number', 'recipient']) + + +# New endpoint to get job statistics the old tests will be refactored away. +def test_get_jobs_for_service_new_endpoint(client, notify_db, notify_db_session, sample_template): + _setup_jobs(notify_db, notify_db_session, sample_template) + + service_id = sample_template.service.id + + path = '/service/{}/job/job-stats'.format(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 + assert resp_json['data'][0]["job_id"] + assert resp_json['data'][0]["created_at"] + assert not resp_json['data'][0]["scheduled_for"] + assert resp_json['data'][0]["sent"] == 0 + assert resp_json['data'][0]["delivered"] == 0 + assert resp_json['data'][0]["failed"] == 0 + + +def test_get_jobs_raises_for_bad_limit_days(client, sample_service): + path = '/service/{}/job/job-stats'.format(sample_service.id) + auth_header = create_authorization_header() + response = client.get(path, + query_string={'limit_days': 'bad_number'}, + headers=[auth_header]) + assert response.status_code == 400 + assert response.get_data(as_text=True) == '{\n "message": {\n "limit_days": [\n ' \ + '"bad_number is not an integer"\n ]\n },\n "result": "error"\n}' + + +def test_parse_status_turns_comma_sep_strings_into_list(): + statuses = "started, finished, pending" + from app.job.rest import _parse_statuses + assert _parse_statuses(statuses) == ["started", "finished", "pending"] + + +def test_parse_status_turns_empty_string_into_empty_list(): + statuses = "" + from app.job.rest import _parse_statuses + assert _parse_statuses(statuses) == [''] From 4fefec6aa34bb8528e78006f907b39fcfeb00fb7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Jun 2017 10:56:03 +0100 Subject: [PATCH 3/5] New endpoints to return job stats. Next step is to use the new endpoints in admin. --- app/dao/jobs_dao.py | 8 +++++++ app/job/rest.py | 18 ++++++++++++-- tests/app/dao/test_jobs_dao.py | 16 +++++++------ tests/app/job/test_rest.py | 44 ++++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 9 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 9d791b8ad..236ddcab8 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -159,6 +159,9 @@ def dao_get_job_statistics_for_job(service_id, job_id): Job.scheduled_for, Job.template_id, Job.template_version, + Job.job_status, + Job.service_id, + Job.notification_count, JobStatistics.sent, JobStatistics.delivered, JobStatistics.failed @@ -179,6 +182,11 @@ def dao_get_job_stats_for_service(service_id, page=1, page_size=50, limit_days=N Job.original_file_name, Job.created_at, Job.scheduled_for, + Job.template_id, + Job.template_version, + Job.job_status, + Job.service_id, + Job.notification_count, JobStatistics.sent, JobStatistics.delivered, JobStatistics.failed diff --git a/app/job/rest.py b/app/job/rest.py index 5e5bc33c1..6a8a86ee4 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -5,6 +5,7 @@ from flask import ( current_app ) +from app import DATETIME_FORMAT from app.dao.jobs_dao import ( dao_create_job, dao_update_job, @@ -12,7 +13,8 @@ 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_job_stats_for_service) + dao_get_job_stats_for_service, + dao_get_job_statistics_for_job) from app.dao.services_dao import ( dao_fetch_service_by_id @@ -57,6 +59,13 @@ def get_job_by_service_and_job_id(service_id, job_id): return jsonify(data=data) +@job_blueprint.route('/job-stats/', methods=['GET']) +def get_job_stats_by_service_and_job_id(service_id, job_id): + statistic = dao_get_job_statistics_for_job(service_id=service_id, job_id=job_id) + + return jsonify(_serialize_job_stats(statistic)) + + @job_blueprint.route('//cancel', methods=['POST']) def cancel_job(service_id, job_id): job = dao_get_future_scheduled_job_by_id_and_service_id(job_id, service_id) @@ -155,8 +164,13 @@ def _serialize_job_stats(stat): return { "job_id": stat.job_id, "original_file_name": stat.original_file_name, - "created_at": stat.created_at, + "created_at": stat.created_at.strftime(DATETIME_FORMAT), "scheduled_for": stat.scheduled_for, + "template_id": stat.template_id, + "template_version": stat.template_version, + "job_status": stat.job_status, + "service_id": stat.service_id, + "requested": stat.notification_count, "sent": stat.sent, "delivered": stat.delivered, "failed": stat.failed diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 32508c628..a4517aee5 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -429,16 +429,16 @@ def test_dao_get_job_statistics_for_job(notify_db, notify_db_session, sample_job update_job_stats_outcome_count(notification_delivered) update_job_stats_outcome_count(notification_failed) result = dao_get_job_statistics_for_job(sample_job.service_id, sample_job.id) - assert_job_stat(job=sample_job, result=result, sent=3, delivered=1, failed=1, with_template=True) + assert_job_stat(job=sample_job, result=result, sent=3, delivered=1, failed=1) def test_dao_get_job_statistics_for_job(notify_db, notify_db_session, sample_service): job_1, job_2 = stats_set_up(notify_db, notify_db_session, sample_service) result = dao_get_job_statistics_for_job(sample_service.id, job_1.id) - assert_job_stat(job=job_1, result=result, sent=2, delivered=1, failed=0, with_template=True) + assert_job_stat(job=job_1, result=result, sent=2, delivered=1, failed=0) result_2 = dao_get_job_statistics_for_job(sample_service.id, job_2.id) - assert_job_stat(job=job_2, result=result_2, sent=1, delivered=0, failed=1, with_template=True) + assert_job_stat(job=job_2, result=result_2, sent=1, delivered=0, failed=1) def test_dao_get_job_stats_for_service(notify_db, notify_db_session, sample_service): @@ -508,17 +508,19 @@ def test_dao_get_job_returns_jobs_for_status( assert results_2.total == 2 -def assert_job_stat(job, result, sent, delivered, failed, with_template=False): +def assert_job_stat(job, result, sent, delivered, failed): assert result.job_id == job.id assert result.original_file_name == job.original_file_name assert result.created_at == job.created_at assert result.scheduled_for == job.scheduled_for + assert result.template_id == job.template_id + assert result.template_version == job.template_version + assert result.job_status == job.job_status + assert result.service_id == job.service_id + assert result.notification_count == job.notification_count assert result.sent == sent assert result.delivered == delivered assert result.failed == failed - if with_template: - assert result.template_id == job.template_id - assert result.template_version == job.template_version def stats_set_up(notify_db, notify_db_session, service): diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 37fbe02dd..b7ba7689d 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -6,6 +6,7 @@ from freezegun import freeze_time import pytest import pytz import app.celery.tasks +from app import DATETIME_FORMAT from tests import create_authorization_header from tests.conftest import set_config @@ -779,6 +780,10 @@ def test_get_jobs_for_service_new_endpoint(client, notify_db, notify_db_session, assert resp_json['data'][0]["job_id"] assert resp_json['data'][0]["created_at"] assert not resp_json['data'][0]["scheduled_for"] + assert resp_json['data'][0]["template_id"] + assert resp_json['data'][0]["template_version"] + assert resp_json['data'][0]["service_id"] + assert resp_json['data'][0]["requested"] assert resp_json['data'][0]["sent"] == 0 assert resp_json['data'][0]["delivered"] == 0 assert resp_json['data'][0]["failed"] == 0 @@ -805,3 +810,42 @@ def test_parse_status_turns_empty_string_into_empty_list(): statuses = "" from app.job.rest import _parse_statuses assert _parse_statuses(statuses) == [''] + + +def test_get_job_stats_by_service_id_and_job_id(client, sample_job): + auth_header = create_authorization_header() + response = client.get("/service/{}/job/job-stats/{}".format(sample_job.service_id, sample_job.id), + headers=[auth_header]) + assert response.status_code == 200 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json["job_id"] == str(sample_job.id) + assert resp_json["created_at"] == sample_job.created_at.strftime(DATETIME_FORMAT) + assert not resp_json["scheduled_for"] + assert resp_json["template_id"] == str(sample_job.template_id) + assert resp_json["template_version"] == sample_job.template_version + assert resp_json["service_id"] == str(sample_job.service_id) + assert resp_json["requested"] == sample_job.notification_count + assert resp_json["sent"] == 0 + assert resp_json["delivered"] == 0 + assert resp_json["failed"] == 0 + + +def test_get_job_stats_with_invalid_job_id_returns404(client, sample_template): + service_id = sample_template.service.id + path = '/service/{}/job/job-=stats{}'.format(service_id, "bad-id") + auth_header = create_authorization_header() + response = client.get(path, headers=[auth_header]) + assert response.status_code == 404 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['result'] == 'error' + assert resp_json['message'] == 'No result found' + + +def test_get_job_stats_with_invalid_service_id_returns404(client, sample_job): + path = '/service/{}/job/job-=stats{}'.format(uuid.uuid4(), sample_job.id) + auth_header = create_authorization_header() + response = client.get(path, headers=[auth_header]) + assert response.status_code == 404 + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json['result'] == 'error' + assert resp_json['message'] == 'No result found' From a83e744c684ef91d69f7e59e1b63b535821de8df Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Jun 2017 11:55:14 +0100 Subject: [PATCH 4/5] Remove join to Template table. Fix typo in unit test. --- app/dao/jobs_dao.py | 7 ++----- tests/app/job/test_rest.py | 10 +++++----- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 236ddcab8..748b7cbaa 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -149,9 +149,8 @@ def dao_get_job_statistics_for_job(service_id, job_id): query = Job.query.join( JobStatistics, Job.id == JobStatistics.job_id ).filter( - Job.id == job_id - ).join( - Template, Template.id == Job.template_id and Template.version == Job.template_version + Job.id == job_id, + Job.service_id == service_id ).add_columns( JobStatistics.job_id, Job.original_file_name, @@ -165,8 +164,6 @@ def dao_get_job_statistics_for_job(service_id, job_id): JobStatistics.sent, JobStatistics.delivered, JobStatistics.failed - ).filter( - Job.service_id == service_id ) return query.one() diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index b7ba7689d..0ed3a533b 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -796,8 +796,9 @@ def test_get_jobs_raises_for_bad_limit_days(client, sample_service): query_string={'limit_days': 'bad_number'}, headers=[auth_header]) assert response.status_code == 400 - assert response.get_data(as_text=True) == '{\n "message": {\n "limit_days": [\n ' \ - '"bad_number is not an integer"\n ]\n },\n "result": "error"\n}' + resp_json = json.loads(response.get_data(as_text=True)) + assert resp_json["result"] == "error" + assert resp_json["message"] == {'limit_days': ['bad_number is not an integer']} def test_parse_status_turns_comma_sep_strings_into_list(): @@ -831,8 +832,7 @@ def test_get_job_stats_by_service_id_and_job_id(client, sample_job): def test_get_job_stats_with_invalid_job_id_returns404(client, sample_template): - service_id = sample_template.service.id - path = '/service/{}/job/job-=stats{}'.format(service_id, "bad-id") + path = '/service/{}/job/job-stats{}'.format(sample_template.service.id, uuid.uuid4()) auth_header = create_authorization_header() response = client.get(path, headers=[auth_header]) assert response.status_code == 404 @@ -842,7 +842,7 @@ def test_get_job_stats_with_invalid_job_id_returns404(client, sample_template): def test_get_job_stats_with_invalid_service_id_returns404(client, sample_job): - path = '/service/{}/job/job-=stats{}'.format(uuid.uuid4(), sample_job.id) + path = '/service/{}/job/job-stats{}'.format(uuid.uuid4(), sample_job.id) auth_header = create_authorization_header() response = client.get(path, headers=[auth_header]) assert response.status_code == 404 From b65ee1d99a9bfead1046be44259400af390e1de9 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 13 Jun 2017 12:08:09 +0100 Subject: [PATCH 5/5] Fix missing slash --- tests/app/job/test_rest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 0ed3a533b..b41dfe065 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -832,7 +832,7 @@ def test_get_job_stats_by_service_id_and_job_id(client, sample_job): def test_get_job_stats_with_invalid_job_id_returns404(client, sample_template): - path = '/service/{}/job/job-stats{}'.format(sample_template.service.id, uuid.uuid4()) + path = '/service/{}/job/job-stats/{}'.format(sample_template.service.id, uuid.uuid4()) auth_header = create_authorization_header() response = client.get(path, headers=[auth_header]) assert response.status_code == 404 @@ -842,7 +842,7 @@ def test_get_job_stats_with_invalid_job_id_returns404(client, sample_template): def test_get_job_stats_with_invalid_service_id_returns404(client, sample_job): - path = '/service/{}/job/job-stats{}'.format(uuid.uuid4(), sample_job.id) + path = '/service/{}/job/job-stats/{}'.format(uuid.uuid4(), sample_job.id) auth_header = create_authorization_header() response = client.get(path, headers=[auth_header]) assert response.status_code == 404