Merge pull request #2815 from alphagov/go-live-without-volumes

Let users go live without filling in the volumes (but tag these tickets)

app/main/views/service_settings.py

@@ -201,9 +201,9 @@ def submit_request_to_go_live(service_id):
             '\nOrganisation type: {organisation_type}'
             '\nAgreement signed: {agreement}'
             '\nChecklist completed: {checklist}'
-            '\nEmails in next year: {volume_email:,.0f}'
+            '\nEmails in next year: {volume_email_formatted}'
-            '\nText messages in next year: {volume_sms:,.0f}'
+            '\nText messages in next year: {volume_sms_formatted}'
-            '\nLetters in next year: {volume_letter:,.0f}'
+            '\nLetters in next year: {volume_letter_formatted}'
             '\nConsent to research: {research_consent}'
             '\nOther live services: {existing_live}'
             '\n'
@@ -225,9 +225,12 @@ def submit_request_to_go_live(service_id):
             organisation_type=str(current_service.organisation_type).title(),
             agreement=AgreementInfo.from_current_user().as_human_readable,
             checklist=current_service.go_live_checklist_completed_as_yes_no,
-            volume_email=current_service.volume_email,
+            volume_email=print_if_number(current_service.volume_email),
-            volume_sms=current_service.volume_sms,
+            volume_email_formatted=format_if_number(current_service.volume_email),
-            volume_letter=current_service.volume_letter,
+            volume_sms=print_if_number(current_service.volume_sms),
+            volume_sms_formatted=format_if_number(current_service.volume_sms),
+            volume_letter=print_if_number(current_service.volume_letter),
+            volume_letter_formatted=format_if_number(current_service.volume_letter),
             research_consent='Yes' if current_service.consent_to_research else 'No',
             existing_live='Yes' if user_api_client.user_has_live_services(current_user) else 'No',
             service_id=current_service.id,
@@ -1061,6 +1064,7 @@ def _get_request_to_go_live_tags(service, agreement_signed):
 
     for test, tag in (
         (True, ''),
+        (not service.volumes, '_volumes'),
         (not service.go_live_checklist_completed, '_checklist'),
         (not agreement_signed, '_mou'),
         (service.needs_to_add_email_reply_to_address, '_email_reply_to'),
@@ -1070,3 +1074,11 @@ def _get_request_to_go_live_tags(service, agreement_signed):
     ):
         if test:
             yield BASE + '_incomplete' + tag
+
+
+def print_if_number(value):
+    return value if isinstance(value, int) else ''
+
+
+def format_if_number(value):
+    return '{:,.0f}'.format(value) if isinstance(value, int) else ''

app/models/service.py

@@ -294,9 +294,18 @@ class Service():
     def get_letter_contact_block(self, id):
         return service_api_client.get_letter_contact(self.id, id)
 
+    @property
+    def volumes(self):
+        return sum(filter(None, (
+            self.volume_email,
+            self.volume_sms,
+            self.volume_letter,
+        )))
+
     @property
     def go_live_checklist_completed(self):
         return all((
+            bool(self.volumes),
             self.has_team_members,
             self.has_templates,
             not self.needs_to_add_email_reply_to_address,

tests/app/main/views/test_service_settings.py

@@ -1017,6 +1017,28 @@ def test_non_gov_users_cant_request_to_go_live(
     )
 
 
+@pytest.mark.parametrize('volumes, displayed_volumes, formatted_displayed_volumes, extra_tags', (
+    (
+        (('email', None), ('sms', None), ('letter', None)),
+        ', , ',
+        (
+            'Emails in next year: \n'
+            'Text messages in next year: \n'
+            'Letters in next year: \n'
+        ),
+        ['notify_request_to_go_live_incomplete_volumes']
+    ),
+    (
+        (('email', 1234), ('sms', 0), ('letter', 999)),
+        '0, 1234, 999',  # This is a different order to match the spreadsheet
+        (
+            'Emails in next year: 1,234\n'
+            'Text messages in next year: 0\n'
+            'Letters in next year: 999\n'
+        ),
+        [],
+    ),
+))
 @freeze_time("2012-12-21")
 def test_should_redirect_after_request_to_go_live(
     client_request,
@@ -1030,7 +1052,18 @@ def test_should_redirect_after_request_to_go_live(
     mock_get_service_settings_page_common,
     mock_get_service_templates,
     mock_get_users_by_service,
+    volumes,
+    displayed_volumes,
+    formatted_displayed_volumes,
+    extra_tags,
 ):
+    for channel, volume in volumes:
+        mocker.patch(
+            'app.models.service.Service.volume_{}'.format(channel),
+            create=True,
+            new_callable=PropertyMock,
+            return_value=volume,
+        )
     mock_post = mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True)
     page = client_request.post(
         'main.request_to_go_live',
@@ -1046,6 +1079,7 @@ def test_should_redirect_after_request_to_go_live(
         tags=[
             'notify_request_to_go_live',
             'notify_request_to_go_live_incomplete',
+        ] + extra_tags + [
             'notify_request_to_go_live_incomplete_checklist',
             'notify_request_to_go_live_incomplete_mou',
             'notify_request_to_go_live_incomplete_team_member',
@@ -1053,21 +1087,24 @@ def test_should_redirect_after_request_to_go_live(
     )
     assert mock_post.call_args[1]['message'] == (
         'Service: service one\n'
-        'http://localhost/services/{}\n'
+        'http://localhost/services/{service_id}\n'
         '\n'
         '---\n'
         'Organisation type: Central\n'
         'Agreement signed: Can’t tell (domain is user.gov.uk)\n'
         'Checklist completed: No\n'
-        'Emails in next year: 111,111\n'
+        '{formatted_displayed_volumes}'
-        'Text messages in next year: 222,222\n'
-        'Letters in next year: 333,333\n'
         'Consent to research: Yes\n'
         'Other live services: No\n'
         '\n'
         '---\n'
-        '{}, None, service one, Test User, test@user.gov.uk, -, 21/12/2012, 222222, 111111, 333333'
+        '{service_id}, None, service one, Test User, test@user.gov.uk, -, 21/12/2012, '
-    ).format(SERVICE_ONE_ID, SERVICE_ONE_ID)
+        '{displayed_volumes}'
+    ).format(
+        service_id=SERVICE_ONE_ID,
+        displayed_volumes=displayed_volumes,
+        formatted_displayed_volumes=formatted_displayed_volumes,
+    )
 
     assert normalize_spaces(page.select_one('.banner-default').text) == (
         'Thanks for your request to go live. We’ll get back to you within one working day.'
@@ -1086,6 +1123,9 @@ def test_should_redirect_after_request_to_go_live(
         'has_email_reply_to_address,'
         'shouldnt_use_govuk_as_sms_sender,'
         'sms_sender_is_govuk,'
+        'volume_email,'
+        'volume_sms,'
+        'volume_letter,'
         'expected_readyness,'
         'agreement_signed,'
         'expected_tags,'
@@ -1099,6 +1139,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             True,
+            1, 1, 1,
             'Yes',
             True,
             [
@@ -1114,6 +1155,7 @@ def test_should_redirect_after_request_to_go_live(
             False,
             True,
             True,
+            1, 1, 1,
             'No',
             True,
             [
@@ -1131,6 +1173,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             False,
+            1, 1, 1,
             'Yes',
             True,
             [
@@ -1146,6 +1189,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             True,
+            1, 1, 1,
             'No',
             True,
             [
@@ -1163,6 +1207,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             False,
+            1, 1, 1,
             'No',
             True,
             [
@@ -1180,6 +1225,7 @@ def test_should_redirect_after_request_to_go_live(
             True,
             True,
             False,
+            1, 1, 1,
             'No',
             True,
             [
@@ -1197,11 +1243,13 @@ def test_should_redirect_after_request_to_go_live(
             False,
             True,
             True,
+            0, None, 0,
             'No',
             False,
             [
                 'notify_request_to_go_live',
                 'notify_request_to_go_live_incomplete',
+                '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',
@@ -1222,6 +1270,9 @@ def test_ready_to_go_live(
     has_email_reply_to_address,
     shouldnt_use_govuk_as_sms_sender,
     sms_sender_is_govuk,
+    volume_email,
+    volume_sms,
+    volume_letter,
     expected_readyness,
     agreement_signed,
     expected_tags,
@@ -1240,6 +1291,15 @@ def test_ready_to_go_live(
             new_callable=PropertyMock
         ).return_value = locals()[prop]
 
+    mocker.patch(
+        'app.models.service.Service.__getattr__',
+        side_effect=lambda prop: {
+            'volume_email': volume_email,
+            'volume_sms': volume_sms,
+            'volume_letter': volume_letter,
+        }.get(prop)
+    )
+
     assert app.models.service.Service({
         'id': SERVICE_ONE_ID
     }).go_live_checklist_completed_as_yes_no == expected_readyness