Merge branch 'master' into add_proxy_header_check

app/main/views/notifications.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from flask import (
+    abort,
     render_template,
     jsonify,
     request,
@@ -34,9 +35,10 @@ def view_notification(service_id, notification_id):
         notification['template'],
         current_service,
         letter_preview_url=url_for(
-            '.view_letter_notification_as_image',
+            '.view_letter_notification_as_preview',
             service_id=service_id,
             notification_id=notification_id,
+            filetype='png',
         ),
         show_recipient=True,
         redact_missing_personalisation=True,
@@ -70,10 +72,13 @@ def view_notification(service_id, notification_id):
     )
 
 
-@main.route("/services/<service_id>/notification/<uuid:notification_id>.png")
+@main.route("/services/<service_id>/notification/<uuid:notification_id>.<filetype>")
 @login_required
 @user_has_permissions('view_activity', admin_override=True)
-def view_letter_notification_as_image(service_id, notification_id):
+def view_letter_notification_as_preview(service_id, notification_id, filetype):
+
+    if filetype not in ('pdf', 'png'):
+        abort(404)
 
     notification = notification_api_client.get_notification(service_id, notification_id)
 
@@ -81,15 +86,16 @@ def view_letter_notification_as_image(service_id, notification_id):
         notification['template'],
         current_service,
         letter_preview_url=url_for(
-            '.view_letter_notification_as_image',
+            '.view_letter_notification_as_preview',
             service_id=service_id,
             notification_id=notification_id,
+            filetype='png',
         ),
     )
 
     template.values = notification['personalisation']
 
-    return TemplatePreview.from_utils_template(template, 'png', page=request.args.get('page'))
+    return TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page'))
 
 
 @main.route("/services/<service_id>/notification/<notification_id>.json")

app/templates/views/check/column-errors.html

@@ -48,7 +48,10 @@
       {% elif not recipients.has_recipient_columns %}
 
         <h1 class='banner-title' data-module="track-error" data-error-type="Missing recipient columns" data-error-label="{{ upload_id }}">
-          Your file needs {{ required_recipient_columns | formatted_list(
+          Your file needs {{ (
+            recipients.missing_column_headers
+            if template.template_type == 'letter' else required_recipient_columns
+          ) | formatted_list(
             prefix='a column called',
             prefix_plural='columns called'
           ) }}

app/templates/views/notifications/notification.html

@@ -34,6 +34,9 @@
       <p>
         Estimated delivery date: {{ estimated_letter_delivery_date|string|format_date_short }}
       </p>
+      <p class="bottom-gutter">
+        <a href="{{ url_for('main.view_letter_notification_as_preview', service_id=current_service.id, notification_id=notification_id, filetype='pdf') }}" download="download">Download as a PDF</a>
+      </p>
     {% endif %}
 
     {{ template|string }}

tests/app/main/views/test_notifications.py

@@ -1,5 +1,6 @@
 from freezegun import freeze_time
 from flask import url_for
+from functools import partial
 import pytest
 
 from notifications_utils.template import LetterImageTemplate
@@ -131,10 +132,14 @@ def test_notification_page_shows_status_of_letter_notification(
     assert page.select('p.notification-status') == []
 
 
+@pytest.mark.parametrize('filetype', [
+    'pdf', 'png'
+])
 def test_should_show_image_of_letter_notification(
     logged_in_client,
     fake_uuid,
-    mocker
+    mocker,
+    filetype,
 ):
 
     mock_get_notification(mocker, fake_uuid, template_type='letter')
@@ -145,15 +150,29 @@ def test_should_show_image_of_letter_notification(
     )
 
     response = logged_in_client.get(url_for(
-        'main.view_letter_notification_as_image',
+        'main.view_letter_notification_as_preview',
         service_id=SERVICE_ONE_ID,
         notification_id=fake_uuid,
+        filetype=filetype
     ))
 
     assert response.status_code == 200
     assert response.get_data(as_text=True) == 'foo'
     assert isinstance(mocked_preview.call_args[0][0], LetterImageTemplate)
-    assert mocked_preview.call_args[0][1] == 'png'
+    assert mocked_preview.call_args[0][1] == filetype
+
+
+def test_should_404_for_unknown_extension(
+    client_request,
+    fake_uuid,
+):
+    client_request.get(
+        'main.view_letter_notification_as_preview',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+        filetype='docx',
+        _expected_status=404,
+    )
 
 
 @pytest.mark.parametrize('service_permissions, template_type, link_expected', [
@@ -182,17 +201,52 @@ def test_notification_page_has_link_to_send_another_for_sms(
     )
 
     last_paragraph = page.select('main p')[-1]
+    conversation_link = url_for(
+        '.conversation',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+        _anchor='n{}'.format(fake_uuid),
+    )
 
     if link_expected:
         assert normalize_spaces(last_paragraph.text) == (
             'See all text messages sent to this phone number'
         )
-        assert last_paragraph.select_one('a')['href'] == url_for(
+        assert last_paragraph.select_one('a')['href'] == conversation_link
-            '.conversation',
-            service_id=SERVICE_ONE_ID,
-            notification_id=fake_uuid,
-            _anchor='n{}'.format(fake_uuid),
-        )
     else:
-        # covers ‘Delivered’, ‘Expected delivery date’
+        assert conversation_link not in str(page.select_one('main'))
-        assert 'deliver' in normalize_spaces(last_paragraph.text).lower()
+
+
+@pytest.mark.parametrize('template_type, expected_link', [
+    ('email', lambda notification_id: None),
+    ('sms', lambda notification_id: None),
+    ('letter', partial(
+        url_for,
+        'main.view_letter_notification_as_preview',
+        service_id=SERVICE_ONE_ID,
+        filetype='pdf'
+    )),
+])
+def test_notification_page_has_link_to_download_letter(
+    client_request,
+    mocker,
+    fake_uuid,
+    service_one,
+    template_type,
+    expected_link,
+):
+
+    mock_get_notification(mocker, fake_uuid, template_type=template_type)
+
+    page = client_request.get(
+        'main.view_notification',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+    )
+
+    try:
+        download_link = page.select_one('a[download]')['href']
+    except TypeError:
+        download_link = None
+
+    assert download_link == expected_link(notification_id=fake_uuid)

tests/app/main/views/test_send.py

@@ -1883,7 +1883,7 @@ def test_check_messages_column_error_doesnt_show_optional_columns(
     )
 
     assert normalize_spaces(page.select_one('.banner-dangerous').text) == (
-        'Your file needs columns called ‘address line 1’, ‘address line 2’ and ‘postcode’ '
+        'Your file needs a column called ‘postcode’ '
         'Right now it has columns called ‘address_line_1’, ‘address_line_2’ and ‘foo’. '
         'Skip to file contents'
     )