From 6b055071f4872f667ed4b06b40cca883f7d76515 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 7 Apr 2020 17:13:28 +0100 Subject: [PATCH] Specify mimetype for PDF letters to be downloaded We had a report that when clicking on the 'Download this letter' link on the notification page the file was not being downloaded as a PDF file but was given a `.htm` file extension instead. We should be able to stop that happening by using Flask's `send_file` function with the right mimetype. This change updates the `view_letter_notification_as_preview` to use `send_file` and splits out code to get the file data into a separate function. Mocks in the tests have been updated and some unused mocks removed. --- app/main/views/notifications.py | 15 ++++++++++++++- tests/app/main/views/test_notifications.py | 22 ++++++---------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 710d8f97d..b392437ba 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -13,6 +13,7 @@ from flask import ( redirect, render_template, request, + send_file, stream_with_context, url_for, ) @@ -57,7 +58,7 @@ def view_notification(service_id, notification_id): error_message = None if notification['template']['is_precompiled_letter']: try: - file_contents, metadata = view_letter_notification_as_preview( + file_contents, metadata = get_letter_file_data( service_id, notification_id, "pdf", with_metadata=True ) page_count = int( @@ -187,6 +188,18 @@ def get_preview_error_image(): def view_letter_notification_as_preview( service_id, notification_id, filetype, with_metadata=False ): + image_data = get_letter_file_data(service_id, notification_id, filetype, with_metadata) + file = io.BytesIO(image_data) + + mimetype = 'image/png' if filetype == 'png' else 'application/pdf' + + return send_file( + filename_or_fp=file, + mimetype=mimetype, + ) + + +def get_letter_file_data(service_id, notification_id, filetype, with_metadata=False): try: preview = notification_api_client.get_notification_letter_preview( service_id, diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 8062a17da..4985af524 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -254,7 +254,7 @@ def test_notification_page_shows_uploaded_letter( fake_uuid, ): mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', + 'app.main.views.notifications.get_letter_file_data', return_value=(b'foo', { 'message': '', 'invalid_pages': '[]', @@ -319,18 +319,13 @@ def test_notification_page_shows_page_for_letter_sent_with_test_key( if is_precompiled_letter: mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', + 'app.main.views.notifications.get_letter_file_data', return_value=(b'foo', { 'message': '', 'invalid_pages': '[]', 'page_count': '1' }) ) - else: - mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', - return_value=b'foo' - ) mocker.patch( 'app.main.views.notifications.pdf_page_count', @@ -383,7 +378,7 @@ def test_notification_page_shows_validation_failed_precompiled_letter( metadata = {"page_count": "1", "status": "validation-failed", "invalid_pages": "[1]", "message": "content-outside-printable-area"} - mocker.patch('app.main.views.notifications.view_letter_notification_as_preview', + mocker.patch('app.main.views.notifications.get_letter_file_data', return_value=("some letter content", metadata)) mocker.patch( 'app.main.views.notifications.get_page_count_for_letter', @@ -627,7 +622,7 @@ def test_should_show_preview_error_image_letter_notification_on_preview_error( side_effect=APIError ) - mocker.patch("builtins.open", mock_open(read_data="preview error image")) + mocker.patch("builtins.open", mock_open(read_data=b"preview error image")) response = logged_in_client.get(url_for( 'main.view_letter_notification_as_preview', @@ -651,7 +646,7 @@ def test_notification_page_shows_error_message_if_precompiled_letter_cannot_be_o is_precompiled_letter=True) mocker.patch('app.notification_api_client.get_notification', return_value=notification) mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', + 'app.main.views.notifications.get_letter_file_data', side_effect=PdfReadError() ) mocker.patch( @@ -783,14 +778,9 @@ def test_notification_page_has_expected_template_link_for_letter( if is_precompiled_letter: mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', + 'app.main.views.notifications.get_letter_file_data', side_effect=[(b'foo', {"message": "", "invalid_pages": "[]", "page_count": "1"}), b'foo'] ) - else: - mocker.patch( - 'app.main.views.notifications.view_letter_notification_as_preview', - return_value=b'foo' - ) mocker.patch( 'app.main.views.notifications.pdf_page_count',