mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-24 16:24:08 -04:00
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]:8f99da525d/app/organisation/rest.py (L34-L47)[2]:70b606a2d4/app/main/views/manage_users.py (L166)[3]:70b606a2d4/app/main/views/templates.py (L499)[4]:70b606a2d4/app/main/views/add_service.py (L30)[5]:70b606a2d4/app/main/views/service_settings.py (L102-L104)[6]:70b606a2d4/app/main/views/organisations.py (L264-L266)[7]:0abc143147/tests/conftest.py (L590-L606)
This commit is contained in:
@@ -62,10 +62,17 @@ def add_organisation():
|
|||||||
form = NewOrganisationForm()
|
form = NewOrganisationForm()
|
||||||
|
|
||||||
if form.validate_on_submit():
|
if form.validate_on_submit():
|
||||||
return redirect(url_for(
|
try:
|
||||||
'.organisation_settings',
|
return redirect(url_for(
|
||||||
org_id=Organisation.create_from_form(form).id,
|
'.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(
|
return render_template(
|
||||||
'views/organisations/add-organisation.html',
|
'views/organisations/add-organisation.html',
|
||||||
|
|||||||
@@ -194,6 +194,37 @@ def test_create_new_organisation_fails_with_incorrect_input(
|
|||||||
assert error_message in page.select_one('.govuk-error-message').text
|
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', (
|
@pytest.mark.parametrize('organisation_type, organisation, expected_status', (
|
||||||
('nhs_gp', None, 200),
|
('nhs_gp', None, 200),
|
||||||
('central', None, 403),
|
('central', None, 403),
|
||||||
|
|||||||
Reference in New Issue
Block a user