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

@@ -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

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('/<uuid:service_id>/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)

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):

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