Merge pull request #1610 from alphagov/allow-updates-to-sms-prefixing

Allow updates to SMS prefixing setting

app/main/forms.py

@@ -721,6 +721,16 @@ class InternationalSMSForm(Form):
     )
 
 
+class SMSPrefixForm(Form):
+    enabled = RadioField(
+        '',
+        choices=[
+            ('on', 'On'),
+            ('off', 'Off'),
+        ],
+    )
+
+
 def get_placeholder_form_instance(
     placeholder_name,
     dict_to_populate_from,

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,30 @@ def service_set_sms(service_id):
     )
 
 
+@main.route("/services/<service_id>/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'
+    ))
+
+    form.enabled.label.text = 'Start all text messages with ‘{}:’'.format(current_service['name'])
+
+    if form.validate_on_submit():
+        service_api_client.update_service(
+            current_service['id'],
+            prefix_sms=(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_id>/service-settings/set-international-sms", methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)

app/notify_client/service_api_client.py

@@ -107,6 +107,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             'permissions',
             'organisation_type',
             'free_sms_fragment_limit',
+            'prefix_sms',
         }
         if disallowed_attributes:
             raise TypeError('Not allowed to update service attributes: {}'.format(

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('Text messages start 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) }}

app/templates/views/service-settings/sms-prefix.html

@@ -0,0 +1,21 @@
+{% extends "withnav_template.html" %}
+{% from "components/radios.html" import radios %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block service_page_title %}
+  Text messages start with service name
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">Text messages start with service name</h1>
+  <form method="post">
+    {{ 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'
+    ) }}
+  </form>
+
+{% endblock %}

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_,

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',
+        'Text messages start 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',
+        'Text messages start 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',
+        'Text messages start 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',
+        'Text messages start 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('legend').text) == (
+        'Start all text messages with ‘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=expected_api_argument,
+    )