Merge pull request #2250 from alphagov/task-list-go-live

Try to get more users to do the things we require before going live

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;
+  }
+
+}

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';

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/components/task-list.html

@@ -0,0 +1,16 @@
+{% macro task_list_item(completed, label) %}
+  <li class="task-list-item">
+    {{ label }}
+    {% if completed %}
+      <span class="task-list-indicator-completed">Completed</span>
+    {% else %}
+      <span class="task-list-indicator-not-completed">Not completed</span>
+    {% endif %}
+  </li>
+{% endmacro %}
+
+{% macro task_list_wrapper() %}
+  <ul class="task-list">
+    {{ caller() }}
+  </ul>
+{% endmacro %}

app/templates/components/tick-cross.html

@@ -14,7 +14,6 @@
   </li>
 {% endmacro %}
 
-
 {% macro tick_cross_done_not_done(yes, label) %}
   {{ tick_cross(yes, label, truthy_hint='Done: ', falsey_hint='Not done: ') }}
 {% endmacro %}

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 %}
   <div class="grid-row">
     <div class="column-whole">
-      <h1 class="heading-large">Request to go live</h1>
-      <p>
-        Before your service can go live on Notify, you’ll need to:
-      </p>
-      <ul class='bottom-gutter'>
-        {{ tick_cross_done_not_done(
+      <h1 class="heading-large">Before you request to go live</h1>
+      {% call task_list_wrapper() %}
+        {{ task_list_item(
           has_team_members,
-          'Have more than one <a href="{}">team member</a> with the ‘Manage service’ permission'.format(
+          '<a href="{}">Add a team member who can manage settings, team and usage</a>
+'.format(
             url_for('main.manage_users', service_id=current_service.id)
           )|safe,
         ) }}
-        {{ tick_cross_done_not_done(
+        {{ task_list_item(
           has_templates,
-          'Create <a href="{}">templates</a> showing the kind of messages you plan to send'.format(
+          '<a href="{}">Add content to
+templates to show the kind of messages you’ll send</a>'.format(
             url_for('main.choose_template', service_id=current_service.id)
           )|safe,
         ) }}
         {% if has_email_templates %}
-          {{ tick_cross_done_not_done(
+          {{ task_list_item(
             has_email_reply_to_address,
-            'Add an <a href="{}">email reply-to address</a>'.format(
+            '<a href="{}">Add an email reply-to address</a>'.format(
               url_for('main.service_email_reply_to', service_id=current_service.id)
             )|safe,
           ) }}
         {% endif %}
-      </ul>
+        {% if has_sms_templates and shouldnt_use_govuk_as_sms_sender %}
+          {{ task_list_item(
+            not sms_sender_is_govuk,
+            '<a href="{}">Change your text message sender name</a>'.format(
+              url_for('main.service_sms_senders', service_id=current_service.id)
+            )|safe
+          ) }}
+        {% endif %}
+      {% endcall %}
       <p>
         You also need to accept our <a href="{{ url_for('.terms') }}">terms of use</a>.
       </p>
-      <p class="bottom-gutter">
-        We’ll make your service live within one working day.
-      </p>
       <p>
         <a href="{{ url_for('main.submit_request_to_go_live', service_id=current_service.id) }}" class="button">Continue</a>
       </p>

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 %}
 
-      <h1 class="heading-large">How do you plan to use Notify?</h1>
+      <h1 class="heading-large">Request to go live</h1>
+
+      <p class="bottom-gutter">
+        Tell us how you plan to use Notify. When we receive your request we’ll make your service live within one working day.
+      </p>
 
       <form method="post" class="top-gutter">
         {{ checkbox_group('What kind of messages will you be sending?', [

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

@@ -505,23 +505,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,
@@ -533,7 +534,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(
@@ -552,9 +554,9 @@ def test_should_show_request_to_go_live_checklist(
     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
@@ -569,19 +571,125 @@ 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 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'),