Merge pull request #3236 from alphagov/returned-letters-tweaks

Returned letters tweaks

app/assets/stylesheets/components/table.scss

@@ -324,11 +324,12 @@
   border-bottom: 1px solid $border-colour;
 }
 
-.table-empty-message {
+.table-empty-message,
-  @include core-16;
+td.table-empty-message {
+  @include core-19;
   color: $secondary-text-colour;
   border-bottom: 1px solid $border-colour;
-  padding: 0.75em 0 0.5625em 0;
+  padding: 20px 0 20px 0;
 }
 
 .table-show-more-link {
@@ -337,7 +338,7 @@
   color: $secondary-text-colour;
   margin-bottom: $gutter * 1.3333;
   border-bottom: 1px solid $border-colour;
-  padding: 10px 0 10px 0;
+  padding: 35px 0 10px 0;
   text-align: center;
 
   .table + & {

app/assets/stylesheets/views/dashboard.scss

@@ -62,6 +62,10 @@
     margin-top: -10px;
   }
 
+  &-filename-unlinked {
+    @include core-19;
+  }
+
   &-hint {
     @include core-16;
     display: block;

app/main/views/returned_letters.py

@@ -7,7 +7,7 @@ from app.main import main
 from app.utils import Spreadsheet, user_has_permissions
 
 
-@main.route("/services/<uuid:service_id>/returned-letter-summary", methods=["GET"])
+@main.route("/services/<uuid:service_id>/returned-letters")
 @user_has_permissions('view_activity')
 def returned_letter_summary(service_id):
     summary = service_api_client.get_returned_letter_summary(service_id)
@@ -17,7 +17,25 @@ def returned_letter_summary(service_id):
     )
 
 
-@main.route("/services/<uuid:service_id>/returned-letters-csv/<simple_date:reported_at>", methods=["GET"])
+@main.route("/services/<uuid:service_id>/returned-letters/<simple_date:reported_at>")
+@user_has_permissions('view_activity')
+def returned_letters(service_id, reported_at):
+
+    page_size = 50
+    returned_letters = service_api_client.get_returned_letters(service_id, reported_at)
+    count_of_returned_letters = len(returned_letters)
+
+    return render_template(
+        'views/returned-letters.html',
+        returned_letters=returned_letters[:page_size],
+        reported_at=reported_at,
+        more_than_one_page=(count_of_returned_letters > page_size),
+        page_size=page_size,
+        count_of_returned_letters=count_of_returned_letters,
+    )
+
+
+@main.route("/services/<uuid:service_id>/returned-letters/<simple_date:reported_at>.csv")
 @user_has_permissions('view_activity')
 def returned_letters_report(service_id, reported_at):
     returned_letters = service_api_client.get_returned_letters(service_id, reported_at)

app/navigation.py

@@ -243,6 +243,7 @@ class HeaderNavigation(Navigation):
         'resend_email_verification',
         'resume_service',
         'returned_letter_summary',
+        'returned_letters',
         'returned_letters_report',
         'revoke_api_key',
         'robots',
@@ -567,6 +568,7 @@ class MainNavigation(Navigation):
         'resend_email_verification',
         'resume_service',
         'returned_letter_summary',
+        'returned_letters',
         'returned_letters_report',
         'roadmap',
         'robots',
@@ -806,6 +808,7 @@ class CaseworkNavigation(Navigation):
         'resend_email_verification',
         'resume_service',
         'returned_letter_summary',
+        'returned_letters',
         'returned_letters_report',
         'revoke_api_key',
         'roadmap',
@@ -1084,6 +1087,7 @@ class OrgNavigation(Navigation):
         'resend_email_verification',
         'resume_service',
         'returned_letter_summary',
+        'returned_letters',
         'returned_letters_report',
         'revoke_api_key',
         'roadmap',

app/templates/views/returned-letter-summary.html

@@ -1,4 +1,4 @@
-{% from "components/table.html" import list_table, field %}
+{% from "components/table.html" import list_table, row_heading %}
 {% from "components/message-count-label.html" import message_count_label %}
 {% extends "withnav_template.html" %}
 
@@ -10,18 +10,23 @@
 <h1 class="heading-large">
     Returned letters
   </h1>
-<div class="body-copy-table" id='pill-selected-item'>
+<div class="dashboard-table">
     {% call(item, row_number) list_table(
         data,
         caption="Returned letters report",
         caption_visible=False,
-        empty_message='If you have returned letter reports they will be listed here'
+        empty_message='If you have returned letter reports they will be listed here',
-      ) %}
+        field_headings=['Report'],
-    {% call field() %}
+        field_headings_visible=False
-      <a target="_blank" class="govuk_link"
+    ) %}
-         href="{{url_for('.returned_letters_report', service_id=current_service.id, reported_at=item.reported_at)}}">Returned letters reported on {{ item.reported_at | format_date}} - {{ item.returned_letter_count}} {{ message_count_label(item.returned_letter_count, 'letter', suffix='')}}</a>
+    {% call row_heading() %}
+      <a class="govuk_link file-list-filename"
+         href="{{url_for('.returned_letters', service_id=current_service.id, reported_at=item.reported_at)}}">{{ item.reported_at | format_date_normal }}</a>
+       <p class="file-list-hint">
+         {{ item.returned_letter_count}} {{ message_count_label(item.returned_letter_count, 'letter', suffix='')}}
+       </p>
     {% endcall %}
   {% endcall %}
 </div>
 
-{% endblock %}
+{% endblock %}

app/templates/views/returned-letters.html

@@ -0,0 +1,57 @@
+{% from "components/table.html" import list_table, field %}
+{% from "components/message-count-label.html" import message_count_label %}
+{% from "components/page-header.html" import page_header %}
+{% extends "withnav_template.html" %}
+
+{% block service_page_title %}
+  Returned letters for {{ reported_at|format_date_normal }}
+{% endblock %}
+
+{% block maincolumn_content %}
+
+{{ page_header(
+  'Returned letters for {}'.format(reported_at|format_date_normal),
+  back_link=url_for('main.returned_letter_summary', service_id=current_service.id)
+) }}
+
+<p class="bottom-gutter">
+  <a download href="{{ url_for('.returned_letters_report', service_id=current_service.id, reported_at=reported_at) }}" class="govuk-link heading-small">Download this report</a>
+</p>
+
+<div class="dashboard-table">
+  {% call(item, row_number) list_table(
+      returned_letters,
+      caption="Returned letters for {}".format(today),
+      caption_visible=False,
+      empty_message='If you have returned letter reports they will be listed here',
+      field_headings=['Template name', 'Originally sent'],
+      field_headings_visible=False
+  ) %}
+    {% call field() %}
+      <span class="file-list-filename file-list-filename-unlinked">{{ item.template_name or item.uploaded_letter_file_name }}</span>
+      <span class="file-list-hint">
+        {% if item.client_reference %}
+          Reference {{ item.client_reference }}
+        {% elif item.original_file_name %}
+          Sent from {{ item.original_file_name }}
+        {% else %}
+          No reference provided
+        {% endif %}
+      </span>
+    {% endcall %}
+    {% call field(align='right') %}
+      <span class="align-with-message-body">
+        <span class="status-hint">
+          Originally sent {{ item.created_at|format_date_normal }}
+        </span>
+      </span>
+    {% endcall %}
+  {% endcall %}
+  {% if more_than_one_page %}
+    <p class="table-show-more-link">
+      Only showing the first {{ page_size }} of {{ count_of_returned_letters }} rows
+    </p>
+  {% endif %}
+</div>
+
+{% endblock %}

tests/app/main/views/test_returned_letters.py

@@ -1,3 +1,5 @@
+import uuid
+
 from flask import url_for
 
 from tests.conftest import SERVICE_ONE_ID, normalize_spaces
@@ -15,12 +17,18 @@ def test_returned_letter_summary(
 
     mock.assert_called_once_with(SERVICE_ONE_ID)
 
-    expected_text = "Returned letters reported on Tuesday 24 December 2019 - 30 letters"
     assert page.h1.string.strip() == 'Returned letters'
-    assert normalize_spaces(page.select('.table-field-left-aligned')[0].text) == expected_text
+    assert normalize_spaces(
-    assert page.select_one('.table-field-left-aligned a')['href'] == url_for('.returned_letters_report',
+        page.select_one('.table-field').text
-                                                                             service_id=SERVICE_ONE_ID,
+    ) == (
-                                                                             reported_at='2019-12-24')
+        '24 December 2019 '
+        '30 letters'
+    )
+    assert page.select_one('.table-field a')['href'] == url_for(
+        '.returned_letters',
+        service_id=SERVICE_ONE_ID,
+        reported_at='2019-12-24',
+    )
 
 
 def test_returned_letter_summary_with_one_letter(
@@ -35,9 +43,101 @@ def test_returned_letter_summary_with_one_letter(
 
     mock.assert_called_once_with(SERVICE_ONE_ID)
 
-    expected_text = "Returned letters reported on Tuesday 24 December 2019 - 1 letter"
     assert page.h1.string.strip() == 'Returned letters'
-    assert normalize_spaces(page.select('.table-field-left-aligned')[0].text) == expected_text
+    assert normalize_spaces(
+        page.select_one('.table-field').text
+    ) == (
+        '24 December 2019 '
+        '1 letter'
+    )
+
+
+def test_returned_letters_page(
+    client_request,
+    mocker
+):
+    data = [
+        {
+            'notification_id': uuid.uuid4(),
+            'client_reference': client_reference,
+            'created_at': '2019-12-24 13:30',
+            'email_address': 'test@gov.uk',
+            'template_name': template_name,
+            'template_id': uuid.uuid4(),
+            'template_version': None,
+            'original_file_name': original_file_name,
+            'job_row_number': None,
+            'uploaded_letter_file_name': 'test_letter.pdf',
+        }
+        for client_reference, template_name, original_file_name, uploaded_letter_file_name in (
+            ('ABC123', 'Example template', None, None),
+            (None, 'Example template', 'Example spreadsheet.xlsx', None),
+            (None, 'Example template', None, None),
+            ('DEF456', None, None, 'Example precompiled.pdf'),
+            (None, None, None, 'Example one-off.pdf'),
+        )
+    ]
+    mocker.patch('app.service_api_client.get_returned_letters', return_value=data)
+
+    page = client_request.get(
+        'main.returned_letters',
+        service_id=SERVICE_ONE_ID,
+        reported_at='2019-12-24',
+    )
+
+    assert [
+        'Template name Originally sent',
+        'Example template Reference ABC123 Originally sent 24 December 2019',
+        'Example template Sent from Example spreadsheet.xlsx Originally sent 24 December 2019',
+        'Example template No reference provided Originally sent 24 December 2019',
+        'test_letter.pdf Reference DEF456 Originally sent 24 December 2019',
+        'test_letter.pdf No reference provided Originally sent 24 December 2019',
+    ] == [
+        normalize_spaces(row.text) for row in page.select('tr')
+    ]
+
+
+def test_returned_letters_page_with_many_letters(
+    client_request,
+    mocker
+):
+    data = [
+        {
+            'notification_id': uuid.uuid4(),
+            'client_reference': None,
+            'created_at': '2019-12-24 13:30',
+            'email_address': 'test@gov.uk',
+            'template_name': 'Example template',
+            'template_id': uuid.uuid4(),
+            'template_version': None,
+            'original_file_name': None,
+            'job_row_number': None,
+            'uploaded_letter_file_name': None,
+        }
+    ] * 51
+    mocker.patch('app.service_api_client.get_returned_letters', return_value=data)
+
+    page = client_request.get(
+        'main.returned_letters',
+        service_id=SERVICE_ONE_ID,
+        reported_at='2019-12-24',
+    )
+
+    assert len(data) == 51
+    assert len(page.select('tbody tr')) == 50
+    assert normalize_spaces(
+        page.select_one('.table-show-more-link').text
+    ) == (
+        'Only showing the first 50 of 51 rows'
+    )
+    assert page.select_one('a[download]').text == (
+        'Download this report'
+    )
+    assert page.select_one('a[download]')['href'] == url_for(
+        '.returned_letters_report',
+        service_id=SERVICE_ONE_ID,
+        reported_at='2019-12-24',
+    )
 
 
 def test_returned_letters_reports(