Check text message sender before going live

We often check that a service has an appropriate text message sender as
a condition of them going live. We don’t mention this anywhere.

The services for whom GOVUK is definitely not an appropriate sender are
those in local government. As we have more of these teams starting to
use Notify, we should streamline the process by making this check
automated.

This commit adds that check, for teams who:
- have text message templates
- have self-declared as NHS or local government

app/main/views/service_settings.py

@@ -53,6 +53,7 @@ from app.utils import (
     AgreementInfo,
     email_safe,
     get_cdn_domain,
+    get_default_sms_sender,
     user_has_permissions,
     user_is_platform_admin,
 )
@@ -83,10 +84,6 @@ def service_settings(service_id):
         (Field(x['contact_block'], html='escape') for x in letter_contact_details if x['is_default']), "Not set"
     )
     sms_senders = service_api_client.get_sms_senders(service_id)
-    sms_sender_count = len(sms_senders)
-    default_sms_sender = next(
-        (Field(x['sms_sender'], html='escape') for x in sms_senders if x['is_default']), "None"
-    )
 
     free_sms_fragment_limit = billing_api_client.get_free_sms_fragment_limit_for_year(service_id)
     data_retention = service_api_client.get_service_data_retention(service_id)
@@ -103,8 +100,8 @@ def service_settings(service_id):
         reply_to_email_address_count=reply_to_email_address_count,
         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,
+        default_sms_sender=get_default_sms_sender(sms_senders),
+        sms_sender_count=len(sms_senders),
         free_sms_fragment_limit=free_sms_fragment_limit,
         prefix_sms=current_service.prefix_sms,
         organisation=organisation,
@@ -192,9 +189,18 @@ def request_to_go_live(service_id):
         has_email_templates=(
             service_api_client.count_service_templates(service_id, template_type='email') > 0
         ),
+        has_sms_templates=(
+            service_api_client.count_service_templates(service_id, template_type='sms') > 0
+        ),
         has_email_reply_to_address=bool(
             service_api_client.get_reply_to_email_addresses(service_id)
-        )
+        ),
+        shouldnt_use_govuk_as_sms_sender=(
+            current_service.organisation_type in {'local', 'nhs'}
+        ),
+        sms_sender_is_govuk=get_default_sms_sender(
+            service_api_client.get_sms_senders(service_id)
+        ) in {'GOVUK', 'None'},
     )
 
 

app/templates/views/service-settings/request-to-go-live.html

@@ -38,6 +38,12 @@
             )|safe,
           ) }}
         {% endif %}
+        {% if has_sms_templates and shouldnt_use_govuk_as_sms_sender %}
+          {{ task_list_item(
+            not sms_sender_is_govuk,
+            'Change your text message sender from GOVUK'
+          ) }}
+        {% endif %}
       {% endcall %}
       <p>
         You also need to accept our <a href="{{ url_for('.terms') }}">terms of use</a>.

app/utils.py

@@ -25,6 +25,7 @@ from flask import (
     url_for,
 )
 from flask_login import current_user
+from notifications_utils.field import Field
 from notifications_utils.formatters import make_quotes_smart
 from notifications_utils.recipients import RecipientCSV
 from notifications_utils.take import Take
@@ -663,3 +664,10 @@ def should_skip_template_page(template_type):
         not current_user.has_permissions('manage_templates', 'manage_api_keys') and
         template_type != 'letter'
     )
+
+
+def get_default_sms_sender(sms_senders):
+    return str(next((
+        Field(x['sms_sender'], html='escape')
+        for x in sms_senders if x['is_default']
+    ), "None"))

tests/app/main/views/test_service_settings.py

@@ -517,9 +517,13 @@ def test_should_raise_duplicate_name_handled(
     (1, [], 'An email reply-to address Not completed'),
     (1, [{}], 'An email reply-to address Completed'),
 ])
+@pytest.mark.parametrize('expected_sms_sender_item', [
+    ('Templates showing the kind of messages you plan to send Not completed'),
+])
 def test_should_show_request_to_go_live_checklist(
     client_request,
     mocker,
+    single_sms_sender,
     count_of_users_with_manage_service,
     expected_user_checklist_item,
     count_of_templates,
@@ -527,11 +531,13 @@ def test_should_show_request_to_go_live_checklist(
     count_of_email_templates,
     reply_to_email_addresses,
     expected_reply_to_checklist_item,
+    expected_sms_sender_item,
 ):
 
     def _count_templates(service_id, template_type=None):
         return {
-            'email': count_of_email_templates
+            'email': count_of_email_templates,
+            'sms': 0,
         }.get(template_type, count_of_templates)
 
     mock_count_users = mocker.patch(
@@ -567,12 +573,118 @@ def test_should_show_request_to_go_live_checklist(
     assert mock_count_templates.call_args_list == [
         call(SERVICE_ONE_ID),
         call(SERVICE_ONE_ID, template_type='email'),
+        call(SERVICE_ONE_ID, template_type='sms'),
     ]
 
     if count_of_email_templates:
         mock_get_reply_to_email_addresses.assert_called_once_with(SERVICE_ONE_ID)
 
 
+@pytest.mark.parametrize('organisation_type,count_of_sms_templates, sms_senders, expected_sms_sender_checklist_item', [
+    pytest.mark.xfail((
+        'local',
+        0,
+        [],
+        '',
+    ), raises=IndexError),
+    pytest.mark.xfail((
+        'local',
+        0,
+        [{'is_default': True, 'sms_sender': 'GOVUK'}],
+        '',
+    ), raises=IndexError),
+    pytest.mark.xfail((
+        None,
+        99,
+        [{'is_default': True, 'sms_sender': 'GOVUK'}],
+        '',
+    ), raises=IndexError),
+    pytest.mark.xfail((
+        'central',
+        99,
+        [{'is_default': True, 'sms_sender': 'GOVUK'}],
+        '',
+    ), raises=IndexError),
+    (
+        'local',
+        1,
+        [],
+        'Change your text message sender from GOVUK Not completed',
+    ),
+    (
+        'local',
+        1,
+        [{'is_default': True, 'sms_sender': 'GOVUK'}],
+        'Change your text message sender from GOVUK Not completed',
+    ),
+    (
+        'local',
+        1,
+        [
+            {'is_default': False, 'sms_sender': 'GOVUK'},
+            {'is_default': True, 'sms_sender': 'KUVOG'},
+        ],
+        'Change your text message sender from GOVUK Completed',
+    ),
+    (
+        'nhs',
+        1,
+        [{'is_default': True, 'sms_sender': 'KUVOG'}],
+        'Change your text message sender from GOVUK Completed',
+    ),
+])
+def test_should_check_for_sms_sender_on_go_live(
+    client_request,
+    service_one,
+    mocker,
+    organisation_type,
+    count_of_sms_templates,
+    sms_senders,
+    expected_sms_sender_checklist_item,
+):
+
+    service_one['organisation_type'] = organisation_type
+
+    def _count_templates(service_id, template_type=None):
+        return {
+            'email': 0,
+            'sms': count_of_sms_templates,
+        }.get(template_type, count_of_sms_templates)
+
+    mocker.patch(
+        'app.main.views.service_settings.user_api_client.get_count_of_users_with_permission',
+        return_value=99,
+    )
+    mock_count_templates = mocker.patch(
+        'app.main.views.service_settings.service_api_client.count_service_templates',
+        side_effect=_count_templates,
+    )
+    mock_get_sms_senders = mocker.patch(
+        'app.main.views.service_settings.service_api_client.get_sms_senders',
+        return_value=sms_senders,
+    )
+    mocker.patch(
+        'app.main.views.service_settings.service_api_client.get_reply_to_email_addresses',
+        return_value=[],
+    )
+
+    page = client_request.get(
+        'main.request_to_go_live', service_id=SERVICE_ONE_ID
+    )
+    assert page.h1.text == 'Request to go live'
+
+    checklist_items = page.select('.task-list .task-list-item')
+    assert normalize_spaces(checklist_items[2].text) == expected_sms_sender_checklist_item
+
+    assert mock_count_templates.call_args_list == [
+        call(SERVICE_ONE_ID),
+        call(SERVICE_ONE_ID, template_type='email'),
+        call(SERVICE_ONE_ID, template_type='sms'),
+    ]
+
+    mock_get_sms_senders.assert_called_once_with(SERVICE_ONE_ID)
+
+
 def test_should_show_request_to_go_live(
     client_request,
 ):