From d86070a7e8c8dcc41ca9643923e3a81ac8757e11 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 26 May 2020 11:57:26 +0100 Subject: [PATCH] redirect from upload preview to notifications if it already exists the upload preview page has a file_id - this corresponds to the file in the transient pdf uploads bucket. However, if the user already hit send (and then navigated back) the file's no longer in that bcuket, it's been moved to the regular letters-pdf bucket. So the s3 get request fails. To avoid this, simply redirect to the notifications page if the file isn't in the transient bucket. This is better for the user as it'll stop them trying to submit it twice, and will provide more clarity on the status of the notification too. --- app/main/views/uploads.py | 16 +++++++++++- .../main/views/uploads/test_upload_letter.py | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index e1093f708..8da369bf5 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -7,6 +7,7 @@ from functools import partial from io import BytesIO from zipfile import BadZipFile +from botocore.exceptions import ClientError from flask import ( abort, current_app, @@ -257,7 +258,20 @@ def _get_error_from_upload_form(form_errors): def uploaded_letter_preview(service_id, file_id): re_upload_form = PDFUploadForm() - metadata = get_letter_metadata(service_id, file_id) + try: + metadata = get_letter_metadata(service_id, file_id) + except ClientError as e: + # if the file's not there, it's probably because we've already created the notification and the letter has been + # moved to the normal letters-pdf bucket. So lets just bounce out to the notification page + if e.response['Error']['Code'] == 'NoSuchKey': + return redirect(url_for( + '.view_notification', + service_id=service_id, + notification_id=file_id, + )) + else: + raise + original_filename = metadata.get('filename') page_count = metadata.get('page_count') status = metadata.get('status') diff --git a/tests/app/main/views/uploads/test_upload_letter.py b/tests/app/main/views/uploads/test_upload_letter.py index 38bab3108..5478f975b 100644 --- a/tests/app/main/views/uploads/test_upload_letter.py +++ b/tests/app/main/views/uploads/test_upload_letter.py @@ -1,6 +1,7 @@ from unittest.mock import ANY, Mock import pytest +from botocore.exceptions import ClientError from flask import make_response, url_for from requests import RequestException @@ -441,6 +442,30 @@ def test_uploaded_letter_preview_does_not_show_send_button_if_service_in_trial_m assert len(page.select('main button[type=submit]')) == 0 +def test_uploaded_letter_preview_redirects_if_file_not_in_s3( + mocker, + client_request, + fake_uuid +): + boto_error_json = {'Error': {'Code': 'NoSuchKey', 'Message': 'The specified key does not exist.'}} + mocker.patch( + 'app.main.views.uploads.get_letter_metadata', + side_effect=ClientError(boto_error_json, 'operation_name') + ) + client_request.get( + 'main.uploaded_letter_preview', + service_id=SERVICE_ONE_ID, + file_id=fake_uuid, + _expected_status=302, + _expected_redirect=url_for( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + _external=True + ) + ) + + @pytest.mark.parametrize('invalid_pages, page_requested, overlay_expected', ( ('[1, 2]', 1, True), ('[1, 2]', 2, True),