From fc8c536fa793020e56a883499448ba5fc61ddd69 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 22 Dec 2021 10:31:36 +0000 Subject: [PATCH 1/2] Show validation error message for a templated letter over 10 pages If a letters that has been posted via the API has more than 10 pages it would not get a validation-failed status. This also happens for letters in a CSV upload, only the first row has been validated for having too many pages, because you need to created the pdf before getting an accurate page count. The API has been updated to mark these letters as invalid and move the letter to the invalid s3 bucket, the meta data is also set with the error message and page count. This PR updates the notification page to display the validation error. https://www.pivotaltracker.com/story/show/169209742 --- app/main/views/notifications.py | 8 ++++++++ tests/app/main/views/test_notifications.py | 23 ++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index af887b48b..158c02635 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -18,6 +18,7 @@ from flask import ( url_for, ) from notifications_python_client.errors import APIError, HTTPError +from notifications_utils import LETTER_MAX_PAGE_COUNT from notifications_utils.letter_timings import ( get_letter_timings, letter_can_be_cancelled, @@ -79,6 +80,13 @@ def view_notification(service_id, notification_id): ) else: page_count = get_page_count_for_letter(notification['template'], values=personalisation) + if page_count and page_count > LETTER_MAX_PAGE_COUNT: + # when a templated letter is sent via the api and the personalisation pushes + # the page count over 10 we need to validate that. The letter is not in validation failed, + # this will enable us to show the error. Perhaps there is a better way to do this. + error_message = get_letter_validation_error( + "letter-too-long", [1], page_count + ) if notification.get('postage'): if notification["status"] == "validation-failed": diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 069b34d78..d827f8878 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -677,6 +677,29 @@ def test_should_show_image_of_letter_notification_that_failed_validation( assert response.get_data(as_text=True) == 'foo', metadata +def test_should_show_image_of_templated_letter_notification_that_failed_validation_because_letter_is_too_long( + client_request, + mocker, + fake_uuid, +): + notification = create_notification( + notification_status='validation-failed', + template_type='letter') + mocker.patch('app.notification_api_client.get_notification', return_value=notification) + mocker.patch('app.main.views.notifications.get_page_count_for_letter', return_value=11) + + 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 because this letter is 11 pages long.Letters must be 10 pages or "\ + "less (5 double-sided sheets of paper)." + + def test_should_show_preview_error_image_letter_notification_on_preview_error( logged_in_client, fake_uuid, From 49138946acc5c0badefbdebe7b3ac8d427873865 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 27 Jan 2022 16:54:18 +0000 Subject: [PATCH 2/2] Update comment to help clarify the decision for this approach to showing letters that are too long. --- app/main/views/notifications.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 158c02635..04c20bbe9 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -81,9 +81,15 @@ def view_notification(service_id, notification_id): else: page_count = get_page_count_for_letter(notification['template'], values=personalisation) if page_count and page_count > LETTER_MAX_PAGE_COUNT: - # when a templated letter is sent via the api and the personalisation pushes - # the page count over 10 we need to validate that. The letter is not in validation failed, - # this will enable us to show the error. Perhaps there is a better way to do this. + # We check page count here to show the right error message for a letter that is too long. + # Another way to do this would be to get the status and error message from letter metadata. + # This would be a significant amount of work though, out of scope for this bug fix. + # This is because currently we do not pull the letter from S3 when showing preview. + # Instead, we generate letter preview based on the letter template and personalisation. + # Additionally, when a templated letter is sent via the api and the personalisation pushes the + # page count over 10 pages, it takes a while for validation status to come through. + # Checking page count here will enable us to show the error message even if the letter is not + # fully processed yet. error_message = get_letter_validation_error( "letter-too-long", [1], page_count )