Merge pull request #3038 from alphagov/add-more-info-when-creating-org

Require more information when creating organisations

app/main/forms.py

@@ -594,6 +594,17 @@ class OrganisationTypeForm(StripWhitespaceForm):
     organisation_type = organisation_type()
 
 
+class NewOrganisationForm(
+    RenameOrganisationForm,
+    OrganisationOrganisationTypeForm,
+    OrganisationCrownStatusForm,
+):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        # Don’t offer the ‘not sure’ choice
+        self.crown_status.choices = self.crown_status.choices[:-1]
+
+
 class FreeSMSAllowance(StripWhitespaceForm):
     free_sms_allowance = IntegerField(
         'Numbers of text message fragments per year',
@@ -1039,11 +1050,6 @@ class PDFUploadForm(StripWhitespaceForm):
     )
 
 
-class CreateOrUpdateOrganisation(StripWhitespaceForm):
-
-    name = StringField('Name', validators=[DataRequired()])
-
-
 class EmailFieldInWhitelist(EmailField, StripWhitespaceStringField):
     pass
 

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,
@@ -31,7 +31,7 @@ from app.main.forms import (
     SetLetterBranding,
 )
 from app.main.views.service_settings import get_branding_as_value_and_label
-from app.models.organisation import Organisations
+from app.models.organisation import Organisation, Organisations
 from app.models.user import InvitedOrgUser, User
 from app.utils import user_has_permissions, user_is_platform_admin
 
@@ -49,14 +49,13 @@ def organisations():
 @main.route("/organisations/add", methods=['GET', 'POST'])
 @user_is_platform_admin
 def add_organisation():
-    form = CreateOrUpdateOrganisation()
+    form = NewOrganisationForm()
 
     if form.validate_on_submit():
-        organisations_client.create_organisation(
+        return redirect(url_for(
-            name=form.name.data,
+            '.organisation_settings',
-        )
+            org_id=Organisation.create_from_form(form).id,
-
+        ))
-        return redirect(url_for('.organisations'))
 
     return render_template(
         'views/organisations/add-organisation.html',

app/models/organisation.py

@@ -38,6 +38,27 @@ class Organisation(JSONModel):
     def from_service(cls, service_id):
         return cls(organisations_client.get_service_organisation(service_id))
 
+    @classmethod
+    def create_from_form(cls, form):
+        return cls.create(
+            name=form.name.data,
+            crown={
+                'crown': True,
+                'non-crown': False,
+                'unknown': None,
+            }.get(form.crown_status.data),
+            organisation_type=form.organisation_type.data,
+        )
+
+    @classmethod
+    def create(cls, name, crown, organisation_type, agreement_signed=False):
+        return cls(organisations_client.create_organisation(
+            name=name,
+            crown=crown,
+            organisation_type=organisation_type,
+            agreement_signed=agreement_signed,
+        ))
+
     def __init__(self, _dict):
 
         super().__init__(_dict)

app/navigation.py

@@ -80,7 +80,6 @@ class HeaderNavigation(Navigation):
             'user_profile_disable_platform_admin_view',
         },
         'platform-admin': {
-            'add_organisation',
             'archive_user',
             'clear_cache',
             'create_email_branding',
@@ -120,6 +119,7 @@ class HeaderNavigation(Navigation):
         'accept_org_invite',
         'action_blocked',
         'add_data_retention',
+        'add_organisation',
         'add_service',
         'add_service_template',
         'agreement',

app/notify_client/organisations_api_client.py

@@ -32,11 +32,16 @@ class OrganisationsClient(NotifyAdminAPIClient):
             raise error
 
     @cache.delete('organisations')
-    def create_organisation(self, name):
+    def create_organisation(self, name, crown, organisation_type, agreement_signed):
-        data = {
+        return self.post(
-            "name": name
+            url="/organisations",
-        }
+            data={
-        return self.post(url="/organisations", data=data)
+                "name": name,
+                "crown": crown,
+                "organisation_type": organisation_type,
+                "agreement_signed": agreement_signed,
+            }
+        )
 
     @cache.delete('domains')
     @cache.delete('organisations')

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

@@ -1,27 +1,39 @@
-{% extends "views/platform-admin/_base_template.html" %}
+{% extends "withoutnav_template.html" %}
 {% 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
+  New organisation
 {% endblock %}
 
-{% block org_page_title %}
+{% block fullwidth_content %}
-  Create an organisation
+  <div id="content">
-{% endblock %}
+    <div class="navigation-service">
+      <div class="navigation-service-name">
+        <a href="{{ url_for('.organisations') }}">All organisations</a>
+      </div>
+      <a href="{{ url_for('main.choose_account') }}" class="navigation-service-switch">Switch service</a>
+    </div>
 
-{% block platform_admin_content %}
+    <main role="main">
-
+      <div class="grid-row">
-  {{ page_header(
+        <div class="column-one-quarter">
-    'New organisation',
+          &nbsp;
-    back_link=url_for('.organisations')
+        </div>
-  ) }}
+        <div class="column-three-quarters">
-
+          {{ page_header('New organisation') }}
-  {% call form_wrapper() %}
+          {% call form_wrapper() %}
-    {{textbox(form.name)}}
+            {{textbox(form.name)}}
-    {{ page_footer('Save') }}
+            {{radios(form.organisation_type)}}
-  {% endcall %}
+            {{radios(form.crown_status)}}
+            {{ page_footer('Save') }}
+          {% endcall %}
+        </div>
+      </div>
+    </main>
+  </div>
 
 {% endblock %}

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,84 @@ def test_view_organisation_shows_the_correct_organisation(
     assert normalize_spaces(page.select_one('h1').text) == 'Usage'
 
 
-def test_create_new_organisation(
+def test_page_to_create_new_organisation(
-    logged_in_platform_admin_client,
+    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'),
+        ('hidden', 'csrf_token', ANY),
+    ]
+
+
+def test_create_new_organisation(
+    client_request,
+    platform_admin_user,
+    mocker,
+):
+    mock_create_organisation = mocker.patch(
+        'app.organisations_client.create_organisation',
+        return_value=organisation_json(ORGANISATION_ID),
+    )
+
+    client_request.login(platform_admin_user)
+    client_request.post(
+        '.add_organisation',
+        _data={
+            'name': 'new name',
+            'organisation_type': 'local',
+            'crown_status': 'non-crown',
+        },
+        _expected_redirect=url_for(
+            'main.organisation_settings',
+            org_id=ORGANISATION_ID,
+            _external=True,
+        ),
+    )
+
+    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,
-    fake_uuid
 ):
     mock_create_organisation = mocker.patch(
         'app.organisations_client.create_organisation'
     )
 
-    org = {'name': 'new name'}
+    client_request.login(platform_admin_user)
-
+    page = client_request.post(
-    logged_in_platform_admin_client.post(
+        '.add_organisation',
-        url_for('.add_organisation'),
+        _expected_status=200,
-        content_type='multipart/form-data',
-        data=org
     )
-
+    assert [
-    mock_create_organisation.assert_called_once_with(name=org['name'])
+        (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(