mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-06-17 19:56:47 -04:00
Fix broken journey when going back from check page
We’ve seen in letters usability testing that people get stuck in a “no-man’s land” when trying to go back from the _Send yourself a test_ page. This was broken for two reasons: - we hadn’t considered that a letter template without placeholder still requires you to fill in - we’ve changed subsequently made the _view template_ page the place where you do your actions, rather than the (old) page with all the templates shown So this commit fixes it so that the back link always take you back to the page you were previously on, and adds some more test cases so we have all the scenarios accounted for.
This commit is contained in:
@@ -236,13 +236,13 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False):
|
||||
|
||||
if request.args.get('from_test'):
|
||||
extra_args = {'help': 1} if request.args.get('help', '0') != '0' else {}
|
||||
if len(template.placeholders):
|
||||
if len(template.placeholders) or template.template_type == 'letter':
|
||||
back_link = url_for(
|
||||
'.send_test', service_id=service_id, template_id=template.id, **extra_args
|
||||
)
|
||||
else:
|
||||
back_link = url_for(
|
||||
'.choose_template', service_id=service_id, **extra_args
|
||||
'.view_template', service_id=service_id, template_id=template.id, **extra_args
|
||||
)
|
||||
choose_time_form = None
|
||||
else:
|
||||
|
||||
@@ -15,6 +15,11 @@ from app.main.views.send import get_check_messages_back_url
|
||||
|
||||
from tests import validate_route_permission, template_json
|
||||
from tests.app.test_utils import normalize_spaces
|
||||
from tests.conftest import (
|
||||
mock_get_service_template,
|
||||
mock_get_service_template_with_placeholders,
|
||||
mock_get_service_letter_template,
|
||||
)
|
||||
|
||||
template_types = ['email', 'sms']
|
||||
|
||||
@@ -688,18 +693,41 @@ def test_route_invalid_permissions(
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'extra_args,expected_url',
|
||||
'template_mock, extra_args, expected_url',
|
||||
[
|
||||
(
|
||||
mock_get_service_template,
|
||||
dict(),
|
||||
partial(url_for, '.send_messages')
|
||||
),
|
||||
(
|
||||
mock_get_service_template_with_placeholders,
|
||||
dict(),
|
||||
partial(url_for, '.send_messages')
|
||||
),
|
||||
(
|
||||
mock_get_service_template,
|
||||
dict(from_test=True),
|
||||
partial(url_for, '.view_template')
|
||||
),
|
||||
(
|
||||
mock_get_service_letter_template, # No placeholders
|
||||
dict(from_test=True),
|
||||
partial(url_for, '.send_test')
|
||||
),
|
||||
(
|
||||
dict(help='0'),
|
||||
mock_get_service_template_with_placeholders,
|
||||
dict(from_test=True),
|
||||
partial(url_for, '.send_test')
|
||||
),
|
||||
(
|
||||
dict(help='2'),
|
||||
mock_get_service_template_with_placeholders,
|
||||
dict(help='0', from_test=True),
|
||||
partial(url_for, '.send_test')
|
||||
),
|
||||
(
|
||||
mock_get_service_template_with_placeholders,
|
||||
dict(help='2', from_test=True),
|
||||
partial(url_for, '.send_test', help='1')
|
||||
)
|
||||
]
|
||||
@@ -711,14 +739,16 @@ def test_check_messages_back_link(
|
||||
mock_get_user_by_email,
|
||||
mock_get_users_by_service,
|
||||
mock_get_service,
|
||||
mock_get_service_template_with_placeholders,
|
||||
mock_has_permissions,
|
||||
mock_get_detailed_service_for_today,
|
||||
mock_s3_download,
|
||||
fake_uuid,
|
||||
mocker,
|
||||
template_mock,
|
||||
extra_args,
|
||||
expected_url
|
||||
):
|
||||
template_mock(mocker)
|
||||
with logged_in_client.session_transaction() as session:
|
||||
session['upload_data'] = {'original_file_name': 'valid.csv',
|
||||
'template_id': fake_uuid,
|
||||
@@ -729,7 +759,6 @@ def test_check_messages_back_link(
|
||||
service_id=fake_uuid,
|
||||
upload_id=fake_uuid,
|
||||
template_type='sms',
|
||||
from_test=True,
|
||||
**extra_args
|
||||
))
|
||||
assert response.status_code == 200
|
||||
|
||||
Reference in New Issue
Block a user