From 52a5da4d175ed5739e21fd8283b094e45faccb77 Mon Sep 17 00:00:00 2001
From: Ben Thorner <ben.thorner@digital.cabinet-office.gov.uk>
Date: Thu, 11 Feb 2021 12:02:42 +0000
Subject: [PATCH] Handle exception is org name already exists

Previously this would return a 500 error, as the 400 exception was
not handled from the API [1]. Note that:

- We tend to rely on exception messages to identify the error that
occurred [2][3], with services being a notable deviation [4]. I've
used the exception message approach, as this is more granular and
broadly consistent with the rest of the app.

- There is already code to cover this scenario when a user changes
the name of an existing organisation or service, but the mechanism
is different [5][6]. It makes sense to just get any error from the
call to try and create the organisation.

- The API mock is based on one for services [7], but I've chosen to
have it inline with the test, since we're unlikely to reuse it, and
it's clearer to have the test setup as part of the test.

[1]: https://github.com/alphagov/notifications-api/blob/8f99da525dad3bf653a4e1f9e4a7b7b689219d78/app/organisation/rest.py#L34-L47
[2]: https://github.com/alphagov/notifications-admin/blob/70b606a2d4cb03f2bd9f3a5742a40970a928e696/app/main/views/manage_users.py#L166
[3]: https://github.com/alphagov/notifications-admin/blob/70b606a2d4cb03f2bd9f3a5742a40970a928e696/app/main/views/templates.py#L499
[4]: https://github.com/alphagov/notifications-admin/blob/70b606a2d4cb03f2bd9f3a5742a40970a928e696/app/main/views/add_service.py#L30
[5]: https://github.com/alphagov/notifications-admin/blob/70b606a2d4cb03f2bd9f3a5742a40970a928e696/app/main/views/service_settings.py#L102-L104
[6]: https://github.com/alphagov/notifications-admin/blob/70b606a2d4cb03f2bd9f3a5742a40970a928e696/app/main/views/organisations.py#L264-L266
[7]: https://github.com/alphagov/notifications-admin/blob/0abc143147c98ac50c059729eb5c1a4755d607fa/tests/conftest.py#L590-L606
---
 app/main/views/organisations.py               | 15 ++++++---
 .../views/organisations/test_organisations.py | 31 +++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py
index 52978bd6d..50c679d50 100644
--- a/app/main/views/organisations.py
+++ b/app/main/views/organisations.py
@@ -62,10 +62,17 @@ def add_organisation():
     form = NewOrganisationForm()
 
     if form.validate_on_submit():
-        return redirect(url_for(
-            '.organisation_settings',
-            org_id=Organisation.create_from_form(form).id,
-        ))
+        try:
+            return redirect(url_for(
+                '.organisation_settings',
+                org_id=Organisation.create_from_form(form).id,
+            ))
+        except HTTPError as e:
+            msg = 'Organisation name already exists'
+            if e.status_code == 400 and msg in e.message:
+                form.name.errors.append("This organisation name is already in use")
+            else:
+                raise e
 
     return render_template(
         'views/organisations/add-organisation.html',
diff --git a/tests/app/main/views/organisations/test_organisations.py b/tests/app/main/views/organisations/test_organisations.py
index bb8f78b38..bfd7b6c7c 100644
--- a/tests/app/main/views/organisations/test_organisations.py
+++ b/tests/app/main/views/organisations/test_organisations.py
@@ -194,6 +194,37 @@ def test_create_new_organisation_fails_with_incorrect_input(
     assert error_message in page.select_one('.govuk-error-message').text
 
 
+def test_create_new_organisation_fails_with_duplicate_name(
+    client_request,
+    platform_admin_user,
+    mocker,
+):
+    def _create(**_kwargs):
+        json_mock = Mock(return_value={'message': 'Organisation name already exists'})
+        resp_mock = Mock(status_code=400, json=json_mock)
+        http_error = HTTPError(response=resp_mock, message="Default message")
+        raise http_error
+
+    mocker.patch(
+        'app.organisations_client.create_organisation',
+        side_effect=_create
+    )
+
+    client_request.login(platform_admin_user)
+    page = client_request.post(
+        '.add_organisation',
+        _data={
+            'name': 'Existing org',
+            'organisation_type': 'local',
+            'crown_status': 'non-crown',
+        },
+        _expected_status=200,
+    )
+
+    error_message = 'This organisation name is already in use'
+    assert error_message in page.select_one('.govuk-error-message').text
+
+
 @pytest.mark.parametrize('organisation_type, organisation, expected_status', (
     ('nhs_gp', None, 200),
     ('central', None, 403),