From 5c22bd56ced6909c60ea8dc1b4cf64bc03574262 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 21 Sep 2020 09:41:19 +0100 Subject: [PATCH 1/2] Add a tour screen once a broadcast is approved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For a training broadcast the user doesn’t get that immediate feedback that something has happened, like they would with a real alert, or even sending themselves a text message. This commit adds another tour-style page which will interrupt their journey and hopefully reinforce the message we’ve given them earlier in the tour. We’re adding this because we’ve found in research that users don’t have a good grasp of the consequences and severity of emergency alerts, versus regular text messages. --- app/main/views/broadcast.py | 9 +++- app/templates/views/broadcast/tour/6.html | 41 +++++++++++++++++++ tests/app/main/views/test_broadcast.py | 50 ++++++++++++++++++----- 3 files changed, 88 insertions(+), 12 deletions(-) create mode 100644 app/templates/views/broadcast/tour/6.html diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 16b820f65..e74555c6b 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -23,7 +23,7 @@ from app.utils import service_has_permission, user_has_permissions @user_has_permissions() @service_has_permission('broadcast') def broadcast_tour(service_id, step_index): - if step_index not in (1, 2, 3, 4, 5): + if step_index not in (1, 2, 3, 4, 5, 6): abort(404) return render_template( f'views/broadcast/tour/{step_index}.html' @@ -275,6 +275,13 @@ def approve_broadcast_message(service_id, broadcast_message_id): broadcast_message.approve_broadcast() + if current_service.trial_mode: + return redirect(url_for( + '.broadcast_tour', + service_id=current_service.id, + step_index=6, + )) + return redirect(url_for( '.view_broadcast_message', service_id=current_service.id, diff --git a/app/templates/views/broadcast/tour/6.html b/app/templates/views/broadcast/tour/6.html new file mode 100644 index 000000000..09084f631 --- /dev/null +++ b/app/templates/views/broadcast/tour/6.html @@ -0,0 +1,41 @@ +{% from "components/banner.html" import banner_wrapper %} + +{% extends "admin_template.html" %} + +{% block per_page_title %} + You’re still in training mode. Notify has not broadcast your alert. +{% endblock %} + +{% set mainClasses = "govuk-!-padding-top-0 govuk-!-padding-bottom-0" %} + +{% block content %} + +
+
+ {{ current_service.name }} Training +
+
+ + + +{% endblock %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 0ee2a41ed..5e5e53547 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -140,6 +140,7 @@ def test_broadcast_pages_403_for_user_without_permission( (3, 'Continue', partial(url_for, '.broadcast_tour', step_index=4)), (4, 'Continue', partial(url_for, '.broadcast_tour', step_index=5)), (5, 'Continue to dashboard', partial(url_for, '.service_dashboard')), + (6, 'Continue to dashboard', partial(url_for, '.service_dashboard')), )) def test_broadcast_tour_pages_have_continue_link( client_request, @@ -165,6 +166,7 @@ def test_broadcast_tour_pages_have_continue_link( pytest.param(3, marks=pytest.mark.xfail), pytest.param(4, marks=pytest.mark.xfail), 5, + 6, )) def test_broadcast_tour_page_4_shows_service_name( client_request, @@ -182,7 +184,7 @@ def test_broadcast_tour_page_4_shows_service_name( ) -@pytest.mark.parametrize('step_index', (0, 6)) +@pytest.mark.parametrize('step_index', (0, 7)) def test_broadcast_tour_page_404s_out_of_range( client_request, service_one, @@ -1105,15 +1107,40 @@ def test_view_only_user_cant_approve_broadcast( assert not page.select_one('.banner a') -@pytest.mark.parametrize('initial_status, expected_approval', ( - ('draft', False,), - ('pending-approval', True), - ('rejected', False), - ('broadcasting', False), - ('cancelled', False), +@pytest.mark.parametrize('trial_mode, initial_status, expected_approval, expected_redirect', ( + (True, 'draft', False, partial( + url_for, + '.view_broadcast_message', + broadcast_message_id=sample_uuid, + )), + (True, 'pending-approval', True, partial( + url_for, + '.broadcast_tour', + step_index=6, + )), + (False, 'pending-approval', True, partial( + url_for, + '.view_broadcast_message', + broadcast_message_id=sample_uuid, + )), + (True, 'rejected', False, partial( + url_for, + '.view_broadcast_message', + broadcast_message_id=sample_uuid, + )), + (True, 'broadcasting', False, partial( + url_for, + '.view_broadcast_message', + broadcast_message_id=sample_uuid, + )), + (True, 'cancelled', False, partial( + url_for, + '.view_broadcast_message', + broadcast_message_id=sample_uuid, + )), )) @freeze_time('2020-02-22T22:22:22.000000') -def test_approve_broadcast( +def test_request_approval( mocker, client_request, service_one, @@ -1123,6 +1150,8 @@ def test_approve_broadcast( mock_update_broadcast_message_status, initial_status, expected_approval, + trial_mode, + expected_redirect, ): mocker.patch( 'app.broadcast_message_api_client.get_broadcast_message', @@ -1135,16 +1164,15 @@ def test_approve_broadcast( status=initial_status, ), ) + service_one['restricted'] = trial_mode service_one['permissions'] += ['broadcast'] client_request.post( '.view_broadcast_message', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, - _expected_redirect=url_for( - '.view_broadcast_message', + _expected_redirect=expected_redirect( service_id=SERVICE_ONE_ID, - broadcast_message_id=fake_uuid, _external=True, ) ) From fec8dc891129158c62c1571ab64acd31a69543a1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 21 Sep 2020 10:53:39 +0100 Subject: [PATCH 2/2] Adjust wording of first paragraph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I’ve swapped the order of the first paragraph so it joins into one sentence, and leads with ‘Notify has not broadcast your alert’ which is the response to what the user has just done. I don’t think we need to say ‘still’ because the idea is users will swap back and forth between training and real depending on the situation. --- app/templates/views/broadcast/tour/6.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/broadcast/tour/6.html b/app/templates/views/broadcast/tour/6.html index 09084f631..1ddb63da9 100644 --- a/app/templates/views/broadcast/tour/6.html +++ b/app/templates/views/broadcast/tour/6.html @@ -3,7 +3,7 @@ {% extends "admin_template.html" %} {% block per_page_title %} - You’re still in training mode. Notify has not broadcast your alert. + Notify has not broadcast your alert because you’re still in training mode. {% endblock %} {% set mainClasses = "govuk-!-padding-top-0 govuk-!-padding-bottom-0" %} @@ -20,7 +20,7 @@

- You’re still in training mode. Notify has not broadcast your alert. + Notify has not broadcast your alert because you’re still in training mode.

In a real emergency, every mobile phone in the area