Merge pull request #4254 from alphagov/fix-go-live-bug-179736794

Fix go-live checks ignoring nested templates

app/models/service.py

@@ -284,13 +284,19 @@ class Service(JSONModel, SortByNameMixin):
             ))
         )
 
+    def has_templates_of_type(self, template_type):
+        return any(
+            template for template in self.all_templates
+            if template['template_type'] == template_type
+        )
+
     @property
     def has_email_templates(self):
-        return len(self.get_templates('email')) > 0
+        return self.has_templates_of_type('email')
 
     @property
     def has_sms_templates(self):
-        return len(self.get_templates('sms')) > 0
+        return self.has_templates_of_type('sms')
 
     @property
     def intending_to_send_email(self):

tests/app/main/views/service_settings/test_service_settings.py

@@ -36,6 +36,7 @@ from tests.conftest import (
     create_platform_admin_user,
     create_reply_to_email_address,
     create_sms_sender,
+    create_template,
     normalize_spaces,
 )
 
@@ -836,20 +837,6 @@ def test_should_check_if_estimated_volumes_provided(
     )
 
 
-@pytest.mark.parametrize((
-    'count_of_users_with_manage_service,'
-    'count_of_invites_with_manage_service,'
-    'expected_user_checklist_item'
-), [
-    (1, 0, 'Add a team member who can manage settings, team and usage Not completed'),
-    (2, 0, 'Add a team member who can manage settings, team and usage Completed'),
-    (1, 1, 'Add a team member who can manage settings, team and usage Completed'),
-])
-@pytest.mark.parametrize('count_of_templates, expected_templates_checklist_item', [
-    (0, 'Add templates with examples of the content you plan to send Not completed'),
-    (1, 'Add templates with examples of the content you plan to send Completed'),
-    (2, 'Add templates with examples of the content you plan to send Completed'),
-])
 @pytest.mark.parametrize((
     'volume_email,'
     'count_of_email_templates,'
@@ -865,6 +852,66 @@ def test_should_check_if_estimated_volumes_provided(
     (1, 0, [], 'Add a reply-to email address Not completed'),
     (1, 0, [{}], 'Add a reply-to email address Completed'),
 ])
+def test_should_check_for_reply_to_on_go_live(
+    client_request,
+    mocker,
+    service_one,
+    fake_uuid,
+    single_sms_sender,
+    volume_email,
+    count_of_email_templates,
+    reply_to_email_addresses,
+    expected_reply_to_checklist_item,
+    mock_get_invites_for_service,
+    mock_get_users_by_service,
+):
+    mocker.patch(
+        'app.service_api_client.get_service_templates',
+        return_value={'data': [
+            create_template(template_type='email')
+            for _ in range(0, count_of_email_templates)
+        ]}
+    )
+
+    mock_get_reply_to_email_addresses = mocker.patch(
+        'app.main.views.service_settings.service_api_client.get_reply_to_email_addresses',
+        return_value=reply_to_email_addresses
+    )
+
+    for channel, volume in (('email', volume_email), ('sms', 0), ('letter', 1)):
+        mocker.patch(
+            'app.models.service.Service.volume_{}'.format(channel),
+            create=True,
+            new_callable=PropertyMock,
+            return_value=volume,
+        )
+
+    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[3].text) == expected_reply_to_checklist_item
+
+    if count_of_email_templates:
+        mock_get_reply_to_email_addresses.assert_called_once_with(SERVICE_ONE_ID)
+
+
+@pytest.mark.parametrize((
+    'count_of_users_with_manage_service,'
+    'count_of_invites_with_manage_service,'
+    'expected_user_checklist_item'
+), [
+    (1, 0, 'Add a team member who can manage settings, team and usage Not completed'),
+    (2, 0, 'Add a team member who can manage settings, team and usage Completed'),
+    (1, 1, 'Add a team member who can manage settings, team and usage Completed'),
+])
+@pytest.mark.parametrize('count_of_templates, expected_templates_checklist_item', [
+    (0, 'Add templates with examples of the content you plan to send Not completed'),
+    (1, 'Add templates with examples of the content you plan to send Completed'),
+    (2, 'Add templates with examples of the content you plan to send Completed'),
+])
 def test_should_check_for_sending_things_right(
     client_request,
     mocker,
@@ -876,19 +923,18 @@ def test_should_check_for_sending_things_right(
     expected_user_checklist_item,
     count_of_templates,
     expected_templates_checklist_item,
-    volume_email,
-    count_of_email_templates,
-    reply_to_email_addresses,
-    expected_reply_to_checklist_item,
     active_user_with_permissions,
     active_user_no_settings_permission,
+    single_reply_to_email_address,
 ):
-    def _templates_by_type(template_type):
-        return {
-            'email': list(range(0, count_of_email_templates)),
-            'sms': [],
-        }.get(template_type)
-    active_user_with_permissions,
+    mocker.patch(
+        'app.service_api_client.get_service_templates',
+        return_value={'data': [
+            create_template(template_type='sms')
+            for _ in range(0, count_of_templates)
+        ]}
+    )
+
     mock_get_users = mocker.patch(
         'app.models.user.Users.client_method',
         return_value=(
@@ -917,35 +963,6 @@ def test_should_check_for_sending_things_right(
         )
     )
 
-    mock_templates = mocker.patch(
-        'app.models.service.Service.all_templates',
-        new_callable=PropertyMock,
-        return_value=list(range(0, count_of_templates)),
-    )
-
-    mocker.patch(
-        'app.models.service.Service.get_templates',
-        side_effect=_templates_by_type,
-    )
-    mocker.patch(
-        'app.models.service.Service.organisation_id',
-        new_callable=PropertyMock,
-        return_value=None,
-    )
-
-    mock_get_reply_to_email_addresses = mocker.patch(
-        'app.main.views.service_settings.service_api_client.get_reply_to_email_addresses',
-        return_value=reply_to_email_addresses
-    )
-
-    for channel, volume in (('email', volume_email), ('sms', 0), ('letter', 1)):
-        mocker.patch(
-            'app.models.service.Service.volume_{}'.format(channel),
-            create=True,
-            new_callable=PropertyMock,
-            return_value=volume,
-        )
-
     page = client_request.get(
         'main.request_to_go_live', service_id=SERVICE_ONE_ID
     )
@@ -954,14 +971,9 @@ def test_should_check_for_sending_things_right(
     checklist_items = page.select('.task-list .task-list-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
 
     mock_get_users.assert_called_once_with(SERVICE_ONE_ID)
     mock_get_invites.assert_called_once_with(SERVICE_ONE_ID)
-    assert mock_templates.called is True
-
-    if count_of_email_templates:
-        mock_get_reply_to_email_addresses.assert_called_once_with(SERVICE_ONE_ID)
 
 
 @pytest.mark.parametrize('checklist_completed, agreement_signed, expected_button', (
@@ -1149,34 +1161,23 @@ def test_should_check_for_sms_sender_on_go_live(
 ):
     service_one['organisation_type'] = organisation_type
 
-    def _templates_by_type(template_type):
-        return list(range(0, {
-            'email': 0,
-            'sms': count_of_sms_templates,
-        }.get(template_type, count_of_sms_templates)))
+    mocker.patch(
+        'app.service_api_client.get_service_templates',
+        return_value={'data': [
+            create_template(template_type='sms')
+            for _ in range(0, count_of_sms_templates)
+        ]}
+    )
 
     mocker.patch(
         'app.models.service.Service.has_team_members',
         return_value=True,
     )
-    mock_templates = mocker.patch(
-        'app.models.service.Service.all_templates',
-        new_callable=PropertyMock,
-        side_effect=partial(_templates_by_type, 'all'),
-    )
-    mocker.patch(
-        'app.models.service.Service.get_templates',
-        side_effect=_templates_by_type,
-    )
 
     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=[],
-    )
 
     for channel, volume in (('email', 0), ('sms', estimated_sms_volume)):
         mocker.patch(
@@ -1194,8 +1195,6 @@ def test_should_check_for_sms_sender_on_go_live(
     checklist_items = page.select('.task-list .task-list-item')
     assert normalize_spaces(checklist_items[3].text) == expected_sms_sender_checklist_item
 
-    assert mock_templates.called is True
-
     mock_get_sms_senders.assert_called_once_with(SERVICE_ONE_ID)
 
 

tests/app/models/test_service.py

@@ -6,7 +6,7 @@ from app.models.organisation import Organisation
 from app.models.service import Service
 from app.models.user import User
 from tests import organisation_json, service_json
-from tests.conftest import ORGANISATION_ID
+from tests.conftest import ORGANISATION_ID, create_folder, create_template
 
 INV_PARENT_FOLDER_ID = '7e979e79-d970-43a5-ac69-b625a8d147b0'
 INV_CHILD_1_FOLDER_ID = '92ee1ee0-e4ee-4dcc-b1a7-a5da9ebcfa2b'
@@ -265,3 +265,23 @@ def test_service_billing_details(purchase_order_number, expected_result):
     service = Service(service_json(purchase_order_number=purchase_order_number))
     service._dict['purchase_order_number'] = purchase_order_number
     assert service.billing_details == expected_result
+
+
+def test_has_templates_of_type_includes_folders(
+    mocker,
+    service_one,
+    mock_get_template_folders,
+):
+    mocker.patch(
+        'app.service_api_client.get_service_templates',
+        return_value={'data': [create_template(
+            folder='something', template_type='sms'
+        )]}
+    )
+
+    mocker.patch(
+        'app.template_folder_api_client.get_template_folders',
+        return_value=[create_folder(id='something')]
+    )
+
+    assert Service(service_one).has_templates_of_type('sms')

tests/conftest.py

@@ -3982,6 +3982,14 @@ def create_notifications(
     )
 
 
+def create_folder(id):
+    return {
+        'id': id,
+        'parent_id': None,
+        'name': 'My folder'
+    }
+
+
 def create_template(
     service_id=SERVICE_ONE_ID,
     template_id=None,
@@ -3990,7 +3998,8 @@ def create_template(
     content='Template content',
     subject='Template subject',
     redact_personalisation=False,
-    postage=None
+    postage=None,
+    folder=None
 ):
     return template_json(
         service_id=service_id,
@@ -4001,6 +4010,7 @@ def create_template(
         subject=subject,
         redact_personalisation=redact_personalisation,
         postage=postage,
+        folder=folder,
     )