diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 82b0373fc..31d3a4f46 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -20,6 +20,7 @@ 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_metadata, get_letter_pdf_and_metadata, get_transient_letter_file_location, upload_letter_to_s3, @@ -43,6 +44,7 @@ def upload_letter(service_id): if form.validate_on_submit(): pdf_file_bytes = form.file.data.read() + original_filename = form.file.data.filename virus_free = antivirus_client.scan(BytesIO(pdf_file_bytes)) if not virus_free: @@ -67,22 +69,29 @@ def upload_letter(service_id): except RequestException as ex: if ex.response is not None and ex.response.status_code == 400: status = 'invalid' - upload_letter_to_s3(pdf_file_bytes, file_location, status) + upload_letter_to_s3( + pdf_file_bytes, + file_location=file_location, + status=status, + page_count=page_count, + filename=original_filename) else: raise ex else: status = 'valid' file_contents = base64.b64decode(response.json()['file'].encode()) - upload_letter_to_s3(file_contents, file_location, status) + upload_letter_to_s3( + file_contents, + file_location=file_location, + status=status, + page_count=page_count, + filename=original_filename) return redirect( url_for( 'main.uploaded_letter_preview', service_id=current_service.id, file_id=upload_id, - original_filename=form.file.data.filename, - page_count=page_count, - status=status, ) ) @@ -97,9 +106,10 @@ def invalid_upload_error(message): @main.route("/services//preview-letter/") @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') + metadata = get_letter_metadata(service_id, file_id) + original_filename = metadata.get('filename') + page_count = metadata.get('page_count') + status = metadata.get('status') template_dict = service_api_client.get_precompiled_template(service_id) @@ -126,8 +136,7 @@ def uploaded_letter_preview(service_id, file_id): @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) + pdf_file, metadata = get_letter_pdf_and_metadata(service_id, file_id) page = request.args.get('page') @@ -140,14 +149,12 @@ def view_letter_upload_as_preview(service_id, file_id): @main.route("/services//upload-letter/send", methods=['POST']) @user_has_permissions('send_messages', restrict_admin_usage=True) def send_uploaded_letter(service_id): - filename = request.form['filename'] - file_id = request.form['file_id'] - if not (current_service.has_permission('letter') and current_service.has_permission('upload_letters')): abort(403) - file_location = get_transient_letter_file_location(service_id, file_id) - _, metadata = get_letter_pdf_and_metadata(file_location) + file_id = request.form['file_id'] + metadata = get_letter_metadata(service_id, file_id) + filename = metadata.get('filename') if metadata.get('status') != 'valid': abort(403) diff --git a/app/s3_client/s3_letter_upload_client.py b/app/s3_client/s3_letter_upload_client.py index 27848f105..8bdeece7b 100644 --- a/app/s3_client/s3_letter_upload_client.py +++ b/app/s3_client/s3_letter_upload_client.py @@ -7,17 +7,22 @@ def get_transient_letter_file_location(service_id, upload_id): return 'service-{}/{}.pdf'.format(service_id, upload_id) -def upload_letter_to_s3(data, file_location, status): +def upload_letter_to_s3(data, *, file_location, status, page_count, filename): utils_s3upload( filedata=data, region=current_app.config['AWS_REGION'], bucket_name=current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location=file_location, - metadata={'status': status} + metadata={ + 'status': status, + 'page_count': str(page_count), + 'filename': filename, + } ) -def get_letter_pdf_and_metadata(file_location): +def get_letter_pdf_and_metadata(service_id, file_id): + file_location = get_transient_letter_file_location(service_id, file_id) s3 = resource('s3') s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() @@ -25,3 +30,11 @@ def get_letter_pdf_and_metadata(file_location): metadata = s3_object['Metadata'] return pdf, metadata + + +def get_letter_metadata(service_id, file_id): + file_location = get_transient_letter_file_location(service_id, file_id) + s3 = resource('s3') + s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() + + return s3_object['Metadata'] diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index c8d954887..5642540e0 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -28,7 +28,6 @@ service_id=current_service.id, )}}" class='page-footer'> - diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index b9b532f84..e58031e0f 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -33,6 +33,8 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'}) ) mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'valid'}) 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: @@ -46,14 +48,15 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): mock_s3.assert_called_once_with( b'The sanitised content', - 'service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), - 'valid', + file_location='service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), + status='valid', + page_count=1, + filename='tests/test_pdf_files/one_page_pdf.pdf' ) assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' assert not page.find(id='validation-error-message') - assert page.find('input', {'type': 'hidden', 'name': 'filename', 'value': 'tests/test_pdf_files/one_page_pdf.pdf'}) assert page.find('input', {'type': 'hidden', 'name': 'file_id', 'value': 'fake-uuid'}) assert page.find('button', {'type': 'submit'}).text == 'Send 1 letter' @@ -73,6 +76,8 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(mocker, client_r ) 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.get_letter_metadata', return_value={ + 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '3', 'status': 'valid'}) 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: @@ -170,6 +175,8 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): 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') + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'invalid'}) with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: file_contents = file.read() @@ -184,8 +191,10 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): mock_s3.assert_called_once_with( file_contents, - 'service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), - 'invalid', + file_location='service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), + status='invalid', + page_count=1, + filename='tests/test_pdf_files/one_page_pdf.pdf' ) assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' @@ -209,6 +218,8 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client 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) + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'invalid'}) with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: page = client_request.post( @@ -253,6 +264,8 @@ def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unk def test_uploaded_letter_preview(mocker, client_request): mocker.patch('app.main.views.uploads.service_api_client') + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + 'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid'}) page = client_request.get( 'main.uploaded_letter_preview', @@ -268,8 +281,11 @@ def test_uploaded_letter_preview(mocker, client_request): def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mocker, service_one, client_request): - mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'valid'})) + metadata = {'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid'} + + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', metadata)) mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=metadata) service_one['permissions'] = ['letter', 'upload_letters'] file_id = 'abcd-1234' @@ -315,8 +331,11 @@ def test_send_uploaded_letter_when_service_does_not_have_correct_permissions( def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, service_one, client_request): - mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'invalid'})) mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + mocker.patch( + 'app.main.views.uploads.get_letter_metadata', + return_value={'filename': 'my_file.pdf', 'page_count': '3', 'status': 'invalid'} + ) service_one['permissions'] = ['letter', 'upload_letters'] file_id = 'abcd-1234' diff --git a/tests/app/s3_client/test_s3_letter_upload_client.py b/tests/app/s3_client/test_s3_letter_upload_client.py index c30ac61b5..5e87a64b2 100644 --- a/tests/app/s3_client/test_s3_letter_upload_client.py +++ b/tests/app/s3_client/test_s3_letter_upload_client.py @@ -6,12 +6,17 @@ from app.s3_client.s3_letter_upload_client import upload_letter_to_s3 def test_upload_letter_to_s3(mocker): s3_mock = mocker.patch('app.s3_client.s3_letter_upload_client.utils_s3upload') - upload_letter_to_s3('pdf_data', 'service_id/upload_id.pdf', 'valid') + upload_letter_to_s3( + 'pdf_data', + file_location='service_id/upload_id.pdf', + status='valid', + page_count=3, + filename='my_doc') s3_mock.assert_called_once_with( bucket_name=current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location='service_id/upload_id.pdf', filedata='pdf_data', - metadata={'status': 'valid'}, + metadata={'status': 'valid', 'page_count': '3', 'filename': 'my_doc'}, region=current_app.config['AWS_REGION'] )