From 2fc0a105f427a7cfe378257993df510837fca7da Mon Sep 17 00:00:00 2001
From: Ben Thorner <ben.thorner@digital.cabinet-office.gov.uk>
Date: Thu, 3 Mar 2022 12:51:38 +0000
Subject: [PATCH] Move branding choices logic into utility module

This was a lot of code to be in a form and it's going to get even
more complicated with email branding pools. Moving it out means we
can also simplify the tests that target this code.
---
 app/main/forms.py     | 57 ++++---------------------------------------
 app/utils/branding.py | 53 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 52 deletions(-)
 create mode 100644 app/utils/branding.py

diff --git a/app/main/forms.py b/app/main/forms.py
index f7c08c1f2..007393b9a 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -67,7 +67,7 @@ from app.main.validators import (
 )
 from app.models.feedback import PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE
 from app.models.organisation import Organisation
-from app.utils import merge_jsonlike
+from app.utils import branding, merge_jsonlike
 from app.utils.user import distinct_email_addresses
 from app.utils.user_permissions import (
     all_ui_permissions,
@@ -2198,57 +2198,10 @@ class ChooseBrandingForm(StripWhitespaceForm):
 
     @staticmethod
     def get_available_choices(service, branding_type):
-        if branding_type == "email":
-            organisation_branding_id = service.organisation.email_branding_id if service.organisation else None
-            service_branding_id = service.email_branding_id
-            service_branding_name = service.email_branding_name
-        elif branding_type == "letter":
-            organisation_branding_id = service.organisation.letter_branding_id if service.organisation else None
-            service_branding_id = service.letter_branding_id
-            service_branding_name = service.letter_branding_name
-
-        if (
-            service.organisation_type == Organisation.TYPE_CENTRAL
-            and organisation_branding_id is None
-            and service_branding_id is not None
-            and branding_type == "email"
-        ):
-            yield ('govuk', 'GOV.UK')
-
-        if (
-            service.organisation_type == Organisation.TYPE_CENTRAL
-            and service.organisation
-            and organisation_branding_id is None
-            and service_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower()
-            and branding_type == "email"
-        ):
-            yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name))
-
-        if (
-            service.organisation_type in {
-                Organisation.TYPE_NHS_CENTRAL,
-                Organisation.TYPE_NHS_LOCAL,
-                Organisation.TYPE_NHS_GP,
-            }
-            and service_branding_name != 'NHS'
-        ):
-            yield ('nhs', 'NHS')
-
-        if (
-            service.organisation
-            and service.organisation_type not in {
-                Organisation.TYPE_NHS_LOCAL,
-                Organisation.TYPE_NHS_CENTRAL,
-                Organisation.TYPE_NHS_GP,
-            }
-            and (
-                service_branding_id is None
-                or service_branding_id != organisation_branding_id
-            )
-        ):
-            yield ('organisation', service.organisation.name)
-
-        yield ChooseBrandingForm.FALLBACK_OPTION
+        return (
+            list(branding.get_available_choices(service, branding_type)) +
+            [ChooseBrandingForm.FALLBACK_OPTION]
+        )
 
     @property
     def something_else_is_only_option(self):
diff --git a/app/utils/branding.py b/app/utils/branding.py
new file mode 100644
index 000000000..dfa9f1585
--- /dev/null
+++ b/app/utils/branding.py
@@ -0,0 +1,53 @@
+from app.models.organisation import Organisation
+
+
+def get_available_choices(service, branding_type):
+    if branding_type == "email":
+        organisation_branding_id = service.organisation.email_branding_id if service.organisation else None
+        service_branding_id = service.email_branding_id
+        service_branding_name = service.email_branding_name
+    elif branding_type == "letter":
+        organisation_branding_id = service.organisation.letter_branding_id if service.organisation else None
+        service_branding_id = service.letter_branding_id
+        service_branding_name = service.letter_branding_name
+
+    if (
+        service.organisation_type == Organisation.TYPE_CENTRAL
+        and organisation_branding_id is None
+        and service_branding_id is not None
+        and branding_type == "email"
+    ):
+        yield ('govuk', 'GOV.UK')
+
+    if (
+        service.organisation_type == Organisation.TYPE_CENTRAL
+        and service.organisation
+        and organisation_branding_id is None
+        and service_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower()
+        and branding_type == "email"
+    ):
+        yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name))
+
+    if (
+        service.organisation_type in {
+            Organisation.TYPE_NHS_CENTRAL,
+            Organisation.TYPE_NHS_LOCAL,
+            Organisation.TYPE_NHS_GP,
+        }
+        and service_branding_name != 'NHS'
+    ):
+        yield ('nhs', 'NHS')
+
+    if (
+        service.organisation
+        and service.organisation_type not in {
+            Organisation.TYPE_NHS_LOCAL,
+            Organisation.TYPE_NHS_CENTRAL,
+            Organisation.TYPE_NHS_GP,
+        }
+        and (
+            service_branding_id is None
+            or service_branding_id != organisation_branding_id
+        )
+    ):
+        yield ('organisation', service.organisation.name)