From ef91c374a5fb9da76f1c0aace7b8371b55c68899 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 19 Jan 2017 18:15:33 +0000 Subject: [PATCH] send users back to beginning of tour if they hit back at end (previously it would have sent them to the choose template page) if the user has added new templates or deleted the example one, they're clearly competent enough to use the app so don't worry (we wouldn't know what URL the tour starts on since the UUID of the example template is random) --- app/main/views/send.py | 15 ++++++- tests/app/main/views/test_send.py | 65 ++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 83c1b3297..0a7065d56 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -241,7 +241,7 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): # NOTE: this is a 301 MOVED PERMANENTLY (httpstatus.es/301), so the browser will cache this redirect, and it'll # *always* happen for that browser. _check_messages is only used by endpoints that contain `upload_id`, which # is a one-time-use id (that ties to a given file in S3 that is already deleted if it's not in the session) - raise RequestRedirect(url_for('main.choose_template', service_id=service_id, template_type=template_type)) + raise RequestRedirect(get_check_messages_back_url(service_id, template_type)) users = user_api_client.get_users_for_service(service_id=service_id) @@ -410,3 +410,16 @@ def go_to_dashboard_after_tour(service_id, example_template_id): return redirect( url_for('main.service_dashboard', service_id=service_id) ) + + +def get_check_messages_back_url(service_id, template_type): + if get_help_argument(): + # if the user is on the introductory tour, then they should be redirected back to the beginning of the tour - + # but to do that we need to find the template_id of the example template. That template *should* be the only + # template for that service, but it's possible they've opened another tab and deleted it for example. In that + # case we should just redirect back to the main page as they clearly know what they're doing. + templates = service_api_client.get_service_templates(service_id)['data'] + if len(templates) == 1: + return url_for('.send_test', service_id=service_id, template_id=templates[0]['id'], help=1) + + return url_for('main.choose_template', service_id=service_id, template_type=template_type) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 452a04ae3..5ba5f6c08 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -6,13 +6,14 @@ import re from itertools import repeat from functools import partial from unittest.mock import patch -from urllib.parse import urlparse import pytest from bs4 import BeautifulSoup from flask import url_for -from tests import validate_route_permission +from app.main.views.send import get_check_messages_back_url + +from tests import validate_route_permission, template_json template_types = ['email', 'sms'] @@ -885,21 +886,59 @@ def test_check_messages_shows_over_max_row_error( ) -def test_check_messages_goes_to_select_template_if_no_upload_data(logged_in_client): - service_id = uuid.uuid4() - +def test_check_messages_redirects_if_no_upload_data(logged_in_client, mocker): + checker = mocker.patch('app.main.views.send.get_check_messages_back_url', return_value='foo') response = logged_in_client.get(url_for( 'main.check_messages', - service_id=service_id, - template_type='sms', - upload_id=uuid.uuid4(), + service_id='0', + template_type='bar', + upload_id='baz' )) - # permanent redirect + checker.assert_called_once_with('0', 'bar') assert response.status_code == 301 - assert response.location == url_for( + assert response.location == 'http://localhost/foo' + + +@pytest.mark.parametrize('template_type', ['sms', 'email']) +def test_get_check_messages_back_url_returns_to_correct_select_template(client, mocker, template_type): + mocker.patch('app.main.views.send.get_help_argument', return_value=False) + + assert get_check_messages_back_url('1234', template_type) == url_for( 'main.choose_template', - service_id=service_id, - template_type='sms', - _external=True + service_id='1234', + template_type=template_type + ) + + +def test_check_messages_back_from_help_goes_to_start_of_help(client, mocker): + mocker.patch('app.main.views.send.get_help_argument', return_value=True) + mocker.patch('app.service_api_client.get_service_templates', lambda service_id: { + 'data': [template_json('000', '111', type_='sms')] + }) + assert get_check_messages_back_url('000', 'sms') == url_for( + 'main.send_test', + service_id='000', + template_id='111', + help='1' + ) + + +@pytest.mark.parametrize('templates', [ + [], + [ + template_json('000', '111', type_='sms'), + template_json('000', '222', type_='sms') + ] +], ids=['no_templates', 'two_templates']) +def test_check_messages_back_from_help_handles_unexpected_templates(client, mocker, templates): + mocker.patch('app.main.views.send.get_help_argument', return_value=True) + mocker.patch('app.service_api_client.get_service_templates', lambda service_id: { + 'data': templates + }) + + assert get_check_messages_back_url('1234', 'sms') == url_for( + 'main.choose_template', + service_id='1234', + template_type='sms' )