From b853c4cdf1d8888f2df0d4b2232b4cda4518d34e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 18 Dec 2019 11:31:33 +0000 Subject: [PATCH] Rename dao method to be more consistent. Fix sort. Add one to job_row_number, rows start at 0 which would confus the user. --- app/dao/returned_letters_dao.py | 17 +++++++++++------ app/service/rest.py | 7 +++++-- tests/app/dao/test_returned_letters_dao.py | 9 +++++---- tests/app/service/test_rest.py | 5 +++-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/app/dao/returned_letters_dao.py b/app/dao/returned_letters_dao.py index 8379a9103..17bf32906 100644 --- a/app/dao/returned_letters_dao.py +++ b/app/dao/returned_letters_dao.py @@ -5,7 +5,14 @@ from sqlalchemy.dialects.postgresql import insert from app import db from app.dao.dao_utils import transactional -from app.models import Notification, NotificationHistory, ReturnedLetter, Job, User, Template +from app.models import ( + Job, + Notification, + NotificationHistory, + ReturnedLetter, + Template, + User, +) def _get_notification_ids_for_references(references): @@ -43,7 +50,7 @@ def insert_or_update_returned_letters(references): db.session.connection().execute(stmt) -def get_returned_letter_summary(service_id): +def fetch_returned_letter_summary(service_id): return db.session.query( func.count(ReturnedLetter.notification_id).label('returned_letter_count'), ReturnedLetter.reported_at @@ -70,7 +77,7 @@ def fetch_returned_letters(service_id, report_date): table.created_by_id, User.name.label('user_name'), Job.original_file_name, - table.job_row_number + (table.job_row_number + 1).label('job_row_number') # row numbers start at 0 ).outerjoin( User, table.created_by_id == User.id ).outerjoin( @@ -84,7 +91,5 @@ def fetch_returned_letters(service_id, report_date): desc(ReturnedLetter.reported_at), desc(table.created_at) ) results = results + query.all() - - results = sorted(results[:4], reverse=True) - + results = sorted(results, key=lambda i: i.created_at, reverse=True) return results diff --git a/app/service/rest.py b/app/service/rest.py index 7fb4d5b39..920cfa408 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -30,7 +30,10 @@ 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, fetch_returned_letters +from app.dao.returned_letters_dao import ( + fetch_returned_letter_summary, + fetch_returned_letters +) from app.dao.service_data_retention_dao import ( fetch_service_data_retention, fetch_service_data_retention_by_id, @@ -945,7 +948,7 @@ def check_if_reply_to_address_already_in_use(service_id, email_address): @service_blueprint.route('//returned-letter-summary', methods=['GET']) def returned_letter_summary(service_id): - results = get_returned_letter_summary(service_id) + results = fetch_returned_letter_summary(service_id) json_results = [{'returned_letter_count': x.returned_letter_count, 'reported_at': x.reported_at.strftime(DATE_FORMAT) diff --git a/tests/app/dao/test_returned_letters_dao.py b/tests/app/dao/test_returned_letters_dao.py index 1d59703e9..dbb036af1 100644 --- a/tests/app/dao/test_returned_letters_dao.py +++ b/tests/app/dao/test_returned_letters_dao.py @@ -3,7 +3,8 @@ from datetime import datetime, timedelta, date from freezegun import freeze_time from app.dao.returned_letters_dao import ( - insert_or_update_returned_letters, get_returned_letter_summary, + insert_or_update_returned_letters, + fetch_returned_letter_summary, fetch_returned_letters ) from app.models import ReturnedLetter, NOTIFICATION_RETURNED_LETTER @@ -94,7 +95,7 @@ def test_get_returned_letter_summary(sample_service): create_returned_letter(sample_service, reported_at=now) create_returned_letter(sample_service, reported_at=now) - results = get_returned_letter_summary(sample_service.id) + results = fetch_returned_letter_summary(sample_service.id) assert len(results) == 1 @@ -112,7 +113,7 @@ def test_get_returned_letter_summary_orders_by_reported_at(sample_service): create_returned_letter(sample_service, reported_at=last_month) create_returned_letter() # returned letter for a different service - results = get_returned_letter_summary(sample_service.id) + results = fetch_returned_letter_summary(sample_service.id) assert len(results) == 2 assert results[0].reported_at == now.date() @@ -163,7 +164,7 @@ def test_fetch_returned_letters_with_jobs(sample_letter_job): 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) + letter_1.created_by_id, None, sample_letter_job.original_file_name, 21) def test_fetch_returned_letters_with_create_by_user(sample_letter_template): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 9525810d8..b2020d385 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -48,7 +48,8 @@ from tests.app.db import ( create_domain, create_email_branding, create_annual_billing, - create_returned_letter, create_notification_history, + create_returned_letter, + create_notification_history, create_job ) from tests.app.db import create_user @@ -3443,4 +3444,4 @@ def test_get_returned_letter(admin_request, sample_letter_template): 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 + assert response[1]['job_row_number'] == 3