diff --git a/app/letters/utils.py b/app/letters/utils.py index 4aa4ee5c7..272cd9f92 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -164,7 +164,7 @@ def get_file_names_from_error_bucket(): return bucket.objects.filter(Prefix="ERROR") -def get_letter_pdf(notification): +def get_letter_pdf_and_metadata(notification): bucket_name, prefix = get_bucket_name_and_prefix_for_notification(notification) s3 = boto3.resource('s3') @@ -174,8 +174,8 @@ def get_letter_pdf(notification): obj = s3.Object( bucket_name=bucket_name, key=item.key - ) - return obj.get()["Body"].read() + ).get() + return obj["Body"].read(), obj["Metadata"] def _move_s3_object(source_bucket, source_filename, target_bucket, target_filename, metadata=None): diff --git a/app/template/rest.py b/app/template/rest.py index 85b610ebb..b77df5741 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -32,7 +32,7 @@ from app.errors import ( register_errors, InvalidRequest ) -from app.letters.utils import get_letter_pdf +from app.letters.utils import get_letter_pdf_and_metadata from app.models import SMS_TYPE, Template, SECOND_CLASS, LETTER_TYPE from app.notifications.validators import service_has_permission, check_reply_to from app.schema_validation import validate @@ -231,11 +231,12 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file notification = get_notification_by_id(notification_id) template = dao_get_template_by_id(notification.template_id) + metadata = {} if template.is_precompiled_letter: try: - pdf_file = get_letter_pdf(notification) + pdf_file, metadata = get_letter_pdf_and_metadata(notification) except botocore.exceptions.ClientError as e: raise InvalidRequest( @@ -245,7 +246,7 @@ 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') + overlay = metadata.get("message") == "content-outside-printable-area" page_number = page if page else "1" if overlay: @@ -300,7 +301,7 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file ) response_content = _get_png_preview_or_overlaid_pdf(url, data, notification.id, json=True) - return jsonify({"content": response_content}) + return jsonify({"content": response_content, "metadata": metadata}) def _get_png_preview_or_overlaid_pdf(url, data, notification_id, json=True): diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index e5d267024..95cf1aa51 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -4,7 +4,7 @@ from flask import jsonify, request, url_for, current_app, send_file from app import api_user, authenticated_service from app.dao import notifications_dao -from app.letters.utils import get_letter_pdf +from app.letters.utils import get_letter_pdf_and_metadata from app.schema_validation import validate from app.v2.errors import BadRequestError, PDFNotReadyError from app.v2.notifications import v2_notification_blueprint @@ -48,7 +48,7 @@ def get_pdf_for_notification(notification_id): raise PDFNotReadyError() try: - pdf_data = get_letter_pdf(notification) + pdf_data, metadata = get_letter_pdf_and_metadata(notification) except Exception: raise PDFNotReadyError() diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 38e61deef..fd41f4b00 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -480,7 +480,7 @@ def test_process_letter_task_check_virus_scan_passed_when_sanitise_fails( "validation_passed": False, "message": "content-outside-printable-area", "invalid_pages": [1, 2], - "page_count": 1 + "page_count": 2 } mock_sanitise = mocker.patch( 'app.celery.letters_pdf_tasks._sanitise_precompiled_pdf', return_value=(sanitise_response, "validation_failed") @@ -500,7 +500,7 @@ def test_process_letter_task_check_virus_scan_passed_when_sanitise_fails( target_bucket=target_bucket_name, target_filename=filename, metadata={ "message": "content-outside-printable-area", "invalid_pages": "[1, 2]", - "page_count": "1" + "page_count": "2" } ) diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index 844ada741..99cc4fbe1 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -10,7 +10,7 @@ from app.letters.utils import ( copy_redaction_failed_pdf, get_bucket_name_and_prefix_for_notification, get_letter_pdf_filename, - get_letter_pdf, + get_letter_pdf_and_metadata, letter_print_day, upload_letter_pdf, ScanErrorType, move_failed_pdf, get_folder_name @@ -194,9 +194,9 @@ def test_get_letter_pdf_gets_pdf_from_correct_bucket( s3 = boto3.client('s3', region_name='eu-west-1') s3.put_object(Bucket=bucket_name, Key=filename, Body=b'pdf_content') - ret = get_letter_pdf(sample_precompiled_letter_notification_using_test_key) + file_data, metadata = get_letter_pdf_and_metadata(sample_precompiled_letter_notification_using_test_key) - assert ret == b'pdf_content' + assert file_data == b'pdf_content' @pytest.mark.parametrize('is_precompiled_letter,bucket_config_name', [ diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index ee6dbb29c..ecce789c5 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1088,7 +1088,14 @@ def test_preview_letter_template_precompiled_pdf_file_type( content = b'\x00\x01' - mock_get_letter_pdf = mocker.patch('app.template.rest.get_letter_pdf', return_value=content) + mock_get_letter_pdf = mocker.patch( + 'app.template.rest.get_letter_pdf_and_metadata', + return_value=(content, { + "message": "", + "invalid_pages": "", + "page_count": "1" + }) + ) resp = admin_request.get( 'template.preview_letter_template_by_notification_id', @@ -1123,7 +1130,7 @@ def test_preview_letter_template_precompiled_s3_error( }): with requests_mock.Mocker(): - mocker.patch('app.template.rest.get_letter_pdf', + mocker.patch('app.template.rest.get_letter_pdf_and_metadata', side_effect=botocore.exceptions.ClientError( {'Error': {'Code': '403', 'Message': 'Unauthorized'}}, 'GetObject' @@ -1143,11 +1150,11 @@ def test_preview_letter_template_precompiled_s3_error( @pytest.mark.parametrize( - "filetype, post_url, overlay", + "filetype, post_url, message", [ - ('png', 'precompiled-preview.png', None), - ('png', 'precompiled/overlay.png?page_number=1', 1), - ('pdf', 'precompiled/overlay.pdf', 1) + ('png', 'precompiled-preview.png', ""), + ('png', 'precompiled/overlay.png?page_number=1', "content-outside-printable-area"), + ('pdf', 'precompiled/overlay.pdf', "content-outside-printable-area") ] ) def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay( @@ -1158,7 +1165,7 @@ def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay( mocker, filetype, post_url, - overlay + message ): template = create_template(sample_service, @@ -1178,7 +1185,16 @@ def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay( pdf_content = b'\x00\x01' expected_returned_content = b'\x00\x02' - mock_get_letter_pdf = mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + metadata = { + "message": message, + "invalid_pages": "[1]", + "page_count": "1" + } + + mock_get_letter_pdf = mocker.patch( + 'app.template.rest.get_letter_pdf_and_metadata', + return_value=(pdf_content, metadata) + ) mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) @@ -1189,18 +1205,18 @@ def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay( status_code=200 ) - resp = admin_request.get( + response = admin_request.get( 'template.preview_letter_template_by_notification_id', service_id=notification.service_id, notification_id=notification.id, 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']) == expected_returned_content + assert base64.b64decode(response['content']) == expected_returned_content + assert response["metadata"] == metadata @pytest.mark.parametrize('page_number,expect_preview_url', [ @@ -1234,7 +1250,14 @@ def test_preview_letter_template_precompiled_png_file_type_hide_notify_tag_only_ png_content = b'\x00\x02' encoded = base64.b64encode(png_content).decode('utf-8') - mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch( + 'app.template.rest.get_letter_pdf_and_metadata', + return_value=(pdf_content, { + "message": "", + "invalid_pages": "", + "page_count": "2" + }) + ) 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_or_overlaid_pdf', return_value=encoded) @@ -1276,7 +1299,11 @@ def test_preview_letter_template_precompiled_png_template_preview_500_error( pdf_content = b'\x00\x01' png_content = b'\x00\x02' - mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch('app.template.rest.get_letter_pdf_and_metadata', return_value=(pdf_content, { + "message": "", + "invalid_pages": "", + "page_count": "1" + })) mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) @@ -1325,7 +1352,11 @@ def test_preview_letter_template_precompiled_png_template_preview_400_error( pdf_content = b'\x00\x01' png_content = b'\x00\x02' - mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch('app.template.rest.get_letter_pdf_and_metadata', return_value=(pdf_content, { + "message": "", + "invalid_pages": "", + "page_count": "1" + })) mocker.patch('app.template.rest.extract_page_from_pdf', return_value=pdf_content) @@ -1373,7 +1404,11 @@ def test_preview_letter_template_precompiled_png_template_preview_pdf_error( pdf_content = b'\x00\x01' png_content = b'\x00\x02' - mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content) + mocker.patch('app.template.rest.get_letter_pdf_and_metadata', return_value=(pdf_content, { + "message": "", + "invalid_pages": "", + "page_count": "1" + })) error_message = "PDF Error message" mocker.patch('app.template.rest.extract_page_from_pdf', side_effect=PdfReadError(error_message)) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index d38d95460..91f5bd752 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -656,7 +656,13 @@ def test_get_pdf_for_notification_returns_pdf_content( sample_letter_notification, mocker, ): - mock_get_letter_pdf = mocker.patch('app.v2.notifications.get_notifications.get_letter_pdf', return_value=b'foo') + mock_get_letter_pdf = mocker.patch( + 'app.v2.notifications.get_notifications.get_letter_pdf_and_metadata', return_value=(b'foo', { + "message": "", + "invalid_pages": "", + "page_count": "1" + }) + ) sample_letter_notification.status = 'created' auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) @@ -677,8 +683,8 @@ def test_get_pdf_for_notification_returns_400_if_pdf_not_found( ): # if no files are returned get_letter_pdf throws StopIteration as the iterator runs out mock_get_letter_pdf = mocker.patch( - 'app.v2.notifications.get_notifications.get_letter_pdf', - side_effect=StopIteration + 'app.v2.notifications.get_notifications.get_letter_pdf_and_metadata', + side_effect=(StopIteration, {}) ) sample_letter_notification.status = 'created' @@ -707,7 +713,13 @@ def test_get_pdf_for_notification_only_returns_pdf_content_if_right_status( status, expected_message ): - mock_get_letter_pdf = mocker.patch('app.v2.notifications.get_notifications.get_letter_pdf', return_value=b'foo') + mock_get_letter_pdf = mocker.patch( + 'app.v2.notifications.get_notifications.get_letter_pdf_and_metadata', return_value=(b'foo', { + "message": "", + "invalid_pages": "", + "page_count": "1" + }) + ) sample_letter_notification.status = status auth_header = create_authorization_header(service_id=sample_letter_notification.service_id)