From 32efb9a03d065d271b91a6bc66e269c0bd973863 Mon Sep 17 00:00:00 2001
From: Ben Thorner <ben.thorner@digital.cabinet-office.gov.uk>
Date: Fri, 27 May 2022 10:31:06 +0100
Subject: [PATCH] Fix go-live checks ignoring nested templates

This is a very low impact bug since a user can always create such
templates after their service is live and not be subject to checks
we do before that point. Still, we may as well fix it.

The main benefit of this change is actually to contribute towards
moving methods like "get_templates" out of the Service class so
we can simplify and cache their results more effectively.

Note: I wanted to simplify the Service class further as the two
"has_" properties are only used once in the app code. Unfortunately
they are tightly coupled in one of the tests as well [^1].

[^1]: https://github.com/alphagov/notifications-admin/blob/bef0382cca16ba87d04430f8cd391092b1dc91ef/tests/app/main/views/service_settings/test_service_settings.py#L1961-L1962
---
 app/models/service.py            | 10 ++++++++--
 tests/app/models/test_service.py | 26 ++------------------------
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/app/models/service.py b/app/models/service.py
index 9d1ca937b..3694d8487 100644
--- a/app/models/service.py
+++ b/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):
diff --git a/tests/app/models/test_service.py b/tests/app/models/test_service.py
index f7ea179d6..908b49037 100644
--- a/tests/app/models/test_service.py
+++ b/tests/app/models/test_service.py
@@ -267,29 +267,7 @@ def test_service_billing_details(purchase_order_number, expected_result):
     assert service.billing_details == expected_result
 
 
-@pytest.mark.xfail
-def test_has_email_templates_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='email'
-        )]}
-    )
-
-    mocker.patch(
-        'app.template_folder_api_client.get_template_folders',
-        return_value=[create_folder(id='something')]
-    )
-
-    assert Service(service_one).has_email_templates
-
-
-@pytest.mark.xfail
-def test_has_sms_templates_includes_folders(
+def test_has_templates_of_type_includes_folders(
     mocker,
     service_one,
     mock_get_template_folders,
@@ -306,4 +284,4 @@ def test_has_sms_templates_includes_folders(
         return_value=[create_folder(id='something')]
     )
 
-    assert Service(service_one).has_sms_templates
+    assert Service(service_one).has_templates_of_type('sms')