mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-09-11 10:28:41 -04:00
Add some summaries of letter validation errors
We show letter validation errors in two places: 1. In response to a user uploading a PDF Here we use the error banner pattern because the problem is as a direct consequence of a user’s action, and is blocking them from continuing. 2. Once a PDF provided through the API has been validated We use a less prominent pattern of red text with no border because the message is reporting on something that’s already happened, and which wasn’t a direct consequence of the user clicking something Because the context and patterns used are different we need slightly different content in each of these situations. Previously we tried to reuse the same content to make the code cleaner and less repetitive. But ultimately a clear interface trumps clear code.
This commit is contained in:
@@ -44,7 +44,7 @@
|
|||||||
</p>
|
</p>
|
||||||
{% elif notification_status == 'validation-failed' %}
|
{% elif notification_status == 'validation-failed' %}
|
||||||
<p class="notification-status-cancelled">
|
<p class="notification-status-cancelled">
|
||||||
Validation failed. {{ message.detail | safe }}
|
{{ message.summary | safe }}
|
||||||
</p>
|
</p>
|
||||||
{% elif notification_status == 'technical-failure' %}
|
{% elif notification_status == 'technical-failure' %}
|
||||||
<p class="notification-status-cancelled">
|
<p class="notification-status-cancelled">
|
||||||
|
|||||||
+29
-1
@@ -579,6 +579,10 @@ LETTER_VALIDATION_MESSAGES = {
|
|||||||
'You need to change the size or orientation of {invalid_pages}. <br>'
|
'You need to change the size or orientation of {invalid_pages}. <br>'
|
||||||
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
||||||
),
|
),
|
||||||
|
'summary': (
|
||||||
|
'Validation failed because {invalid_pages} {invalid_pages_are_or_is} not A4 portrait size.<br>'
|
||||||
|
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
||||||
|
),
|
||||||
},
|
},
|
||||||
'content-outside-printable-area': {
|
'content-outside-printable-area': {
|
||||||
'title': 'Your content is outside the printable area',
|
'title': 'Your content is outside the printable area',
|
||||||
@@ -586,6 +590,10 @@ LETTER_VALIDATION_MESSAGES = {
|
|||||||
'You need to edit {invalid_pages}.<br>'
|
'You need to edit {invalid_pages}.<br>'
|
||||||
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
||||||
),
|
),
|
||||||
|
'summary': (
|
||||||
|
'Validation failed because content is outside the printable area on {invalid_pages}.<br>'
|
||||||
|
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
||||||
|
),
|
||||||
},
|
},
|
||||||
'letter-too-long': {
|
'letter-too-long': {
|
||||||
'title': 'Your letter is too long',
|
'title': 'Your letter is too long',
|
||||||
@@ -593,6 +601,10 @@ LETTER_VALIDATION_MESSAGES = {
|
|||||||
'Letters must be 10 pages or less. <br>'
|
'Letters must be 10 pages or less. <br>'
|
||||||
'Your letter is {page_count} pages long.'
|
'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': {
|
'no-encoded-string': {
|
||||||
'title': 'Sanitise failed - No encoded string'
|
'title': 'Sanitise failed - No encoded string'
|
||||||
@@ -603,6 +615,10 @@ LETTER_VALIDATION_MESSAGES = {
|
|||||||
'Notify cannot read this PDF.'
|
'Notify cannot read this PDF.'
|
||||||
'<br>Save a new copy of your file and try again.'
|
'<br>Save a new copy of your file and try again.'
|
||||||
),
|
),
|
||||||
|
'summary': (
|
||||||
|
'Letters must be 10 pages or less. <br>'
|
||||||
|
'This letter is {page_count} pages long.'
|
||||||
|
),
|
||||||
},
|
},
|
||||||
'address-is-empty': {
|
'address-is-empty': {
|
||||||
'title': 'The address block is empty',
|
'title': 'The address block is empty',
|
||||||
@@ -610,6 +626,10 @@ LETTER_VALIDATION_MESSAGES = {
|
|||||||
'You need to add a recipient address.<br>'
|
'You need to add a recipient address.<br>'
|
||||||
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
||||||
),
|
),
|
||||||
|
'summary': (
|
||||||
|
'Validation failed because the address block is empty.<br>'
|
||||||
|
f'Files must meet our <a href="{LETTER_SPECIFICATION_URL}" target="_blank">letter specification</a>.'
|
||||||
|
),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -618,6 +638,8 @@ def get_letter_validation_error(validation_message, invalid_pages=None, page_cou
|
|||||||
if validation_message not in LETTER_VALIDATION_MESSAGES:
|
if validation_message not in LETTER_VALIDATION_MESSAGES:
|
||||||
return {'title': 'Validation failed'}
|
return {'title': 'Validation failed'}
|
||||||
|
|
||||||
|
invalid_pages_are_or_is = 'is' if len(invalid_pages) == 1 else 'are'
|
||||||
|
|
||||||
invalid_pages = unescaped_formatted_list(
|
invalid_pages = unescaped_formatted_list(
|
||||||
invalid_pages or [],
|
invalid_pages or [],
|
||||||
before_each='',
|
before_each='',
|
||||||
@@ -630,8 +652,14 @@ def get_letter_validation_error(validation_message, invalid_pages=None, page_cou
|
|||||||
'title': LETTER_VALIDATION_MESSAGES[validation_message]['title'],
|
'title': LETTER_VALIDATION_MESSAGES[validation_message]['title'],
|
||||||
'detail': LETTER_VALIDATION_MESSAGES[validation_message]['detail'].format(
|
'detail': LETTER_VALIDATION_MESSAGES[validation_message]['detail'].format(
|
||||||
invalid_pages=invalid_pages,
|
invalid_pages=invalid_pages,
|
||||||
|
invalid_pages_are_or_is=invalid_pages_are_or_is,
|
||||||
page_count=page_count,
|
page_count=page_count,
|
||||||
)
|
),
|
||||||
|
'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,
|
||||||
|
),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -332,7 +332,7 @@ def test_notification_page_shows_validation_failed_precompiled_letter(
|
|||||||
|
|
||||||
error_message = page.find('p', class_='notification-status-cancelled').text
|
error_message = page.find('p', class_='notification-status-cancelled').text
|
||||||
assert normalize_spaces(error_message) == (
|
assert normalize_spaces(error_message) == (
|
||||||
'Validation failed. You need to edit page 1.'
|
'Validation failed because content is outside the printable area on page 1.'
|
||||||
'Files must meet our letter specification.'
|
'Files must meet our letter specification.'
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
+44
-11
@@ -413,24 +413,57 @@ def test_get_letter_validation_error_for_unknown_error():
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('error_message, expected_title, expected_content', [
|
@pytest.mark.parametrize('error_message, 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" '
|
'Your letter is not A4 portrait size',
|
||||||
'target="_blank">letter specification</a>.'),
|
(
|
||||||
('content-outside-printable-area', 'Your content is outside the printable area',
|
'You need to change the size or orientation of page 2. <br>Files must meet our '
|
||||||
'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" '
|
||||||
'<a href="https://docs.notifications.service.gov.uk/documentation/images/notify-pdf-letter-spec-v2.4.pdf" '
|
'target="_blank">letter specification</a>.'
|
||||||
'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.<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>.'
|
||||||
|
),
|
||||||
|
(
|
||||||
|
'Validation failed because content is outside the printable area '
|
||||||
|
'on 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.'
|
||||||
|
),
|
||||||
|
(
|
||||||
|
'Validation failed because this letter is 13 pages long.<br>'
|
||||||
|
'Letters must be 10 pages or less.'
|
||||||
|
),
|
||||||
|
),
|
||||||
])
|
])
|
||||||
def test_get_letter_validation_error_for_known_errors(
|
def test_get_letter_validation_error_for_known_errors(
|
||||||
error_message,
|
error_message,
|
||||||
expected_title,
|
expected_title,
|
||||||
expected_content,
|
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=[2], page_count=13)
|
||||||
|
|
||||||
assert error['title'] == expected_title
|
assert error['title'] == expected_title
|
||||||
assert expected_content in error['detail']
|
assert expected_content in error['detail']
|
||||||
|
assert error['summary'] == expected_summary
|
||||||
|
|||||||
Reference in New Issue
Block a user