From 6d3855bba440fcac5d2530d280a338d2306cd419 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 3 Nov 2017 15:01:41 +0000 Subject: [PATCH] Allow updates to SMS prefixing setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’re extracting this from being determined based on what the sender name is to its own setting. This commit will let users set it independently. Until the explicitly set it, it will still be determined based on whether their default sender name matches the default for the platform. --- app/main/forms.py | 10 ++++ app/main/views/service_settings.py | 26 +++++++++- app/notify_client/service_api_client.py | 1 + app/templates/views/service-settings.html | 6 +++ .../views/service-settings/sms-prefix.html | 24 +++++++++ tests/__init__.py | 4 +- tests/app/main/views/test_service_settings.py | 49 ++++++++++++++++++- 7 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 app/templates/views/service-settings/sms-prefix.html diff --git a/app/main/forms.py b/app/main/forms.py index 242be6879..e80fa0023 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -721,6 +721,16 @@ class InternationalSMSForm(Form): ) +class SMSPrefixForm(Form): + enabled = RadioField( + 'Start all text messages with service name', + choices=[ + ('on', 'On'), + ('off', 'Off'), + ], + ) + + def get_placeholder_form_instance( placeholder_name, dict_to_populate_from, diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 9446b69ff..ecadb06cb 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -38,6 +38,7 @@ from app.main.forms import ( OrganisationTypeForm, FreeSMSAllowance, ServiceEditInboundNumberForm, + SMSPrefixForm, ) from app import user_api_client, current_service, organisations_client, inbound_number_client from notifications_utils.formatters import formatted_list @@ -103,7 +104,8 @@ def service_settings(service_id): default_letter_contact_block=default_letter_contact_block, letter_contact_details_count=letter_contact_details_count, default_sms_sender=default_sms_sender, - sms_sender_count=sms_sender_count + sms_sender_count=sms_sender_count, + prefix_sms_with_service_name=current_service['prefix_sms_with_service_name'], ) @@ -486,6 +488,28 @@ def service_set_sms(service_id): ) +@main.route("/services//service-settings/sms-prefix", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def service_set_sms_prefix(service_id): + + form = SMSPrefixForm(enabled=( + 'on' if current_service['prefix_sms_with_service_name'] else 'off' + )) + + if form.validate_on_submit(): + service_api_client.update_service( + current_service['id'], + prefix_sms_with_service_name=(form.enabled.data == 'on') + ) + return redirect(url_for('.service_settings', service_id=service_id)) + + return render_template( + 'views/service-settings/sms-prefix.html', + form=form + ) + + @main.route("/services//service-settings/set-international-sms", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index e374487d5..3667ecba2 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -107,6 +107,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): 'permissions', 'organisation_type', 'free_sms_fragment_limit', + 'prefix_sms_with_service_name', } if disallowed_attributes: raise TypeError('Not allowed to update service attributes: {}'.format( diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index b31854876..7d417e58e 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -92,6 +92,12 @@ {{ edit_field('Manage' if sms_sender_count else 'Change', url_for('.service_sms_senders', service_id=current_service.id)) }} {% endcall %} + {% call row() %} + {{ text_field('Start all text messages with service name') }} + {{ boolean_field(prefix_sms_with_service_name) }} + {{ edit_field('Change', url_for('.service_set_sms_prefix', service_id=current_service.id)) }} + {% endcall %} + {% call row() %} {{ text_field('International text messages') }} {{ boolean_field('international_sms' in current_service.permissions) }} diff --git a/app/templates/views/service-settings/sms-prefix.html b/app/templates/views/service-settings/sms-prefix.html new file mode 100644 index 000000000..5a99b86d6 --- /dev/null +++ b/app/templates/views/service-settings/sms-prefix.html @@ -0,0 +1,24 @@ +{% extends "withnav_template.html" %} +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} + +{% block service_page_title %} + Start all text messages with service name +{% endblock %} + +{% block maincolumn_content %} + +

Start all text messages with service name

+

+ Your service name is {{ current_service.name}}. +

+
+ {{ radios(form.enabled) }} + {{ page_footer( + button_text="Save", + back_link=url_for('.service_settings', service_id=current_service.id), + back_link_text='Back to settings' + ) }} +
+ +{% endblock %} diff --git a/tests/__init__.py b/tests/__init__.py index b467f6daa..f59633f55 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -56,7 +56,7 @@ def service_json( permissions=None, organisation_type='central', free_sms_fragment_limit=250000, - prefix_sms_with_service_name=None, + prefix_sms_with_service_name='Treat as None', ): if users is None: users = [] @@ -64,7 +64,7 @@ def service_json( permissions = ['email', 'sms'] if inbound_api is None: inbound_api = [] - if prefix_sms_with_service_name is None: + if prefix_sms_with_service_name == 'Treat as None': prefix_sms_with_service_name = (sms_sender == 'GOVUK') return { 'id': id_, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 27080aaee..1f09d38c4 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -44,6 +44,7 @@ from tests.conftest import ( 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Start all text messages with service name On Change', 'International text messages Off Change', 'Receive text messages Off Change', @@ -63,6 +64,7 @@ from tests.conftest import ( 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Start all text messages with service name On Change', 'International text messages Off Change', 'Receive text messages Off Change', @@ -119,6 +121,7 @@ def test_should_show_overview( 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Start all text messages with service name On Change', 'International text messages On Change', 'Receive text messages On Change', 'Callback URL for received text messages Not set Change', @@ -138,6 +141,7 @@ def test_should_show_overview( 'Label Value Action', 'Send text messages On Change', 'Text message sender GOVUK Manage', + 'Start all text messages with service name On Change', 'International text messages Off Change', 'Receive text messages Off Change', @@ -713,7 +717,7 @@ def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender( assert get_row(page, 2) == "Email reply to addresses test@example.com …and 2 more Manage" assert get_row(page, 4) == "Text message sender Example …and 2 more Manage" - assert get_row(page, 8) == "Sender addresses 1 Example Street …and 2 more Manage" + assert get_row(page, 9) == "Sender addresses 1 Example Street …and 2 more Manage" @pytest.mark.parametrize('sender_list_page, expected_output', [ @@ -2063,3 +2067,46 @@ def test_empty_letter_contact_block_returns_error( page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') error_message = page.find('span', class_='error-message').text.strip() assert error_message == 'Can’t be empty' + + +def test_show_sms_prefixing_setting_page( + client_request, + mock_update_service, +): + page = client_request.get( + 'main.service_set_sms_prefix', service_id=SERVICE_ONE_ID + ) + assert normalize_spaces(page.select_one('main p').text) == ( + 'Your service name is service one.' + ) + radios = page.select('input[type=radio]') + assert len(radios) == 2 + assert radios[0]['value'] == 'on' + assert radios[0]['checked'] == '' + assert radios[1]['value'] == 'off' + with pytest.raises(KeyError): + assert radios[1]['checked'] + + +@pytest.mark.parametrize('post_value, expected_api_argument', [ + ('on', True), + ('off', False), +]) +def test_updates_sms_prefixing( + client_request, + mock_update_service, + post_value, + expected_api_argument, +): + client_request.post( + 'main.service_set_sms_prefix', service_id=SERVICE_ONE_ID, + _data={'enabled': post_value}, + _expected_redirect=url_for( + 'main.service_settings', service_id=SERVICE_ONE_ID, + _external=True + ) + ) + mock_update_service.assert_called_once_with( + service_id=SERVICE_ONE_ID, + prefix_sms_with_service_name=expected_api_argument, + )