From 2f52a8fd64f1418dbc246c57aa38080ebcd15ad4 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 4 Apr 2019 12:07:08 +0100 Subject: [PATCH 1/4] Call overlaid preview for precompiled letters that fail validation Also test precompiled letter preview with overlay --- app/template/rest.py | 16 ++++++++++++---- tests/app/template/test_rest.py | 13 ++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index ac4bc9a64..be09cb74e 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -249,10 +249,18 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file status_code=500 ) - url = '{}/precompiled-preview.png{}'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'], - '?hide_notify=true' if page_number == '1' else '' - ) + if request.args.get('overlay'): + content = pdf_page + url = '{}/precompiled/overlay.png{}{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?invert=1', + '&hide_notify=true' if page_number == '1' else '' + ) + else: + url = '{}/precompiled-preview.png{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?hide_notify=true' if page_number == '1' else '' + ) content = _get_png_preview(url, content, notification.id, json=False) else: diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 2963bd3ec..030ac6674 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1098,12 +1098,18 @@ def test_preview_letter_template_precompiled_s3_error( "when calling the GetObject operation: Unauthorized".format(notification.id) +@pytest.mark.parametrize( + "post_url, overlay", + [('precompiled-preview.png', None), ('precompiled/overlay.png?invert=1', 1)] +) def test_preview_letter_template_precompiled_png_file_type( notify_api, client, admin_request, sample_service, - mocker + mocker, + post_url, + overlay ): template = create_template(sample_service, @@ -1128,7 +1134,7 @@ def test_preview_letter_template_precompiled_png_file_type( mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) mock_post = request_mock.post( - 'http://localhost/notifications-template-preview/precompiled-preview.png', + 'http://localhost/notifications-template-preview/{}'.format(post_url), content=png_content, headers={'X-pdf-page-count': '1'}, status_code=200 @@ -1138,7 +1144,8 @@ def test_preview_letter_template_precompiled_png_file_type( 'template.preview_letter_template_by_notification_id', service_id=notification.service_id, notification_id=notification.id, - file_type='png' + file_type='png', + overlay=overlay, ) with pytest.raises(ValueError): From 63fe21020216afe134272612706b82eccdc80b27 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 4 Apr 2019 18:38:31 +0100 Subject: [PATCH 2/4] Also show overlay on document download of failed letter And of course test this new flex --- app/letters/utils.py | 1 - app/template/rest.py | 16 +++++++++++----- tests/app/template/test_rest.py | 19 ++++++++++++------- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/app/letters/utils.py b/app/letters/utils.py index c7e558244..786edbc57 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -136,7 +136,6 @@ def get_letter_pdf(notification): s3 = boto3.resource('s3') bucket = s3.Bucket(bucket_name) - item = next(x for x in bucket.objects.filter(Prefix=prefix)) obj = s3.Object( diff --git a/app/template/rest.py b/app/template/rest.py index be09cb74e..914520cda 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -251,10 +251,9 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file if request.args.get('overlay'): content = pdf_page - url = '{}/precompiled/overlay.png{}{}'.format( + url = '{}/precompiled/overlay.png{}'.format( current_app.config['TEMPLATE_PREVIEW_API_HOST'], '?invert=1', - '&hide_notify=true' if page_number == '1' else '' ) else: url = '{}/precompiled-preview.png{}'.format( @@ -262,7 +261,14 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file '?hide_notify=true' if page_number == '1' else '' ) - content = _get_png_preview(url, content, notification.id, json=False) + content = _get_png_preview_or_overlaid_pdf(url, content, notification.id, json=False) + elif file_type == 'pdf' and request.args.get('overlay'): + content = pdf_file + url = '{}/precompiled/overlay.pdf{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?invert=1', + ) + content = _get_png_preview_or_overlaid_pdf(url, content, notification.id, json=False) else: template_for_letter_print = { @@ -287,12 +293,12 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file file_type, '?page={}'.format(page) if page else '' ) - content = _get_png_preview(url, data, notification.id, json=True) + content = _get_png_preview_or_overlaid_pdf(url, data, notification.id, json=True) return jsonify({"content": content}) -def _get_png_preview(url, data, notification_id, json=True): +def _get_png_preview_or_overlaid_pdf(url, data, notification_id, json=True): if json: resp = requests_post( url, diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 030ac6674..dc7dbbd2c 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1099,15 +1099,20 @@ def test_preview_letter_template_precompiled_s3_error( @pytest.mark.parametrize( - "post_url, overlay", - [('precompiled-preview.png', None), ('precompiled/overlay.png?invert=1', 1)] + "filetype, post_url, overlay", + [ + ('png', 'precompiled-preview.png', None), + ('png', 'precompiled/overlay.png?invert=1', 1), + ('pdf', 'precompiled/overlay.pdf?invert=1', 1) + ] ) -def test_preview_letter_template_precompiled_png_file_type( +def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay( notify_api, client, admin_request, sample_service, mocker, + filetype, post_url, overlay ): @@ -1127,7 +1132,7 @@ def test_preview_letter_template_precompiled_png_file_type( with requests_mock.Mocker() as request_mock: pdf_content = b'\x00\x01' - png_content = b'\x00\x02' + expected_returned_content = b'\x00\x02' mock_get_letter_pdf = mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) @@ -1135,7 +1140,7 @@ def test_preview_letter_template_precompiled_png_file_type( mock_post = request_mock.post( 'http://localhost/notifications-template-preview/{}'.format(post_url), - content=png_content, + content=expected_returned_content, headers={'X-pdf-page-count': '1'}, status_code=200 ) @@ -1144,14 +1149,14 @@ def test_preview_letter_template_precompiled_png_file_type( 'template.preview_letter_template_by_notification_id', service_id=notification.service_id, notification_id=notification.id, - file_type='png', + file_type=filetype, overlay=overlay, ) with pytest.raises(ValueError): mock_post.last_request.json() assert mock_get_letter_pdf.called_once_with(notification) - assert base64.b64decode(resp['content']) == png_content + assert base64.b64decode(resp['content']) == expected_returned_content @pytest.mark.parametrize('page_number,expect_preview_url', [ From dd9e2856848bfa0d8f236f59e62b072a7fc2e87c Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 10 Apr 2019 16:08:08 +0100 Subject: [PATCH 3/4] Refactor preview_letter_template_by_notification_id --- app/template/rest.py | 45 +++++++++++++++------------------ tests/app/template/test_rest.py | 2 +- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index 914520cda..2c3755b74 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -235,13 +235,23 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file ) content = base64.b64encode(pdf_file).decode('utf-8') + overlay = request.args.get('overlay') + page_number = page if page else "1" + + if overlay: + path = '/precompiled/overlay.{}'.format(file_type) + query_string = '?invert=1' + content = pdf_file + elif file_type == 'png': + path = '/precompiled-preview.png' + query_string = '?hide_notify=true' if page_number == '1' else '' + else: + path = None if file_type == 'png': try: - page_number = page if page else "1" - pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page_number) - 1) - content = base64.b64encode(pdf_page).decode('utf-8') + content = pdf_page if overlay else 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( @@ -249,26 +259,11 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file status_code=500 ) - if request.args.get('overlay'): - content = pdf_page - url = '{}/precompiled/overlay.png{}'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'], - '?invert=1', - ) - else: - url = '{}/precompiled-preview.png{}'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'], - '?hide_notify=true' if page_number == '1' else '' - ) - - content = _get_png_preview_or_overlaid_pdf(url, content, notification.id, json=False) - elif file_type == 'pdf' and request.args.get('overlay'): - content = pdf_file - url = '{}/precompiled/overlay.pdf{}'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'], - '?invert=1', - ) - content = _get_png_preview_or_overlaid_pdf(url, content, notification.id, json=False) + if path: + url = current_app.config['TEMPLATE_PREVIEW_API_HOST'] + path + query_string + response_content = _get_png_preview_or_overlaid_pdf(url, content, notification.id, json=False) + else: + response_content = content else: template_for_letter_print = { @@ -293,9 +288,9 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file file_type, '?page={}'.format(page) if page else '' ) - content = _get_png_preview_or_overlaid_pdf(url, data, notification.id, json=True) + response_content = _get_png_preview_or_overlaid_pdf(url, data, notification.id, json=True) - return jsonify({"content": content}) + return jsonify({"content": response_content}) def _get_png_preview_or_overlaid_pdf(url, data, notification_id, json=True): diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index dc7dbbd2c..b0ff45ee8 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1192,7 +1192,7 @@ def test_preview_letter_template_precompiled_png_file_type_hide_notify_tag_only_ mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) mocker.patch('app.template.rest.extract_page_from_pdf', return_value=png_content) - mock_get_png_preview = mocker.patch('app.template.rest._get_png_preview', return_value=encoded) + mock_get_png_preview = mocker.patch('app.template.rest._get_png_preview_or_overlaid_pdf', return_value=encoded) admin_request.get( 'template.preview_letter_template_by_notification_id', From 2d71aea7a147ad8cd3fe6c92ce1e66115142347d Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 11 Apr 2019 16:33:40 +0100 Subject: [PATCH 4/4] Get rid of an unneeded flag - destination endpoint is slimmer now and doesnt need it --- app/template/rest.py | 5 ++--- tests/app/template/test_rest.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/template/rest.py b/app/template/rest.py index 2c3755b74..7c44ec405 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -240,11 +240,10 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file if overlay: path = '/precompiled/overlay.{}'.format(file_type) - query_string = '?invert=1' content = pdf_file elif file_type == 'png': - path = '/precompiled-preview.png' query_string = '?hide_notify=true' if page_number == '1' else '' + path = '/precompiled-preview.png' + query_string else: path = None @@ -260,7 +259,7 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file ) if path: - url = current_app.config['TEMPLATE_PREVIEW_API_HOST'] + path + query_string + url = current_app.config['TEMPLATE_PREVIEW_API_HOST'] + path response_content = _get_png_preview_or_overlaid_pdf(url, content, notification.id, json=False) else: response_content = content diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index b0ff45ee8..e2d5caeb3 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1102,8 +1102,8 @@ def test_preview_letter_template_precompiled_s3_error( "filetype, post_url, overlay", [ ('png', 'precompiled-preview.png', None), - ('png', 'precompiled/overlay.png?invert=1', 1), - ('pdf', 'precompiled/overlay.pdf?invert=1', 1) + ('png', 'precompiled/overlay.png', 1), + ('pdf', 'precompiled/overlay.pdf', 1) ] ) def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay(