From 99b7d8a66fd5343ace04a89c92166be2d99c564c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 20 Nov 2020 15:42:52 +0000 Subject: [PATCH] Add flow for composing an alert without a template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We think that in some cases alerts will be composed in the moment, and therefore making people first create a template is: - not a good use of their time - adding some conceptual complexity which they don’t need This commit makes it possible to type some words and have them go straight into the `content` field in the database. In the future we might want to progressively enhance the radio buttons so they show on the same page (like we do with the grey buttons on the templates page). --- app/main/forms.py | 15 ++ app/main/views/broadcast.py | 49 +++++ app/models/broadcast_message.py | 11 + app/navigation.py | 8 + .../broadcast_message_api_client.py | 9 +- app/templates/views/broadcast/dashboard.html | 12 ++ .../views/broadcast/new-broadcast.html | 23 ++ .../views/broadcast/write-new-broadcast.html | 49 +++++ tests/app/main/views/test_broadcast.py | 200 ++++++++++++++++++ .../test_broadcast_message_client.py | 2 + tests/conftest.py | 3 + 11 files changed, 380 insertions(+), 1 deletion(-) create mode 100644 app/templates/views/broadcast/new-broadcast.html create mode 100644 app/templates/views/broadcast/write-new-broadcast.html diff --git a/app/main/forms.py b/app/main/forms.py index 8d0b14427..dcb4d05d6 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1293,6 +1293,21 @@ class ConfirmPasswordForm(StripWhitespaceForm): raise ValidationError('Invalid password') +class NewBroadcastForm(StripWhitespaceForm): + content = GovukRadiosField( + "How to do you want to add content to the alert?", + choices=[ + ('freeform', 'Write your own message'), + ('template', 'Use a template'), + ], + param_extensions={'fieldset': {'legend': {'classes': 'govuk-visually-hidden'}}} + ) + + @property + def use_template(self): + return self.content.data == 'template' + + class BaseTemplateForm(StripWhitespaceForm): name = GovukTextInputField( "Template name", diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 51baa5457..c42a00f5d 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -13,6 +13,8 @@ from app.main import main from app.main.forms import ( BroadcastAreaForm, BroadcastAreaFormWithSelectAll, + BroadcastTemplateForm, + NewBroadcastForm, SearchByNameForm, ) from app.models.broadcast_message import BroadcastMessage, BroadcastMessages @@ -71,6 +73,53 @@ def get_broadcast_dashboard_partials(service_id): ) +@main.route('/services//new-broadcast', methods=['GET', 'POST']) +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def new_broadcast(service_id): + form = NewBroadcastForm() + + if form.validate_on_submit(): + if form.use_template: + return redirect(url_for( + '.choose_template', + service_id=current_service.id, + )) + return redirect(url_for( + '.write_new_broadcast', + service_id=current_service.id, + )) + + return render_template( + 'views/broadcast/new-broadcast.html', + form=form, + ) + + +@main.route('/services//write-new-broadcast', methods=['GET', 'POST']) +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def write_new_broadcast(service_id): + form = BroadcastTemplateForm() + + if form.validate_on_submit(): + broadcast_message = BroadcastMessage.create_from_content( + service_id=current_service.id, + content=form.template_content.data, + reference=form.name.data, + ) + return redirect(url_for( + '.preview_broadcast_areas', + service_id=current_service.id, + broadcast_message_id=broadcast_message.id, + )) + + return render_template( + 'views/broadcast/write-new-broadcast.html', + form=form, + ) + + @main.route('/services//new-broadcast/') @user_has_permissions('send_messages') @service_has_permission('broadcast') diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 337aa7683..43f8fec3e 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -55,6 +55,17 @@ class BroadcastMessage(JSONModel): return cls(broadcast_message_api_client.create_broadcast_message( service_id=service_id, template_id=template_id, + content=None, + reference=None, + )) + + @classmethod + def create_from_content(cls, *, service_id, content, reference): + return cls(broadcast_message_api_client.create_broadcast_message( + service_id=service_id, + template_id=None, + content=content, + reference=reference, )) @classmethod diff --git a/app/navigation.py b/app/navigation.py index 97938d829..22f5b7924 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -374,6 +374,8 @@ class HeaderNavigation(Navigation): 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', + 'new_broadcast', + 'write_new_broadcast', } # header HTML now comes from GOVUK Frontend so requires a boolean, not an attribute @@ -400,6 +402,8 @@ class MainNavigation(Navigation): 'broadcast_dashboard', 'broadcast_dashboard_updates', 'view_current_broadcast', + 'new_broadcast', + 'write_new_broadcast', }, 'previous-broadcasts': { 'broadcast_dashboard_previous', @@ -1053,6 +1057,8 @@ class CaseworkNavigation(Navigation): 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', + 'new_broadcast', + 'write_new_broadcast', } @@ -1386,4 +1392,6 @@ class OrgNavigation(Navigation): 'approve_broadcast_message', 'reject_broadcast_message', 'cancel_broadcast_message', + 'new_broadcast', + 'write_new_broadcast', } diff --git a/app/notify_client/broadcast_message_api_client.py b/app/notify_client/broadcast_message_api_client.py index 9fb7e1587..2bf8b8d9a 100644 --- a/app/notify_client/broadcast_message_api_client.py +++ b/app/notify_client/broadcast_message_api_client.py @@ -8,12 +8,19 @@ class BroadcastMessageAPIClient(NotifyAdminAPIClient): *, service_id, template_id, + content, + reference, ): data = { "service_id": service_id, - "template_id": template_id, "personalisation": {}, } + if template_id: + data.update(template_id=template_id) + if content: + data.update(content=content) + if reference: + data.update(reference=reference) data = _attach_current_user(data) diff --git a/app/templates/views/broadcast/dashboard.html b/app/templates/views/broadcast/dashboard.html index 405bbc9de..a1d67d7cc 100644 --- a/app/templates/views/broadcast/dashboard.html +++ b/app/templates/views/broadcast/dashboard.html @@ -1,4 +1,5 @@ {% from 'components/ajax-block.html' import ajax_block %} +{% from "components/button/macro.njk" import govukButton %} {% extends "withnav_template.html" %} @@ -24,4 +25,15 @@ 'current_broadcasts' ) }} + {% if current_user.has_permissions('send_messages') %} +
+ {{ govukButton({ + "element": "a", + "text": "New alert", + "href": url_for('.new_broadcast', service_id=current_service.id), + "classes": "govuk-button--secondary" + }) }} +
+ {% endif %} + {% endblock %} diff --git a/app/templates/views/broadcast/new-broadcast.html b/app/templates/views/broadcast/new-broadcast.html new file mode 100644 index 000000000..323e1e869 --- /dev/null +++ b/app/templates/views/broadcast/new-broadcast.html @@ -0,0 +1,23 @@ +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + New alert +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header( + 'New alert', + back_link=url_for('.broadcast_dashboard', service_id=current_service.id), + )}} + + {% call form_wrapper() %} + {{ form.content }} + {{ page_footer('Continue') }} + {% endcall %} + +{% endblock %} diff --git a/app/templates/views/broadcast/write-new-broadcast.html b/app/templates/views/broadcast/write-new-broadcast.html new file mode 100644 index 000000000..97386596e --- /dev/null +++ b/app/templates/views/broadcast/write-new-broadcast.html @@ -0,0 +1,49 @@ +{% from "components/form.html" import form_wrapper %} +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/textbox.html" import textbox %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + New alert +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header( + 'New alert', + back_link=url_for('.new_broadcast', service_id=current_service.id), + )}} + + {% call form_wrapper() %} +
+
+ {{ form.name(param_extensions={ + "classes": "govuk-!-width-full", + "hint": {"text": "Your recipients will not see this"}, + "label": {"text": "Title"} + }) }} +
+
+ {{ textbox( + form.template_content, + highlight_placeholders=False, + autosize=True, + width='1-1', + rows=5, + extra_form_group_classes='govuk-!-margin-bottom-2' + ) }} +
+
+
+
+   +
+
+ {{ page_footer('Continue') }} +
+
+ {% endcall %} + +{% endblock %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 99c502b92..cdad71f35 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -26,6 +26,14 @@ sample_uuid = sample_uuid() '.broadcast_dashboard_previous', {}, 403, 405, ), + ( + '.new_broadcast', {}, + 403, 403, + ), + ( + '.write_new_broadcast', {}, + 403, 403, + ), ( '.broadcast', {'template_id': sample_uuid}, @@ -86,6 +94,14 @@ def test_broadcast_pages_403_without_permission( @pytest.mark.parametrize('endpoint, extra_args, expected_get_status, expected_post_status', ( + ( + '.new_broadcast', {}, + 403, 403, + ), + ( + '.write_new_broadcast', {}, + 403, 403, + ), ( '.broadcast', {'template_id': sample_uuid}, @@ -331,6 +347,30 @@ def test_broadcast_dashboard( 'Example template This is a test Live since today at 1:20am England Scotland', ] + button = page.select_one( + '.js-stick-at-bottom-when-scrolling a.govuk-button.govuk-button--secondary' + ) + assert normalize_spaces(button.text) == 'New alert' + assert button['href'] == url_for( + 'main.new_broadcast', + service_id=SERVICE_ONE_ID, + ) + + +def test_broadcast_dashboard_does_not_have_button_for_view_only_user( + client_request, + service_one, + active_user_view_permissions, + mock_get_broadcast_messages, +): + service_one['permissions'] += ['broadcast'] + client_request.login(active_user_view_permissions) + page = client_request.get( + '.broadcast_dashboard', + service_id=SERVICE_ONE_ID, + ) + assert not page.select('a.govuk-button') + @freeze_time('2020-02-20 02:20') def test_broadcast_dashboard_json( @@ -381,6 +421,166 @@ def test_previous_broadcasts_page( ] +def test_new_broadcast_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + page = client_request.get( + '.new_broadcast', + service_id=SERVICE_ONE_ID, + ) + + assert normalize_spaces(page.select_one('h1').text) == 'New alert' + + form = page.select_one('form') + assert form['method'] == 'post' + assert 'action' not in form + + assert [ + ( + choice.select_one('input')['name'], + choice.select_one('input')['value'], + normalize_spaces(choice.select_one('label').text), + ) + for choice in form.select('.govuk-radios__item') + ] == [ + ('content', 'freeform', 'Write your own message'), + ('content', 'template', 'Use a template'), + ] + + +@pytest.mark.parametrize('value, expected_redirect_endpoint', ( + ('freeform', 'main.write_new_broadcast'), + ('template', 'main.choose_template'), +)) +def test_new_broadcast_page_redirects( + client_request, + service_one, + value, + expected_redirect_endpoint, +): + service_one['permissions'] += ['broadcast'] + client_request.post( + '.new_broadcast', + service_id=SERVICE_ONE_ID, + _data={ + 'content': value, + }, + _expected_redirect=url_for( + expected_redirect_endpoint, + service_id=SERVICE_ONE_ID, + _external=True, + ) + ) + + +def test_write_new_broadcast_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + page = client_request.get( + '.write_new_broadcast', + service_id=SERVICE_ONE_ID, + ) + + assert normalize_spaces(page.select_one('h1').text) == 'New alert' + + form = page.select_one('form') + assert form['method'] == 'post' + assert 'action' not in form + + assert page.select_one('input[type=text]')['name'] == 'name' + + assert page.select_one('textarea')['name'] == 'template_content' + assert page.select_one('textarea')['data-module'] == 'enhanced-textbox' + assert page.select_one('textarea')['data-highlight-placeholders'] == 'false' + + assert ( + page.select_one('[data-module=update-status]')['data-updates-url'] + ) == url_for( + '.count_content_length', + service_id=SERVICE_ONE_ID, + template_type='broadcast', + ) + + assert ( + page.select_one('[data-module=update-status]')['data-target'] + ) == ( + page.select_one('textarea')['id'] + ) == ( + 'template_content' + ) + + assert ( + page.select_one('[data-module=update-status]')['aria-live'] + ) == ( + 'polite' + ) + + +def test_write_new_broadcast_posts( + client_request, + service_one, + mock_create_broadcast_message, + fake_uuid, +): + service_one['permissions'] += ['broadcast'] + client_request.post( + '.write_new_broadcast', + service_id=SERVICE_ONE_ID, + _data={ + 'name': 'My new alert', + 'template_content': 'This is a test', + }, + _expected_redirect=url_for( + '.preview_broadcast_areas', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _external=True, + ), + ) + mock_create_broadcast_message.assert_called_once_with( + service_id=SERVICE_ONE_ID, + reference='My new alert', + content='This is a test', + template_id=None, + ) + + +@pytest.mark.parametrize('content, expected_error_message', ( + ('', 'Cannot be empty'), + ('ŵ' * 616, 'Content must be 615 characters or fewer because it contains ŵ'), + ('w' * 1_396, 'Content must be 1,395 characters or fewer'), + ('hello ((name))', 'You can’t use ((double brackets)) to personalise this message'), +)) +def test_write_new_broadcast_bad_content( + client_request, + service_one, + mock_create_broadcast_message, + fake_uuid, + content, + expected_error_message, +): + service_one['permissions'] += ['broadcast'] + page = client_request.post( + '.write_new_broadcast', + service_id=SERVICE_ONE_ID, + _data={ + 'name': 'My new alert', + 'template_content': content, + }, + _expected_status=200, + ) + assert normalize_spaces( + page.select_one('.error-message').text + ) == ( + expected_error_message + ) + assert mock_create_broadcast_message.called is False + + def test_broadcast_page( client_request, service_one, diff --git a/tests/app/notify_client/test_broadcast_message_client.py b/tests/app/notify_client/test_broadcast_message_client.py index af739d2cc..fba03e265 100644 --- a/tests/app/notify_client/test_broadcast_message_client.py +++ b/tests/app/notify_client/test_broadcast_message_client.py @@ -12,6 +12,8 @@ def test_create_broadcast_message(mocker): client.create_broadcast_message( service_id='12345', template_id='67890', + content=None, + reference=None, ) mock_post.assert_called_once_with( '/service/12345/broadcast-message', diff --git a/tests/conftest.py b/tests/conftest.py index 0ce2ad7fc..e1d8eee1d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4272,8 +4272,11 @@ def mock_create_broadcast_message( fake_uuid, ): def _create( + *, service_id, template_id, + content, + reference, ): return { 'id': fake_uuid,