From ffd844b2a782906407cde3ad435b33b2e04451a9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 10 May 2021 22:00:09 +0100 Subject: [PATCH 1/7] Add confirmation step to emergency alert settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It feels quite dangerous that it’s just one click to make an emergency alerts service live. This commit adds a confirmation step which explains the consequences of what you’re about to do. --- app/main/forms.py | 2 + app/main/views/service_settings.py | 28 ++++- app/navigation.py | 1 + ...ervice-confirm-broadcast-account-type.html | 53 +++++++++ .../service-set-broadcast-account-type.html | 4 +- tests/app/main/views/test_service_settings.py | 105 +++++++++++++++++- tests/app/test_navigation.py | 1 + 7 files changed, 185 insertions(+), 9 deletions(-) create mode 100644 app/templates/views/service-settings/service-confirm-broadcast-account-type.html diff --git a/app/main/forms.py b/app/main/forms.py index e9a11b538..f2cf33f04 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2316,6 +2316,8 @@ class ServiceBroadcastAccountTypeField(GovukRadiosField): # (service_mode, broadcast_channel, allowed_broadcast_provider) # to a value to be used in our form such as "live-severe-ee" def process_data(self, value): + if isinstance(value, str): + return super().process_data(value) (live, broadcast_channel, allowed_broadcast_provider) = value account_type = None if broadcast_channel: diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 29387c35e..8473a9e99 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -327,6 +327,31 @@ def service_set_broadcast_account_type(service_id): ) ) + if form.validate_on_submit(): + return redirect(url_for( + '.service_confirm_broadcast_account_type', + service_id=current_service.id, + account_type=form.account_type.data, + )) + + return render_template( + 'views/service-settings/service-set-broadcast-account-type.html', + form=form, + ) + + +@main.route( + "/services//service-settings/broadcasts/", + methods=["GET", "POST"] +) +@user_is_platform_admin +def service_confirm_broadcast_account_type(service_id, account_type): + form = ServiceBroadcastAccountTypeForm(account_type=account_type) + form.validate() + + if form.account_type.errors: + abort(404) + if form.validate_on_submit(): service_api_client.set_service_broadcast_settings( current_service.id, @@ -341,11 +366,10 @@ def service_set_broadcast_account_type(service_id): broadcast_channel=form.account_type.broadcast_channel, provider_restriction=form.account_type.provider_restriction, ) - return redirect(url_for(".service_settings", service_id=service_id)) return render_template( - 'views/service-settings/service-set-broadcast-account-type.html', + 'views/service-settings/service-confirm-broadcast-account-type.html', form=form, ) diff --git a/app/navigation.py b/app/navigation.py index 81fdf53cd..ce0ffe8bb 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -244,6 +244,7 @@ class MainNavigation(Navigation): 'service_set_channel', 'send_files_by_email_contact_details', 'service_set_broadcast_account_type', + 'service_confirm_broadcast_account_type', 'service_set_email_branding', 'service_set_inbound_number', 'service_set_inbound_sms', diff --git a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html new file mode 100644 index 000000000..986ff5afc --- /dev/null +++ b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html @@ -0,0 +1,53 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% block service_page_title %} + Confirm emergency alert settings +{% endblock %} + +{% block maincolumn_content %} + +
+
+ {{ page_header( + 'Confirm emergency alert settings', + back_link=url_for('.service_set_broadcast_account_type', service_id=current_service.id) + ) }} + {% if form.account_type.service_mode == 'training' %} +

+ Training +

+

+ No phones will receive alerts sent from this service. +

+ {% else %} +

+ + {% if form.account_type.broadcast_channel == 'severe' %} + Live + {% elif form.account_type.broadcast_channel == 'test' %} + Test + {% endif %} + {% if form.account_type.provider_restriction != 'all' %} + ({{ form.account_type.provider_restriction|format_mobile_network }}) + {% endif %} + +

+

+ Members of the public + {% if form.account_type.broadcast_channel == 'test' %} + who have switched on the test channel on their phones + {% endif %} + will receive alerts sent from this service. +

+ {% endif %} + + {% call form_wrapper() %} + {{ page_footer('Confirm') }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/app/templates/views/service-settings/service-set-broadcast-account-type.html b/app/templates/views/service-settings/service-set-broadcast-account-type.html index 83b739d16..78d64c049 100644 --- a/app/templates/views/service-settings/service-set-broadcast-account-type.html +++ b/app/templates/views/service-settings/service-set-broadcast-account-type.html @@ -6,7 +6,7 @@ {% block service_page_title %} Send cell broadcasts {% endblock %} - + {% block maincolumn_content %}
@@ -24,7 +24,7 @@ } } }) }} - {{ page_footer('Save') }} + {{ page_footer('Continue') }} {% endcall %}
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 596fa0987..94fbbe129 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -5521,6 +5521,103 @@ def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast assert selected_label.text.strip() == expected_text +@pytest.mark.parametrize( + 'value', + ( + 'training-test', + 'live-test-ee', + 'live-test-o2', + 'live-test-three', + 'live-test-vodafone', + 'live-test', + 'live-severe', + pytest.param('foo', marks=pytest.mark.xfail), + ), +) +def test_post_service_set_broadcast_account_type_confirms( + client_request, + platform_admin_user, + mocker, + value, +): + client_request.login(platform_admin_user) + client_request.post( + 'main.service_set_broadcast_account_type', + service_id=SERVICE_ONE_ID, + _data={ + 'account_type': value, + }, + _expected_status=302, + _expected_redirect=url_for( + 'main.service_confirm_broadcast_account_type', + service_id=SERVICE_ONE_ID, + account_type=value, + _external=True, + ) + ) + + +@pytest.mark.parametrize( + 'value, expected_paragraphs', + [ + ('training-test', [ + 'Training', + 'No phones will receive alerts sent from this service.', + ]), + ('live-test-ee', [ + 'Test (EE)', + 'Members of the public who have switched on the test ' + 'channel on their phones will receive alerts sent from ' + 'this service.', + ]), + ('live-test-o2', [ + 'Test (O2)', + 'Members of the public who have switched on the test ' + 'channel on their phones will receive alerts sent from ' + 'this service.', + ]), + ('live-test-three', [ + 'Test (Three)', + 'Members of the public who have switched on the test ' + 'channel on their phones will receive alerts sent from ' + 'this service.', + ]), + ('live-test-vodafone', [ + 'Test (Vodafone)', + 'Members of the public who have switched on the test ' + 'channel on their phones will receive alerts sent from ' + 'this service.', + ]), + ('live-test', [ + 'Test', + 'Members of the public who have switched on the test ' + 'channel on their phones will receive alerts sent from ' + 'this service.', + ]), + ('live-severe', [ + 'Live', + 'Members of the public will receive alerts sent from this ' + 'service.', + ]), + ] +) +def test_post_service_set_broadcast_account_type_confirmation_page( + client_request, + platform_admin_user, + value, + expected_paragraphs, +): + client_request.login(platform_admin_user) + page = client_request.get( + 'main.service_confirm_broadcast_account_type', + service_id=SERVICE_ONE_ID, + account_type=value, + ) + assert [ + normalize_spaces(p.text) for p in page.select('main p') + ] == expected_paragraphs + + @pytest.mark.parametrize( 'value,service_mode,broadcast_channel,allowed_broadcast_provider', [ @@ -5543,12 +5640,10 @@ def test_post_service_set_broadcast_account_type_posts_data_to_api_and_redirects response = platform_admin_client.post( url_for( - 'main.service_set_broadcast_account_type', + 'main.service_confirm_broadcast_account_type', service_id=SERVICE_ONE_ID, - ), - data={ - 'account_type': value - } + account_type=value, + ) ) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=SERVICE_ONE_ID, _external=True) diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 15b25c089..6df041e79 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -246,6 +246,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'service_preview_letter_branding', 'service_set_auth_type', 'service_set_broadcast_account_type', + 'service_confirm_broadcast_account_type', 'service_set_channel', 'service_set_email_branding', 'service_set_inbound_number', From f640767f3d7468402881441345bcd4ab983c6eb3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 11 May 2021 09:50:18 +0100 Subject: [PATCH 2/7] Add government channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have been asked to support the government channel so that: - it can be tested - the option to use it is available for the most severe of emergencies, where the public’s choice to opt-out is outweighed by the widespread risk to life --- app/main/forms.py | 1 + app/templates/views/service-settings.html | 1 + ...ervice-confirm-broadcast-account-type.html | 9 ++- app/templates/withnav_template.html | 12 ++- tests/app/main/views/test_broadcast.py | 75 ++++++++++++------- tests/app/main/views/test_service_settings.py | 7 ++ 6 files changed, 69 insertions(+), 36 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index f2cf33f04..327873385 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2351,6 +2351,7 @@ class ServiceBroadcastAccountTypeForm(StripWhitespaceForm): ("live-test-vodafone", "Test channel (Vodafone)"), ("live-test", "Test channel (all networks)"), ("live-severe", "Live (all networks)"), + ("live-government", "Government channel (all networks)"), ], validators=[DataRequired()] ) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index e28f0a168..ea1e3b3dd 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -434,6 +434,7 @@ {% if not current_service.broadcast_channel %} Off {% else %} + {% if current_service.live and current_service.broadcast_channel == "government" %}Government{% endif %} {% if current_service.live and current_service.broadcast_channel == "severe" %}Live{% endif %} {% if current_service.live and current_service.broadcast_channel == "test" %}Test {% if current_service.allowed_broadcast_provider != "all" %}({{ current_service.allowed_broadcast_provider|format_mobile_network }}){% else %}(All networks){% endif %}{%endif%} {% if not current_service.live%}Training {% endif%} diff --git a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html index 986ff5afc..79fc1bd40 100644 --- a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html +++ b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html @@ -27,8 +27,8 @@ {% if form.account_type.broadcast_channel == 'severe' %} Live - {% elif form.account_type.broadcast_channel == 'test' %} - Test + {% else %} + {{ form.account_type.broadcast_channel|title }} {% endif %} {% if form.account_type.provider_restriction != 'all' %} ({{ form.account_type.provider_restriction|format_mobile_network }}) @@ -40,7 +40,10 @@ {% if form.account_type.broadcast_channel == 'test' %} who have switched on the test channel on their phones {% endif %} - will receive alerts sent from this service. + will receive alerts sent from this service + {%- if form.account_type.broadcast_channel == 'government' -%} + , even if they’ve opted out + {%- endif %}.

{% endif %} diff --git a/app/templates/withnav_template.html b/app/templates/withnav_template.html index 296f2a029..34f7fefae 100644 --- a/app/templates/withnav_template.html +++ b/app/templates/withnav_template.html @@ -22,11 +22,15 @@ {% elif current_service.has_permission('broadcast') %} {% if current_service.trial_mode %} Training - {% else %} + {% elif current_service.broadcast_channel == 'severe' %} Live - {% endif %} - {% if current_service.allowed_broadcast_provider != "all" %} - ({{ current_service.allowed_broadcast_provider }}) + {% elif current_service.broadcast_channel == 'test' %} + {{ current_service.broadcast_channel|title }} + {% if current_service.allowed_broadcast_provider != "all" %} + ({{ current_service.allowed_broadcast_provider }}) + {% endif %} + {% else %} + {{ current_service.broadcast_channel|title }} {% endif %} {% endif %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 83709ce98..4b4c3af9f 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -286,37 +286,52 @@ def test_broadcast_tour_page_4_shows_service_name( ) -@pytest.mark.parametrize('trial_mode, allowed_broadcast_provider, selector, expected_text, expected_tagged_text', ( +@pytest.mark.parametrize( + 'trial_mode, channel, allowed_broadcast_provider, selector, expected_text, expected_tagged_text', ( - True, - "all", - '.navigation-service-type.navigation-service-type--training', - 'service one Training Switch service', - 'Training', - ), - ( - False, - "all", - '.navigation-service-type.navigation-service-type--live', - 'service one Live Switch service', - 'Live', - ), + ( + True, + "all", + None, + '.navigation-service-type.navigation-service-type--training', + 'service one Training Switch service', + 'Training', + ), + ( + False, + 'severe', + "all", + '.navigation-service-type.navigation-service-type--live', + 'service one Live Switch service', + 'Live', + ), - ( - True, - 'vodafone', - '.navigation-service-type.navigation-service-type--training', - 'service one Training (vodafone) Switch service', - 'Training (vodafone)', - ), - ( - False, - 'vodafone', - '.navigation-service-type.navigation-service-type--live', - 'service one Live (vodafone) Switch service', - 'Live (vodafone)', - ), -)) + ( + False, + 'test', + 'vodafone', + '.navigation-service-type.navigation-service-type--live', + 'service one Test (vodafone) Switch service', + 'Test (vodafone)', + ), + ( + False, + 'test', + 'all', + '.navigation-service-type.navigation-service-type--live', + 'service one Test Switch service', + 'Test', + ), + ( + False, + 'government', + 'all', + '.navigation-service-type.navigation-service-type--live', + 'service one Government Switch service', + 'Government', + ), + ) +) def test_broadcast_service_shows_live_or_training( client_request, service_one, @@ -324,6 +339,7 @@ def test_broadcast_service_shows_live_or_training( mock_get_service_templates_when_no_templates_exist, trial_mode, allowed_broadcast_provider, + channel, selector, expected_text, expected_tagged_text, @@ -331,6 +347,7 @@ def test_broadcast_service_shows_live_or_training( service_one['allowed_broadcast_provider'] = allowed_broadcast_provider service_one['permissions'] += ['broadcast'] service_one['restricted'] = trial_mode + service_one['broadcast_channel'] = channel page = client_request.get( '.broadcast_dashboard', service_id=SERVICE_ONE_ID, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 94fbbe129..1bca91636 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -5435,6 +5435,7 @@ def test_get_service_set_broadcast_account_type( "Test channel (Vodafone)", "Test channel (all networks)", "Live (all networks)", + "Government channel (all networks)", ] labels = page.find_all('label', class_="govuk-radios__label") assert len(labels) == len(expected_labels) @@ -5599,6 +5600,11 @@ def test_post_service_set_broadcast_account_type_confirms( 'Members of the public will receive alerts sent from this ' 'service.', ]), + ('live-government', [ + 'Government', + 'Members of the public will receive alerts sent from this ' + 'service, even if they’ve opted out.' + ]), ] ) def test_post_service_set_broadcast_account_type_confirmation_page( @@ -5624,6 +5630,7 @@ def test_post_service_set_broadcast_account_type_confirmation_page( ("training-test", "training", "test", "all"), ("live-test-vodafone", "live", "test", "vodafone"), ("live-severe", "live", "severe", "all"), + ("live-government", "live", "government", "all"), ] ) def test_post_service_set_broadcast_account_type_posts_data_to_api_and_redirects( From a0f54539ccab1ac8d9850a4d891289b1ad917bfb Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 11 May 2021 11:07:30 +0100 Subject: [PATCH 3/7] Add a second step for choosing networks Only the test channel has the option to isolate messages to one network. This commits makes the choices less confusing by only showing the network choice to those who have selected the test channel. --- app/main/forms.py | 31 +++- app/main/views/service_settings.py | 41 ++++- app/navigation.py | 3 +- app/templates/views/service-settings.html | 2 +- ...ervice-confirm-broadcast-account-type.html | 2 +- .../service-set-broadcast-channel.html | 32 ++++ .../service-set-broadcast-network.html | 32 ++++ tests/app/main/views/test_service_settings.py | 169 +++++++++++++++--- tests/app/test_navigation.py | 3 +- 9 files changed, 284 insertions(+), 31 deletions(-) create mode 100644 app/templates/views/service-settings/service-set-broadcast-channel.html create mode 100644 app/templates/views/service-settings/service-set-broadcast-network.html diff --git a/app/main/forms.py b/app/main/forms.py index 327873385..223970de8 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2316,7 +2316,7 @@ class ServiceBroadcastAccountTypeField(GovukRadiosField): # (service_mode, broadcast_channel, allowed_broadcast_provider) # to a value to be used in our form such as "live-severe-ee" def process_data(self, value): - if isinstance(value, str): + if not value or isinstance(value, str): return super().process_data(value) (live, broadcast_channel, allowed_broadcast_provider) = value account_type = None @@ -2332,13 +2332,40 @@ class ServiceBroadcastAccountTypeField(GovukRadiosField): # broadcast_channel and provider_restriction to be used by the flask route to send to the # API def post_validate(self, form, validation_stopped): - if not validation_stopped: + if not validation_stopped and self.data: split_values = self.data.split("-") self.service_mode = split_values[0] self.broadcast_channel = split_values[1] self.provider_restriction = split_values[2] if len(split_values) == 3 else 'all' +class ServiceBroadcastChannelForm(StripWhitespaceForm): + channel = ServiceBroadcastAccountTypeField( + 'Emergency alerts settings', + thing='mode or channel', + choices=[ + ("training-test", "Training mode"), + ("live-test", "Test channel"), + ("live-severe", "Live channel"), + ("live-government", "Government channel"), + ], + ) + + +class ServiceBroadcastNetworkForm(StripWhitespaceForm): + network = ServiceBroadcastAccountTypeField( + 'Choose a mobile network', + thing='mobile network', + choices=[ + ("live-test", "All networks"), + ("live-test-ee", "EE only"), + ("live-test-o2", "O2 only"), + ("live-test-vodafone", "Vodafone only"), + ("live-test-three", "Three only"), + ], + ) + + class ServiceBroadcastAccountTypeForm(StripWhitespaceForm): account_type = ServiceBroadcastAccountTypeField( 'Change cell broadcast service type', diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 8473a9e99..0167a2843 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -46,6 +46,8 @@ from app.main.forms import ( RenameServiceForm, SearchByNameForm, ServiceBroadcastAccountTypeForm, + ServiceBroadcastChannelForm, + ServiceBroadcastNetworkForm, ServiceContactDetailsForm, ServiceDataRetentionEditForm, ServiceDataRetentionForm, @@ -318,9 +320,38 @@ def service_set_permission(service_id, permission): @main.route("/services//service-settings/broadcasts", methods=["GET", "POST"]) @user_is_platform_admin -def service_set_broadcast_account_type(service_id): - form = ServiceBroadcastAccountTypeForm( - account_type=( +def service_set_broadcast_channel(service_id): + form = ServiceBroadcastChannelForm( + channel=( + current_service.live, + current_service.broadcast_channel, + 'all', + ) + ) + + if form.validate_on_submit(): + if form.channel.data == 'live-test': + return redirect(url_for( + '.service_set_broadcast_network', + service_id=current_service.id, + )) + return redirect(url_for( + '.service_confirm_broadcast_account_type', + service_id=current_service.id, + account_type=form.channel.data, + )) + + return render_template( + 'views/service-settings/service-set-broadcast-channel.html', + form=form, + ) + + +@main.route("/services//service-settings/broadcasts/network", methods=["GET", "POST"]) +@user_is_platform_admin +def service_set_broadcast_network(service_id): + form = ServiceBroadcastNetworkForm( + network=( current_service.live, current_service.broadcast_channel, current_service.allowed_broadcast_provider @@ -331,11 +362,11 @@ def service_set_broadcast_account_type(service_id): return redirect(url_for( '.service_confirm_broadcast_account_type', service_id=current_service.id, - account_type=form.account_type.data, + account_type=form.network.data, )) return render_template( - 'views/service-settings/service-set-broadcast-account-type.html', + 'views/service-settings/service-set-broadcast-network.html', form=form, ) diff --git a/app/navigation.py b/app/navigation.py index ce0ffe8bb..77e1f59e3 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -243,8 +243,9 @@ class MainNavigation(Navigation): 'service_set_auth_type', 'service_set_channel', 'send_files_by_email_contact_details', - 'service_set_broadcast_account_type', 'service_confirm_broadcast_account_type', + 'service_set_broadcast_channel', + 'service_set_broadcast_network', 'service_set_email_branding', 'service_set_inbound_number', 'service_set_inbound_sms', diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index ea1e3b3dd..4f665378e 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -442,7 +442,7 @@ {% endcall %} {{ edit_field( 'Change', - url_for('.service_set_broadcast_account_type', service_id=current_service.id), + url_for('.service_set_broadcast_channel', service_id=current_service.id), suffix='your settings for Send cell broadcasts' ) }} diff --git a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html index 79fc1bd40..98fb1a52c 100644 --- a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html +++ b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html @@ -13,7 +13,7 @@
{{ page_header( 'Confirm emergency alert settings', - back_link=url_for('.service_set_broadcast_account_type', service_id=current_service.id) + back_link=url_for('.service_set_broadcast_channel', service_id=current_service.id) ) }} {% if form.account_type.service_mode == 'training' %}

diff --git a/app/templates/views/service-settings/service-set-broadcast-channel.html b/app/templates/views/service-settings/service-set-broadcast-channel.html new file mode 100644 index 000000000..b8a07b1d1 --- /dev/null +++ b/app/templates/views/service-settings/service-set-broadcast-channel.html @@ -0,0 +1,32 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} +{% from "components/back-link/macro.njk" import govukBackLink %} + +{% block service_page_title %} + Emergency alerts settings +{% endblock %} + +{% block maincolumn_content %} + +

+
+ {{ govukBackLink({ + "text": "Back", + "href": url_for('.service_settings', service_id=current_service.id) + }) }} + {% call form_wrapper() %} + {{ form.channel(param_extensions={ + 'fieldset': { + 'legend': { + 'isPageHeading': True, + 'classes': 'govuk-fieldset__legend--l' + } + } + }) }} + {{ page_footer('Continue') }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/app/templates/views/service-settings/service-set-broadcast-network.html b/app/templates/views/service-settings/service-set-broadcast-network.html new file mode 100644 index 000000000..bb0e68561 --- /dev/null +++ b/app/templates/views/service-settings/service-set-broadcast-network.html @@ -0,0 +1,32 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} +{% from "components/back-link/macro.njk" import govukBackLink %} + +{% block service_page_title %} + Choose a mobile network +{% endblock %} + +{% block maincolumn_content %} + +
+
+ {{ govukBackLink({ + "text": "Back", + "href": url_for('.service_set_broadcast_channel', service_id=current_service.id) + }) }} + {% call form_wrapper() %} + {{ form.network(param_extensions={ + 'fieldset': { + 'legend': { + 'isPageHeading': True, + 'classes': 'govuk-fieldset__legend--l' + } + } + }) }} + {{ page_footer('Continue') }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 1bca91636..c11f39872 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -5418,24 +5418,20 @@ def test_get_service_set_broadcast_account_type( ): response = platform_admin_client.get( url_for( - 'main.service_set_broadcast_account_type', + 'main.service_set_broadcast_channel', service_id=SERVICE_ONE_ID, ) ) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.select_one('h1').text.strip() == "Change cell broadcast service type" + assert page.select_one('h1').text.strip() == 'Emergency alerts settings' expected_labels = [ "Training mode", - "Test channel (EE)", - "Test channel (O2)", - "Test channel (Three)", - "Test channel (Vodafone)", - "Test channel (all networks)", - "Live (all networks)", - "Government channel (all networks)", + "Test channel", + "Live channel", + "Government channel", ] labels = page.find_all('label', class_="govuk-radios__label") assert len(labels) == len(expected_labels) @@ -5452,7 +5448,7 @@ def test_get_service_set_broadcast_account_type_has_no_radio_selected_for_non_br ): response = platform_admin_client.get( url_for( - 'main.service_set_broadcast_account_type', + 'main.service_set_broadcast_channel', service_id=SERVICE_ONE_ID, ) ) @@ -5475,16 +5471,23 @@ def test_get_service_set_broadcast_account_type_has_no_radio_selected_for_non_br "live", "test", "vodafone", - "Test channel (Vodafone)", - "live-test-vodafone", + "Test channel", + "live-test", ), ( "live", "severe", "all", - "Live (all networks)", + "Live channel", "live-severe", ), + ( + "live", + "government", + "all", + "Government channel", + "live-government", + ), ] ) def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast_service( @@ -5507,7 +5510,7 @@ def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast response = platform_admin_client.get( url_for( - 'main.service_set_broadcast_account_type', + 'main.service_set_broadcast_channel', service_id=SERVICE_ONE_ID, ) ) @@ -5522,16 +5525,136 @@ def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast assert selected_label.text.strip() == expected_text +@pytest.mark.parametrize( + 'account_type,expected_redirect_endpoint,extra_args', + [ + ( + 'training-test', + '.service_confirm_broadcast_account_type', + {'account_type': 'training-test'}, + ), + ( + 'live-test', + '.service_set_broadcast_network', + {}, + ), + ( + 'live-severe', + '.service_confirm_broadcast_account_type', + {'account_type': 'live-severe'}, + ), + ( + 'live-government', + '.service_confirm_broadcast_account_type', + {'account_type': 'live-government'}, + ), + ] +) +def test_get_service_set_broadcast_channel_redirects( + client_request, + platform_admin_user, + mocker, + account_type, + expected_redirect_endpoint, + extra_args, +): + client_request.login(platform_admin_user) + client_request.post( + 'main.service_set_broadcast_channel', + service_id=SERVICE_ONE_ID, + _data={ + 'channel': account_type, + }, + _expected_redirect=url_for( + expected_redirect_endpoint, + service_id=SERVICE_ONE_ID, + _external=True, + **extra_args, + ) + ) + + +@pytest.mark.parametrize( + 'service_mode,broadcast_channel,allowed_broadcast_provider,expected_text,expected_value', + [ + ( + "training", + "test", + "all", + "All networks", + "live-test", + ), + ( + "live", + "test", + "ee", + "EE only", + "live-test-ee", + ), + ( + "live", + "test", + "o2", + "O2 only", + "live-test-o2", + ), + ( + "live", + "test", + "three", + "Three only", + "live-test-three", + ), + ( + "live", + "test", + "vodafone", + "Vodafone only", + "live-test-vodafone", + ), + ] +) +def test_get_service_set_broadcast_network_has_radio_selected( + client_request, + platform_admin_user, + mocker, + service_mode, + broadcast_channel, + allowed_broadcast_provider, + expected_text, + expected_value +): + client_request.login(platform_admin_user) + service_one = service_json( + SERVICE_ONE_ID, + permissions=['broadcast'], + restricted=False, + broadcast_channel=broadcast_channel, + allowed_broadcast_provider=allowed_broadcast_provider, + ) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) + + page = client_request.get( + 'main.service_set_broadcast_network', + service_id=SERVICE_ONE_ID, + ) + selected_radios = page.select('input[checked]') + assert len(selected_radios) == 1 + + selected_radio = selected_radios[0] + assert selected_radio.get('value') == expected_value + selected_label = selected_radio.find_next_sibling('label') + assert normalize_spaces(selected_label.text) == expected_text + + @pytest.mark.parametrize( 'value', ( - 'training-test', 'live-test-ee', 'live-test-o2', 'live-test-three', 'live-test-vodafone', 'live-test', - 'live-severe', pytest.param('foo', marks=pytest.mark.xfail), ), ) @@ -5543,10 +5666,10 @@ def test_post_service_set_broadcast_account_type_confirms( ): client_request.login(platform_admin_user) client_request.post( - 'main.service_set_broadcast_account_type', + 'main.service_set_broadcast_network', service_id=SERVICE_ONE_ID, _data={ - 'account_type': value, + 'network': value, }, _expected_status=302, _expected_redirect=url_for( @@ -5669,21 +5792,27 @@ def test_post_service_set_broadcast_account_type_posts_data_to_api_and_redirects ) +@pytest.mark.parametrize('endpoint, expected_error', ( + ('main.service_set_broadcast_channel', 'Error: Select mode or channel'), + ('main.service_set_broadcast_network', 'Error: Select mobile network'), +)) def test_post_service_set_broadcast_account_type_shows_errors_if_no_radio_selected( platform_admin_client, mocker, + endpoint, + expected_error, ): set_service_broadcast_settings_mock = mocker.patch('app.service_api_client.set_service_broadcast_settings') mock_event_handler = mocker.patch('app.main.views.service_settings.create_broadcast_account_type_change_event') response = platform_admin_client.post( url_for( - 'main.service_set_broadcast_account_type', + endpoint, service_id=SERVICE_ONE_ID, ) ) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert "This field is required" in page.find("span", {"class": "govuk-error-message"}).text + assert expected_error in page.find("span", {"class": "govuk-error-message"}).text assert not set_service_broadcast_settings_mock.called assert not mock_event_handler.called diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 6df041e79..79b78376d 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -245,8 +245,9 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'service_preview_email_branding', 'service_preview_letter_branding', 'service_set_auth_type', - 'service_set_broadcast_account_type', 'service_confirm_broadcast_account_type', + 'service_set_broadcast_channel', + 'service_set_broadcast_network', 'service_set_channel', 'service_set_email_branding', 'service_set_inbound_number', From d720b0e47a969cd81248bae075f428dc1e685908 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 11 May 2021 11:14:32 +0100 Subject: [PATCH 4/7] Rename cell broadcasts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ‘Emergency alerts’ is the confirmed name of the service now. --- app/templates/views/service-settings.html | 4 ++-- tests/app/main/views/test_service_settings.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 4f665378e..ac2560348 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -429,7 +429,7 @@ {% endfor %} {% call row() %} - {{ text_field('Send cell broadcasts')}} + {{ text_field('Emergency alerts')}} {% call field(wrap=True) %} {% if not current_service.broadcast_channel %} Off @@ -443,7 +443,7 @@ {{ edit_field( 'Change', url_for('.service_set_broadcast_channel', service_id=current_service.id), - suffix='your settings for Send cell broadcasts' + suffix='your settings for emergency alerts' ) }} {% endcall %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index c11f39872..1fd9d5f34 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -113,7 +113,7 @@ def mock_get_service_settings_page_common( 'Custom data retention Email – 7 days Change data retention', 'Receive inbound SMS Off Change your settings for Receive inbound SMS', 'Email authentication Off Change your settings for Email authentication', - 'Send cell broadcasts Off Change your settings for Send cell broadcasts', + 'Emergency alerts Off Change your settings for emergency alerts', ]), ]) def test_should_show_overview( @@ -186,7 +186,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( 'Label Value Action', 'Notes None Change the notes for the service', 'Email authentication Off Change your settings for Email authentication', - 'Send cell broadcasts Training Change your settings for Send cell broadcasts', + 'Emergency alerts Training Change your settings for emergency alerts', ] assert len(rows) == len(expected_rows) @@ -238,7 +238,7 @@ def test_platform_admin_sees_correct_description_of_broadcast_service_setting( )) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - broadcast_setting_row = page.find(string=re.compile("Send cell broadcasts")).find_parent('tr') + broadcast_setting_row = page.find(string=re.compile("Emergency alerts")).find_parent('tr') broadcast_setting_description = broadcast_setting_row.select('td')[1].text.strip() assert broadcast_setting_description == expected_text From e45bd485e802275b0b1af0433da3804f0e15dbf2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 11 May 2021 12:39:52 +0100 Subject: [PATCH 5/7] Add conditional reveal to network selection Rather than try to explain all/only just through words we can use some interaction design to make the hierarchy of choices more explicit. --- app/main/forms.py | 35 ++++- app/main/views/service_settings.py | 24 +++- .../service-set-broadcast-network.html | 29 ++-- tests/app/main/views/test_service_settings.py | 134 +++++++++++++----- 4 files changed, 161 insertions(+), 61 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 223970de8..065b62a6d 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2339,6 +2339,13 @@ class ServiceBroadcastAccountTypeField(GovukRadiosField): self.provider_restriction = split_values[2] if len(split_values) == 3 else 'all' +class OptionalServiceBroadcastAccountTypeField(ServiceBroadcastAccountTypeField): + def pre_validate(self, form): + if self.data is None: + return + super().pre_validate(form) + + class ServiceBroadcastChannelForm(StripWhitespaceForm): channel = ServiceBroadcastAccountTypeField( 'Emergency alerts settings', @@ -2353,18 +2360,32 @@ class ServiceBroadcastChannelForm(StripWhitespaceForm): class ServiceBroadcastNetworkForm(StripWhitespaceForm): - network = ServiceBroadcastAccountTypeField( + + network_variant = ServiceBroadcastAccountTypeField( 'Choose a mobile network', - thing='mobile network', + thing='a mobile network', choices=[ - ("live-test", "All networks"), - ("live-test-ee", "EE only"), - ("live-test-o2", "O2 only"), - ("live-test-vodafone", "Vodafone only"), - ("live-test-three", "Three only"), + ('live-test', 'All networks'), + ('', 'A single network'), + ] + ) + network = OptionalServiceBroadcastAccountTypeField( + 'Choose a mobile network', + thing='a mobile network', + choices=[ + ('live-test-ee', 'EE'), + ('live-test-o2', 'O2'), + ('live-test-vodafone', 'Vodafone'), + ('live-test-three', 'Three'), ], ) + def validate_network(self, field): + if not self.network_variant.data and not field.data: + raise ValidationError('Select a mobile network') + if self.network_variant.data == 'all': + field.data = '' + class ServiceBroadcastAccountTypeForm(StripWhitespaceForm): account_type = ServiceBroadcastAccountTypeField( diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 0167a2843..5153351a3 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -350,19 +350,29 @@ def service_set_broadcast_channel(service_id): @main.route("/services//service-settings/broadcasts/network", methods=["GET", "POST"]) @user_is_platform_admin def service_set_broadcast_network(service_id): - form = ServiceBroadcastNetworkForm( - network=( - current_service.live, - current_service.broadcast_channel, - current_service.allowed_broadcast_provider + if current_service.allowed_broadcast_provider == 'all': + form = ServiceBroadcastNetworkForm( + network_variant=( + current_service.live, + current_service.broadcast_channel, + current_service.allowed_broadcast_provider, + ), + ) + else: + form = ServiceBroadcastNetworkForm( + network_variant='', + network=( + current_service.live, + current_service.broadcast_channel, + current_service.allowed_broadcast_provider + ) ) - ) if form.validate_on_submit(): return redirect(url_for( '.service_confirm_broadcast_account_type', service_id=current_service.id, - account_type=form.network.data, + account_type=form.network_variant.data or form.network.data, )) return render_template( diff --git a/app/templates/views/service-settings/service-set-broadcast-network.html b/app/templates/views/service-settings/service-set-broadcast-network.html index bb0e68561..a32f24819 100644 --- a/app/templates/views/service-settings/service-set-broadcast-network.html +++ b/app/templates/views/service-settings/service-set-broadcast-network.html @@ -1,6 +1,9 @@ {% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} {% from "components/page-footer.html" import page_footer %} {% from "components/form.html" import form_wrapper %} +{% from "components/radios.html" import radio, conditional_radio_panel %} +{% from "components/select-input.html" import select_wrapper %} {% from "components/back-link/macro.njk" import govukBackLink %} {% block service_page_title %} @@ -11,19 +14,21 @@
- {{ govukBackLink({ - "text": "Back", - "href": url_for('.service_set_broadcast_channel', service_id=current_service.id) - }) }} + {{ page_header( + 'Choose a mobile network', + back_link=url_for('.service_set_broadcast_channel', service_id=current_service.id) + ) }} {% call form_wrapper() %} - {{ form.network(param_extensions={ - 'fieldset': { - 'legend': { - 'isPageHeading': True, - 'classes': 'govuk-fieldset__legend--l' - } - } - }) }} + {% call select_wrapper(form.network_variant, hide_legend=True) %} + {% for option in form.network_variant %} + {{ radio(option, data_target='single-network' if option.data == "" else None) }} + {% endfor %} + {% endcall %} + {% call conditional_radio_panel('single-network') %} + {{ form.network( + param_extensions={'fieldset': {'legend': {'classes': 'govuk-visually-hidden'}}} + ) }} + {% endcall %} {{ page_footer('Continue') }} {% endcall %}
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 1fd9d5f34..d4297c74d 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -5575,42 +5575,69 @@ def test_get_service_set_broadcast_channel_redirects( @pytest.mark.parametrize( - 'service_mode,broadcast_channel,allowed_broadcast_provider,expected_text,expected_value', + 'service_mode,broadcast_channel,allowed_broadcast_provider,expected_selected', [ ( "training", "test", "all", - "All networks", - "live-test", + [], + ), + ( + "live", + "severe", + "all", + [], + ), + ( + "live", + "government", + "all", + [], + ), + ( + "live", + "test", + "all", + [ + ("All networks", "live-test"), + ], ), ( "live", "test", "ee", - "EE only", - "live-test-ee", + [ + ("A single network", ""), + ("EE", "live-test-ee"), + ], ), ( "live", "test", "o2", - "O2 only", - "live-test-o2", + [ + ("A single network", ""), + ("O2", "live-test-o2"), + ], ), ( "live", "test", "three", - "Three only", - "live-test-three", + [ + ("A single network", ""), + ("Three", "live-test-three"), + ], ), ( "live", "test", "vodafone", - "Vodafone only", - "live-test-vodafone", + [ + ("A single network", ""), + ("Vodafone", "live-test-vodafone"), + ], ), ] ) @@ -5621,14 +5648,13 @@ def test_get_service_set_broadcast_network_has_radio_selected( service_mode, broadcast_channel, allowed_broadcast_provider, - expected_text, - expected_value + expected_selected, ): client_request.login(platform_admin_user) service_one = service_json( SERVICE_ONE_ID, permissions=['broadcast'], - restricted=False, + restricted=False if service_mode == 'live' else True, broadcast_channel=broadcast_channel, allowed_broadcast_provider=allowed_broadcast_provider, ) @@ -5638,49 +5664,87 @@ def test_get_service_set_broadcast_network_has_radio_selected( 'main.service_set_broadcast_network', service_id=SERVICE_ONE_ID, ) - selected_radios = page.select('input[checked]') - assert len(selected_radios) == 1 - selected_radio = selected_radios[0] - assert selected_radio.get('value') == expected_value - selected_label = selected_radio.find_next_sibling('label') - assert normalize_spaces(selected_label.text) == expected_text + assert [ + ( + normalize_spaces(radio.find_next_sibling('label').text), + radio['value'], + ) + for radio in page.select('input[checked]') + ] == expected_selected @pytest.mark.parametrize( - 'value', + 'data, expected_result', ( - 'live-test-ee', - 'live-test-o2', - 'live-test-three', - 'live-test-vodafone', - 'live-test', - pytest.param('foo', marks=pytest.mark.xfail), + ( + {'network_variant': 'live-test'}, + 'live-test' + ), + ( + {'network_variant': '', 'network': 'live-test-ee'}, + 'live-test-ee' + ), + ( + {'network_variant': '', 'network': 'live-test-o2'}, + 'live-test-o2' + ), + ( + {'network_variant': '', 'network': 'live-test-three'}, + 'live-test-three' + ), + ( + {'network_variant': '', 'network': 'live-test-vodafone'}, + 'live-test-vodafone' + ), ), ) -def test_post_service_set_broadcast_account_type_confirms( +def test_post_service_set_broadcast_network( client_request, platform_admin_user, - mocker, - value, + data, + expected_result, ): client_request.login(platform_admin_user) client_request.post( 'main.service_set_broadcast_network', service_id=SERVICE_ONE_ID, - _data={ - 'network': value, - }, + _data=data, _expected_status=302, _expected_redirect=url_for( 'main.service_confirm_broadcast_account_type', service_id=SERVICE_ONE_ID, - account_type=value, + account_type=expected_result, _external=True, ) ) +@pytest.mark.parametrize( + 'data', + ( + {}, + {'network_variant': ''}, # Missing choice of MNO + ), +) +def test_post_service_set_broadcast_network_makes_you_choose( + client_request, + platform_admin_user, + mocker, + data, +): + client_request.login(platform_admin_user) + page = client_request.post( + 'main.service_set_broadcast_network', + service_id=SERVICE_ONE_ID, + _data=data, + _expected_status=200, + ) + assert normalize_spaces( + page.select_one('.govuk-error-message').text + ) == 'Error: Select a mobile network' + + @pytest.mark.parametrize( 'value, expected_paragraphs', [ @@ -5794,7 +5858,7 @@ def test_post_service_set_broadcast_account_type_posts_data_to_api_and_redirects @pytest.mark.parametrize('endpoint, expected_error', ( ('main.service_set_broadcast_channel', 'Error: Select mode or channel'), - ('main.service_set_broadcast_network', 'Error: Select mobile network'), + ('main.service_set_broadcast_network', 'Error: Select a mobile network'), )) def test_post_service_set_broadcast_account_type_shows_errors_if_no_radio_selected( platform_admin_client, From d38f44ec6961d435dddbe9ac6dcb3e7cf347a26d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 May 2021 11:33:09 +0100 Subject: [PATCH 6/7] =?UTF-8?q?Be=20explicit=20that=20=E2=80=98test?= =?UTF-8?q?=E2=80=99=20goes=20out=20on=20real=20networks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding ‘all networks’ whenever we mention the using the test channel without a restriction to a single network should help reinforce that this sends real alerts. --- app/templates/views/service-settings.html | 2 +- ...ervice-confirm-broadcast-account-type.html | 3 ++ app/templates/withnav_template.html | 4 ++- tests/app/main/views/test_broadcast.py | 29 ++++++++++++------- tests/app/main/views/test_service_settings.py | 4 +-- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index ac2560348..567003446 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -436,7 +436,7 @@ {% else %} {% if current_service.live and current_service.broadcast_channel == "government" %}Government{% endif %} {% if current_service.live and current_service.broadcast_channel == "severe" %}Live{% endif %} - {% if current_service.live and current_service.broadcast_channel == "test" %}Test {% if current_service.allowed_broadcast_provider != "all" %}({{ current_service.allowed_broadcast_provider|format_mobile_network }}){% else %}(All networks){% endif %}{%endif%} + {% if current_service.live and current_service.broadcast_channel == "test" %}Test {% if current_service.allowed_broadcast_provider != "all" %}({{ current_service.allowed_broadcast_provider|format_mobile_network }}){% else %}(all networks){% endif %}{%endif%} {% if not current_service.live%}Training {% endif%} {% endif %} {% endcall %} diff --git a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html index 98fb1a52c..e36045331 100644 --- a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html +++ b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html @@ -33,6 +33,9 @@ {% if form.account_type.provider_restriction != 'all' %} ({{ form.account_type.provider_restriction|format_mobile_network }}) {% endif %} + {% if form.account_type.broadcast_channel == 'test' and form.account_type.provider_restriction == 'all'%} + (all networks) + {% endif %}

diff --git a/app/templates/withnav_template.html b/app/templates/withnav_template.html index 34f7fefae..9e142d5f7 100644 --- a/app/templates/withnav_template.html +++ b/app/templates/withnav_template.html @@ -26,7 +26,9 @@ Live {% elif current_service.broadcast_channel == 'test' %} {{ current_service.broadcast_channel|title }} - {% if current_service.allowed_broadcast_provider != "all" %} + {% if current_service.allowed_broadcast_provider == "all" %} + (all networks) + {% else %} ({{ current_service.allowed_broadcast_provider }}) {% endif %} {% else %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 4b4c3af9f..63ad7f956 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -291,8 +291,16 @@ def test_broadcast_tour_page_4_shows_service_name( ( ( True, - "all", None, + 'all', + '.navigation-service-type.navigation-service-type--training', + 'service one Training Switch service', + 'Training', + ), + ( + True, + 'test', + 'all', '.navigation-service-type.navigation-service-type--training', 'service one Training Switch service', 'Training', @@ -300,12 +308,19 @@ def test_broadcast_tour_page_4_shows_service_name( ( False, 'severe', - "all", + 'all', '.navigation-service-type.navigation-service-type--live', 'service one Live Switch service', 'Live', ), - + ( + False, + 'test', + 'all', + '.navigation-service-type.navigation-service-type--live', + 'service one Test (all networks) Switch service', + 'Test (all networks)', + ), ( False, 'test', @@ -314,14 +329,6 @@ def test_broadcast_tour_page_4_shows_service_name( 'service one Test (vodafone) Switch service', 'Test (vodafone)', ), - ( - False, - 'test', - 'all', - '.navigation-service-type.navigation-service-type--live', - 'service one Test Switch service', - 'Test', - ), ( False, 'government', diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index d4297c74d..6d2f3f2de 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -203,7 +203,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( (True, "training", "test", "all", "Training"), (True, "live", "test", "ee", "Test (EE)"), (True, "live", "test", "three", "Test (Three)"), - (True, "live", "test", "all", "Test (All networks)"), + (True, "live", "test", "all", "Test (all networks)"), (True, "live", "severe", "all", "Live"), ] ) @@ -5777,7 +5777,7 @@ def test_post_service_set_broadcast_network_makes_you_choose( 'this service.', ]), ('live-test', [ - 'Test', + 'Test (all networks)', 'Members of the public who have switched on the test ' 'channel on their phones will receive alerts sent from ' 'this service.', From ad0b7537def1c60d22d6c5a06eee31d6bea47040 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 May 2021 11:50:30 +0100 Subject: [PATCH 7/7] Make the government channel visually distinct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s really serious, so this sets it apart from the other live channels. --- .../stylesheets/components/navigation.scss | 6 +++++ ...ervice-confirm-broadcast-account-type.html | 27 ++++++++++--------- app/templates/withnav_template.html | 4 +-- tests/app/main/views/test_broadcast.py | 2 +- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/app/assets/stylesheets/components/navigation.scss b/app/assets/stylesheets/components/navigation.scss index e98bad1dc..1bcf79d19 100644 --- a/app/assets/stylesheets/components/navigation.scss +++ b/app/assets/stylesheets/components/navigation.scss @@ -47,6 +47,12 @@ box-shadow: 0 -3px 0 0 #F6D7D2; } + &--government { + background: #942514; + color: #F6D7D2; + box-shadow: 0 -3px 0 0 #942514; + } + } &-service-switch, diff --git a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html index e36045331..229f55856 100644 --- a/app/templates/views/service-settings/service-confirm-broadcast-account-type.html +++ b/app/templates/views/service-settings/service-confirm-broadcast-account-type.html @@ -24,19 +24,20 @@

{% else %}

- - {% if form.account_type.broadcast_channel == 'severe' %} - Live - {% else %} - {{ form.account_type.broadcast_channel|title }} - {% endif %} - {% if form.account_type.provider_restriction != 'all' %} - ({{ form.account_type.provider_restriction|format_mobile_network }}) - {% endif %} - {% if form.account_type.broadcast_channel == 'test' and form.account_type.provider_restriction == 'all'%} - (all networks) - {% endif %} - + {% if form.account_type.broadcast_channel == 'severe' %} + Live + {% elif form.account_type.broadcast_channel == 'government' %} + Government + {% else %} + {{ form.account_type.broadcast_channel|title }} + {% endif %} + {% if form.account_type.provider_restriction != 'all' %} + ({{ form.account_type.provider_restriction|format_mobile_network }}) + {% endif %} + {% if form.account_type.broadcast_channel == 'test' and form.account_type.provider_restriction == 'all'%} + (all networks) + {% endif %} +

Members of the public diff --git a/app/templates/withnav_template.html b/app/templates/withnav_template.html index 9e142d5f7..f13c62b43 100644 --- a/app/templates/withnav_template.html +++ b/app/templates/withnav_template.html @@ -31,8 +31,8 @@ {% else %} ({{ current_service.allowed_broadcast_provider }}) {% endif %} - {% else %} - {{ current_service.broadcast_channel|title }} + {% elif current_service.broadcast_channel == 'government' %} + Government {% endif %} {% endif %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 63ad7f956..804505afb 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -333,7 +333,7 @@ def test_broadcast_tour_page_4_shows_service_name( False, 'government', 'all', - '.navigation-service-type.navigation-service-type--live', + '.navigation-service-type.navigation-service-type--government', 'service one Government Switch service', 'Government', ),