diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index f9d381edc..d0f2f7622 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import base64 import io +import json import os from datetime import datetime @@ -39,6 +40,7 @@ from app.utils import ( generate_notifications_csv, get_help_argument, get_letter_printing_statement, + get_letter_validation_error, get_template, parse_filter_args, set_status_filters, @@ -53,13 +55,21 @@ def view_notification(service_id, notification_id): notification['template'].update({'reply_to_text': notification['reply_to_text']}) personalisation = get_all_personalisation_from_notification(notification) - + error_message = None if notification['template']['is_precompiled_letter']: try: - file_contents = view_letter_notification_as_preview( - service_id, notification_id, "pdf" + file_contents, metadata = view_letter_notification_as_preview( + service_id, notification_id, "pdf", with_metadata=True ) - page_count = pdf_page_count(io.BytesIO(file_contents)) + page_count = int( + metadata["page_count"] + ) if metadata.get("page_count") else pdf_page_count(io.BytesIO(file_contents)) + if notification["status"] == "validation-failed": + invalid_pages = metadata.get("invalid_pages") + invalid_pages = json.loads(invalid_pages) if invalid_pages else invalid_pages + error_message = get_letter_validation_error( + metadata.get("message"), invalid_pages, page_count + ) except PdfReadError: return render_template( 'views/notifications/invalid_precompiled_letter.html', @@ -119,6 +129,7 @@ def view_notification(service_id, notification_id): 'views/notifications/notification.html', finished=(notification['status'] in (DELIVERED_STATUSES + FAILURE_STATUSES)), notification_status=notification['status'], + message=error_message, uploaded_file_name='Report', template=template, job=job, @@ -171,31 +182,28 @@ def get_preview_error_image(): @main.route("/services//notification/.") @user_has_permissions('view_activity') -def view_letter_notification_as_preview(service_id, notification_id, filetype): +def view_letter_notification_as_preview( + service_id, notification_id, filetype, with_metadata=False +): if filetype not in ('pdf', 'png'): abort(404) - notification = notification_api_client.get_notification(service_id, str(notification_id)) + try: - if notification['status'] == "validation-failed": - preview = notification_api_client.get_notification_letter_preview_with_overlay( - service_id, - notification_id, - filetype, - page=request.args.get('page') - ) - else: - preview = notification_api_client.get_notification_letter_preview( - service_id, - notification_id, - filetype, - page=request.args.get('page') - ) + preview = notification_api_client.get_notification_letter_preview( + service_id, + notification_id, + filetype, + page=request.args.get('page') + ) display_file = base64.b64decode(preview['content']) except APIError: display_file = get_preview_error_image() + preview = {"metadata": {}} + if with_metadata: + return display_file, preview['metadata'] return display_file diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 0ce251f6b..a6982b553 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -103,17 +103,6 @@ class NotificationApiClient(NotifyAdminAPIClient): return self.get(url=get_url) - def get_notification_letter_preview_with_overlay(self, service_id, notification_id, file_type, page=None): - get_url = '/service/{}/template/preview/{}/{}{}{}'.format( - service_id, - notification_id, - file_type, - '?overlay=1', - '&page={}'.format(page) if page else '', - ) - - return self.get(url=get_url) - def update_notification_to_cancelled(self, service_id, notification_id): return self.post( url='/service/{}/notifications/{}/cancel'.format(service_id, notification_id), diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index 0446406ef..4e5ece0cf 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -15,7 +15,6 @@ message_count_label(1, template.template_type, suffix='') | capitalize, back_link=back_link ) }} -

{% if is_precompiled_letter %} Provided as PDF @@ -45,7 +44,7 @@

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

- Validation failed – content is outside the printable area + Validation failed. {{ message.detail | safe }}

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

diff --git a/app/utils.py b/app/utils.py index dffa23ebe..98254c291 100644 --- a/app/utils.py +++ b/app/utils.py @@ -557,7 +557,7 @@ LETTER_VALIDATION_MESSAGES = { }, 'content-outside-printable-area': { 'title': 'We cannot print your letter', - 'detail': 'The content appears outside the printable area on {invalid_pages}
' + 'detail': 'The content appears outside the printable area on {invalid_pages}.
' '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 d2ca2db49..3aba1d535 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -263,10 +263,20 @@ def test_notification_page_shows_page_for_letter_sent_with_test_key( expected_postage, ): - mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', - return_value=b'foo' - ) + if is_precompiled_letter: + mocker.patch( + 'app.main.views.notifications.view_letter_notification_as_preview', + return_value=(b'foo', { + 'message': '', + 'invalid_pages': '[]', + 'page_count': '1' + }) + ) + else: + mocker.patch( + 'app.main.views.notifications.view_letter_notification_as_preview', + return_value=b'foo' + ) mocker.patch( 'app.main.views.notifications.pdf_page_count', @@ -319,7 +329,7 @@ def test_notification_page_shows_page_for_letter_sent_with_test_key( ), ( 'validation-failed', - 'Validation failed – content is outside the printable area', + 'Validation failed.', ), ( 'technical-failure', @@ -334,7 +344,6 @@ def test_notification_page_shows_cancelled_or_failed_letter( notification_status, expected_message, ): - mock_get_notification( mocker, fake_uuid, @@ -343,7 +352,7 @@ def test_notification_page_shows_cancelled_or_failed_letter( ) mocker.patch( 'app.main.views.notifications.get_page_count_for_letter', - return_value=1 + return_value=1, ) page = client_request.get( @@ -507,10 +516,16 @@ def test_should_show_image_of_letter_notification_that_failed_validation( mock_get_notification(mocker, fake_uuid, template_type='letter', notification_status='validation-failed') + metadata = { + 'message': 'content-outside-printable-area', + 'invalid_pages': '[1]', + 'page_count': '1' + } mocker.patch( - 'app.main.views.notifications.notification_api_client.get_notification_letter_preview_with_overlay', + 'app.main.views.notifications.notification_api_client.get_notification_letter_preview', return_value={ - 'content': base64.b64encode(b'foo').decode('utf-8') + 'content': base64.b64encode(b'foo').decode('utf-8'), + 'metadata': metadata } ) @@ -518,11 +533,12 @@ def test_should_show_image_of_letter_notification_that_failed_validation( 'main.view_letter_notification_as_preview', service_id=SERVICE_ONE_ID, notification_id=fake_uuid, - filetype='png' + filetype='png', + with_metadata=True )) assert response.status_code == 200 - assert response.get_data(as_text=True) == 'foo' + assert response.get_data(as_text=True) == 'foo', metadata def test_should_show_preview_error_image_letter_notification_on_preview_error( @@ -692,10 +708,16 @@ def test_notification_page_has_expected_template_link_for_letter( has_template_link ): - mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', - return_value=b'foo' - ) + if is_precompiled_letter: + mocker.patch( + 'app.main.views.notifications.view_letter_notification_as_preview', + side_effect=[(b'foo', {"message": "", "invalid_pages": "[]", "page_count": "1"}), b'foo'] + ) + else: + mocker.patch( + 'app.main.views.notifications.view_letter_notification_as_preview', + return_value=b'foo' + ) mocker.patch( 'app.main.views.notifications.pdf_page_count', diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 1733a8aec..3aa529046 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -750,7 +750,7 @@ def test_letter_validation_preview_renders_correctly(mocker, platform_admin_clie @pytest.mark.parametrize("passed_validation,message,expected_class", [ (True, 'Your PDF passed the layout check', 'banner-with-tick'), - (False, 'content-otside-printable-area', "banner-dangerous") + (False, 'content-outside-printable-area', "banner-dangerous") ]) def test_letter_validation_preview_calls_template_preview_when_data_correct_and_displays_correct_message( mocker, platform_admin_client, passed_validation, message, expected_class