From 0cdbb850aa61224d0041b6113be3e0fae41ab2d9 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 26 Jun 2020 20:39:20 +0100 Subject: [PATCH 1/2] Convert radios in edit org type page (basic) Changes the OrganisationTypeField class used by OrganisationOrganisationTypeForm.organisation_type OrganisationTypeField is also used by the forms in /add-service: - CreateServiceForm - CreateNhsServiceForm Because of that, this commit also includes changes to the template for that route. Note: this also moves where OrganisationTypeField appears in app/main/forms.py so it can use GovukRadiosField. --- app/main/forms.py | 40 +++++++++---------- app/templates/views/add-service.html | 3 +- .../organisation/settings/edit-type.html | 3 +- tests/app/main/views/test_add_service.py | 12 +++--- 4 files changed, 28 insertions(+), 30 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index c8c76c816..918178907 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -426,26 +426,6 @@ class ForgivingIntegerField(GovukTextInputField): return super().__call__(value=value, **kwargs) -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 - ], - thing='the type of organisation', - validators=validators or [], - **kwargs - ) - - class FieldWithNoneOption(): # This is a special value that is specific to our forms. This is @@ -951,6 +931,26 @@ class OnOffField(GovukRadiosField): ) +class OrganisationTypeField(GovukRadiosField): + 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 + ], + thing='the type of organisation', + validators=validators or [], + **kwargs + ) + + # guard against data entries that aren't a role in permissions def filter_by_permissions(valuelist): if valuelist is None: diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index 66d25fa0e..d15373641 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -1,5 +1,4 @@ {% extends "withoutnav_template.html" %} -{% from "components/radios.html" import radios %} {% from "components/page-header.html" import page_header %} {% from "components/page-footer.html" import page_footer %} {% from "components/form.html" import form_wrapper %} @@ -20,7 +19,7 @@ {{ form.name(param_extensions={"hint": {"text": "You can change this later"}}) }} {% if not default_organisation_type %} - {{ radios(form.organisation_type) }} + {{ form.organisation_type }} {% endif %} {{ page_footer('Add service') }} diff --git a/app/templates/views/organisations/organisation/settings/edit-type.html b/app/templates/views/organisations/organisation/settings/edit-type.html index 4ce32d672..7761cc302 100644 --- a/app/templates/views/organisations/organisation/settings/edit-type.html +++ b/app/templates/views/organisations/organisation/settings/edit-type.html @@ -1,4 +1,3 @@ -{% from "components/radios.html" import radios %} {% from "components/page-footer.html" import page_footer %} {% from "components/page-header.html" import page_header %} {% from "components/form.html" import form_wrapper %} @@ -15,7 +14,7 @@ back_link=url_for('.organisation_settings', org_id=current_org.id) ) }} {% call form_wrapper() %} - {{ radios(form.organisation_type) }} + {{ form.organisation_type }} {{ page_footer('Save') }} {% endcall %} {% endblock %} diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 150077515..29f11b92c 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -37,7 +37,7 @@ def test_get_should_render_add_service_template( assert page.select_one('h1').text.strip() == 'About your service' assert page.select_one('input[name=name]').get('value') is None assert [ - label.text.strip() for label in page.select('.multiple-choice label') + label.text.strip() for label in page.select('.govuk-radios__item label') ] == [ 'Central government', 'Local government', @@ -49,7 +49,7 @@ def test_get_should_render_add_service_template( 'Other', ] assert [ - radio['value'] for radio in page.select('.multiple-choice input') + radio['value'] for radio in page.select('.govuk-radios__item input') ] == [ 'central', 'local', @@ -193,8 +193,8 @@ def test_add_service_has_to_choose_org_type( }, _expected_status=200, ) - assert normalize_spaces(page.select_one('.error-message').text) == ( - 'Select the type of organisation' + assert normalize_spaces(page.select_one('.govuk-error-message').text) == ( + 'Error: Select the type of organisation' ) assert mock_create_service.called is False assert mock_create_service_template.called is False @@ -223,14 +223,14 @@ def test_get_should_only_show_nhs_org_types_radios_if_user_has_nhs_email( assert page.select_one('h1').text.strip() == 'About your service' assert page.select_one('input[name=name]').get('value') is None assert [ - label.text.strip() for label in page.select('.multiple-choice label') + label.text.strip() for label in page.select('.govuk-radios__item 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') + radio['value'] for radio in page.select('.govuk-radios__item input') ] == [ 'nhs_central', 'nhs_local', From b1d0d216e0bfb055ab7428733777c47ae53281cb Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 26 Jun 2020 17:47:16 +0100 Subject: [PATCH 2/2] Convert radios on add org page Changes OrganisationCrownStatusForm.crown_status. This also effects NewOrganisationForm, which inherits from OrganisationCrownStatusForm. Because of that this commit also updates the template used for the edit org crown status page, which uses NewOrganisationForm for its form. --- app/main/forms.py | 6 ++---- app/templates/views/organisations/add-organisation.html | 5 ++--- .../organisation/settings/edit-crown-status.html | 3 +-- tests/app/main/views/organisations/test_organisation.py | 6 +++--- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 918178907..726195f49 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1168,10 +1168,8 @@ class OrganisationOrganisationTypeForm(StripWhitespaceForm): class OrganisationCrownStatusForm(StripWhitespaceForm): - crown_status = RadioField( - ( - 'Is this organisation a crown body?' - ), + crown_status = GovukRadiosField( + 'Is this organisation a crown body?', choices=[ ('crown', 'Yes'), ('non-crown', 'No'), diff --git a/app/templates/views/organisations/add-organisation.html b/app/templates/views/organisations/add-organisation.html index 8414df2ba..30c09bd8f 100644 --- a/app/templates/views/organisations/add-organisation.html +++ b/app/templates/views/organisations/add-organisation.html @@ -1,7 +1,6 @@ {% extends "withoutnav_template.html" %} {% from "components/page-header.html" import page_header %} {% from "components/page-footer.html" import page_footer %} -{% from "components/radios.html" import radios %} {% from "components/form.html" import form_wrapper %} {% block per_page_title %} @@ -26,8 +25,8 @@ {{ page_header('New organisation') }} {% call form_wrapper() %} {{ form.name }} - {{ radios(form.organisation_type) }} - {{ radios(form.crown_status) }} + {{ form.organisation_type }} + {{ form.crown_status }} {{ page_footer('Save') }} {% endcall %} {% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/edit-crown-status.html b/app/templates/views/organisations/organisation/settings/edit-crown-status.html index 1ab642015..4f325eabf 100644 --- a/app/templates/views/organisations/organisation/settings/edit-crown-status.html +++ b/app/templates/views/organisations/organisation/settings/edit-crown-status.html @@ -1,4 +1,3 @@ -{% from "components/radios.html" import radios %} {% from "components/page-footer.html" import page_footer %} {% from "components/page-header.html" import page_header %} {% from "components/form.html" import form_wrapper %} @@ -17,7 +16,7 @@
{% call form_wrapper() %} - {{ radios(form.crown_status) }} + {{ form.crown_status }} {{ page_footer('Save') }} {% endcall %}
diff --git a/tests/app/main/views/organisations/test_organisation.py b/tests/app/main/views/organisations/test_organisation.py index 28ee9606c..7d3e12de6 100644 --- a/tests/app/main/views/organisations/test_organisation.py +++ b/tests/app/main/views/organisations/test_organisation.py @@ -155,11 +155,11 @@ def test_create_new_organisation_validates( ) assert [ (error['data-error-label'], normalize_spaces(error.text)) - for error in page.select('.govuk-error-message, .error-message') + for error in page.select('.govuk-error-message') ] == [ ('name', 'Error: Cannot be empty'), - ('organisation_type', 'Select the type of organisation'), - ('crown_status', 'Select whether this organisation is a crown body'), + ('organisation_type', 'Error: Select the type of organisation'), + ('crown_status', 'Error: Select whether this organisation is a crown body'), ] assert mock_create_organisation.called is False