Merge pull request #3170 from alphagov/show-users-why-their-precompiled-letter-failed-validation

Show users why their precompiled letter failed validation

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',

tests/app/main/views/test_platform_admin.py

@@ -748,9 +748,12 @@ def test_letter_validation_preview_renders_correctly(mocker, platform_admin_clie
     assert page.find_all('input', class_='file-upload-field')
 
 
-@pytest.mark.parametrize("result,expected_class", [(True, 'banner-with-tick'), (False, "banner-dangerous")])
+@pytest.mark.parametrize("passed_validation,message,expected_class", [
+    (True, 'Your PDF passed the layout check', 'banner-with-tick'),
+    (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, result, expected_class
+    mocker, platform_admin_client, passed_validation, message, expected_class
 ):
     endpoint = '{}/precompiled/validate?include_preview=true'.format(current_app.config['TEMPLATE_PREVIEW_API_HOST'])
     mocker.patch('app.main.views.platform_admin.antivirus_client.scan', return_value=True)
@@ -759,7 +762,7 @@ def test_letter_validation_preview_calls_template_preview_when_data_correct_and_
         rmock.request(
             "POST",
             endpoint,
-            json={"pages": [], "message": "bazinga!", "result": result},
+            json={"pages": [], "message": message, "result": passed_validation},
             status_code=200
         )
         with open('tests/test_pdf_files/multi_page_pdf.pdf', 'rb') as file:
@@ -773,7 +776,10 @@ def test_letter_validation_preview_calls_template_preview_when_data_correct_and_
         assert rmock.request_history[0].url == endpoint
 
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert page.find('div', class_=expected_class).text.strip() == "bazinga!"
+    if passed_validation:
+        assert page.find('div', class_=expected_class).text.strip() == message
+    else:
+        assert page.find('div', class_=expected_class).find('h1', {"data-error-type": message})
 
 
 def test_letter_validation_preview_doesnt_call_template_preview_when_no_file(mocker, platform_admin_client):

tests/app/main/views/test_send.py

@@ -2778,8 +2778,7 @@ def test_check_messages_does_not_allow_to_send_letter_longer_than_10_pages(
         upload_id=fake_uuid,
         _test_page_title=False,
     )
-
-    assert page.select('#letter-too-long')
+    assert page.find('h1', {"data-error-type": "letter-too-long"})
 
     assert len(page.select('.letter img')) == 10  # if letter longer than 10 pages, only 10 first pages are displayed
     assert not page.select('[type=submit]')
@@ -3149,7 +3148,7 @@ def test_send_one_off_letter_errors_if_letter_longer_than_10_pages(
         _test_page_title=False,
     )
 
-    assert page.select('#letter-too-long')
+    assert page.find('h1', {"data-error-type": "letter-too-long"})
     assert len(page.select('.letter img')) == 10
 
     assert not page.select('[type=submit]')

tests/app/main/views/test_templates.py

@@ -545,7 +545,7 @@ def test_view_letter_template_does_not_display_send_button_if_template_over_10_p
     )
 
     assert "Send" not in page.text
-    assert page.select('#letter-too-long')
+    assert page.find('h1', {"data-error-type": "letter-too-long"})
 
 
 def test_edit_letter_template_postage_page_displays_correctly(

tests/app/main/views/test_uploads.py

@@ -249,11 +249,7 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request):
             message='content-outside-printable-area'
         )
 
-    assert page.find('div', class_='banner-dangerous').find('h1').text == 'We cannot print your letter'
-    assert page.find(
-        'div', class_='banner-dangerous').find('p').text == (
-        'The content appears outside the printable area on page 1 Files must meet our letter specification.'
-    )
+    assert page.find('div', class_='banner-dangerous').find('h1', {"data-error-type": 'content-outside-printable-area'})
     assert not page.find('button', {'type': 'submit'})