From 70276cb5268ea33708de479fa0027d4d70eb3f01 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 10 Jan 2020 14:42:56 +0000 Subject: [PATCH] Added a new error message when the letter is mising an address block. Before deploying a change to template-preview to return a validation error for letters that are missing the address block, we need to add the new erorr message to admin. Some content changes have been made to other messages. The format of the message has changed. --- .../invalid_precompiled_letter.html | 2 +- .../views/notifications/notification.html | 2 +- app/utils.py | 14 ++++-- tests/app/main/views/test_notifications.py | 45 ++++++++++++++++--- tests/app/test_utils.py | 19 +++++--- 5 files changed, 64 insertions(+), 18 deletions(-) diff --git a/app/templates/views/notifications/invalid_precompiled_letter.html b/app/templates/views/notifications/invalid_precompiled_letter.html index 99530d918..1c2f9af63 100644 --- a/app/templates/views/notifications/invalid_precompiled_letter.html +++ b/app/templates/views/notifications/invalid_precompiled_letter.html @@ -12,6 +12,6 @@ Provided as PDF on {{ created_at|format_datetime_short }}

- Validation failed – Notify cannot read this PDF file + Validation failed – There’s a problem with your letter.
Notify cannot read this PDF.

{% endblock %} diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index 12663f3fc..f9ffe85ef 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -44,7 +44,7 @@

{% elif notification_status == 'validation-failed' %}

- Validation failed. {{ message.detail | safe }} + Validation failed – {{ message.title | safe }}. {{ message.detail | safe }}

{% elif notification_status == 'technical-failure' %}

diff --git a/app/utils.py b/app/utils.py index 3bd002238..b7ce5b69e 100644 --- a/app/utils.py +++ b/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}
' + 'title': 'Your letter is not A4 portrait size', + 'detail': 'You need to change the size or orientation of {invalid_pages}.
' 'Files must meet our letter specification.' }, 'content-outside-printable-area': { - 'title': 'We cannot print your letter', - 'detail': 'The content appears outside the printable area on {invalid_pages}.
' + 'title': 'Your content is outside the printable area', + 'detail': 'You need to edit {invalid_pages}.
' 'Files must meet our letter specification.' }, @@ -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.
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.
' + 'Files must meet our letter specification.' } } diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 98a634897..9e1c1bcdf 100644 --- a/tests/app/main/views/test_notifications.py +++ b/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( diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index cb493adeb..a81032b5b 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -414,14 +414,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.
Files must meet our ' + 'letter specification.'), + ('content-outside-printable-area', 'Your content is outside the printable area', + 'You need to edit page 2.
Files must meet our ' + 'letter specification.'), + ('letter-too-long', 'Your letter is too long', + 'Letters must be 10 pages or less.
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)