Merge pull request #3254 from alphagov/letter-validation-short-messages

Add separate messages for precompiled letters that have failed validation

app/main/views/index.py

@@ -343,3 +343,11 @@ def old_page_redirects():
         'main.old_integration_testing': 'main.integration_testing',
     }
     return redirect(url_for(redirects[request.endpoint]), code=301)
+
+
+@main.route('/docs/notify-pdf-letter-spec-latest.pdf')
+def letter_spec():
+    return redirect(
+        'https://docs.notifications.service.gov.uk'
+        '/documentation/images/notify-pdf-letter-spec-v2.4.pdf'
+    )

app/navigation.py

@@ -208,6 +208,7 @@ class HeaderNavigation(Navigation):
         'invite_org_user',
         'invite_user',
         'no_cookie.letter_branding_preview_image',
+        'letter_spec',
         'letter_template',
         'link_service_to_organisation',
         'manage_org_users',
@@ -533,6 +534,7 @@ class MainNavigation(Navigation):
         'no_cookie.letter_branding_preview_image',
         'live_services',
         'live_services_csv',
+        'letter_spec',
         'letter_template',
         'message_status',
         'manage_org_users',
@@ -763,6 +765,7 @@ class CaseworkNavigation(Navigation):
         'invite_user',
         'no_cookie.letter_branding_preview_image',
         'letter_branding',
+        'letter_spec',
         'letter_template',
         'link_service_to_organisation',
         'live_services',
@@ -1049,6 +1052,7 @@ class OrgNavigation(Navigation):
         'invite_user',
         'letter_branding',
         'no_cookie.letter_branding_preview_image',
+        'letter_spec',
         'letter_template',
         'link_service_to_organisation',
         'live_services',

app/templates/views/features/letters.html

@@ -33,7 +33,7 @@
 
   <h3 class="heading heading-small" id="upload-letters">Upload your own letters</h3>
   <p>You can create reusable letter templates in Notify, or upload and send your own letters with the Notify API.</p>
-  <p>Use the <a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf">letter specification document</a> to help you set up your letter, save it as a PDF, then upload it to Notify.<p>
+  <p>Use the <a href="{{ url_for('.letter_spec') }}">letter specification document</a> to help you set up your letter, save it as a PDF, then upload it to Notify.<p>
   <p>Read our <a href="{{ url_for('.documentation') }}">API documentation</a> for more information.</p>
 
   <h2 class="heading heading-medium">Pricing</h2>

app/templates/views/notifications/notification.html

@@ -44,7 +44,7 @@
         </p>
       {% elif notification_status == 'validation-failed' %}
         <p class="notification-status-cancelled">
-          Validation failed – {{ message.title | safe }}. {{ message.detail | safe }}
+          {{ message.summary | safe }}
         </p>
       {% elif notification_status == 'technical-failure' %}
         <p class="notification-status-cancelled">

app/templates/views/uploads/choose-file.html

@@ -33,7 +33,7 @@
     )}}
   </p>
   <p>You can upload a single letter as a PDF.</p>
-  <p>Your file must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf">letter specification</a>.</p>
+  <p>Your file must meet our <a href="{{ url_for('.letter_spec') }}">letter specification</a>.</p>
   <p>To help you set up your letter you can download a <a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-word-template-v1.0.docx">Word document template</a>.</p>
 
   </div>

app/utils.py

@@ -569,32 +569,61 @@ def get_letter_printing_statement(status, created_at):
 LETTER_VALIDATION_MESSAGES = {
     'letter-not-a4-portrait-oriented': {
         'title': 'Your letter is not A4 portrait size',
-        'detail': 'You need to change the size or orientation of {invalid_pages}. <br>'
+        'detail': (
-                  'Files must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/'
+            'You need to change the size or orientation of {invalid_pages}. <br>'
-                  'notify-pdf-letter-spec-v2.4.pdf" target="_blank">letter specification</a>.'
+            'Files must meet our <a href="{letter_spec}" target="_blank">letter specification</a>.'
+        ),
+        'summary': (
+            'Validation failed because {invalid_pages} {invalid_pages_are_or_is} not A4 portrait size.<br>'
+            'Files must meet our <a href="{letter_spec}" target="_blank">letter specification</a>.'
+        ),
     },
     'content-outside-printable-area': {
         'title': 'Your content is outside the printable area',
-        'detail': 'You need to edit {invalid_pages}.<br>'
+        'detail': (
-                  'Files must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/'
+            'You need to edit {invalid_pages}.<br>'
-                  'notify-pdf-letter-spec-v2.4.pdf" target="_blank">letter specification</a>.'
+            'Files must meet our <a href="{letter_spec}" target="_blank">letter specification</a>.'
+        ),
+        'summary': (
+            'Validation failed because content is outside the printable area on {invalid_pages}.<br>'
+            'Files must meet our <a href="{letter_spec}" target="_blank">letter specification</a>.'
+        ),
     },
     'letter-too-long': {
         'title': 'Your letter is too long',
-        'detail': 'Letters must be 10 pages or less. <br>Your letter is {page_count} pages long.'
+        'detail': (
+            'Letters must be 10 pages or less. <br>'
+            'Your letter is {page_count} pages long.'
+        ),
+        'summary': (
+            'Validation failed because this letter is {page_count} pages long.<br>'
+            'Letters must be 10 pages or less.'
+        ),
     },
     'no-encoded-string': {
         'title': 'Sanitise failed - No encoded string'
     },
     'unable-to-read-the-file': {
         'title': 'There’s a problem with your file',
-        'detail': 'Notify cannot read this PDF.<br>Save a new copy of your file and try again.'
+        'detail': (
+            'Notify cannot read this PDF.'
+            '<br>Save a new copy of your file and try again.'
+        ),
+        'summary': (
+            'Validation failed because Notify cannot read this PDF.<br>'
+            'Save a new copy of your file and try again.'
+        ),
     },
     'address-is-empty': {
         'title': 'The address block is empty',
-        'detail': 'You need to add a recipient address.<br>'
+        'detail': (
-                  'Files must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/'
+            'You need to add a recipient address.<br>'
-                  'notify-pdf-letter-spec-v2.4.pdf" target="_blank">letter specification</a>.'
+            'Files must meet our <a href="{letter_spec}" target="_blank">letter specification</a>.'
+        ),
+        'summary': (
+            'Validation failed because the address block is empty.<br>'
+            'Files must meet our <a href="{letter_spec}" target="_blank">letter specification</a>.'
+        ),
     }
 }
 
@@ -603,6 +632,8 @@ def get_letter_validation_error(validation_message, invalid_pages=None, page_cou
     if validation_message not in LETTER_VALIDATION_MESSAGES:
         return {'title': 'Validation failed'}
 
+    invalid_pages_are_or_is = 'is' if len(invalid_pages) == 1 else 'are'
+
     invalid_pages = unescaped_formatted_list(
         invalid_pages or [],
         before_each='',
@@ -615,8 +646,16 @@ def get_letter_validation_error(validation_message, invalid_pages=None, page_cou
         'title': LETTER_VALIDATION_MESSAGES[validation_message]['title'],
         'detail': LETTER_VALIDATION_MESSAGES[validation_message]['detail'].format(
             invalid_pages=invalid_pages,
+            invalid_pages_are_or_is=invalid_pages_are_or_is,
             page_count=page_count,
-        )
+            letter_spec=url_for('.letter_spec'),
+        ),
+        'summary': LETTER_VALIDATION_MESSAGES[validation_message]['summary'].format(
+            invalid_pages=invalid_pages,
+            invalid_pages_are_or_is=invalid_pages_are_or_is,
+            page_count=page_count,
+            letter_spec=url_for('.letter_spec'),
+        ),
     }
 
 

tests/app/main/views/test_index.py

@@ -254,3 +254,26 @@ def test_letter_template_preview_headers(
     )
 
     assert response.headers.get('X-Frame-Options') == 'SAMEORIGIN'
+
+
+def test_letter_spec_redirect(client_request):
+    client_request.get(
+        'main.letter_spec',
+        _expected_status=302,
+        _expected_redirect=(
+            'https://docs.notifications.service.gov.uk'
+            '/documentation/images/notify-pdf-letter-spec-v2.4.pdf'
+        ),
+    )
+
+
+def test_letter_spec_redirect_with_non_logged_in_user(client_request):
+    client_request.logout()
+    client_request.get(
+        'main.letter_spec',
+        _expected_status=302,
+        _expected_redirect=(
+            'https://docs.notifications.service.gov.uk'
+            '/documentation/images/notify-pdf-letter-spec-v2.4.pdf'
+        ),
+    )

tests/app/main/views/test_notifications.py

@@ -331,9 +331,10 @@ def test_notification_page_shows_validation_failed_precompiled_letter(
     )
 
     error_message = page.find('p', class_='notification-status-cancelled').text
-    assert normalize_spaces(error_message) == \
+    assert normalize_spaces(error_message) == (
-        "Validation failed – Your content is outside the printable area. " \
+        'Validation failed because content is outside the printable area on page 1.'
-        "You need to edit page 1.Files must meet our letter specification."
+        'Files must meet our letter specification.'
+    )
 
     assert not page.select('p.notification-status')
 

tests/app/test_utils.py

@@ -4,6 +4,8 @@ from io import StringIO
 from pathlib import Path
 
 import pytest
+from bs4 import BeautifulSoup
+from flask import url_for
 from freezegun import freeze_time
 
 from app import format_datetime_relative
@@ -413,24 +415,94 @@ def test_get_letter_validation_error_for_unknown_error():
     }
 
 
-@pytest.mark.parametrize('error_message, expected_title, expected_content', [
+@pytest.mark.parametrize('error_message, invalid_pages, expected_title, expected_content, expected_summary', [
-    ('letter-not-a4-portrait-oriented', 'Your letter is not A4 portrait size',
+    (
-     'You need to change the size or orientation of page 2. <br>Files must meet our '
+        'letter-not-a4-portrait-oriented',
-     '<a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf" '
+        [2],
-     'target="_blank">letter specification</a>.'),
+        'Your letter is not A4 portrait size',
-    ('content-outside-printable-area', 'Your content is outside the printable area',
+        (
-     'You need to edit page 2.<br>Files must meet our '
+            'You need to change the size or orientation of page 2. '
-     '<a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf" '
+            'Files must meet our letter specification.'
-     'target="_blank">letter specification</a>.'),
+        ),
-    ('letter-too-long', 'Your letter is too long',
+        (
-     'Letters must be 10 pages or less. <br>Your letter is 13 pages long.')
+            'Validation failed because page 2 is not A4 portrait size.'
+            'Files must meet our letter specification.'
+        ),
+    ),
+    (
+        'letter-not-a4-portrait-oriented',
+        [2, 3, 4],
+        'Your letter is not A4 portrait size',
+        (
+            'You need to change the size or orientation of pages 2, 3 and 4. '
+            'Files must meet our letter specification.'
+        ),
+        (
+            'Validation failed because pages 2, 3 and 4 are not A4 portrait size.'
+            'Files must meet our letter specification.'
+        ),
+    ),
+    (
+        'content-outside-printable-area',
+        [2],
+        'Your content is outside the printable area',
+        (
+            'You need to edit page 2.'
+            'Files must meet our letter specification.'
+        ),
+        (
+            'Validation failed because content is outside the printable area '
+            'on page 2.'
+            'Files must meet our letter specification.'
+        ),
+    ),
+    (
+        'letter-too-long',
+        [2],
+        'Your letter is too long',
+        (
+            'Letters must be 10 pages or less. '
+            'Your letter is 13 pages long.'
+        ),
+        (
+            'Validation failed because this letter is 13 pages long.'
+            'Letters must be 10 pages or less.'
+        ),
+    ),
+    (
+        'unable-to-read-the-file',
+        [2],
+        'There’s a problem with your file',
+        (
+            'Notify cannot read this PDF.'
+            'Save a new copy of your file and try again.'
+        ),
+        (
+            'Validation failed because Notify cannot read this PDF.'
+            'Save a new copy of your file and try again.'
+        ),
+    ),
 ])
 def test_get_letter_validation_error_for_known_errors(
-        error_message,
+    client_request,
-        expected_title,
+    error_message,
-        expected_content,
+    invalid_pages,
+    expected_title,
+    expected_content,
+    expected_summary,
 ):
-    error = get_letter_validation_error(error_message, invalid_pages=[2], page_count=13)
+    error = get_letter_validation_error(error_message, invalid_pages=invalid_pages, page_count=13)
+    detail = BeautifulSoup(error['detail'], 'html.parser')
+    summary = BeautifulSoup(error['summary'], 'html.parser')
 
     assert error['title'] == expected_title
-    assert expected_content in error['detail']
+
+    assert detail.text == expected_content
+    if detail.select_one('a'):
+        assert detail.select_one('a')['href'] == url_for('.letter_spec')
+        assert detail.select_one('a')['target'] == '_blank'
+
+    assert summary.text == expected_summary
+    if summary.select_one('a'):
+        assert summary.select_one('a')['href'] == url_for('.letter_spec')
+        assert summary.select_one('a')['target'] == '_blank'