From 1c5e6612640edc33ccd6fc17b4bee6b6f6b1cb11 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 19 May 2020 17:26:55 +0100 Subject: [PATCH] Accept file id in URL, not as form parameter In the future we need to get the metadata from the file in order to work out what form validation rules should apply (postage is only required for UK letters). To start doing this we need all instances of the app accepting `post` requests with the `file_id` in the URL, as well as in the form data (for backwards compatibility). --- app/main/views/uploads.py | 7 ++-- tests/app/main/views/test_uploads.py | 59 ++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 4a640c864..672f5b83c 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -339,13 +339,14 @@ def view_letter_upload_as_preview(service_id, file_id): @main.route("/services//upload-letter/send", methods=['POST']) +@main.route("/services//upload-letter/send/", methods=['POST']) @user_has_permissions('send_messages', restrict_admin_usage=True) -def send_uploaded_letter(service_id): +def send_uploaded_letter(service_id, file_id=None): if not (current_service.has_permission('letter') and current_service.has_permission('upload_letters')): abort(403) - form = LetterUploadPostageForm() - file_id = form.file_id.data + form = LetterUploadPostageForm(file_id=file_id) + file_id = file_id or form.file_id.data if not form.validate_on_submit(): return uploaded_letter_preview(service_id, file_id) diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 714240ec9..26845b415 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -790,6 +790,65 @@ def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mo mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', file_id, 'first', 'address') +@pytest.mark.parametrize('form_data', ( + {'filename': 'my_file.pdf', 'postage': 'first'}, + {'filename': 'my_file.pdf', 'postage': 'first', 'file_id': 'Ignored in favour of URL'}, +)) +def test_send_uploaded_letter_accepts_file_id_in_url( + mocker, + service_one, + client_request, + fake_uuid, + form_data, +): + 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') + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=metadata) + + service_one['permissions'] = ['letter', 'upload_letters'] + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + file_id=fake_uuid, + _data=form_data, + _expected_redirect=url_for( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + _external=True + ) + ) + mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', fake_uuid, 'first', 'address') + + +def test_send_uploaded_letter_needs_file_id_in_form_if_not_in_url( + mocker, + service_one, + client_request, + mock_template_preview, + fake_uuid, +): + metadata = LetterMetadata({'filename': 'my_file.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'address'}) + + 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) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template') + + service_one['permissions'] = ['letter', 'upload_letters'] + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'postage': 'first'}, + _expected_status=200, + _expected_redirect=None, + ) + assert mock_send.called is False + + @pytest.mark.parametrize('permissions', [ ['email'], ['letter'],