From 7368245c9a3bbfa6e7d7ef75a3e2c9ef8b31f4fd Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 9 Sep 2019 10:59:32 +0100 Subject: [PATCH] Show letter preview once file is uploaded This shows the sanitised letter preview if the file had no validation errors or the preview with the overlay if it failed validation. --- app/main/views/uploads.py | 45 ++++++++-- app/navigation.py | 4 + app/notify_client/service_api_client.py | 6 ++ app/s3_client/s3_letter_upload_client.py | 11 +++ app/template_previews.py | 34 ++++++++ app/templates/views/uploads/preview.html | 4 + tests/app/main/views/test_uploads.py | 83 ++++++++++++++++++- .../notify_client/test_service_api_client.py | 8 ++ tests/app/test_template_previews.py | 39 +++++++++ 9 files changed, 228 insertions(+), 6 deletions(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index ef47b61da..28e40b8f6 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -13,16 +13,17 @@ from notifications_utils.pdf import pdf_page_count from PyPDF2.utils import PdfReadError from requests import RequestException -from app import current_service +from app import current_service, service_api_client from app.extensions import antivirus_client from app.main import main from app.main.forms import PDFUploadForm from app.s3_client.s3_letter_upload_client import ( + get_letter_pdf_and_metadata, get_transient_letter_file_location, upload_letter_to_s3, ) -from app.template_previews import sanitise_letter -from app.utils import user_has_permissions +from app.template_previews import TemplatePreview, sanitise_letter +from app.utils import get_template, user_has_permissions MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @@ -49,7 +50,7 @@ def upload_letter(service_id): return invalid_upload_error('Your file must be smaller than 2MB') try: - pdf_page_count(BytesIO(pdf_file_bytes)) + page_count = pdf_page_count(BytesIO(pdf_file_bytes)) except PdfReadError: current_app.logger.info('Invalid PDF uploaded for service_id: {}'.format(service_id)) return invalid_upload_error('Your file must be a valid PDF') @@ -76,6 +77,7 @@ def upload_letter(service_id): service_id=current_service.id, file_id=upload_id, original_filename=form.file.data.filename, + page_count=page_count, status=status, ) ) @@ -92,6 +94,39 @@ def invalid_upload_error(message): @user_has_permissions('send_messages') def uploaded_letter_preview(service_id, file_id): original_filename = request.args.get('original_filename') + page_count = request.args.get('page_count') status = request.args.get('status') - return render_template('views/uploads/preview.html', original_filename=original_filename, status=status) + template_dict = service_api_client.get_precompiled_template(service_id) + + template = get_template( + template_dict, + service_id, + letter_preview_url=url_for( + '.view_letter_upload_as_preview', + service_id=service_id, + file_id=file_id + ), + page_count=page_count + ) + + return render_template( + 'views/uploads/preview.html', + original_filename=original_filename, + template=template, + status=status, + ) + + +@main.route("/services//preview-letter-image/") +@user_has_permissions('send_messages') +def view_letter_upload_as_preview(service_id, file_id): + file_location = get_transient_letter_file_location(service_id, file_id) + pdf_file, metadata = get_letter_pdf_and_metadata(file_location) + + page = request.args.get('page') + + if metadata['status'] == 'invalid': + return TemplatePreview.from_invalid_pdf_file(pdf_file, page) + else: + return TemplatePreview.from_valid_pdf_file(pdf_file, page) diff --git a/app/navigation.py b/app/navigation.py index 39b937dd4..cbd05f76d 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -314,6 +314,7 @@ class HeaderNavigation(Navigation): 'view_jobs', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification', 'view_notification_updates', 'view_notifications', @@ -607,6 +608,7 @@ class MainNavigation(Navigation): 'view_job_updates', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification_updates', 'view_notifications_csv', 'view_provider', @@ -887,6 +889,7 @@ class CaseworkNavigation(Navigation): 'view_job_updates', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification_updates', 'view_notifications_csv', 'view_provider', @@ -1170,6 +1173,7 @@ class OrgNavigation(Navigation): 'view_jobs', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification', 'view_notification_updates', 'view_notifications', diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 04448e67e..f247597be 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -261,6 +261,12 @@ class ServiceAPIClient(NotifyAdminAPIClient): ) return self.get(endpoint) + def get_precompiled_template(self, service_id): + """ + Returns the precompiled template for a service, creating it if it doesn't already exist + """ + return self.get('/service/{}/template/precompiled'.format(service_id)) + @cache.set('service-{service_id}-templates') def get_service_templates(self, service_id): """ diff --git a/app/s3_client/s3_letter_upload_client.py b/app/s3_client/s3_letter_upload_client.py index e73266c79..27848f105 100644 --- a/app/s3_client/s3_letter_upload_client.py +++ b/app/s3_client/s3_letter_upload_client.py @@ -1,3 +1,4 @@ +from boto3 import resource from flask import current_app from notifications_utils.s3 import s3upload as utils_s3upload @@ -14,3 +15,13 @@ def upload_letter_to_s3(data, file_location, status): file_location=file_location, metadata={'status': status} ) + + +def get_letter_pdf_and_metadata(file_location): + s3 = resource('s3') + s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() + + pdf = s3_object['Body'].read() + metadata = s3_object['Metadata'] + + return pdf, metadata diff --git a/app/template_previews.py b/app/template_previews.py index 14a7ece98..03bb15f77 100644 --- a/app/template_previews.py +++ b/app/template_previews.py @@ -1,5 +1,9 @@ +import base64 +from io import BytesIO + import requests from flask import current_app, json +from notifications_utils.pdf import extract_page_from_pdf from app import current_service @@ -24,6 +28,36 @@ class TemplatePreview: ) return (resp.content, resp.status_code, resp.headers.items()) + @classmethod + def from_valid_pdf_file(cls, pdf_file, page): + pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page) - 1) + + response = requests.post( + '{}/precompiled-preview.png{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?hide_notify=true' if page == '1' else '' + ), + data=base64.b64encode(pdf_page).decode('utf-8'), + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + + return (response.content, response.status_code, response.headers.items()) + + @classmethod + def from_invalid_pdf_file(cls, pdf_file, page): + pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page) - 1) + + response = requests.post( + '{}/precompiled/overlay.png{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?page_number={}'.format(page) + ), + data=pdf_page, + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + + return (response.content, response.status_code, response.headers.items()) + @classmethod def from_example_template(cls, template, filename): data = { diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index 2597b1c0e..9001411b0 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -16,4 +16,8 @@ Validation failed

{% endif %} + +
+ {{ template|string }} +
{% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index ee98153a2..609ff116b 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -30,6 +30,7 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): antivirus_mock = mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) mocker.patch('app.main.views.uploads.sanitise_letter', return_value=Mock(content='The sanitised content')) mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template') with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: page = client_request.post( @@ -50,6 +51,43 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): assert not page.find(id='validation-error-message') +def test_post_upload_letter_shows_letter_preview_for_valid_file(mocker, client_request): + letter_template = {'template_type': 'letter', + 'reply_to_text': '', + 'postage': 'second', + 'subject': 'hi', + 'content': 'my letter'} + + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mocker.patch('app.main.views.uploads.sanitise_letter', return_value=Mock(content='The sanitised content')) + mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mocker.patch('app.main.views.uploads.pdf_page_count', return_value=3) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True, + ) + + assert len(page.select('.letter-postage')) == 1 + assert normalize_spaces(page.select_one('.letter-postage').text) == ('Postage: second class') + assert page.select_one('.letter-postage')['class'] == ['letter-postage', 'letter-postage-second'] + + letter_images = page.select('main img') + assert len(letter_images) == 3 + + for page_no, img in enumerate(letter_images, start=1): + assert img['src'] == url_for( + '.view_letter_upload_as_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + page=page_no) + + def test_post_upload_letter_shows_error_when_file_is_not_a_pdf(client_request): with open('tests/non_spreadsheet_files/actually_a_png.csv', 'rb') as file: page = client_request.post( @@ -121,6 +159,7 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): mock_sanitise_response = Mock() mock_sanitise_response.raise_for_status.side_effect = RequestException(response=Mock(status_code=400)) mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template') with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: file_contents = file.read() @@ -145,6 +184,43 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): ) == 'Validation failed' +def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request): + letter_template = {'template_type': 'letter', + 'reply_to_text': '', + 'postage': 'first', + 'subject': 'hi', + 'content': 'my letter'} + + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mock_sanitise_response = Mock() + mock_sanitise_response.raise_for_status.side_effect = RequestException(response=Mock(status_code=400)) + mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True, + ) + + assert len(page.select('.letter-postage')) == 1 + assert normalize_spaces(page.select_one('.letter-postage').text) == ('Postage: first class') + assert page.select_one('.letter-postage')['class'] == ['letter-postage', 'letter-postage-first'] + + letter_images = page.select('main img') + assert len(letter_images) == 1 + assert letter_images[0]['src'] == url_for( + '.view_letter_upload_as_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + page=1 + ) + + def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unknown_error(mocker, client_request): mocker.patch('uuid.uuid4', return_value='fake-uuid') mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) @@ -164,12 +240,17 @@ def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unk assert not mock_s3.called -def test_uploaded_letter_preview(client_request): +def test_uploaded_letter_preview(mocker, client_request): + mocker.patch('app.main.views.uploads.service_api_client') + page = client_request.get( 'main.uploaded_letter_preview', service_id=SERVICE_ONE_ID, file_id='fake-uuid', original_filename='my_letter.pdf', + page_count=1, + status='valid', ) assert page.find('h1').text == 'my_letter.pdf' + assert page.find('div', class_='letter-sent') diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 801b7e742..1a1ee7c3a 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -93,6 +93,14 @@ def test_client_creates_service_with_correct_data( ) +def test_get_precompiled_template(mocker): + client = ServiceAPIClient() + mock_get = mocker.patch.object(client, 'get') + + client.get_precompiled_template(SERVICE_ONE_ID) + mock_get.assert_called_once_with('/service/{}/template/precompiled'.format(SERVICE_ONE_ID)) + + @pytest.mark.parametrize('template_data, extra_args, expected_count', ( ( [], diff --git a/tests/app/test_template_previews.py b/tests/app/test_template_previews.py index f37464e8d..4fdd73bbf 100644 --- a/tests/app/test_template_previews.py +++ b/tests/app/test_template_previews.py @@ -1,3 +1,4 @@ +import base64 from functools import partial from unittest.mock import Mock @@ -79,6 +80,44 @@ def test_from_database_object_makes_request( request_mock.assert_called_once_with(expected_url, json=data, headers=headers) +@pytest.mark.parametrize('page_number, expected_url', [ + ('1', 'http://localhost:9999/precompiled-preview.png?hide_notify=true'), + ('2', 'http://localhost:9999/precompiled-preview.png'), +]) +def test_from_valid_pdf_file_makes_request(mocker, page_number, expected_url): + mocker.patch('app.template_previews.extract_page_from_pdf', return_value=b'pdf page') + request_mock = mocker.patch( + 'app.template_previews.requests.post', + return_value=Mock(content='a', status_code='b', headers={'c': 'd'}) + ) + + response = TemplatePreview.from_valid_pdf_file(b'pdf file', page_number) + + assert response == ('a', 'b', {'c': 'd'}.items()) + request_mock.assert_called_once_with( + expected_url, + data=base64.b64encode(b'pdf page').decode('utf-8'), + headers={'Authorization': 'Token my-secret-key'}, + ) + + +def test_from_invalid_pdf_file_makes_request(mocker): + mocker.patch('app.template_previews.extract_page_from_pdf', return_value=b'pdf page') + request_mock = mocker.patch( + 'app.template_previews.requests.post', + return_value=Mock(content='a', status_code='b', headers={'c': 'd'}) + ) + + response = TemplatePreview.from_invalid_pdf_file(b'pdf file', '1') + + assert response == ('a', 'b', {'c': 'd'}.items()) + request_mock.assert_called_once_with( + 'http://localhost:9999/precompiled/overlay.png?page_number=1', + data=b'pdf page', + headers={'Authorization': 'Token my-secret-key'}, + ) + + @pytest.mark.parametrize('template_type', [ 'email', 'sms' ])