From 4576bdd64fda07866c4bcb228df4aa536bba5bfa Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 13 Feb 2018 14:47:03 +0000 Subject: [PATCH] Reraise integrity errors on org name unique contraints as 400 - currently being raised as 500s, but should really be 400s as the request data has the duplicate name --- app/organisation/rest.py | 18 ++++++++++++++- tests/app/organisation/test_rest.py | 36 +++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 3ebd953b3..68bbe9119 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,4 +1,5 @@ -from flask import Blueprint, jsonify, request +from flask import Blueprint, current_app, jsonify, request +from sqlalchemy.exc import IntegrityError from app.dao.organisation_dao import ( dao_create_organisation, @@ -22,6 +23,21 @@ organisation_blueprint = Blueprint('organisation', __name__) register_errors(organisation_blueprint) +@organisation_blueprint.errorhandler(IntegrityError) +def handle_integrity_error(exc): + """ + Handle integrity errors caused by the unique contraint on ix_organisation_name + """ + if 'ix_organisation_name' in str(exc): + current_app.logger.exception('Unique constraint ix_organisation_name triggered') + return jsonify( + result='error', + message='Organisation name already exists' + ), 400 + + raise + + @organisation_blueprint.route('', methods=['GET']) def get_organisations(): organisations = [ diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index b7135f854..0bcd56381 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -53,6 +53,24 @@ def test_post_create_organisation(admin_request, notify_db_session): assert len(organisation) == 1 +def test_post_create_organisation_existing_name_raises_400(admin_request, sample_organisation): + data = { + 'name': sample_organisation.name, + 'active': True + } + + response = admin_request.post( + 'organisation.create_organisation', + _data=data, + _expected_status=400 + ) + + organisation = Organisation.query.all() + + assert len(organisation) == 1 + assert response['message'] == 'Organisation name already exists' + + def test_post_create_organisation_with_missing_name_gives_validation_error(admin_request, notify_db_session): data = { 'active': False @@ -91,6 +109,24 @@ def test_post_update_organisation_updates_fields(admin_request, notify_db_sessio assert organisation[0].active == data['active'] +def test_post_update_organisation_raises_400_on_existing_org_name( + admin_request, notify_db_session, sample_organisation): + org = create_organisation() + data = { + 'name': sample_organisation.name, + 'active': False + } + + response = admin_request.post( + 'organisation.update_organisation', + _data=data, + organisation_id=org.id, + _expected_status=400 + ) + + assert response['message'] == 'Organisation name already exists' + + def test_post_update_organisation_gives_404_status_if_org_does_not_exist(admin_request, notify_db_session): data = {'name': 'new organisation name'}