Merge pull request #2827 from alphagov/depend-on-service-volumes

When going live, make reply-to address and text message sender dependent on estimated sending volumes

app/models/service.py

@@ -226,7 +226,7 @@ class Service():
 
     @property
     def needs_to_add_email_reply_to_address(self):
-        return self.has_email_templates and not self.has_email_reply_to_address
+        return self.volume_email and not self.has_email_reply_to_address
 
     @property
     def shouldnt_use_govuk_as_sms_sender(self):
@@ -269,7 +269,7 @@ class Service():
     @property
     def needs_to_change_sms_sender(self):
         return all((
-            self.has_sms_templates,
+            self.volume_sms,
             self.shouldnt_use_govuk_as_sms_sender,
             self.sms_sender_is_govuk,
         ))

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

@@ -27,14 +27,21 @@
           'Add templates with examples of the content you plan to send',
           url_for('main.choose_template', service_id=current_service.id),
         ) }}
-        {% if current_service.has_email_templates %}
+        {% if (
+          current_service.has_email_templates
+          and (current_service.volume_email != 0)
+        ) %}
           {{ task_list_item(
             current_service.has_email_reply_to_address,
             'Add an email reply-to address',
             url_for('main.service_email_reply_to', service_id=current_service.id),
           ) }}
         {% endif %}
-        {% if current_service.has_sms_templates and current_service.shouldnt_use_govuk_as_sms_sender %}
+        {% if (
+          current_service.has_sms_templates
+          and current_service.shouldnt_use_govuk_as_sms_sender
+          and (current_service.volume_sms != 0)
+        ) %}
           {{ task_list_item(
             not current_service.sms_sender_is_govuk,
             'Change your text message sender name',

tests/app/main/views/test_service_settings.py

@@ -522,6 +522,45 @@ def test_should_raise_duplicate_name_handled(
     ((1, 0, 0), True, 'Tell us how many messages you expect to send Completed'),
     ((9, 99, 999), True, 'Tell us how many messages you expect to send Completed'),
 ])
+def test_should_check_if_estimated_volumes_provided(
+    client_request,
+    mocker,
+    single_sms_sender,
+    single_reply_to_email_address,
+    mock_get_service_templates,
+    mock_get_users_by_service,
+    volumes,
+    consent_to_research,
+    expected_estimated_volumes_item,
+):
+
+    for volume, channel in zip(volumes, ('sms', 'email', 'letter')):
+        mocker.patch(
+            'app.models.service.Service.volume_{}'.format(channel),
+            create=True,
+            new_callable=PropertyMock,
+            return_value=volume,
+        )
+
+    mocker.patch(
+        'app.models.service.Service.consent_to_research',
+        create=True,
+        new_callable=PropertyMock,
+        return_value=consent_to_research,
+    )
+
+    page = client_request.get(
+        'main.request_to_go_live', service_id=SERVICE_ONE_ID
+    )
+    assert page.h1.text == 'Before you request to go live'
+
+    assert normalize_spaces(
+        page.select_one('.task-list .task-list-item').text
+    ) == (
+        expected_estimated_volumes_item
+    )
+
+
 @pytest.mark.parametrize('count_of_users_with_manage_service, expected_user_checklist_item', [
     (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'),
@@ -537,7 +576,7 @@ def test_should_raise_duplicate_name_handled(
     (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(
+def test_should_check_for_sending_things_right(
     client_request,
     mocker,
     single_sms_sender,
@@ -548,9 +587,6 @@ def test_should_show_request_to_go_live_checklist(
     count_of_email_templates,
     reply_to_email_addresses,
     expected_reply_to_checklist_item,
-    volumes,
-    consent_to_research,
-    expected_estimated_volumes_item,
 ):
 
     def _templates_by_type(template_type):
@@ -580,21 +616,14 @@ def test_should_show_request_to_go_live_checklist(
         return_value=reply_to_email_addresses
     )
 
-    for volume, channel in zip(volumes, ('sms', 'email', 'letter')):
+    for channel, volume in (('email', 1), ('sms', 0), ('letter', 1)):
         mocker.patch(
             'app.models.service.Service.volume_{}'.format(channel),
             create=True,
             new_callable=PropertyMock,
-            return_value=volume,
+            return_value=1,
         )
 
-    mocker.patch(
-        'app.models.service.Service.consent_to_research',
-        create=True,
-        new_callable=PropertyMock,
-        return_value=consent_to_research,
-    )
-
     page = client_request.get(
         'main.request_to_go_live', service_id=SERVICE_ONE_ID
     )
@@ -602,7 +631,6 @@ def test_should_show_request_to_go_live_checklist(
 
     checklist_items = page.select('.task-list .task-list-item')
 
-    assert normalize_spaces(checklist_items[0].text) == expected_estimated_volumes_item
     assert normalize_spaces(checklist_items[1].text) == expected_user_checklist_item
     assert normalize_spaces(checklist_items[2].text) == expected_templates_checklist_item
     assert normalize_spaces(checklist_items[3].text) == expected_reply_to_checklist_item
@@ -623,6 +651,11 @@ def test_should_show_request_to_go_live_checklist(
         mock_get_reply_to_email_addresses.assert_called_once_with(SERVICE_ONE_ID)
 
 
+@pytest.mark.parametrize('estimated_sms_volume', (
+    pytest.param(None),
+    pytest.param(1),
+    pytest.param(0, marks=pytest.mark.xfail(raises=IndexError)),
+))
 @pytest.mark.parametrize('organisation_type,count_of_sms_templates, sms_senders, expected_sms_sender_checklist_item', [
     pytest.param(
         'local',
@@ -688,6 +721,7 @@ def test_should_check_for_sms_sender_on_go_live(
     count_of_sms_templates,
     sms_senders,
     expected_sms_sender_checklist_item,
+    estimated_sms_volume,
 ):
 
     service_one['organisation_type'] = organisation_type
@@ -721,6 +755,13 @@ def test_should_check_for_sms_sender_on_go_live(
         return_value=[],
     )
 
+    mocker.patch(
+        'app.models.service.Service.volume_sms',
+        create=True,
+        new_callable=PropertyMock,
+        return_value=estimated_sms_volume,
+    )
+
     page = client_request.get(
         'main.request_to_go_live', service_id=SERVICE_ONE_ID
     )
@@ -1160,7 +1201,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             True,
-            1, 1, 1,
+            1, 0, 0,
             'Yes',
             True,
             [
@@ -1176,7 +1217,7 @@ def test_should_redirect_after_request_to_go_live(
             False,
             True,
             True,
-            1, 1, 1,
+            1, 0, 1,
             'No',
             True,
             [
@@ -1194,7 +1235,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             False,
-            1, 1, 1,
+            0, 1, 0,
             'Yes',
             True,
             [
@@ -1210,7 +1251,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             True,
-            1, 1, 1,
+            0, 1, 0,
             'No',
             True,
             [
@@ -1228,7 +1269,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             False,
-            1, 1, 1,
+            1, 0, 0,
             'No',
             True,
             [
@@ -1246,7 +1287,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             False,
-            1, 1, 1,
+            0, 1, 0,
             'No',
             True,
             [
@@ -1256,7 +1297,7 @@ def test_should_redirect_after_request_to_go_live(
                 'notify_request_to_go_live_incomplete_template_content',
             ],
         ),
-        (  # Everything is wrong
+        (  # Not done anything yet
             False,
             False,
             True,
@@ -1273,10 +1314,8 @@ def test_should_redirect_after_request_to_go_live(
                 'notify_request_to_go_live_incomplete_volumes',
                 'notify_request_to_go_live_incomplete_checklist',
                 'notify_request_to_go_live_incomplete_mou',
-                'notify_request_to_go_live_incomplete_email_reply_to',
                 'notify_request_to_go_live_incomplete_team_member',
                 'notify_request_to_go_live_incomplete_template_content',
-                'notify_request_to_go_live_incomplete_sms_sender',
             ],
         ),
     ),