darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-08-11 09:28:27 -04:00
Code Issues Packages Projects Releases Wiki Activity

if on the tour check notification page (step 2), show back link

Browse Source 
Back link redirects to beginning of tour.

Additionally, this fixes a problem where you'd get a 500 when using
the browser back button
This commit is contained in:
Leo Hemsted
2017-06-30 12:33:31 +01:00
parent ef4d3d111f
commit 0b6b659bc0
2 changed files with 55 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
app/main/views/send.py
View File

@@ -345,7 +345,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(get_check_messages_back_url(service_id, template_type))
raise RequestRedirect(url_for('main.choose_template', service_id=service_id))
users = user_api_client.get_users_for_service(service_id=service_id)
@@ -494,19 +494,6 @@ def go_to_dashboard_after_tour(service_id, example_template_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)
def fields_to_fill_in(template, prefill_current_user=False):
recipient_columns = first_column_headings[template.template_type]
@@ -577,7 +564,16 @@ def get_send_test_page_title(template_type, help_argument):
def get_back_link(service_id, template_id, step_index):
if get_help_argument():
return None
# if we're on the check page, redirect back to the beginning. anywhere else, don't return the back link
if request.endpoint == 'main.check_notification':
return url_for(
'main.send_test',
service_id=service_id,
template_id=template_id,
help=get_help_argument()
)
else:
return None
elif step_index == 0:
return url_for(
'.view_template',
@@ -613,11 +609,8 @@ def _check_notification(service_id, template_id, exception=None):
back_link = get_back_link(service_id, template_id, 0)
if (
(
not session.get('recipient') or
not all_placeholders_in_session(template.placeholders)
)
and back_link
not session.get('recipient') or
not all_placeholders_in_session(template.placeholders)
):
return redirect(back_link)

93
tests/app/main/views/test_send.py
View File

@@ -14,9 +14,7 @@ from notifications_python_client.errors import HTTPError
from notifications_utils.template import LetterPreviewTemplate, LetterImageTemplate
from notifications_utils.recipients import RecipientCSV
from app.main.views.send import get_check_messages_back_url
from tests import validate_route_permission, template_json
from tests import validate_route_permission
from tests.app.test_utils import normalize_spaces
from tests.conftest import (
mock_get_service_template,
@@ -1519,8 +1517,7 @@ def test_non_ascii_characters_in_letter_recipients_file_shows_error(
assert page.find('span', class_='table-field-error-label').text == u'Can’t include П, е, т or я'
def test_check_messages_redirects_if_no_upload_data(logged_in_client, service_one, mocker):
checker = mocker.patch('app.main.views.send.get_check_messages_back_url', return_value='foo')
def test_check_messages_redirects_if_no_upload_data(logged_in_client, service_one):
response = logged_in_client.get(url_for(
'main.check_messages',
service_id=service_one['id'],
@@ -1528,51 +1525,8 @@ def test_check_messages_redirects_if_no_upload_data(logged_in_client, service_on
upload_id='baz'
))
checker.assert_called_once_with(service_one['id'], 'bar')
assert response.status_code == 301
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='1234'
)
def test_check_messages_back_from_help_goes_to_start_of_help(client, service_one, 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(service_one['id'], '111', type_='sms')]
})
assert get_check_messages_back_url(service_one['id'], 'sms') == url_for(
'main.send_test',
service_id=service_one['id'],
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',
)
assert response.location == url_for('main.choose_template', service_id=service_one['id'], _external=True)
@pytest.mark.parametrize('existing_session_items', [
@@ -1604,6 +1558,37 @@ def test_check_notification_redirects_if_session_not_populated(
)
@pytest.mark.parametrize('existing_session_items', [
{},
{'recipient': '07700900001'},
{'name': 'Jo'}
])
def test_check_notification_redirects_with_help_if_session_not_populated(
logged_in_client,
service_one,
fake_uuid,
existing_session_items,
mock_get_service_template_with_placeholders
):
with logged_in_client.session_transaction() as session:
session.update(existing_session_items)
resp = logged_in_client.get(url_for(
'main.check_notification',
service_id=service_one['id'],
template_id=fake_uuid,
help='2'
))
assert resp.location == url_for(
'main.send_test',
service_id=service_one['id'],
template_id=fake_uuid,
help='2',
_external=True
)
def test_check_notification_shows_preview(
client_request,
service_one,
@@ -1651,13 +1636,19 @@ def test_check_notification_shows_help(
template_id=fake_uuid,
help='2'
)
assert page.select('.banner-tour')
assert page.select_one('.banner-tour')
assert page.form.attrs['action'] == url_for(
'main.send_notification',
service_id=service_one['id'],
template_id=fake_uuid,
help='3'
)
assert page.select_one('.page-footer-back-link')['href'] == url_for(
'main.send_test',
service_id=service_one['id'],
template_id=fake_uuid,
help='2'
)
def test_send_notification_submits_data(
@@ -1751,8 +1742,8 @@ def test_send_notification_redirects_to_view_page(
'.view_notification',
service_id=service_one['id'],
notification_id=fake_uuid,
**extra_redirect_args,
_external=True
**extra_redirect_args,
)