From aabaa4a971b1ce9e1d25cddf2d069a89d918a45a Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 16 Dec 2019 17:25:57 +0000 Subject: [PATCH] Added joins to template, job and user for returned letter query. Added unit tests Comleted endpoint to get returned letter details --- app/dao/returned_letters_dao.py | 43 +++++++++++---- app/service/rest.py | 21 ++++++-- tests/app/dao/test_returned_letters_dao.py | 60 ++++++++++++++++++--- tests/app/db.py | 7 +-- tests/app/service/test_rest.py | 61 +++++++++++++++++++++- 5 files changed, 164 insertions(+), 28 deletions(-) diff --git a/app/dao/returned_letters_dao.py b/app/dao/returned_letters_dao.py index ebeb30055..8379a9103 100644 --- a/app/dao/returned_letters_dao.py +++ b/app/dao/returned_letters_dao.py @@ -5,7 +5,7 @@ from sqlalchemy.dialects.postgresql import insert from app import db from app.dao.dao_utils import transactional -from app.models import Notification, NotificationHistory, ReturnedLetter +from app.models import Notification, NotificationHistory, ReturnedLetter, Job, User, Template def _get_notification_ids_for_references(references): @@ -56,14 +56,35 @@ def get_returned_letter_summary(service_id): ).all() - def fetch_returned_letters(service_id, report_date): - return db.session.query( - ReturnedLetter.notification_id, - ReturnedLetter.reported_at - ).filter( - ReturnedLetter.service_id == service_id, - func.date(ReturnedLetter.reported_at) == report_date - ).order_by( - desc(ReturnedLetter.reported_at) - ).all() + results = [] + for table in [Notification, NotificationHistory]: + query = db.session.query( + ReturnedLetter.notification_id, + ReturnedLetter.reported_at, + table.client_reference, + table.created_at, + Template.name.label('template_name'), + table.template_id, + table.template_version, + table.created_by_id, + User.name.label('user_name'), + Job.original_file_name, + table.job_row_number + ).outerjoin( + User, table.created_by_id == User.id + ).outerjoin( + Job, table.job_id == Job.id + ).filter( + ReturnedLetter.service_id == service_id, + ReturnedLetter.reported_at == report_date, + ReturnedLetter.notification_id == table.id, + table.template_id == Template.id + ).order_by( + desc(ReturnedLetter.reported_at), desc(table.created_at) + ) + results = results + query.all() + + results = sorted(results[:4], reverse=True) + + return results diff --git a/app/service/rest.py b/app/service/rest.py index 0d7518c4f..7fb4d5b39 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 +from app import DATE_FORMAT, DATETIME_FORMAT from app.config import QueueNames from app.dao import fact_notification_status_dao, notifications_dao from app.dao.dao_utils import dao_rollback @@ -30,7 +30,7 @@ from app.dao.fact_notification_status_dao import ( ) from app.dao.inbound_numbers_dao import dao_allocate_number_for_service from app.dao.organisation_dao import dao_get_organisation_by_service_id -from app.dao.returned_letters_dao import get_returned_letter_summary +from app.dao.returned_letters_dao import get_returned_letter_summary, fetch_returned_letters from app.dao.service_data_retention_dao import ( fetch_service_data_retention, fetch_service_data_retention_by_id, @@ -956,8 +956,19 @@ def returned_letter_summary(service_id): @service_blueprint.route('//returned-letters', methods=['GET']) def get_returned_letters(service_id): - results = get_returned_letter_summary(service_id) + results = fetch_returned_letters(service_id=service_id, report_date=request.args.get('reported_at')) - json_results = [{'returned_letter_count': x.returned_letter_count, 'reported_at': x.reported_at} for x in results] + json_results = [ + {'notification_id': x.notification_id, + 'client_reference': x.client_reference, + '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, + 'original_file_name': x.original_file_name, + 'job_row_number': x.job_row_number + } for x in results] - return jsonify(json_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 ce3677716..1d59703e9 100644 --- a/tests/app/dao/test_returned_letters_dao.py +++ b/tests/app/dao/test_returned_letters_dao.py @@ -6,7 +6,7 @@ from app.dao.returned_letters_dao import ( insert_or_update_returned_letters, get_returned_letter_summary, fetch_returned_letters ) -from app.models import ReturnedLetter +from app.models import ReturnedLetter, NOTIFICATION_RETURNED_LETTER from tests.app.db import create_notification, create_notification_history, create_returned_letter @@ -121,14 +121,62 @@ def test_get_returned_letter_summary_orders_by_reported_at(sample_service): assert results[1].returned_letter_count == 2 -def test_fetch_returned_letters(sample_service): +def test_fetch_returned_letters_from_notifications_and_notification_history(sample_letter_template): today = datetime.now() last_month = datetime.now() - timedelta(days=30) - create_returned_letter(service=sample_service, reported_at=today) - create_returned_letter(service=sample_service, reported_at=today) - create_returned_letter(service=sample_service, reported_at=last_month) + letter_1 = create_notification(template=sample_letter_template, client_reference='letter_1', + status=NOTIFICATION_RETURNED_LETTER, + created_at=datetime.utcnow() - timedelta(days=1)) + returned_letter_1 = create_returned_letter(service=sample_letter_template.service, reported_at=today, + notification_id=letter_1.id) + letter_2 = create_notification_history(template=sample_letter_template, client_reference='letter_2', + status=NOTIFICATION_RETURNED_LETTER, created_at=datetime.utcnow()) + returned_letter_2 = create_returned_letter(service=sample_letter_template.service, reported_at=today, + notification_id=letter_2.id) + letter_3 = create_notification_history(template=sample_letter_template, client_reference='letter_3', + status=NOTIFICATION_RETURNED_LETTER) + create_returned_letter(service=sample_letter_template.service, reported_at=last_month, + notification_id=letter_3.id) - results = fetch_returned_letters(service_id=sample_service.id, report_date=today.date()) + results = fetch_returned_letters(service_id=sample_letter_template.service_id, report_date=today.date()) 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, + letter_2.created_by_id, 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, + letter_1.created_by_id, None, None, None) + + +def test_fetch_returned_letters_with_jobs(sample_letter_job): + today = datetime.now() + letter_1 = create_notification_history(template=sample_letter_job.template, client_reference='letter_1', + status=NOTIFICATION_RETURNED_LETTER, + job=sample_letter_job, job_row_number=20, + created_at=datetime.utcnow() - timedelta(minutes=1)) + returned_letter_1 = create_returned_letter(service=sample_letter_job.service, reported_at=today, + notification_id=letter_1.id) + + 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, + letter_1.created_by_id, None, sample_letter_job.original_file_name, letter_1.job_row_number) + + +def test_fetch_returned_letters_with_create_by_user(sample_letter_template): + today = datetime.now() + letter_1 = create_notification_history(template=sample_letter_template, client_reference='letter_1', + status=NOTIFICATION_RETURNED_LETTER, + created_at=datetime.utcnow() - timedelta(minutes=1), + created_by_id=sample_letter_template.service.users[0].id) + returned_letter_1 = create_returned_letter(service=sample_letter_template.service, reported_at=today, + notification_id=letter_1.id) + + 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, + letter_1.created_by_id, sample_letter_template.service.users[0].name, None, None) diff --git a/tests/app/db.py b/tests/app/db.py index 8959d8660..6f5feed9f 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -60,7 +60,6 @@ from app.models import ( LetterBranding, Domain, NotificationHistory, - NOTIFICATION_RETURNED_LETTER, ReturnedLetter ) @@ -944,15 +943,13 @@ def set_up_usage_data(start_date): return org, org_3, service, service_3, service_4, service_sms_only -def create_returned_letter(service=None, reported_at=None): +def create_returned_letter(service=None, reported_at=None, notification_id=None): if not service: service = create_service(service_name='a - with sms and letter') - template = create_template(service=service, template_type=LETTER_TYPE) - notification = create_notification(template=template, status=NOTIFICATION_RETURNED_LETTER) returned_letter = ReturnedLetter( service_id=service.id, reported_at=reported_at or datetime.utcnow(), - notification_id=notification.id, + notification_id=notification_id or uuid.uuid4(), created_at=datetime.utcnow(), ) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ec736abd1..9525810d8 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -27,6 +27,7 @@ from app.models import ( KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE, + NOTIFICATION_RETURNED_LETTER ) from tests import create_authorization_header from tests.app.db import ( @@ -47,7 +48,9 @@ from tests.app.db import ( create_domain, create_email_branding, create_annual_billing, - create_returned_letter) + create_returned_letter, create_notification_history, + create_job +) from tests.app.db import create_user @@ -3385,3 +3388,59 @@ def test_get_returned_letter_summary(admin_request, sample_service): assert len(response) == 2 assert response[0] == {'returned_letter_count': 2, 'reported_at': '2019-12-11'} assert response[1] == {'returned_letter_count': 1, 'reported_at': '2019-12-08'} + + +@freeze_time('2019-12-11 13:30') +def test_get_returned_letter(admin_request, sample_letter_template): + letter_1 = create_notification_history(template=sample_letter_template, client_reference='letter_1', + status=NOTIFICATION_RETURNED_LETTER, + created_at=datetime.utcnow() - timedelta(minutes=1), + created_by_id=sample_letter_template.service.users[0].id) + create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow() - timedelta(days=3), + 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)) + create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), + notification_id=letter_2.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) + create_returned_letter(service=sample_letter_template.service, reported_at=datetime.utcnow(), + notification_id=letter_3.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, + status=NOTIFICATION_RETURNED_LETTER) + create_returned_letter(service=not_included_in_results.service, reported_at=datetime.utcnow(), + notification_id=letter_4.id) + 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 response[0]['reported_at'] == '2019-12-11' + assert response[0]['created_at'] == '2019-12-10T13:30:00.000000Z' + 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[1]['notification_id'] == str(letter_2.id) + assert response[1]['client_reference'] == 'letter_2' + assert response[1]['reported_at'] == '2019-12-11' + assert response[1]['created_at'] == '2019-12-06T13:30:00.000000Z' + 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'] == 2