diff --git a/app/s3_client/s3_letter_upload_client.py b/app/s3_client/s3_letter_upload_client.py index bf263fb6f..e5ddad695 100644 --- a/app/s3_client/s3_letter_upload_client.py +++ b/app/s3_client/s3_letter_upload_client.py @@ -48,9 +48,20 @@ def get_letter_pdf_and_metadata(service_id, file_id): 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 + return pdf, LetterMetadata(s3_object['Metadata']) + + +class LetterMetadata: + KEYS_TO_DECODE = [] + + def __init__(self, metadata): + self._metadata = metadata + + def get(self, key, default=None): + if key in self.KEYS_TO_DECODE: + return urllib.parse.unquote(self._metadata.get(key, default)) + return self._metadata.get(key, default) def get_letter_metadata(service_id, file_id): @@ -58,4 +69,4 @@ def get_letter_metadata(service_id, file_id): s3 = resource('s3') s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() - return s3_object['Metadata'] + return LetterMetadata(s3_object['Metadata']) diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 98246762a..c77eb9bc6 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -10,6 +10,7 @@ from freezegun import freeze_time from requests import RequestException from app.main.views.uploads import format_recipient +from app.s3_client.s3_letter_upload_client import LetterMetadata from app.utils import normalize_spaces from tests.conftest import ( SERVICE_ONE_ID, @@ -166,12 +167,12 @@ def test_post_upload_letter_redirects_for_valid_file( ) ) mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') - mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'The Queen' - }) + })) mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template') service_one['restricted'] = False @@ -232,12 +233,12 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file( ) 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={ + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '3', 'status': 'valid', 'recipient': 'The Queen' - }) + })) mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) service_one['restricted'] = False @@ -351,9 +352,9 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request, fake_uuid) } 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={ + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'invalid', - 'message': 'content-outside-printable-area', 'invalid_pages': '[1]'}) + 'message': 'content-outside-printable-area', 'invalid_pages': '[1]'})) with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: file_contents = file.read() @@ -395,9 +396,9 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client mock_sanitise_response.json = lambda: {"message": "template preview error", "recipient_address": "The Queen"} 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={ + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'invalid', - 'message': 'template-preview-error'}) + 'message': 'template-preview-error'})) with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: page = client_request.post( @@ -455,10 +456,10 @@ def test_uploaded_letter_preview( ): mocker.patch('app.main.views.uploads.service_api_client') recipient = 'Bugs Bunny\n123 Big Hole\rLooney Town' - mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ 'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid', 'recipient': urllib.parse.quote(recipient) - }) + })) service_one['restricted'] = False client_request.login(active_user_with_permissions, service=service_one) @@ -485,8 +486,8 @@ def test_uploaded_letter_preview_does_not_show_send_button_if_service_in_trial_m fake_uuid, ): 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', 'recipient': 'The Queen'}) + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ + 'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'The Queen'})) # client_request uses service_one, which is in trial mode page = client_request.get( @@ -603,7 +604,7 @@ def test_uploaded_letter_preview_image_400s_for_bad_page_type( def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mocker, service_one, client_request): - metadata = {'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'address'} + metadata = LetterMetadata({'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'address'}) 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') @@ -656,8 +657,8 @@ def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, servic 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', - 'message': 'error', 'invalid_pages': '[1]'} + return_value=LetterMetadata({'filename': 'my_file.pdf', 'page_count': '3', 'status': 'invalid', + 'message': 'error', 'invalid_pages': '[1]'}) ) service_one['permissions'] = ['letter', 'upload_letters']