From ef4d3d111fd119e8f1fa40c4b254dbaed1637fd7 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 30 Jun 2017 11:49:03 +0100 Subject: [PATCH] help (aka tour) now works correctly throughout the notification flow --- app/main/views/send.py | 16 ++--- app/templates/views/jobs/job.html | 10 ++-- app/templates/views/notifications/check.html | 8 ++- .../views/notifications/notification.html | 10 ++-- tests/app/main/views/test_send.py | 58 ++++++++++++++----- 5 files changed, 64 insertions(+), 38 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 5572b7036..ea681bc60 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -183,6 +183,8 @@ def get_notification_check_endpoint(service_id, template): 'main.check_notification', service_id=service_id, template_id=template.id, + # at check phase we should move to help stage 2 ("the template pulls in the data you provide") + help='2' if 'help' in request.args else None )) @@ -381,15 +383,8 @@ 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) 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( - '.view_template', service_id=service_id, template_id=template.id, **extra_args - ) + # only happens if generating a letter preview test + back_link = url_for('.send_test', service_id=service_id, template_id=template.id) choose_time_form = None else: back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) @@ -689,5 +684,6 @@ def send_notification(service_id, template_id): return redirect(url_for( '.view_notification', service_id=service_id, - notification_id=noti['id'] + notification_id=noti['id'], + help=request.args.get('help') )) diff --git a/app/templates/views/jobs/job.html b/app/templates/views/jobs/job.html index 14aa297bf..9483766d4 100644 --- a/app/templates/views/jobs/job.html +++ b/app/templates/views/jobs/job.html @@ -19,11 +19,9 @@ {{ ajax_block(partials, updates_url, 'counts', finished=finished) }} {{ ajax_block(partials, updates_url, 'notifications', finished=finished) }} - {% if not help %} - {{ page_footer( - secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id), - secondary_link_text='Back to {}'.format(template.name) - ) }} - {% endif %} + {{ page_footer( + secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id), + secondary_link_text='Back to {}'.format(template.name) + ) }} {% endblock %} diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index 02656faae..fb336b631 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -28,9 +28,13 @@ {{ template|string }}
-
+ - {% if not error %} {% if template.template_type != 'letter' or not request.args.from_test %} diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index f4936fb53..e56b1066a 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -19,9 +19,11 @@ {{ ajax_block(partials, updates_url, 'counts', finished=finished) }} {{ ajax_block(partials, updates_url, 'notifications', finished=finished) }} - {{ page_footer( - secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id), - secondary_link_text='Back to {}'.format(template.name) - ) }} + {% if not help %} + {{ page_footer( + secondary_link=url_for('.view_template', service_id=current_service.id, template_id=template.id), + secondary_link_text='Back to {}'.format(template.name) + ) }} + {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index dce07f62e..4d63ada63 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1288,11 +1288,6 @@ def test_route_invalid_permissions( 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), @@ -1302,16 +1297,6 @@ def test_route_invalid_permissions( mock_get_service_template_with_placeholders, dict(from_test=True), partial(url_for, '.send_test') - ), - ( - 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') ) ] ) @@ -1640,6 +1625,40 @@ def test_check_notification_shows_preview( page.findAll('a', {'class': 'page-footer-back-link'})[0]['href'] ) == url_for('main.view_template', service_id=service_one['id'], template_id=fake_uuid) + # assert tour not visible + assert not page.select('.banner-tour') + assert page.form.attrs['action'] == url_for( + 'main.send_notification', + service_id=service_one['id'], + template_id=fake_uuid, + help='0' + ) + + +def test_check_notification_shows_help( + client_request, + service_one, + fake_uuid, + mock_get_service_template +): + with client_request.session_transaction() as session: + session['recipient'] = '07700900001' + session['placeholders'] = {} + + page = client_request.get( + 'main.check_notification', + service_id=service_one['id'], + template_id=fake_uuid, + help='2' + ) + assert page.select('.banner-tour') + assert page.form.attrs['action'] == url_for( + 'main.send_notification', + service_id=service_one['id'], + template_id=fake_uuid, + help='3' + ) + def test_send_notification_submits_data( client_request, @@ -1707,18 +1726,24 @@ def test_send_notification_redirects_if_missing_data( ) +@pytest.mark.parametrize('extra_args, extra_redirect_args', [ + ({}, {}), + ({'help': '3'}, {'help': '3'}) +]) def test_send_notification_redirects_to_view_page( logged_in_client, service_one, fake_uuid, mock_send_notification, + extra_args, + extra_redirect_args ): with logged_in_client.session_transaction() as session: session['recipient'] = '07700900001' session['placeholders'] = {'a': 'b'} resp = logged_in_client.post( - url_for('main.send_notification', service_id=service_one['id'], template_id=fake_uuid) + url_for('main.send_notification', service_id=service_one['id'], template_id=fake_uuid, **extra_args) ) assert resp.status_code == 302 @@ -1726,6 +1751,7 @@ def test_send_notification_redirects_to_view_page( '.view_notification', service_id=service_one['id'], notification_id=fake_uuid, + **extra_redirect_args, _external=True )