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

@@ -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(
+    results = []
-        ReturnedLetter.notification_id,
+    for table in [Notification, NotificationHistory]:
-        ReturnedLetter.reported_at
+        query = db.session.query(
-    ).filter(
+            ReturnedLetter.notification_id,
-        ReturnedLetter.service_id == service_id,
+            ReturnedLetter.reported_at,
-        func.date(ReturnedLetter.reported_at) == report_date
+            table.client_reference,
-    ).order_by(
+            table.created_at,
-        desc(ReturnedLetter.reported_at)
+            Template.name.label('template_name'),
-    ).all()
+            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

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

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)
+    letter_1 = create_notification(template=sample_letter_template, client_reference='letter_1',
-    create_returned_letter(service=sample_service, reported_at=today)
+                                   status=NOTIFICATION_RETURNED_LETTER,
-    create_returned_letter(service=sample_service, reported_at=last_month)
+                                   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)

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(),
     )
 

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