From 483c00b3a2b9934a15045413da2ffc4d4ada87e2 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Wed, 11 Mar 2020 14:10:48 +0000 Subject: [PATCH] Undo bug for previewing precompiled letters https://github.com/alphagov/notifications-api/pull/2742/files#diff-d9a6761afaff4a491e60b64f6063654fL265 introduced a change in behaviour which causes template preview to 500 for a precompiled letter that is invalid but not because the content is outside the printable area. The changes it back to the previous behaviour, where we b64encode and utf-8 decode for a page that is invalid for reasons that are not content outside the printable area, for example a non portrait a4 letter. I didn't end up writing a unit test for this because I really can't figure out the existing behaviour we expect for this use case. This method is too big and complex but I'm confident by looking at the change in the commit mentioned above that this puts behaviour back to how it was. Manual testing also confirms the fix. I've also taken this opportunity to improve two test names whilst I was looking at things. --- app/template/rest.py | 5 ++++- tests/app/template/test_rest.py | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index 411cd6c82..0db1e4786 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -263,7 +263,10 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file if file_type == 'png': try: pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page_number) - 1) - content = pdf_page if page_is_in_invalid_pages else base64.b64encode(pdf_page).decode('utf-8') + if content_outside_printable_area and page_is_in_invalid_pages: + content = pdf_page + else: + content = base64.b64encode(pdf_page).decode('utf-8') except PdfReadError as e: raise InvalidRequest( 'Error extracting requested page from PDF file for notification_id {} type {} {}'.format( diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index ffe660cea..cb92e0b66 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1149,6 +1149,8 @@ def test_preview_letter_template_precompiled_s3_error( ("", "", 'precompiled-preview.png'), # page is valid, no overlay shown ("1", "", 'precompiled-preview.png'), + # page is invalid but not because content is outside printable area so no overlay + ("1", "letter-not-a4-portrait-oriented", 'precompiled-preview.png'), # page is invalid, overlay shown ("1", "content-outside-printable-area", 'precompiled/overlay.png?page_number=1'), # page is valid, no overlay shown @@ -1157,7 +1159,7 @@ def test_preview_letter_template_precompiled_s3_error( ("3", "content-outside-printable-area", 'precompiled/overlay.png?page_number=3'), ] ) -def test_preview_letter_template_precompiled_for_png_file_shows_overlay_where_appropriate( +def test_preview_letter_template_precompiled_for_png_shows_overlay_on_pages_with_content_outside_printable_area( notify_api, client, admin_request, @@ -1227,7 +1229,7 @@ def test_preview_letter_template_precompiled_for_png_file_shows_overlay_where_ap "[2,4]", # it shouldn't make a difference if the error was on the first page or not ] ) -def test_preview_letter_template_precompiled_for_pdf_file_shows_overlay_if_content_outside_printable_area( +def test_preview_letter_template_precompiled_for_pdf_shows_overlay_on_all_pages_if_content_outside_printable_area( notify_api, client, admin_request,