Only show nhs radios if user has nhs domain email

Also split local NHS into two groups following designer advice
on readability.

app/main/forms.py

@@ -249,7 +249,8 @@ def organisation_type(label='Who runs this service?'):
             ('central', 'Central government'),
             ('local', 'Local government'),
             ('nhs_central', 'NHS – central government agency or public body'),
-            ('nhs_local', 'NHS Trust, GP surgery or Clinical Commissioning Group'),
+            ('nhs_local', 'NHS Trust or Clinical Commissioning Group'),
+            ('nhs_local', 'GP practice'),
             ('emergency_service', 'Emergency service'),
             ('school_or_college', 'School or college'),
             ('other', 'Other'),
@@ -258,6 +259,18 @@ def organisation_type(label='Who runs this service?'):
     )
 
 
+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 FieldWithNoneOption():
 
     # This is a special value that is specific to our forms. This is
@@ -588,13 +601,17 @@ class OrganisationDomainsForm(StripWhitespaceForm):
 
 class CreateServiceForm(StripWhitespaceForm):
     name = StringField(
-        u'What’s your service called?',
+        "What’s your service called?",
         validators=[
             DataRequired(message='Can’t be empty')
         ])
     organisation_type = organisation_type()
 
 
+class CreateNhsServiceForm(CreateServiceForm):
+    organisation_type = nhs_organisation_type()
+
+
 class NewOrganisationForm(
     RenameOrganisationForm,
     OrganisationOrganisationTypeForm,

app/main/views/add_service.py

@@ -4,7 +4,7 @@ from notifications_python_client.errors import HTTPError
 
 from app import billing_api_client, service_api_client
 from app.main import main
-from app.main.forms import CreateServiceForm
+from app.main.forms import CreateNhsServiceForm, CreateServiceForm
 from app.utils import email_safe, user_is_gov_user, user_is_logged_in
 
 
@@ -47,9 +47,14 @@ def _create_example_template(service_id):
 @user_is_logged_in
 @user_is_gov_user
 def add_service():
-    form = CreateServiceForm(
-        organisation_type=current_user.default_organisation_type
-    )
+    default_organisation_type = current_user.default_organisation_type
+    if default_organisation_type == 'nhs':
+        form = CreateNhsServiceForm()
+        default_organisation_type = None
+    else:
+        form = CreateServiceForm(
+            organisation_type=default_organisation_type
+        )
     heading = 'About your service'
 
     if form.validate_on_submit():
@@ -58,7 +63,7 @@ def add_service():
 
         service_id, error = _create_service(
             service_name,
-            current_user.default_organisation_type or form.organisation_type.data,
+            default_organisation_type or form.organisation_type.data,
             email_from,
             form,
         )
@@ -72,11 +77,12 @@ def add_service():
         return redirect(url_for(
             'main.start_tour',
             service_id=service_id,
-            template_id=example_sms_template['data']['id'],
+            template_id=example_sms_template['data']['id']
         ))
     else:
         return render_template(
             'views/add-service.html',
             form=form,
-            heading=heading
+            heading=heading,
+            default_organisation_type=default_organisation_type,
         )

app/models/user.py

@@ -322,7 +322,7 @@ class User(JSONModel, UserMixin):
         if self.default_organisation:
             return self.default_organisation.organisation_type
         if self.has_nhs_email_address:
-            return 'nhs_local'
+            return 'nhs'
         return None
 
     @property

app/templates/views/add-service.html

@@ -20,7 +20,7 @@
 
       {{ textbox(form.name, hint="You can change this later") }}
 
-      {% if not current_user.default_organisation_type %}
+      {% if not default_organisation_type %}
         {{ radios(form.organisation_type) }}
       {% endif %}
 

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

@@ -28,8 +28,9 @@
         {{ optional_text_field({
           'central': 'Central government',
           'local': 'Local government',
+          'nhs': 'NHS',
           'nhs_central': 'NHS – central government agency or public body',
-          'nhs_local': 'NHS Trust, GP surgery or Clinical Commissioning Group',
+          'nhs_local': 'NHS Trust, GP practice or Clinical Commissioning Group',
           'emergency_service': 'Emergency service',
           'school_or_college': 'School or college',
           'other': 'Other',

app/templates/views/service-settings.html

@@ -322,6 +322,11 @@
                     'central': 'Central government',
                     'local': 'Local government',
                     'nhs': 'NHS',
+                    '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) }}
                 </div>
               {% endif %}

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

@@ -92,6 +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', 'emergency_service'),
         ('radio', 'organisation_type', 'school_or_college'),
         ('radio', 'organisation_type', 'other'),
@@ -280,7 +281,8 @@ def test_organisation_settings_for_platform_admin(
             ('central', 'Central government'),
             ('local', 'Local government'),
             ('nhs_central', 'NHS – central government agency or public body'),
-            ('nhs_local', 'NHS Trust, GP surgery or Clinical Commissioning Group'),
+            ('nhs_local', 'NHS Trust or Clinical Commissioning Group'),
+            ('nhs_local', 'GP practice'),
             ('emergency_service', 'Emergency service'),
             ('school_or_college', 'School or college'),
             ('other', 'Other'),

tests/app/main/views/test_add_service.py

@@ -42,7 +42,8 @@ def test_get_should_render_add_service_template(
         'Central government',
         'Local government',
         'NHS – central government agency or public body',
-        'NHS Trust, GP surgery or Clinical Commissioning Group',
+        'NHS Trust or Clinical Commissioning Group',
+        'GP practice',
         'Emergency service',
         'School or college',
         'Other',
@@ -54,6 +55,7 @@ def test_get_should_render_add_service_template(
         'local',
         'nhs_central',
         'nhs_local',
+        'nhs_local',
         'emergency_service',
         'school_or_college',
         'other',
@@ -175,20 +177,11 @@ def test_add_service_has_to_choose_org_type(
     'test@nhs.uk',
     'test@example.NhS.uK',
     'test@EXAMPLE.NHS.NET',
-    pytest.param(
-        'test@not-nhs.uk',
-        marks=pytest.mark.xfail(raises=AssertionError)
-    )
 ))
-def test_add_service_guesses_org_type_for_unknown_nhs_orgs(
-    mocker,
+def test_get_should_only_show_nhs_org_types_radios_if_user_has_nhs_email(
     client_request,
-    mock_create_service,
-    mock_create_service_template,
-    mock_get_services_with_no_services,
+    mocker,
     api_user_active,
-    mock_create_or_update_free_sms_fragment_limit,
-    mock_get_all_email_branding,
     email_address,
 ):
     api_user_active['email_address'] = email_address
@@ -197,11 +190,23 @@ def test_add_service_guesses_org_type_for_unknown_nhs_orgs(
         'app.organisations_client.get_organisation_by_domain',
         return_value=None,
     )
-    client_request.post(
-        'main.add_service',
-        _data={'name': 'example'},
-    )
-    assert mock_create_service.call_args[1]['organisation_type'] == 'nhs_local'
+    page = client_request.get('main.add_service')
+    assert page.select_one('h1').text.strip() == 'About your service'
+    assert page.select_one('input[name=name]')['value'] == ''
+    assert [
+        label.text.strip() for label in page.select('.multiple-choice label')
+    ] == [
+        'NHS – central government agency or public body',
+        'NHS Trust or Clinical Commissioning Group',
+        'GP practice',
+    ]
+    assert [
+        radio['value'] for radio in page.select('.multiple-choice input')
+    ] == [
+        'nhs_central',
+        'nhs_local',
+        'nhs_local',
+    ]
 
 
 @pytest.mark.parametrize('organisation_type, free_allowance', [

tests/app/main/views/test_service_settings.py

@@ -955,7 +955,7 @@ def test_should_not_show_go_live_button_if_checklist_not_complete(
     ),
     (
         None,
-        'nhs',
+        'nhs_local',
         1,
         [{'is_default': True, 'sms_sender': 'KUVOG'}],
         'Change your text message sender name Completed',