diff --git a/app/main/forms.py b/app/main/forms.py index a23408385..b710316dd 100644 --- a/app/main/forms.py +++ b/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, diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 86019caa1..b1b726189 100644 --- a/app/main/views/add_service.py +++ b/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, ) diff --git a/app/models/user.py b/app/models/user.py index abd7804a2..8810719bc 100644 --- a/app/models/user.py +++ b/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 diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index b284e972e..adf2191cc 100644 --- a/app/templates/views/add-service.html +++ b/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 %} diff --git a/app/templates/views/organisations/organisation/settings/index.html b/app/templates/views/organisations/organisation/settings/index.html index c102b84ac..ce5e4f342 100644 --- a/app/templates/views/organisations/organisation/settings/index.html +++ b/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', diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 1f8bf3f43..05fe3e84b 100644 --- a/app/templates/views/service-settings.html +++ b/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) }} {% endif %} diff --git a/tests/app/main/views/organisations/test_organisation.py b/tests/app/main/views/organisations/test_organisation.py index 357ec7347..7e5eb5d34 100644 --- a/tests/app/main/views/organisations/test_organisation.py +++ b/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'), diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 3106da51c..2a6910067 100644 --- a/tests/app/main/views/test_add_service.py +++ b/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', [ diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index f20ec3f93..4967ef871 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/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',