Grab metadata when getting pdf letter preview from S3

Also use this metadata to decide whether preview pages need
overlay or not. So far we have always added overlay when validation
has failed. Now we will only show it when validation failed due to
content being outside of printable area.
This commit is contained in:
Pea Tyczynska
2019-10-29 16:19:50 +00:00
parent ca96c5d81b
commit c2825e10b1
7 changed files with 81 additions and 33 deletions

View File

@@ -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"
}
)

View File

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

View File

@@ -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))

View File

@@ -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)