diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index e7c87f6a7..ef734e901 100644 --- a/app/main/views/service_settings.py +++ b/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'}, ) diff --git a/app/templates/views/service-settings/request-to-go-live.html b/app/templates/views/service-settings/request-to-go-live.html index 4858714ca..fbd228fea 100644 --- a/app/templates/views/service-settings/request-to-go-live.html +++ b/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 %}

You also need to accept our terms of use. diff --git a/app/utils.py b/app/utils.py index ac3251bf4..0590dd585 100644 --- a/app/utils.py +++ b/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")) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index c26ab1194..c7118e540 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/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, ):