From 054f75a8edb802b23dcf2de48e35c0578972d1d7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 30 Apr 2018 12:55:56 +0100 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20load=20the=20check=20page=20if?= =?UTF-8?q?=20a=20job=20exists=20already?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a user clicks ‘back’ once they’ve sent a job we don’t want them to land on the ‘check’ page again. This would suggest that they can send the same job again (they can’t because that `job_id` is in the database already). That said, it’s confusing to see that page; the natural thing is to go jump back another step, to where you uploaded the file. --- app/main/views/send.py | 15 +++++++++++++++ tests/app/main/views/test_send.py | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/app/main/views/send.py b/app/main/views/send.py index 19b3c5a84..479bb44e0 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -1,5 +1,6 @@ import itertools import json +from contextlib import suppress from string import ascii_uppercase from zipfile import BadZipFile @@ -21,6 +22,7 @@ from notifications_utils.recipients import ( optional_address_columns, ) from orderedset import OrderedSet +from werkzeug.routing import RequestRedirect from xlrd.biffh import XLRDError from xlrd.xldate import XLDateError @@ -477,6 +479,19 @@ def send_test_preview(service_id, template_id, filetype): def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_pdf=False): + with suppress(HTTPError): + # The happy path is that the job doesn’t already exist, so the + # API will return a 404 and the client will raise HTTPError. + job_api_client.get_job(service_id, upload_id) + # If we just return a `redirect` (302) object here, we'll get + # errors when we try and unpack in the check_messages route. + # Rasing a werkzeug.routing redirect means that doesn't happen. + raise RequestRedirect(url_for( + '.send_messages', + service_id=service_id, + template_id=template_id + )) + users = user_api_client.get_users_for_service(service_id=service_id) statistics = service_api_client.get_detailed_service_for_today(service_id)['data']['statistics'] diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index bfdbdc194..de6333617 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2819,3 +2819,26 @@ def test_sms_sender_is_previewed( assert sms_sender_on_page.text.strip() == 'From: GOVUK' else: assert not sms_sender_on_page + + +def test_redirects_to_template_if_job_exists_already( + client_request, + mock_get_service_email_template, + mock_get_job, + fake_uuid, +): + + client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + upload_id=fake_uuid, + original_file_name='example.csv', + _expected_status=301, + _expected_redirect=url_for( + 'main.send_messages', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _external=True, + ) + )