From 8601b1359540a4404c1a34c6855e9574b4be62c2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 25 Feb 2020 16:05:39 +0000 Subject: [PATCH 1/2] Return template type for jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is so we can display letter jobs in a different way on the admin app (because it doesn’t make sense for them to have failed/delivered counts like it does for email and text message jobs). As elsewhere we use `fields.Method` to avoid serializing the whole template object. --- app/schemas.py | 5 +++++ tests/app/job/test_rest.py | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/app/schemas.py b/app/schemas.py index fc740c53e..5dd616617 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -380,6 +380,11 @@ class JobSchema(BaseSchema): service_name = fields.Nested( ServiceSchema, attribute="service", dump_to="service_name", only=["name"], dump_only=True) + template_type = fields.Method('get_template_type', dump_only=True) + + def get_template_type(self, job): + return job.template.template_type + @validates('scheduled_for') def validate_scheduled_for(self, value): _validate_datetime_not_in_past(value) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index e94cb2076..80383804a 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -1,6 +1,7 @@ import json import uuid from datetime import datetime, timedelta, date +from unittest.mock import ANY from freezegun import freeze_time import pytest @@ -653,6 +654,7 @@ def test_get_job_by_id_with_stats_for_old_job_where_notifications_have_been_purg assert resp_json['data']['created_by']['name'] == 'Test User' +@freeze_time('2017-07-17 07:17') def test_get_jobs(admin_request, sample_template): _setup_jobs(sample_template) @@ -660,6 +662,28 @@ def test_get_jobs(admin_request, sample_template): resp_json = admin_request.get('job.get_jobs_by_service', service_id=service_id) assert len(resp_json['data']) == 5 + assert resp_json['data'][0] == { + 'archived': False, + 'created_at': '2017-07-17T07:17:00+00:00', + 'created_by': { + 'id': ANY, + 'name': 'Test User', + }, + 'id': ANY, + 'job_status': 'pending', + 'notification_count': 1, + 'original_file_name': 'some.csv', + 'processing_finished': None, + 'processing_started': None, + 'scheduled_for': None, + 'service': str(sample_template.service.id), + 'service_name': {'name': sample_template.service.name}, + 'statistics': [], + 'template': str(sample_template.id), + 'template_type': 'sms', + 'template_version': 1, + 'updated_at': None, + } def test_get_jobs_with_limit_days(admin_request, sample_template): From 57e671267c1fa5fe95fd915ad875482586f59bd2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 25 Feb 2020 17:13:01 +0000 Subject: [PATCH 2/2] Return template type in uploads response We need template type for the uploads response, which will eventually supercede the jobs response. At the moment the page uses both. --- app/dao/uploads_dao.py | 4 ++++ app/upload/rest.py | 4 +++- tests/app/dao/test_uploads_dao.py | 28 ++++++++++++++++++++++++---- tests/app/upload/test_rest.py | 5 +++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/app/dao/uploads_dao.py b/app/dao/uploads_dao.py index 3fb23173e..fe5aa69f6 100644 --- a/app/dao/uploads_dao.py +++ b/app/dao/uploads_dao.py @@ -25,11 +25,14 @@ def dao_get_uploads_by_service_id(service_id, limit_days=None, page=1, page_size Job.id, Job.original_file_name, Job.notification_count, + Template.template_type, Job.created_at.label("created_at"), Job.scheduled_for.label("scheduled_for"), Job.processing_started.label('processing_started'), Job.job_status.label("status"), literal('job').label('upload_type') + ).join( + Template, Job.template_id == Template.id ).filter( *jobs_query_filter ) @@ -49,6 +52,7 @@ def dao_get_uploads_by_service_id(service_id, limit_days=None, page=1, page_size Notification.id, Notification.client_reference.label('original_file_name'), literal('1').label('notification_count'), + literal(None).label('template_type'), Notification.created_at.label("created_at"), literal(None).label('scheduled_for'), # letters don't have a processing_started date but we want created_at to be used for sorting diff --git a/app/upload/rest.py b/app/upload/rest.py index 048f714cc..8859e925d 100644 --- a/app/upload/rest.py +++ b/app/upload/rest.py @@ -43,7 +43,8 @@ def get_paginated_uploads(service_id, limit_days, page): 'notification_count': upload.notification_count, 'created_at': upload.scheduled_for.strftime( "%Y-%m-%d %H:%M:%S") if upload.scheduled_for else upload.created_at.strftime("%Y-%m-%d %H:%M:%S"), - 'upload_type': upload.upload_type + 'upload_type': upload.upload_type, + 'template_type': None, } if upload.upload_type == 'job': start = upload.processing_started @@ -58,6 +59,7 @@ def get_paginated_uploads(service_id, limit_days, page): statistics = dao_get_notification_outcomes_for_job(service_id, upload.id) upload_dict['statistics'] = [{'status': statistic.status, 'count': statistic.count} for statistic in statistics] + upload_dict['template_type'] = upload.template_type else: upload_dict['statistics'] = [{'status': upload.status, 'count': 1}] data.append(upload_dict) diff --git a/tests/app/dao/test_uploads_dao.py b/tests/app/dao/test_uploads_dao.py index 1fde9c58e..c8a52f057 100644 --- a/tests/app/dao/test_uploads_dao.py +++ b/tests/app/dao/test_uploads_dao.py @@ -46,15 +46,34 @@ def test_get_uploads_for_service(sample_template): assert len(uploads_from_db) == 2 - assert uploads_from_db[0] == (letter.id, letter.client_reference, 1, letter.created_at, - None, letter.created_at, letter.status, "letter") - assert uploads_from_db[1] == (job.id, job.original_file_name, job.notification_count, job.created_at, - job.scheduled_for, job.processing_started, job.job_status, "job") + assert uploads_from_db[0] == ( + letter.id, + letter.client_reference, + 1, + None, + letter.created_at, + None, + letter.created_at, + letter.status, + "letter", + ) + assert uploads_from_db[1] == ( + job.id, + job.original_file_name, + job.notification_count, + 'sms', + job.created_at, + job.scheduled_for, + job.processing_started, + job.job_status, + "job", + ) assert len(other_uploads_from_db) == 2 assert other_uploads_from_db[0] == (other_letter.id, other_letter.client_reference, 1, + None, other_letter.created_at, None, other_letter.created_at, @@ -63,6 +82,7 @@ def test_get_uploads_for_service(sample_template): assert other_uploads_from_db[1] == (other_job.id, other_job.original_file_name, other_job.notification_count, + other_job.template.template_type, other_job.created_at, other_job.scheduled_for, other_job.processing_started, diff --git a/tests/app/upload/test_rest.py b/tests/app/upload/test_rest.py index 3b508ccba..dc11f1ed0 100644 --- a/tests/app/upload/test_rest.py +++ b/tests/app/upload/test_rest.py @@ -56,12 +56,14 @@ def test_get_uploads(admin_request, sample_template): assert data[0] == {'id': str(upload_5.id), 'original_file_name': 'some.csv', 'notification_count': 10, + 'template_type': 'sms', 'created_at': upload_5.created_at.strftime("%Y-%m-%d %H:%M:%S"), 'statistics': [], 'upload_type': 'job'} assert data[1] == {'id': str(upload_4.id), 'original_file_name': 'some.csv', 'notification_count': 1, + 'template_type': 'sms', 'created_at': upload_4.created_at.strftime( "%Y-%m-%d %H:%M:%S"), 'statistics': [], @@ -69,12 +71,14 @@ def test_get_uploads(admin_request, sample_template): assert data[2] == {'id': str(upload_3.id), 'original_file_name': "file-name", 'notification_count': 1, + 'template_type': None, 'created_at': upload_3.created_at.strftime("%Y-%m-%d %H:%M:%S"), 'statistics': [{'count': 1, 'status': 'delivered'}], 'upload_type': 'letter'} assert data[3] == {'id': str(upload_2.id), 'original_file_name': "some.csv", 'notification_count': 1, + 'template_type': 'sms', 'created_at': upload_2.created_at.strftime( "%Y-%m-%d %H:%M:%S"), 'statistics': [], @@ -82,6 +86,7 @@ def test_get_uploads(admin_request, sample_template): assert data[4] == {'id': str(upload_1.id), 'original_file_name': "file-name", 'notification_count': 1, + 'template_type': None, 'created_at': upload_1.created_at.strftime("%Y-%m-%d %H:%M:%S"), 'statistics': [{'count': 1, 'status': 'delivered'}], 'upload_type': 'letter'}