Update the resultset to relect how users will consume the information.

- Do not show "hidden" or precompiled templates, users don't know about them.
- Remove the client reference if it is the file name of an uploaded file.
- Format the date for created_at
- Added a test for all the different types of letters.
 1) One off templated letter
 2) Letter created by a csv upload or job.
 3) Uploaded letter
 4) Templated letter sent by the API
 5) Precompiled letter sent by the API

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,

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),
+         'created_at': x.created_at.strftime("%Y-%m-%d %H:%M:%S"),
-         'template_name': x.template_name,
+         # it doesn't make sense to show hidden/precompiled templates
-         'template_id': x.template_id,
+         'template_name': x.template_name if not x.hidden else None,
-         'template_version': x.template_version,
+         'template_id': x.template_id if not x.hidden else None,
-         'user_name': x.user_name,
+         'template_version': x.template_version if not x.hidden else None,
-         'email_address': x.email_address,
+         '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))

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,
+                          sample_letter_template.name, letter_1.template_id, letter_1.template_version, False,
-                          None, None, None, None, None)
+                          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)

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',
+    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=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)
+                           notification_id=letter_from_job.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=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 len(response) == 5
-    assert response[0]['notification_id'] == str(letter_3.id)
+    assert response[0]['notification_id'] == str(letter_from_job.id)
-    assert response[0]['client_reference'] == 'letter_3'
+    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 response[0]['original_file_name'] == job.original_file_name
-    assert not response[0]['job_row_number']
+    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]['notification_id'] == str(one_off_letter.id)
-    assert response[1]['client_reference'] == 'letter_2'
+    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]['user_name'] == sample_letter_template.service.users[0].name
-    assert response[1]['original_file_name'] == job.original_file_name
+    assert not response[1]['original_file_name']
-    assert response[1]['job_row_number'] == 3
+    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'