diff --git a/app/dao/returned_letters_dao.py b/app/dao/returned_letters_dao.py index a70c0ca9e..09dc95e00 100644 --- a/app/dao/returned_letters_dao.py +++ b/app/dao/returned_letters_dao.py @@ -74,6 +74,8 @@ def fetch_returned_letters(service_id, report_date): Template.name.label('template_name'), table.template_id, table.template_version, + Template.hidden, + table.api_key_id, table.created_by_id, User.name.label('user_name'), User.email_address, diff --git a/app/service/rest.py b/app/service/rest.py index befd7b22f..e266911b4 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -12,7 +12,7 @@ from notifications_utils.timezones import convert_utc_to_bst from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound -from app import DATE_FORMAT, DATETIME_FORMAT +from app import DATE_FORMAT from app.config import QueueNames from app.dao import fact_notification_status_dao, notifications_dao from app.dao.dao_utils import dao_rollback @@ -963,17 +963,20 @@ def get_returned_letters(service_id): json_results = [ {'notification_id': x.notification_id, - 'client_reference': x.client_reference, + # client reference can only be added on API letters + 'client_reference': x.client_reference if x.api_key_id else None, 'reported_at': x.reported_at.strftime(DATE_FORMAT), - 'created_at': x.created_at.strftime(DATETIME_FORMAT), - 'template_name': x.template_name, - 'template_id': x.template_id, - 'template_version': x.template_version, - 'user_name': x.user_name, - 'email_address': x.email_address, + 'created_at': x.created_at.strftime("%Y-%m-%d %H:%M:%S"), + # it doesn't make sense to show hidden/precompiled templates + 'template_name': x.template_name if not x.hidden else None, + 'template_id': x.template_id if not x.hidden else None, + 'template_version': x.template_version if not x.hidden else None, + 'user_name': x.user_name or 'API', + 'email_address': x.email_address or 'API', 'original_file_name': x.original_file_name, 'job_row_number': x.job_row_number, - 'uploaded_letter': x.client_reference if x.user_name and not x.original_file_name else None + # the file name for a letter uploaded via the UI + 'uploaded_letter': x.client_reference if x.hidden and not x.api_key_id else None } for x in results] return jsonify(sorted(json_results, key=lambda i: i['created_at'], reverse=True)) diff --git a/tests/app/dao/test_returned_letters_dao.py b/tests/app/dao/test_returned_letters_dao.py index 78f23c2c0..17312ddfa 100644 --- a/tests/app/dao/test_returned_letters_dao.py +++ b/tests/app/dao/test_returned_letters_dao.py @@ -144,11 +144,11 @@ def test_fetch_returned_letters_from_notifications_and_notification_history(samp assert len(results) == 2 assert results[0] == (letter_2.id, returned_letter_2.reported_at, letter_2.client_reference, letter_2.created_at, - sample_letter_template.name, letter_2.template_id, letter_2.template_version, + sample_letter_template.name, letter_2.template_id, letter_2.template_version, False, None, None, None, None, None, None) assert results[1] == (letter_1.id, returned_letter_1.reported_at, letter_1.client_reference, letter_1.created_at, - sample_letter_template.name, letter_1.template_id, letter_1.template_version, - None, None, None, None, None) + sample_letter_template.name, letter_1.template_id, letter_1.template_version, False, + letter_1.api_key_id, None, None, None, None, None) def test_fetch_returned_letters_with_jobs(sample_letter_job): @@ -163,7 +163,7 @@ def test_fetch_returned_letters_with_jobs(sample_letter_job): results = fetch_returned_letters(service_id=sample_letter_job.service_id, report_date=today.date()) assert len(results) == 1 assert results[0] == (letter_1.id, returned_letter_1.reported_at, letter_1.client_reference, letter_1.created_at, - sample_letter_job.template.name, letter_1.template_id, letter_1.template_version, + sample_letter_job.template.name, letter_1.template_id, letter_1.template_version, False, None, None, None, None, sample_letter_job.original_file_name, 21) @@ -179,6 +179,6 @@ def test_fetch_returned_letters_with_create_by_user(sample_letter_template): results = fetch_returned_letters(service_id=sample_letter_template.service_id, report_date=today.date()) assert len(results) == 1 assert results[0] == (letter_1.id, returned_letter_1.reported_at, letter_1.client_reference, letter_1.created_at, - sample_letter_template.name, letter_1.template_id, letter_1.template_version, + sample_letter_template.name, letter_1.template_id, letter_1.template_version, False, None, letter_1.created_by_id, sample_letter_template.service.users[0].name, sample_letter_template.service.users[0].email_address, None, None) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index b2020d385..70159bd0a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -50,7 +50,8 @@ from tests.app.db import ( create_annual_billing, create_returned_letter, create_notification_history, - create_job + create_job, + create_api_key ) from tests.app.db import create_user @@ -3401,19 +3402,43 @@ def test_get_returned_letter(admin_request, sample_letter_template): notification_id=letter_1.id) job = create_job(template=sample_letter_template) - letter_2 = create_notification(template=sample_letter_template, client_reference='letter_2', - status=NOTIFICATION_RETURNED_LETTER, - job=job, job_row_number=2, - created_at=datetime.utcnow() - timedelta(days=5)) + letter_from_job = create_notification(template=sample_letter_template, client_reference='letter_from_job', + status=NOTIFICATION_RETURNED_LETTER, + job=job, job_row_number=2, + created_at=datetime.utcnow() - timedelta(days=1), + created_by_id=sample_letter_template.service.users[0].id) create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), - notification_id=letter_2.id) + notification_id=letter_from_job.id) - letter_3 = create_notification(template=sample_letter_template, client_reference='letter_3', - status=NOTIFICATION_RETURNED_LETTER, - created_at=datetime.utcnow() - timedelta(days=1), - created_by_id=sample_letter_template.service.users[0].id) + one_off_letter = create_notification(template=sample_letter_template, + status=NOTIFICATION_RETURNED_LETTER, + created_at=datetime.utcnow() - timedelta(days=2), + created_by_id=sample_letter_template.service.users[0].id) create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), - notification_id=letter_3.id) + notification_id=one_off_letter.id) + + api_key = create_api_key(service=sample_letter_template.service) + api_letter = create_notification(template=sample_letter_template, client_reference='api_letter', + status=NOTIFICATION_RETURNED_LETTER, + created_at=datetime.utcnow() - timedelta(days=3), + api_key=api_key) + create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), + notification_id=api_letter.id) + + precompiled_template = create_template(service=sample_letter_template.service, template_type='letter', hidden=True, + template_name='hidden template') + precompiled_letter = create_notification_history(template=precompiled_template, api_key=api_key, + client_reference='precompiled letter', + created_at=datetime.utcnow() - timedelta(days=4)) + create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), + notification_id=precompiled_letter.id) + + uploaded_letter = create_notification_history(template=precompiled_template, client_reference='filename.pdf', + created_at=datetime.utcnow() - timedelta(days=5), + created_by_id=sample_letter_template.service.users[0].id) + create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), + notification_id=uploaded_letter.id) + not_included_in_results = create_template(service=create_service(service_name='not included in results'), template_type='letter') letter_4 = create_notification_history(template=not_included_in_results, @@ -3423,25 +3448,64 @@ def test_get_returned_letter(admin_request, sample_letter_template): response = admin_request.get('service.get_returned_letters', service_id=sample_letter_template.service_id, reported_at='2019-12-11') - assert len(response) == 2 - assert response[0]['notification_id'] == str(letter_3.id) - assert response[0]['client_reference'] == 'letter_3' + assert len(response) == 5 + assert response[0]['notification_id'] == str(letter_from_job.id) + assert not response[0]['client_reference'] assert response[0]['reported_at'] == '2019-12-11' - assert response[0]['created_at'] == '2019-12-10T13:30:00.000000Z' + assert response[0]['created_at'] == '2019-12-10 13:30:00' assert response[0]['template_name'] == sample_letter_template.name assert response[0]['template_id'] == str(sample_letter_template.id) assert response[0]['template_version'] == sample_letter_template.version assert response[0]['user_name'] == sample_letter_template.service.users[0].name - assert not response[0]['original_file_name'] - assert not response[0]['job_row_number'] + assert response[0]['original_file_name'] == job.original_file_name + assert response[0]['job_row_number'] == 3 + assert not response[0]['uploaded_letter'] - assert response[1]['notification_id'] == str(letter_2.id) - assert response[1]['client_reference'] == 'letter_2' + assert response[1]['notification_id'] == str(one_off_letter.id) + assert not response[1]['client_reference'] assert response[1]['reported_at'] == '2019-12-11' - assert response[1]['created_at'] == '2019-12-06T13:30:00.000000Z' + assert response[1]['created_at'] == '2019-12-09 13:30:00' assert response[1]['template_name'] == sample_letter_template.name assert response[1]['template_id'] == str(sample_letter_template.id) assert response[1]['template_version'] == sample_letter_template.version - assert not response[1]['user_name'] - assert response[1]['original_file_name'] == job.original_file_name - assert response[1]['job_row_number'] == 3 + assert response[1]['user_name'] == sample_letter_template.service.users[0].name + assert not response[1]['original_file_name'] + assert not response[1]['job_row_number'] + assert not response[1]['uploaded_letter'] + + assert response[2]['notification_id'] == str(api_letter.id) + assert response[2]['client_reference'] == 'api_letter' + assert response[2]['reported_at'] == '2019-12-11' + assert response[2]['created_at'] == '2019-12-08 13:30:00' + assert response[2]['template_name'] == sample_letter_template.name + assert response[2]['template_id'] == str(sample_letter_template.id) + assert response[2]['template_version'] == sample_letter_template.version + assert response[2]['user_name'] == 'API' + assert not response[2]['original_file_name'] + assert not response[2]['job_row_number'] + assert not response[2]['uploaded_letter'] + + assert response[3]['notification_id'] == str(precompiled_letter.id) + assert response[3]['client_reference'] == 'precompiled letter' + assert response[3]['reported_at'] == '2019-12-11' + assert response[3]['created_at'] == '2019-12-07 13:30:00' + assert not response[3]['template_name'] + assert not response[3]['template_id'] + assert not response[3]['template_version'] + assert response[3]['user_name'] == 'API' + assert not response[3]['original_file_name'] + assert not response[3]['job_row_number'] + assert not response[3]['uploaded_letter'] + + assert response[4]['notification_id'] == str(uploaded_letter.id) + assert not response[4]['client_reference'] + assert response[4]['reported_at'] == '2019-12-11' + assert response[4]['created_at'] == '2019-12-06 13:30:00' + assert not response[4]['template_name'] + assert not response[4]['template_id'] + assert not response[4]['template_version'] + assert response[4]['user_name'] == sample_letter_template.service.users[0].name + assert response[4]['email_address'] == sample_letter_template.service.users[0].email_address + assert not response[4]['original_file_name'] + assert not response[4]['job_row_number'] + assert response[4]['uploaded_letter'] == 'filename.pdf'