Don’t load the check page if a job exists already

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.
This commit is contained in:
Chris Hill-Scott
2018-04-30 12:55:56 +01:00
parent b44074bf3a
commit 054f75a8ed
2 changed files with 38 additions and 0 deletions

View File

@@ -1,5 +1,6 @@
import itertools import itertools
import json import json
from contextlib import suppress
from string import ascii_uppercase from string import ascii_uppercase
from zipfile import BadZipFile from zipfile import BadZipFile
@@ -21,6 +22,7 @@ from notifications_utils.recipients import (
optional_address_columns, optional_address_columns,
) )
from orderedset import OrderedSet from orderedset import OrderedSet
from werkzeug.routing import RequestRedirect
from xlrd.biffh import XLRDError from xlrd.biffh import XLRDError
from xlrd.xldate import XLDateError 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): 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 doesnt 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) 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'] statistics = service_api_client.get_detailed_service_for_today(service_id)['data']['statistics']

View File

@@ -2819,3 +2819,26 @@ def test_sms_sender_is_previewed(
assert sms_sender_on_page.text.strip() == 'From: GOVUK' assert sms_sender_on_page.text.strip() == 'From: GOVUK'
else: else:
assert not sms_sender_on_page 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,
)
)