Let GPs nominate service name

Most GP practice services are named after the practice, which is the
organisation.

So rather than make people re-type the name of their organisation (and
potentially make a typo) let’s just let them say ‘yes, that’s the name
of my organisation’.

app/assets/stylesheets/components/conditional-radios.scss

@@ -17,8 +17,8 @@ $top-spacing: $gutter - 5px;
 
   &-panel {
     border-left: $border-thickness solid $border-colour;
-    margin: 0 0 (-$top-spacing - $gutter) ($gutter-half + ($border-thickness / 2));
-    padding: $top-spacing 0 0 ($gutter - ($border-thickness / 2));
+    margin: 0 0 (-$gutter-half) ($gutter-half + ($border-thickness / 2));
+    padding: $gutter-half 0 0 ($gutter - ($border-thickness / 2));
     position: relative;
     top: -$top-spacing;
     z-index: 1;

app/assets/stylesheets/components/radios.scss

@@ -111,9 +111,3 @@
   }
 
 }
-
-.conditional-radio-panel {
-  border-left: 4px solid $border-colour;
-  margin: -20px 0 0 17px;
-  padding: 10px 0 0 28px;
-}

app/main/forms.py

@@ -358,6 +358,31 @@ class StripWhitespaceStringField(StringField):
         super(StringField, self).__init__(label, **kwargs)
 
 
+class OnOffField(RadioField):
+
+    def __init__(self, label, choices=None, *args, **kwargs):
+        super().__init__(label, choices=choices or [
+            (True, 'On'),
+            (False, 'Off'),
+        ], *args, **kwargs)
+
+    def process_formdata(self, valuelist):
+        if valuelist:
+            value = valuelist[0]
+            self.data = (value == 'True') if value in ['True', 'False'] else value
+
+    def iter_choices(self):
+        for value, label in self.choices:
+            # This overrides WTForms default behaviour which is to check
+            # self.coerce(value) == self.data
+            # where self.coerce returns a string for a boolean input
+            yield (
+                value,
+                label,
+                (self.data in {value, self.coerce(value)})
+            )
+
+
 class LoginForm(StripWhitespaceForm):
     email_address = EmailField('Email address', validators=[
         Length(min=5, max=255),
@@ -534,6 +559,38 @@ class RenameOrganisationForm(StripWhitespaceForm):
         ])
 
 
+class AddGPOrganisationForm(StripWhitespaceForm):
+
+    def __init__(self, *args, service_name='unknown', **kwargs):
+        super().__init__(*args, **kwargs)
+        self.same_as_service_name.label.text = 'Is your GP practice called ‘{}’?'.format(service_name)
+        self.service_name = service_name
+
+    def get_organisation_name(self):
+        if self.same_as_service_name.data:
+            return self.service_name
+        return self.name.data
+
+    same_as_service_name = OnOffField(
+        'Is your GP practice called the same name as your service?',
+        choices=(
+            (True, 'Yes'),
+            (False, 'No'),
+        ),
+    )
+
+    name = StringField(
+        'What’s your practice called?',
+    )
+
+    def validate_name(self, field):
+        if self.same_as_service_name.data is False:
+            if not field.data:
+                raise ValidationError('Can’t be empty')
+        else:
+            field.data = ''
+
+
 class OrganisationOrganisationTypeForm(StripWhitespaceForm):
     organisation_type = OrganisationTypeField('What type of organisation is this?')
 
@@ -941,19 +998,6 @@ class ServiceLetterContactBlockForm(StripWhitespaceForm):
             )
 
 
-class OnOffField(RadioField):
-    def __init__(self, label, *args, **kwargs):
-        super().__init__(label, choices=[
-            (True, 'On'),
-            (False, 'Off'),
-        ], *args, **kwargs)
-
-    def process_formdata(self, valuelist):
-        if valuelist:
-            value = valuelist[0]
-            self.data = (value == 'True') if value in ['True', 'False'] else value
-
-
 class ServiceOnOffSettingForm(StripWhitespaceForm):
 
     def __init__(self, name, *args, truthy='On', falsey='Off', **kwargs):

app/main/views/organisations.py

@@ -16,6 +16,7 @@ from app import (
 )
 from app.main import main
 from app.main.forms import (
+    AddGPOrganisationForm,
     ConfirmPasswordForm,
     GoLiveNotesForm,
     InviteOrgUserForm,
@@ -67,16 +68,14 @@ def add_organisation():
 @main.route('/services/<uuid:service_id>/add-gp-organisation', methods=['GET', 'POST'])
 @user_has_permissions('manage_service')
 def add_organisation_from_gp_service(service_id):
-    print(current_service.organisation_type)
-    print(current_service.organisation)
     if (not current_service.organisation_type == 'nhs_gp') or current_service.organisation:
         abort(403)
 
-    form = RenameOrganisationForm()
+    form = AddGPOrganisationForm(service_name=current_service.name)
 
     if form.validate_on_submit():
         Organisation.create(
-            form.name.data,
+            form.get_organisation_name(),
             crown=False,
             organisation_type='nhs_gp',
             agreement_signed=False,

app/templates/components/radios.html

@@ -57,7 +57,7 @@
 
 
 {% macro conditional_radio_panel(id) %}
-  <div class="conditional-radios-panel" id="panel-{{ id }}">
+  <div class="conditional-radios-panel js-hidden" id="{{ id }}">
     {{ caller() }}
   </div>
 {% endmacro %}

app/templates/views/agreement/agreement-accept.html

@@ -1,6 +1,6 @@
 {% extends "withnav_template.html" %}
 {% from "components/textbox.html" import textbox %}
-{% from "components/radios.html" import radio %}
+{% from "components/radios.html" import radio, conditional_radio_panel %}
 {% from "components/select-input.html" import select_wrapper %}
 {% from "components/form.html" import form_wrapper %}
 {% from "components/page-footer.html" import page_footer %}
@@ -27,10 +27,10 @@
           {{ radio(option, data_target='on-behalf-of' if option.data == 'someone-else' else None) }}
         {% endfor %}
       {% endcall %}
-      <div class="conditional-radio-panel js-hidden" id="on-behalf-of">
+      {% call conditional_radio_panel('on-behalf-of') %}
         {{ textbox(form.on_behalf_of_name, width='1-1') }}
         {{ textbox(form.on_behalf_of_email, width='1-1') }}
-      </div>
+      {% endcall %}
       {{ textbox(form.version, width='1-3', hint='The version number is on the front page, for example ‘3.6’') }}
 
       {{ page_footer('Continue') }}

app/templates/views/organisations/add-gp-organisation.html

@@ -2,17 +2,28 @@
 {% from "components/page-header.html" import page_header %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/textbox.html" import textbox %}
-{% from "components/radios.html" import radios %}
+{% from "components/radios.html" import radio, conditional_radio_panel %}
+{% from "components/select-input.html" import select_wrapper %}
 {% from "components/form.html" import form_wrapper %}
 
 {% block per_page_title %}
-  What is the name of your organisation?
+  Accept our data sharing and financial agreement
 {% endblock %}
 
 {% block maincolumn_content %}
-  {{ page_header('What is the name of your organisation?') }}
+  {{ page_header(
+    'Accept our data sharing and financial agreement',
+    back_link=url_for('main.request_to_go_live', service_id=current_service.id)
+  ) }}
   {% call form_wrapper() %}
-    {{textbox(form.name)}}
+    {% call select_wrapper(form.same_as_service_name) %}
+      {% for option in form.same_as_service_name %}
+        {{ radio(option, data_target='custom-organisation-name' if option.data == False else '') }}
+      {% endfor %}
+    {% endcall %}
+    {% call conditional_radio_panel('custom-organisation-name') %}
+      {{ textbox(form.name) }}
+    {% endcall %}
     {{ page_footer('Continue') }}
   {% endcall %}
 {% endblock %}

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

@@ -193,11 +193,29 @@ def test_only_gps_can_create_own_organisations(
     )
 
 
+@pytest.mark.parametrize('data, expected_service_name', (
+    (
+        {
+            'same_as_service_name': False,
+            'name': 'Dr. Example',
+        },
+        'Dr. Example',
+    ),
+    (
+        {
+            'same_as_service_name': True,
+            'name': 'This is ignored',
+        },
+        'service one',
+    ),
+))
 def test_gps_can_name_their_organisation(
     client_request,
     mocker,
     service_one,
     mock_update_service_organisation,
+    data,
+    expected_service_name,
 ):
     mocker.patch('app.organisations_client.get_service_organisation', return_value=None)
     service_one['organisation_type'] = 'nhs_gp'
@@ -209,9 +227,7 @@ def test_gps_can_name_their_organisation(
     client_request.post(
         '.add_organisation_from_gp_service',
         service_id=SERVICE_ONE_ID,
-        _data={
-            'name': 'Dr. Example',
-        },
+        _data=data,
         _expected_status=302,
         _expected_redirect=url_for(
             'main.service_agreement',
@@ -221,7 +237,7 @@ def test_gps_can_name_their_organisation(
     )
 
     mock_create_organisation.assert_called_once_with(
-        name='Dr. Example',
+        name=expected_service_name,
         organisation_type='nhs_gp',
         agreement_signed=False,
         crown=False,
@@ -229,6 +245,39 @@ def test_gps_can_name_their_organisation(
     mock_update_service_organisation.assert_called_once_with(SERVICE_ONE_ID, ORGANISATION_ID)
 
 
+@pytest.mark.parametrize('data, expected_error', (
+    (
+        {
+            'name': 'Dr. Example',
+        },
+        'Not a valid choice',
+    ),
+    (
+        {
+            'same_as_service_name': False,
+            'name': '',
+        },
+        'Can’t be empty',
+    ),
+))
+def test_validation_of_gps_creating_organisations(
+    client_request,
+    mocker,
+    service_one,
+    data,
+    expected_error,
+):
+    mocker.patch('app.organisations_client.get_service_organisation', return_value=None)
+    service_one['organisation_type'] = 'nhs_gp'
+    page = client_request.post(
+        '.add_organisation_from_gp_service',
+        service_id=SERVICE_ONE_ID,
+        _data=data,
+        _expected_status=200,
+    )
+    assert normalize_spaces(page.select_one('.error-message').text) == expected_error
+
+
 def test_organisation_services_shows_live_services_only(
     client_request,
     mock_get_organisation,

tests/app/main/views/test_agreement.py

@@ -222,7 +222,7 @@ def test_show_accept_agreement_page(
     assert page.select('.multiple-choice')[1]['data-target'] == 'on-behalf-of'
     assert [
         field['name']
-        for field in page.select('#on-behalf-of.conditional-radio-panel input')
+        for field in page.select('#on-behalf-of.conditional-radios-panel input')
     ] == [
         'on_behalf_of_name', 'on_behalf_of_email'
     ]