From 06d7962d3d1d8a9af16b918849041aa7a4465505 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Aug 2020 11:26:02 +0100 Subject: [PATCH] Let people approve own broadcasts in training mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to let users learn the system by trying it out. At the moment they can’t explore it fully by themselves because they’re blocked at the point of approving the broadcast, unless they involve in another member of their team. Having to involve another person is friction that will discourage people from exploring. So this adds a button that lets people approve their own broadcasts in trial mode, with some rough-and-ready content that explains training mode and how it would be different to trial mode. --- .../views/broadcast/view-message.html | 76 ++++++++++---- tests/app/main/views/test_broadcast.py | 99 +++++++++++++++++-- 2 files changed, 148 insertions(+), 27 deletions(-) diff --git a/app/templates/views/broadcast/view-message.html b/app/templates/views/broadcast/view-message.html index 45ddcd714..8722d7eca 100644 --- a/app/templates/views/broadcast/view-message.html +++ b/app/templates/views/broadcast/view-message.html @@ -1,3 +1,4 @@ +{% from "components/back-link/macro.njk" import govukBackLink %} {% from "components/button/macro.njk" import govukButton %} {% from "components/form.html" import form_wrapper %} {% from "components/banner.html" import banner %} @@ -7,46 +8,87 @@ {% extends "withnav_template.html" %} {% block service_page_title %} - {{ broadcast_message.template_name }} + {% if broadcast_message.status == 'pending-approval' %} + {% if broadcast_message.created_by == current_user and current_user.has_permissions('send_messages') %} + {{ broadcast_message.template_name }} is waiting for approval + {% elif current_user.has_permissions('send_messages') %} + {{ broadcast_message.created_by.name }} wants to broadcast + {{ broadcast_message.template_name }} + {% else %} + This alert is waiting for approval + {% endif %} + {% else %} + {{ broadcast_message.template_name }} + {% endif %} {% endblock %} {% block maincolumn_content %} - {{ page_header( - broadcast_message.template_name, - back_link=url_for('.broadcast_dashboard', service_id=current_service.id) - ) }} + {{ govukBackLink({ "href": url_for('.broadcast_dashboard', service_id=current_service.id) }) }} {% if broadcast_message.status == 'pending-approval' %} {% if broadcast_message.created_by == current_user and current_user.has_permissions('send_messages') %} {% elif current_user.has_permissions('send_messages') %} {% call form_wrapper(class="banner govuk-!-margin-bottom-6") %} -

- {{ broadcast_message.created_by.name }} wants to broadcast this - message. -

+

+ {{ broadcast_message.created_by.name }} wants to broadcast + {{ broadcast_message.template_name }} +

{{ page_footer( "Start broadcasting now", delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id), - delete_link_text='Reject this broadcast' + delete_link_text='Reject this alert' ) }} {% endcall %} {% else %} {% endif %} {% else %} + {{ page_header(broadcast_message.template_name) }} +

Created by {{ broadcast_message.created_by.name }} and approved by {{ broadcast_message.approved_by.name }}. diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 8722a1f54..a3ffb3d1f 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -706,8 +706,8 @@ def test_view_pending_broadcast( assert ( normalize_spaces(page.select_one('.banner').text) ) == ( - 'Test User wants to broadcast this message. ' - 'Start broadcasting now Reject this broadcast' + 'Test User wants to broadcast Example template ' + 'Start broadcasting now Reject this alert' ) form = page.select_one('form.banner') @@ -716,7 +716,7 @@ def test_view_pending_broadcast( assert form.select_one('button[type=submit]') link = form.select_one('a.govuk-link.govuk-link--destructive') - assert link.text == 'Reject this broadcast' + assert link.text == 'Reject this alert' assert link['href'] == url_for( '.reject_broadcast_message', service_id=SERVICE_ONE_ID, @@ -732,6 +732,60 @@ def test_cant_approve_own_broadcast( active_user_with_permissions, mock_get_broadcast_template, fake_uuid, +): + service_one['restricted'] = False + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + created_by_id=fake_uuid, + finishes_at='2020-02-23T23:23:23.000000', + status='pending-approval', + ), + ) + mocker.patch('app.user_api_client.get_user', side_effect=[ + active_user_with_permissions, # Current user + active_user_with_permissions, # User who created broadcast (the same) + ]) + service_one['permissions'] += ['broadcast'] + + page = client_request.get( + '.view_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + + assert ( + normalize_spaces(page.select_one('.banner h1').text) + ) == ( + 'Example template is waiting for approval' + ) + assert ( + normalize_spaces(page.select_one('.banner p').text) + ) == ( + 'You need another member of your team to approve your alert.' + ) + assert not page.select('form') + + link = page.select_one('.banner a.govuk-link.govuk-link--destructive') + assert link.text == 'Withdraw this alert' + assert link['href'] == url_for( + '.reject_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + + +@freeze_time('2020-02-22T22:22:22.000000') +def test_can_approve_own_broadcast_in_trial_mode( + mocker, + client_request, + service_one, + active_user_with_permissions, + mock_get_broadcast_template, + fake_uuid, ): mocker.patch( 'app.broadcast_message_api_client.get_broadcast_message', @@ -757,16 +811,41 @@ def test_cant_approve_own_broadcast( ) assert ( - normalize_spaces(page.select_one('.banner').text) + normalize_spaces(page.select_one('.banner h1').text) ) == ( - 'Your broadcast is waiting for approval from another member of your team ' - 'Withdraw this broadcast' + 'Example template is waiting for approval' + ) + assert ( + normalize_spaces(page.select_one('.banner p').text) + ) == ( + 'When you use a live account you’ll need another member of ' + 'your team to approve your alert.' + ) + assert ( + normalize_spaces(page.select_one('.banner details summary').text) + ) == ( + 'Approve your own alert' + ) + assert ( + normalize_spaces(page.select_one('.banner details ').text) + ) == ( + 'Approve your own alert ' + 'Because you’re in training mode you can approve your own ' + 'alerts, to see how it works. ' + 'No real alerts will be broadcast to anyone’s phone. ' + 'Start broadcasting now ' + 'Cancel this alert' ) - assert not page.select_one('form') + form = page.select_one('.banner details form') + assert form['method'] == 'post' + assert 'action' not in form + assert normalize_spaces(form.select_one('button[type=submit]').text) == ( + 'Start broadcasting now' + ) link = page.select_one('.banner a.govuk-link.govuk-link--destructive') - assert link.text == 'Withdraw this broadcast' + assert link.text == 'Cancel this alert' assert link['href'] == url_for( '.reject_broadcast_message', service_id=SERVICE_ONE_ID, @@ -810,8 +889,8 @@ def test_view_only_user_cant_approve_broadcast( assert ( normalize_spaces(page.select_one('.banner').text) ) == ( - 'This broadcast is waiting for approval ' - 'You don’t have permission to approve broadcasts.' + 'This alert is waiting for approval ' + 'You don’t have permission to approve alerts.' ) assert not page.select_one('form')