Merge pull request #3253 from alphagov/new-no-address-message

Added a new error message when the letter is missing an address block.

app/templates/views/notifications/invalid_precompiled_letter.html

@@ -12,6 +12,6 @@
       Provided as PDF on {{ created_at|format_datetime_short }}
     </p>
     <p class="notification-status-cancelled">
-      Validation failed – Notify cannot read this PDF file
+      Validation failed – There’s a problem with your letter. <br>Notify cannot read this PDF.
     </p>
 {% endblock %}

app/templates/views/notifications/notification.html

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

app/utils.py

@@ -566,14 +566,14 @@ def get_letter_printing_statement(status, created_at):
 
 LETTER_VALIDATION_MESSAGES = {
     'letter-not-a4-portrait-oriented': {
-        'title': 'We cannot print your letter',
-        'detail': 'Your letter is not A4 portrait size on {invalid_pages} <br>'
+        'title': 'Your letter is not A4 portrait size',
+        'detail': 'You need to change the size or orientation of {invalid_pages}. <br>'
                   'Files must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/'
                   'notify-pdf-letter-spec-v2.4.pdf" target="_blank">letter specification</a>.'
     },
     'content-outside-printable-area': {
-        'title': 'We cannot print your letter',
-        'detail': 'The content appears outside the printable area on {invalid_pages}.<br>'
+        'title': 'Your content is outside the printable area',
+        'detail': 'You need to edit {invalid_pages}.<br>'
                   'Files must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/'
                   'notify-pdf-letter-spec-v2.4.pdf" target="_blank">letter specification</a>.'
     },
@@ -587,6 +587,12 @@ LETTER_VALIDATION_MESSAGES = {
     '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.'
+    },
+    'address-is-empty': {
+        'title': 'The address block is empty',
+        'detail': 'You need to add a recipient address.<br>'
+                  'Files must meet our <a href="https://docs.notifications.service.gov.uk/documentation/images/'
+                  'notify-pdf-letter-spec-v2.4.pdf" target="_blank">letter specification</a>.'
     }
 }
 

tests/app/main/views/test_notifications.py

@@ -304,6 +304,42 @@ def test_notification_page_shows_page_for_letter_sent_with_test_key(
     assert page.select('p.notification-status') == []
 
 
+def test_notification_page_shows_validation_failed_precompiled_letter(
+    client_request,
+    mocker,
+    fake_uuid,
+):
+    notification = create_notification(template_type='letter',
+                                       notification_status='validation-failed',
+                                       is_precompiled_letter=True
+                                       )
+    mocker.patch('app.notification_api_client.get_notification', return_value=notification)
+    metadata = {"page_count": "1", "status": "validation-failed",
+                "invalid_pages": "[1]",
+                "message": "content-outside-printable-area"}
+    mocker.patch('app.main.views.notifications.view_letter_notification_as_preview',
+                 return_value=("some letter content", metadata))
+    mocker.patch(
+        'app.main.views.notifications.get_page_count_for_letter',
+        return_value=1,
+    )
+
+    page = client_request.get(
+        'main.view_notification',
+        service_id=SERVICE_ONE_ID,
+        notification_id=fake_uuid,
+    )
+
+    error_message = page.find('p', class_='notification-status-cancelled').text
+    assert normalize_spaces(error_message) == \
+        "Validation failed – Your content is outside the printable area. " \
+        "You need to edit page 1.Files must meet our letter specification."
+
+    assert not page.select('p.notification-status')
+
+    assert page.select_one('main img')['src'].endswith('.png?page=1')
+
+
 @pytest.mark.parametrize('notification_status, expected_message', (
     (
         'permanent-failure',
@@ -313,10 +349,6 @@ def test_notification_page_shows_page_for_letter_sent_with_test_key(
         'cancelled',
         'Cancelled 1 January at 1:02am',
     ),
-    (
-        'validation-failed',
-        'Validation failed.',
-    ),
     (
         'technical-failure',
         'Technical failure – Notify will resend once the team have fixed the problem',
@@ -538,7 +570,7 @@ def test_should_show_preview_error_image_letter_notification_on_preview_error(
     assert response.get_data(as_text=True) == 'preview error image'
 
 
-def test_notifification_page_shows_error_message_if_precompiled_letter_cannot_be_opened(
+def test_notification_page_shows_error_message_if_precompiled_letter_cannot_be_opened(
     client_request,
     mocker,
     fake_uuid,
@@ -563,7 +595,8 @@ def test_notifification_page_shows_error_message_if_precompiled_letter_cannot_be
     )
 
     error_message = page.find('p', class_='notification-status-cancelled').text
-    assert normalize_spaces(error_message) == "Validation failed – Notify cannot read this PDF file"
+    assert normalize_spaces(error_message) == \
+        "Validation failed – There’s a problem with your letter. Notify cannot read this PDF."
 
 
 def test_should_404_for_unknown_extension(

tests/app/test_utils.py

@@ -413,14 +413,21 @@ def test_get_letter_validation_error_for_unknown_error():
 
 
 @pytest.mark.parametrize('error_message, expected_title, expected_content', [
-    ('letter-not-a4-portrait-oriented', 'We cannot print your letter', 'A4 portrait size on page 2'),
-    ('content-outside-printable-area', 'We cannot print your letter', 'outside the printable area on page 2'),
-    ('letter-too-long', 'Your letter is too long', 'letter is 13 pages long.')
+    ('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 '
+     '<a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf" '
+     'target="_blank">letter specification</a>.'),
+    ('content-outside-printable-area', 'Your content is outside the printable area',
+     'You need to edit page 2.<br>Files must meet our '
+     '<a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf" '
+     '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.')
 ])
 def test_get_letter_validation_error_for_known_errors(
-    error_message,
-    expected_title,
-    expected_content,
+        error_message,
+        expected_title,
+        expected_content,
 ):
     error = get_letter_validation_error(error_message, invalid_pages=[2], page_count=13)