diff --git a/app/assets/stylesheets/components/task-list.scss b/app/assets/stylesheets/components/task-list.scss new file mode 100644 index 0000000000..35f42eb159 --- /dev/null +++ b/app/assets/stylesheets/components/task-list.scss @@ -0,0 +1,41 @@ +$indicator-colour: $black; + +%task-list-indicator { + @include bold-16; + display: inline-block; + padding: 3px 8px 1px 8px; + position: absolute; + right: 0; + top: 50%; + margin-top: -15px; + border: 2px solid $indicator-colour; +} + +.task-list { + + border-bottom: 1px solid $border-colour; + margin: $gutter 0; + + &-item { + border-top: 1px solid $border-colour; + padding: 15px 0; + padding-right: 20%; + position: relative; + } + + &-indicator-completed { + @extend %task-list-indicator; + background-color: $indicator-colour; + color: $grey-4; + // Just a pinch of letter spacing to make reversed-out text a bit + // easier to read + letter-spacing: 0.02em; + } + + &-indicator-not-completed { + @extend %task-list-indicator; + background-color: transparent; + color: $indicator-colour; + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 9f5955cae8..74e3784d55 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -62,6 +62,7 @@ $path: '/static/images/'; @import 'components/vendor/breadcrumbs'; @import 'components/vendor/responsive-embed'; @import 'components/email-preview-pane'; +@import 'components/task-list'; @import 'views/dashboard'; @import 'views/users'; diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index e7c87f6a7d..ef734e901c 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -53,6 +53,7 @@ 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/components/task-list.html b/app/templates/components/task-list.html new file mode 100644 index 0000000000..38585ee541 --- /dev/null +++ b/app/templates/components/task-list.html @@ -0,0 +1,16 @@ +{% macro task_list_item(completed, label) %} +
  • + {{ label }} + {% if completed %} + Completed + {% else %} + Not completed + {% endif %} +
  • +{% endmacro %} + +{% macro task_list_wrapper() %} + +{% endmacro %} diff --git a/app/templates/components/tick-cross.html b/app/templates/components/tick-cross.html index f6fbc9ecf0..d8c0c297d6 100644 --- a/app/templates/components/tick-cross.html +++ b/app/templates/components/tick-cross.html @@ -14,7 +14,6 @@ {% endmacro %} - {% macro tick_cross_done_not_done(yes, label) %} {{ tick_cross(yes, label, truthy_hint='Done: ', falsey_hint='Not done: ') }} {% endmacro %} 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 2833dac489..f80287c1a1 100644 --- a/app/templates/views/service-settings/request-to-go-live.html +++ b/app/templates/views/service-settings/request-to-go-live.html @@ -4,47 +4,51 @@ {% from "components/radios.html" import radios %} {% from "components/page-footer.html" import page_footer %} {% from "components/banner.html" import banner_wrapper %} -{% from "components/tick-cross.html" import tick_cross_done_not_done %} +{% from "components/task-list.html" import task_list_wrapper, task_list_item %} {% block service_page_title %} - Request to go live + Before you request to go live {% endblock %} {% block maincolumn_content %}
    -

    Request to go live

    -

    - Before your service can go live on Notify, you’ll need to: -

    - + {% if has_sms_templates and shouldnt_use_govuk_as_sms_sender %} + {{ task_list_item( + not sms_sender_is_govuk, + 'Change your text message sender name'.format( + url_for('main.service_sms_senders', service_id=current_service.id) + )|safe + ) }} + {% endif %} + {% endcall %}

    You also need to accept our terms of use.

    -

    - We’ll make your service live within one working day. -

    Continue

    diff --git a/app/templates/views/service-settings/submit-request-to-go-live.html b/app/templates/views/service-settings/submit-request-to-go-live.html index 517afd2780..c6a352d114 100644 --- a/app/templates/views/service-settings/submit-request-to-go-live.html +++ b/app/templates/views/service-settings/submit-request-to-go-live.html @@ -6,12 +6,16 @@ {% from "components/banner.html" import banner_wrapper %} {% block service_page_title %} - How do you plan to use Notify? + Request to go live {% endblock %} {% block maincolumn_content %} -

    How do you plan to use Notify?

    +

    Request to go live

    + +

    + Tell us how you plan to use Notify. When we receive your request we’ll make your service live within one working day. +

    {{ checkbox_group('What kind of messages will you be sending?', [ diff --git a/app/utils.py b/app/utils.py index ac3251bf43..0590dd5851 100644 --- a/app/utils.py +++ b/app/utils.py @@ -25,6 +25,7 @@ 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 b87f834bd6..c1fa32ff83 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -503,23 +503,24 @@ def test_should_raise_duplicate_name_handled( @pytest.mark.parametrize('count_of_users_with_manage_service, expected_user_checklist_item', [ - (1, 'Not done: Have more than one team member with the ‘Manage service’ permission'), - (2, 'Done: Have more than one team member with the ‘Manage service’ permission'), + (1, 'Add a team member who can manage settings, team and usage Not completed'), + (2, 'Add a team member who can manage settings, team and usage Completed'), ]) @pytest.mark.parametrize('count_of_templates, expected_templates_checklist_item', [ - (0, 'Not done: Create templates showing the kind of messages you plan to send'), - (1, 'Done: Create templates showing the kind of messages you plan to send'), - (2, 'Done: Create templates showing the kind of messages you plan to send'), + (0, 'Add content to templates to show the kind of messages you’ll send Not completed'), + (1, 'Add content to templates to show the kind of messages you’ll send Completed'), + (2, 'Add content to templates to show the kind of messages you’ll send Completed'), ]) @pytest.mark.parametrize('count_of_email_templates, reply_to_email_addresses, expected_reply_to_checklist_item', [ pytest.mark.xfail((0, [], ''), raises=IndexError), pytest.mark.xfail((0, [{}], ''), raises=IndexError), - (1, [], 'Not done: Add an email reply-to address'), - (1, [{}], 'Done: Add an email reply-to address'), + (1, [], 'Add an email reply-to address Not completed'), + (1, [{}], 'Add an email reply-to address 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, @@ -531,7 +532,8 @@ def test_should_show_request_to_go_live_checklist( 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( @@ -550,9 +552,9 @@ def _count_templates(service_id, template_type=None): page = client_request.get( 'main.request_to_go_live', service_id=SERVICE_ONE_ID ) - assert page.h1.text == 'Request to go live' + assert page.h1.text == 'Before you request to go live' - checklist_items = page.select('main ul[class=bottom-gutter] li') + checklist_items = page.select('.task-list .task-list-item') assert normalize_spaces(checklist_items[0].text) == expected_user_checklist_item assert normalize_spaces(checklist_items[1].text) == expected_templates_checklist_item @@ -567,19 +569,125 @@ def _count_templates(service_id, template_type=None): 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 name Not completed', + ), + ( + 'local', + 1, + [{'is_default': True, 'sms_sender': 'GOVUK'}], + 'Change your text message sender name Not completed', + ), + ( + 'local', + 1, + [ + {'is_default': False, 'sms_sender': 'GOVUK'}, + {'is_default': True, 'sms_sender': 'KUVOG'}, + ], + 'Change your text message sender name Completed', + ), + ( + 'nhs', + 1, + [{'is_default': True, 'sms_sender': 'KUVOG'}], + 'Change your text message sender name 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 == 'Before you 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, ): page = client_request.get( 'main.submit_request_to_go_live', service_id=SERVICE_ONE_ID ) - assert page.h1.text == 'How do you plan to use Notify?' + assert page.h1.text == 'Request to go live' for channel, label in ( ('email', 'Emails'), ('sms', 'Text messages'),