From efabf0e87d3babf3c0d3a55de8a189ce56b3bb59 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 27 Aug 2019 15:52:42 +0100
Subject: [PATCH] Refactor to avoid redefinition of org types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We’re defining the list of org types in a few different places. This
makes it more likely we’ll forget to update one of these places, thereby
introducing a bug.

This commit moves the definition to be on the organisation model, which
feels like a sensible enough place for it.
---
 app/main/forms.py          | 54 +++++++++++++++++---------------------
 app/models/organisation.py | 11 ++++++++
 2 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index b33f3ef42..e5a084cd7 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -45,6 +45,7 @@ from app.main.validators import (
     ValidEmail,
     ValidGovEmail,
 )
+from app.models.organisation import Organisation
 from app.models.roles_and_permissions import permissions, roles
 from app.utils import guess_name_from_email_address
 
@@ -242,33 +243,23 @@ class ForgivingIntegerField(StringField):
         return super().__call__(value=value, **kwargs)
 
 
-def organisation_type(label='Who runs this service?'):
-    return RadioField(
-        label,
-        choices=[
-            ('central', 'Central government'),
-            ('local', 'Local government'),
-            ('nhs_central', 'NHS – central government agency or public body'),
-            ('nhs_local', 'NHS Trust or Clinical Commissioning Group'),
-            ('nhs_local', 'GP practice'),
-            ('emergency_service', 'Emergency service'),
-            ('school_or_college', 'School or college'),
-            ('other', 'Other'),
-        ],
-        validators=[DataRequired()],
-    )
-
-
-def nhs_organisation_type(label='Who runs this service?'):
-    return RadioField(
-        label,
-        choices=[
-            ('nhs_central', 'NHS – central government agency or public body'),
-            ('nhs_local', 'NHS Trust or Clinical Commissioning Group'),
-            ('nhs_local', 'GP practice'),
-        ],
-        validators=[DataRequired()],
-    )
+class OrganisationTypeField(RadioField):
+    def __init__(
+        self,
+        *args,
+        include_only=None,
+        validators=None,
+        **kwargs
+    ):
+        super().__init__(
+            *args,
+            choices=[
+                (value, label) for value, label in Organisation.TYPES
+                if not include_only or value in include_only
+            ],
+            validators=[DataRequired()] + (validators or []),
+            **kwargs
+        )
 
 
 class FieldWithNoneOption():
@@ -544,7 +535,7 @@ class RenameOrganisationForm(StripWhitespaceForm):
 
 
 class OrganisationOrganisationTypeForm(StripWhitespaceForm):
-    organisation_type = organisation_type(label='What type of organisation is this?')
+    organisation_type = OrganisationTypeField('What type of organisation is this?')
 
 
 class OrganisationCrownStatusForm(StripWhitespaceForm):
@@ -605,11 +596,14 @@ class CreateServiceForm(StripWhitespaceForm):
         validators=[
             DataRequired(message='Can’t be empty')
         ])
-    organisation_type = organisation_type()
+    organisation_type = OrganisationTypeField('Who runs this service?')
 
 
 class CreateNhsServiceForm(CreateServiceForm):
-    organisation_type = nhs_organisation_type()
+    organisation_type = OrganisationTypeField(
+        'Who runs this service?',
+        include_only={'nhs_central', 'nhs_local'}
+    )
 
 
 class NewOrganisationForm(
diff --git a/app/models/organisation.py b/app/models/organisation.py
index 5c6748f16..960b38b5f 100644
--- a/app/models/organisation.py
+++ b/app/models/organisation.py
@@ -7,6 +7,17 @@ from app.notify_client.organisations_api_client import organisations_client
 
 class Organisation(JSONModel):
 
+    TYPES = (
+        ('central', 'Central government'),
+        ('local', 'Local government'),
+        ('nhs_central', 'NHS – central government agency or public body'),
+        ('nhs_local', 'NHS Trust or Clinical Commissioning Group'),
+        ('nhs_local', 'GP practice'),
+        ('emergency_service', 'Emergency service'),
+        ('school_or_college', 'School or college'),
+        ('other', 'Other'),
+    )
+
     ALLOWED_PROPERTIES = {
         'id',
         'name',