diff --git a/app/assets/stylesheets/components/conditional-radios.scss b/app/assets/stylesheets/components/conditional-radios.scss index 6a85cbb06..e23eccae4 100644 --- a/app/assets/stylesheets/components/conditional-radios.scss +++ b/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; diff --git a/app/assets/stylesheets/components/radios.scss b/app/assets/stylesheets/components/radios.scss index e0dc263d1..ca6b343dc 100644 --- a/app/assets/stylesheets/components/radios.scss +++ b/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; -} diff --git a/app/main/forms.py b/app/main/forms.py index 4dcf5818e..96db524be 100644 --- a/app/main/forms.py +++ b/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): diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index c9b790ed5..430bb609f 100644 --- a/app/main/views/organisations.py +++ b/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//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, diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index 8702490b3..dd56baba0 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -57,7 +57,7 @@ {% macro conditional_radio_panel(id) %} -
+
{{ caller() }}
{% endmacro %} diff --git a/app/templates/views/agreement/agreement-accept.html b/app/templates/views/agreement/agreement-accept.html index 4d1b7e90f..ad5dc1e8b 100644 --- a/app/templates/views/agreement/agreement-accept.html +++ b/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 %} -
+ {% 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') }} -
+ {% endcall %} {{ textbox(form.version, width='1-3', hint='The version number is on the front page, for example ‘3.6’') }} {{ page_footer('Continue') }} diff --git a/app/templates/views/organisations/add-gp-organisation.html b/app/templates/views/organisations/add-gp-organisation.html index d1b713719..91d923f79 100644 --- a/app/templates/views/organisations/add-gp-organisation.html +++ b/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 %} diff --git a/tests/app/main/views/organisations/test_organisation.py b/tests/app/main/views/organisations/test_organisation.py index b9fc159ad..d9bab799b 100644 --- a/tests/app/main/views/organisations/test_organisation.py +++ b/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, diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index 5d487c31e..ee2e70bda 100644 --- a/tests/app/main/views/test_agreement.py +++ b/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' ]