From f8f3d44511067b2ba20a52b83a42a178df2e0bd6 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 15 Feb 2021 21:02:37 +0000 Subject: [PATCH] Add form to set service broadcast account type Note, no option at the moment to set the service broadcast account type as None, or back to without the broadcast permission. This has been done for speed of development given the chance of us needing this is very low. We can add it later if we need to. --- app/__init__.py | 2 + app/formatters.py | 6 + app/main/forms.py | 48 ++++ app/main/views/service_settings.py | 28 +++ app/models/service.py | 1 + app/navigation.py | 4 + app/notify_client/service_api_client.py | 17 ++ app/templates/views/service-settings.html | 17 ++ .../service-set-broadcast-account-type.html | 32 +++ tests/__init__.py | 6 +- tests/app/main/views/test_service_settings.py | 232 ++++++++++++++++++ .../notify_client/test_service_api_client.py | 1 + 12 files changed, 393 insertions(+), 1 deletion(-) create mode 100644 app/templates/views/service-settings/service-set-broadcast-account-type.html diff --git a/app/__init__.py b/app/__init__.py index 1e9ce53e5..4e6e8f21f 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -53,6 +53,7 @@ from app.formatters import ( format_delta, format_delta_days, format_list_items, + format_mobile_network, format_notification_status, format_notification_status_as_field_status, format_notification_status_as_time, @@ -560,6 +561,7 @@ def add_template_filters(application): message_count_label, message_count, message_count_noun, + format_mobile_network, ]: application.add_template_filter(fn) diff --git a/app/formatters.py b/app/formatters.py index 28470c1ec..2f9223a44 100644 --- a/app/formatters.py +++ b/app/formatters.py @@ -509,3 +509,9 @@ def character_count(count): if count == 1: return '1 character' return f'{format_thousands(count)} characters' + + +def format_mobile_network(network): + if network in ('three', 'vodafone', 'o2'): + return network.capitalize() + return 'EE' diff --git a/app/main/forms.py b/app/main/forms.py index 74f956125..b49715f4f 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2295,6 +2295,54 @@ class GoLiveNotesForm(StripWhitespaceForm): ) +class ServiceBroadcastAccountTypeField(GovukRadiosField): + # When receiving Python data, eg when instantiating the form object + # we want to convert it from a tuple of + # (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): + (live, broadcast_channel, allowed_broadcast_provider) = value + account_type = None + if broadcast_channel: + account_type = "live" if live else "training" + account_type += f"-{broadcast_channel}" + if allowed_broadcast_provider: + account_type += f"-{allowed_broadcast_provider}" + + self.data = account_type + + # After validation we split the value back into its parts of service_mode + # 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: + 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 None + + +class ServiceBroadcastAccountTypeForm(StripWhitespaceForm): + account_type = ServiceBroadcastAccountTypeField( + 'Change cell broadcast service type', + thing='which type of account this cell broadcast service is', + choices=[ + ("training-test-ee", "Training mode - EE network - Test channel only"), + ("training-test-o2", "Training mode - O2 network - Test channel only"), + ("training-test-three", "Training mode - Three network - Test channel only"), + ("training-test-vodafone", "Training mode - Vodafone network - Test channel only"), + ("training-severe", "Training mode - All networks - Public channel"), + ("live-test-ee", "Live - EE network - Test channel only"), + ("live-test-o2", "Live - O2 network - Test channel only"), + ("live-test-three", "Live - Three network - Test channel only"), + ("live-test-vodafone", "Live - Vodafone network - Test channel only"), + ("live-test", "Live - All networks - Test channel only"), + ("live-severe", "Live - All networks - Public channel"), + ], + validators=[DataRequired()] + ) + + class AcceptAgreementForm(StripWhitespaceForm): @classmethod diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 60307f847..4544d619f 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -44,6 +44,7 @@ from app.main.forms import ( RateLimit, RenameServiceForm, SearchByNameForm, + ServiceBroadcastAccountTypeForm, ServiceContactDetailsForm, ServiceDataRetentionEditForm, ServiceDataRetentionForm, @@ -314,6 +315,33 @@ 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=( + current_service.live, + current_service.broadcast_channel, + current_service.allowed_broadcast_provider + ) + ) + + if form.validate_on_submit(): + service_api_client.set_service_broadcast_settings( + current_service.id, + service_mode=form.account_type.service_mode, + 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', + form=form, + ) + + @main.route("/services//service-settings/archive", methods=['GET', 'POST']) @user_has_permissions('manage_service') def archive_service(service_id): diff --git a/app/models/service.py b/app/models/service.py index 4448dcd1c..c37b19aec 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -34,6 +34,7 @@ class Service(JSONModel): 'billing_contact_email_addresses', 'billing_contact_names', 'billing_reference', + 'broadcast_channel', 'consent_to_research', 'contact_link', 'count_as_live', diff --git a/app/navigation.py b/app/navigation.py index 4985bd680..58027c8c8 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -302,6 +302,7 @@ class HeaderNavigation(Navigation): 'service_set_auth_type', 'service_set_channel', 'send_files_by_email_contact_details', + 'service_set_broadcast_account_type', 'service_set_email_branding', 'service_set_inbound_number', 'service_set_inbound_sms', @@ -504,6 +505,7 @@ class MainNavigation(Navigation): 'service_set_auth_type', 'service_set_channel', 'send_files_by_email_contact_details', + 'service_set_broadcast_account_type', 'service_set_email_branding', 'service_set_inbound_number', 'service_set_inbound_sms', @@ -976,6 +978,7 @@ class CaseworkNavigation(Navigation): 'service_set_auth_type', 'service_set_channel', 'send_files_by_email_contact_details', + 'service_set_broadcast_account_type', 'service_set_email_branding', 'service_set_inbound_number', 'service_set_inbound_sms', @@ -1297,6 +1300,7 @@ class OrgNavigation(Navigation): 'service_set_auth_type', 'service_set_channel', 'send_files_by_email_contact_details', + 'service_set_broadcast_account_type', 'service_set_email_branding', 'service_set_inbound_number', 'service_set_inbound_sms', diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index eab9e595c..034e940f3 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -616,5 +616,22 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_returned_letters(self, service_id, reported_at): return self.get("service/{}/returned-letters?reported_at={}".format(service_id, reported_at)) + @cache.delete('service-{service_id}') + def set_service_broadcast_settings( + self, service_id, service_mode, broadcast_channel, provider_restriction + ): + """ + service_mode is one of "training" or "live" + broadcast channel is one of "test" or "severe" + provider_restriction is one of None, "three", "o2", "vodafone", "ee" + """ + data = { + "service_mode": service_mode, + "broadcast_channel": broadcast_channel, + "provider_restriction": provider_restriction + } + + return self.post("/service/{}/set-as-broadcast-service".format(service_id), data) + service_api_client = ServiceAPIClient() diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 564ea3e28..b41491dde 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -428,6 +428,23 @@ {% endif %} {% endfor %} + {% call row() %} + {{ text_field('Send cell broadcasts')}} + {% call field(wrap=True) %} + {% if not current_service.broadcast_channel %} + Off + {% else %} + {% if current_service.live %}Live{% else %}Training mode{% endif %} - {% if current_service.allowed_broadcast_provider %}{{ current_service.allowed_broadcast_provider|format_mobile_network }} network{% else %}All networks{% endif %} - {% if current_service.broadcast_channel == "test" %}Test channel only{% else %}Public channel{% endif %} + {% endif %} + {% endcall %} + {{ edit_field( + 'Change', + url_for('.service_set_broadcast_account_type', service_id=current_service.id), + suffix='your settings for Send cell broadcasts' + ) + }} + {% endcall %} + {% endcall %} 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 new file mode 100644 index 000000000..83b739d16 --- /dev/null +++ b/app/templates/views/service-settings/service-set-broadcast-account-type.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 %} + Send cell broadcasts +{% endblock %} + +{% block maincolumn_content %} + +
+
+ {{ govukBackLink({ + "text": "Back", + "href": url_for('.service_settings', service_id=current_service.id) + }) }} + {% call form_wrapper() %} + {{ form.account_type(param_extensions={ + 'fieldset': { + 'legend': { + 'isPageHeading': True, + 'classes': 'govuk-fieldset__legend--l' + } + } + }) }} + {{ page_footer('Save') }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/tests/__init__.py b/tests/__init__.py index e39b22797..cd3143915 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -154,7 +154,9 @@ def service_json( billing_contact_email_addresses=None, billing_contact_names=None, billing_reference=None, - purchase_order_number=None + purchase_order_number=None, + broadcast_channel=None, + allowed_broadcast_provider=None, ): if users is None: users = [] @@ -198,6 +200,8 @@ def service_json( 'billing_contact_names': billing_contact_names, 'billing_reference': billing_reference, 'purchase_order_number': purchase_order_number, + 'broadcast_channel': broadcast_channel, + 'allowed_broadcast_provider': allowed_broadcast_provider, } diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index c596f6949..f33ca5c34 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,3 +1,4 @@ +import re from datetime import datetime from functools import partial from unittest.mock import ANY, Mock, PropertyMock, call @@ -165,6 +166,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( permissions=['broadcast'], organisation_id=ORGANISATION_ID, contact_link='contact_us@gov.uk', + broadcast_channel="severe", ) mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) @@ -185,6 +187,8 @@ 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 mode - All networks - Public channel ' + + 'Change your settings for Send cell broadcasts', ] assert len(rows) == len(expected_rows) @@ -193,6 +197,55 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service( app.service_api_client.get_service.assert_called_with(SERVICE_ONE_ID) +@pytest.mark.parametrize( + 'has_broadcast_permission,service_mode,broadcast_channel,allowed_broadcast_provider,expected_text', + [ + (False, "training", None, None, "Off"), + (False, "live", None, None, "Off"), + (True, "training", "severe", None, "Training mode - All networks - Public channel"), + (True, "training", "test", "ee", "Training mode - EE network - Test channel only"), + (True, "live", "test", "three", "Live - Three network - Test channel only"), + (True, "live", "test", None, "Live - All networks - Test channel only"), + (True, "live", "severe", None, "Live - All networks - Public channel"), + ] +) +def test_platform_admin_sees_correct_description_of_broadcast_service_setting( + client, + mocker, + api_user_active, + no_reply_to_email_addresses, + no_letter_contact_blocks, + mock_get_organisation, + single_sms_sender, + mock_get_service_settings_page_common, + has_broadcast_permission, + service_mode, + broadcast_channel, + allowed_broadcast_provider, + expected_text +): + service_one = service_json( + SERVICE_ONE_ID, + users=[api_user_active['id']], + permissions=['broadcast'] if has_broadcast_permission else ['email'], + organisation_id=ORGANISATION_ID, + restricted=True if service_mode == "training" else False, + broadcast_channel=broadcast_channel, + allowed_broadcast_provider=allowed_broadcast_provider, + ) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) + + client.login(create_platform_admin_user(), mocker, service_one) + response = client.get(url_for( + 'main.service_settings', service_id=SERVICE_ONE_ID + )) + 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_description = broadcast_setting_row.select('td')[1].text.strip() + assert broadcast_setting_description == expected_text + + def test_no_go_live_link_for_service_without_organisation( client_request, mocker, @@ -5402,3 +5455,182 @@ def test_update_service_billing_details( purchase_order_number='PO1234', notes='very fluffy, give extra allowance' ) + + +def test_get_service_set_broadcast_account_type( + platform_admin_client, +): + response = platform_admin_client.get( + url_for( + 'main.service_set_broadcast_account_type', + 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" + + expected_labels = [ + "Training mode - EE network - Test channel only", + "Training mode - O2 network - Test channel only", + "Training mode - Three network - Test channel only", + "Training mode - Vodafone network - Test channel only", + "Training mode - All networks - Public channel", + "Live - EE network - Test channel only", + "Live - O2 network - Test channel only", + "Live - Three network - Test channel only", + "Live - Vodafone network - Test channel only", + "Live - All networks - Test channel only", + "Live - All networks - Public channel", + ] + labels = page.find_all('label', class_="govuk-radios__label") + assert len(labels) == len(expected_labels) + for label in labels: + assert label.text.strip() in expected_labels + + assert page.select_one('.govuk-back-link')['href'] == url_for( + 'main.service_settings', service_id=SERVICE_ONE_ID, + ) + + +def test_get_service_set_broadcast_account_type_has_no_radio_selected_for_non_broadcast_service( + platform_admin_client +): + response = platform_admin_client.get( + url_for( + 'main.service_set_broadcast_account_type', + service_id=SERVICE_ONE_ID, + ) + ) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert len(page.select('input[checked]')) == 0 + + +@pytest.mark.parametrize( + 'service_mode,broadcast_channel,allowed_broadcast_provider,expected_text,expected_value', + [ + ( + "training", + "severe", + None, + "Training mode - All networks - Public channel", + "training-severe", + ), + ( + "training", + "test", + "vodafone", + "Training mode - Vodafone network - Test channel only", + "training-test-vodafone", + ), + ( + "live", + "test", + "o2", + "Live - O2 network - Test channel only", + "live-test-o2", + ), + ( + "live", + "test", + None, + "Live - All networks - Test channel only", + "live-test", + ), + ( + "live", + "severe", + None, + "Live - All networks - Public channel", + "live-severe", + ), + ] +) +def test_get_service_set_broadcast_account_type_has_radio_selected_for_broadcast_service( + platform_admin_client, + mocker, + service_mode, + broadcast_channel, + allowed_broadcast_provider, + expected_text, + expected_value +): + service_one = service_json( + SERVICE_ONE_ID, + permissions=['broadcast'], + restricted=True if service_mode == "training" else False, + broadcast_channel=broadcast_channel, + allowed_broadcast_provider=allowed_broadcast_provider, + ) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) + + response = platform_admin_client.get( + url_for( + 'main.service_set_broadcast_account_type', + service_id=SERVICE_ONE_ID, + ) + ) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + 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 selected_label.text.strip() == expected_text + + +@pytest.mark.parametrize( + 'value,service_mode,broadcast_channel,allowed_broadcast_provider', + [ + ("training-severe", "training", "severe", None), + ("training-test-vodafone", "training", "test", "vodafone"), + ("live-test-o2", "live", "test", "o2"), + ("live-test", "live", "test", None), + ("live-severe", "live", "severe", None), + ] +) +def test_post_service_set_broadcast_account_type_posts_data_to_api_and_redirects( + platform_admin_client, + mocker, + value, + service_mode, + broadcast_channel, + allowed_broadcast_provider, +): + set_service_broadcast_settings_mock = mocker.patch('app.service_api_client.set_service_broadcast_settings') + + response = platform_admin_client.post( + url_for( + 'main.service_set_broadcast_account_type', + service_id=SERVICE_ONE_ID, + ), + data={ + 'account_type': value + } + ) + assert response.status_code == 302 + assert response.location == url_for('main.service_settings', service_id=SERVICE_ONE_ID, _external=True) + set_service_broadcast_settings_mock.assert_called_once_with( + SERVICE_ONE_ID, + service_mode=service_mode, + broadcast_channel=broadcast_channel, + provider_restriction=allowed_broadcast_provider, + ) + + +def test_post_service_set_broadcast_account_type_shows_errors_if_no_radio_selected( + platform_admin_client +): + response = platform_admin_client.post( + url_for( + 'main.service_set_broadcast_account_type', + 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 diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 252612202..674ac612e 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -410,6 +410,7 @@ def test_returns_value_from_cache( (service_api_client, 'delete_sms_sender', [SERVICE_ONE_ID, ''], {}), (service_api_client, 'update_service_callback_api', [SERVICE_ONE_ID] + [''] * 4, {}), (service_api_client, 'create_service_callback_api', [SERVICE_ONE_ID] + [''] * 3, {}), + (service_api_client, 'set_service_broadcast_settings', [SERVICE_ONE_ID, 'training', 'severe', None], {}), (user_api_client, 'add_user_to_service', [SERVICE_ONE_ID, uuid4(), [], []], {}), (invite_api_client, 'accept_invite', [SERVICE_ONE_ID, uuid4()], {}), ])