Merge pull request #3104 from alphagov/gp

Add ‘GP’ as an organisation type

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', 'nhs_gp'},
+    )
 
 
 class NewOrganisationForm(

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_gp', 'GP practice'),
+        ('emergency_service', 'Emergency service'),
+        ('school_or_college', 'School or college'),
+        ('other', 'Other'),
+    )
+
     ALLOWED_PROPERTIES = {
         'id',
         'name',
@@ -98,6 +109,10 @@ class Organisation(JSONModel):
     def as_info_for_branding_request(self, fallback_domain):
         return self.name or 'Can’t tell (domain is {})'.format(fallback_domain)
 
+    @property
+    def organisation_type_label(self):
+        return dict(self.TYPES).get(self.organisation_type)
+
     @property
     def crown_status_or_404(self):
         if self.crown is None:

app/models/service.py

@@ -439,6 +439,10 @@ class Service(JSONModel):
     def organisation_type(self):
         return self.organisation.organisation_type or self._dict['organisation_type']
 
+    @property
+    def organisation_type_label(self):
+        return dict(Organisation.TYPES).get(self.organisation_type)
+
     @cached_property
     def inbound_number(self):
         return inbound_number_client.get_inbound_sms_number_for_service(self.id)['data'].get('number', '')

app/templates/views/organisations/organisation/settings/index.html

@@ -25,15 +25,7 @@
       {% endcall %}
       {% call row() %}
         {{ text_field('Sector') }}
-        {{ optional_text_field({
-          'central': 'Central government',
-          'local': 'Local government',
-          'nhs_central': 'NHS – central government agency or public body',
-          'nhs_local': 'NHS Trust, GP practice or Clinical Commissioning Group',
-          'emergency_service': 'Emergency service',
-          'school_or_college': 'School or college',
-          'other': 'Other',
-        }.get(current_org.organisation_type)) }}
+        {{ optional_text_field(current_org.organisation_type_label) }}
         {{ edit_field(
             'Change',
             url_for('.edit_organisation_type', org_id=current_org.id)

app/templates/views/service-settings.html

@@ -260,7 +260,7 @@
 
       <p>
         {% if current_user.has_permissions('manage_service') %}
-          To remove these restrictions, you can send us a 
+          To remove these restrictions, you can send us a
           <a href="{{ url_for('.request_to_go_live', service_id=current_service.id) }}">request to go live</a>.
         {% else %}
           Your service manager can ask to have these restrictions removed.
@@ -324,15 +324,7 @@
               {% endif %}
               {% if current_service.organisation_type %}
                 <div class="hint">
-                  {{ {
-                    'central': 'Central government',
-                    'local': 'Local government',
-                    'nhs_central': 'NHS – central government agency or public body',
-                    'nhs_local': 'NHS Trust, GP practice or Clinical Commissioning Group',
-                    'emergency_service': 'Emergency service',
-                    'school_or_college': 'School or college',
-                    'other': 'Other'
-                  }.get(current_service.organisation_type) }}
+                  {{ current_service.organisation_type_label }}
                 </div>
               {% endif %}
             {% endcall %}

tests/app/main/views/organisations/test_organisation.py

@@ -92,7 +92,7 @@ def test_page_to_create_new_organisation(
         ('radio', 'organisation_type', 'local'),
         ('radio', 'organisation_type', 'nhs_central'),
         ('radio', 'organisation_type', 'nhs_local'),
-        ('radio', 'organisation_type', 'nhs_local'),
+        ('radio', 'organisation_type', 'nhs_gp'),
         ('radio', 'organisation_type', 'emergency_service'),
         ('radio', 'organisation_type', 'school_or_college'),
         ('radio', 'organisation_type', 'other'),
@@ -282,7 +282,7 @@ def test_organisation_settings_for_platform_admin(
             ('local', 'Local government'),
             ('nhs_central', 'NHS – central government agency or public body'),
             ('nhs_local', 'NHS Trust or Clinical Commissioning Group'),
-            ('nhs_local', 'GP practice'),
+            ('nhs_gp', 'GP practice'),
             ('emergency_service', 'Emergency service'),
             ('school_or_college', 'School or college'),
             ('other', 'Other'),

tests/app/main/views/test_add_service.py

@@ -55,7 +55,7 @@ def test_get_should_render_add_service_template(
         'local',
         'nhs_central',
         'nhs_local',
-        'nhs_local',
+        'nhs_gp',
         'emergency_service',
         'school_or_college',
         'other',
@@ -205,7 +205,7 @@ def test_get_should_only_show_nhs_org_types_radios_if_user_has_nhs_email(
     ] == [
         'nhs_central',
         'nhs_local',
-        'nhs_local',
+        'nhs_gp',
     ]