From 0916b2ba6b84286dac8f923243cc9d42fa8a5812 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 1 Jul 2019 10:19:33 +0100 Subject: [PATCH] Require more information when creating organisations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we set not-very-useful defaults for organisation type and crown status when creating an organisation. This commit adds two field to the form (in addition to the existing name field) to explicitly ask for: - organisation type - crown status We need these for all organisations before we can make any of their services live. This commit also records any new organisation as not having accepted the data sharing and financial agreement, because if we don’t know about the organisation already then they definitely won’t have signed it. --- app/main/forms.py | 13 ++-- app/main/views/organisations.py | 11 ++- .../views/organisations/add-organisation.html | 9 +-- .../views/organisations/test_organisation.py | 77 ++++++++++++++++--- 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 2dc749e6c..3267248c6 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -594,6 +594,14 @@ class OrganisationTypeForm(StripWhitespaceForm): organisation_type = organisation_type() +class NewOrganisationForm( + RenameOrganisationForm, + OrganisationTypeForm, + OrganisationCrownStatusForm, +): + pass + + class FreeSMSAllowance(StripWhitespaceForm): free_sms_allowance = IntegerField( 'Numbers of text message fragments per year', @@ -1039,11 +1047,6 @@ class PDFUploadForm(StripWhitespaceForm): ) -class CreateOrUpdateOrganisation(StripWhitespaceForm): - - name = StringField('Name', validators=[DataRequired()]) - - class EmailFieldInWhitelist(EmailField, StripWhitespaceStringField): pass diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index 78ebfec2d..0fdd0b832 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -16,9 +16,9 @@ from app import ( from app.main import main from app.main.forms import ( ConfirmPasswordForm, - CreateOrUpdateOrganisation, GoLiveNotesForm, InviteOrgUserForm, + NewOrganisationForm, OrganisationAgreementSignedForm, OrganisationCrownStatusForm, OrganisationDomainsForm, @@ -51,11 +51,18 @@ def organisations(): @login_required @user_is_platform_admin def add_organisation(): - form = CreateOrUpdateOrganisation() + form = NewOrganisationForm() if form.validate_on_submit(): organisations_client.create_organisation( name=form.name.data, + crown={ + 'crown': True, + 'non-crown': False, + 'unknown': None, + }.get(form.crown_status.data), + organisation_type=form.organisation_type.data, + agreement_signed=False, ) return redirect(url_for('.organisations')) diff --git a/app/templates/views/organisations/add-organisation.html b/app/templates/views/organisations/add-organisation.html index 19218dcd3..f27b91ebd 100644 --- a/app/templates/views/organisations/add-organisation.html +++ b/app/templates/views/organisations/add-organisation.html @@ -2,14 +2,11 @@ {% 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/form.html" import form_wrapper %} {% block per_page_title %} - Create an organisation -{% endblock %} - -{% block org_page_title %} - Create an organisation + New organisation {% endblock %} {% block platform_admin_content %} @@ -21,6 +18,8 @@ {% call form_wrapper() %} {{textbox(form.name)}} + {{radios(form.organisation_type)}} + {{radios(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 bfe84636d..0430a2802 100644 --- a/tests/app/main/views/organisations/test_organisation.py +++ b/tests/app/main/views/organisations/test_organisation.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock +from unittest.mock import ANY, Mock import pytest from bs4 import BeautifulSoup @@ -75,24 +75,79 @@ def test_view_organisation_shows_the_correct_organisation( assert normalize_spaces(page.select_one('h1').text) == 'Usage' -def test_create_new_organisation( - logged_in_platform_admin_client, +def test_page_to_create_new_organisation( + client_request, + platform_admin_user, + mocker, +): + client_request.login(platform_admin_user) + page = client_request.get('.add_organisation') + + assert [ + (input['type'], input['name'], input['value']) + for input in page.select('input') + ] == [ + ('text', 'name', ''), + ('radio', 'organisation_type', 'central'), + ('radio', 'organisation_type', 'local'), + ('radio', 'organisation_type', 'nhs'), + ('radio', 'crown_status', 'crown'), + ('radio', 'crown_status', 'non-crown'), + ('radio', 'crown_status', 'unknown'), + ('hidden', 'csrf_token', ANY), + ] + + +def test_create_new_organisation( + client_request, + platform_admin_user, mocker, - fake_uuid ): mock_create_organisation = mocker.patch( 'app.organisations_client.create_organisation' ) - org = {'name': 'new name'} - - logged_in_platform_admin_client.post( - url_for('.add_organisation'), - content_type='multipart/form-data', - data=org + client_request.login(platform_admin_user) + client_request.post( + '.add_organisation', + _data={ + 'name': 'new name', + 'organisation_type': 'local', + 'crown_status': 'non-crown', + } ) - mock_create_organisation.assert_called_once_with(name=org['name']) + mock_create_organisation.assert_called_once_with( + name='new name', + organisation_type='local', + crown=False, + agreement_signed=False, + ) + + +def test_create_new_organisation_validates( + client_request, + platform_admin_user, + mocker, +): + mock_create_organisation = mocker.patch( + 'app.organisations_client.create_organisation' + ) + + client_request.login(platform_admin_user) + page = client_request.post( + '.add_organisation', + _expected_status=200, + ) + assert [ + (error['data-error-label'], normalize_spaces(error.text)) + for error in page.select('.error-message') + ] == [ + ('name', 'Can’t be empty'), + ('organisation_type', 'Not a valid choice'), + ('crown_status', 'Not a valid choice'), + ] + assert mock_create_organisation.called is False def test_organisation_services_shows_live_services_only(