diff --git a/.gitignore b/.gitignore index 18542dd47..e4f8ff58e 100644 --- a/.gitignore +++ b/.gitignore @@ -72,3 +72,5 @@ celerybeat-schedule # CloudFoundry .cf + +/scripts/run_my_tests.sh diff --git a/app/errors.py b/app/errors.py index 1a2d3e479..c2461f3cd 100644 --- a/app/errors.py +++ b/app/errors.py @@ -3,7 +3,7 @@ from flask import ( current_app, json) from notifications_utils.recipients import InvalidEmailError -from sqlalchemy.exc import SQLAlchemyError, DataError +from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from marshmallow import ValidationError from jsonschema import ValidationError as JsonSchemaValidationError @@ -95,20 +95,6 @@ def register_errors(blueprint): current_app.logger.info(e) return jsonify(result='error', message="No result found"), 404 - @blueprint.errorhandler(SQLAlchemyError) - def db_error(e): - if hasattr(e, 'orig') and hasattr(e.orig, 'pgerror') and e.orig.pgerror and \ - ('duplicate key value violates unique constraint "services_name_key"' in e.orig.pgerror or - 'duplicate key value violates unique constraint "services_email_from_key"' in e.orig.pgerror): - return jsonify( - result='error', - message={'name': ["Duplicate service name '{}'".format( - e.params.get('name', e.params.get('email_from', '')) - )]} - ), 400 - current_app.logger.exception(e) - return jsonify(result='error', message="Internal server error"), 500 - # this must be defined after all other error handlers since it catches the generic Exception object @blueprint.app_errorhandler(500) @blueprint.errorhandler(Exception) diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 0bb5ff3ae..6e29c9e89 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,4 +1,4 @@ -from flask import Blueprint, jsonify, request +from flask import Blueprint, jsonify, request, current_app from sqlalchemy.exc import IntegrityError from app.dao.organisation_dao import ( @@ -26,12 +26,14 @@ 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 + Handle integrity errors caused by the unique constraint on ix_organisation_name """ if 'ix_organisation_name' in str(exc): - raise InvalidRequest('Organisation name already exists', 400) + return jsonify(result="error", + message="Organisation name already exists"), 400 - raise + current_app.logger.exception(exc) + return jsonify(result='error', message="Internal server error"), 500 @organisation_blueprint.route('', methods=['GET']) @@ -57,7 +59,6 @@ def create_organisation(): organisation = Organisation(**data) dao_create_organisation(organisation) - return jsonify(organisation.serialize()), 201 @@ -66,7 +67,6 @@ def update_organisation(organisation_id): data = request.get_json() validate(data, post_update_organisation_schema) result = dao_update_organisation(organisation_id, **data) - if result: return '', 204 else: diff --git a/app/service/rest.py b/app/service/rest.py index 8d74124a2..99f03cee9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -7,6 +7,7 @@ from flask import ( current_app, Blueprint ) +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound from app.dao import notifications_dao @@ -94,6 +95,22 @@ service_blueprint = Blueprint('service', __name__) register_errors(service_blueprint) +@service_blueprint.errorhandler(IntegrityError) +def handle_integrity_error(exc): + """ + Handle integrity errors caused by the unique constraint on ix_organisation_name + """ + if 'services_name_key' or 'services_email_from_key' in str(exc): + return jsonify( + result='error', + message={'name': ["Duplicate service name '{}'".format( + exc.params.get('name', exc.params.get('email_from', '')) + )]} + ), 400 + current_app.logger.exception(exc) + return jsonify(result='error', message="Internal server error"), 500 + + @service_blueprint.route('/platform-stats', methods=['GET']) def get_platform_stats(): include_from_test_key = request.args.get('include_from_test_key', 'True') != 'False' diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index a6fa6daaa..676d72a22 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -1,6 +1,3 @@ -import pytest - -from app.errors import InvalidRequest from app.models import Organisation from app.dao.organisation_dao import dao_add_service_to_organisation from tests.app.db import create_organisation, create_service @@ -62,17 +59,16 @@ def test_post_create_organisation_existing_name_raises_400(admin_request, sample 'active': True } - with pytest.raises(InvalidRequest) as e: - admin_request.post( - 'organisation.create_organisation', - _data=data, - _expected_status=400 - ) + response = admin_request.post( + 'organisation.create_organisation', + _data=data, + _expected_status=400 + ) organisation = Organisation.query.all() assert len(organisation) == 1 - assert 'Organisation name already exists' in str(e.value) + assert response['message'] == 'Organisation name already exists' def test_post_create_organisation_with_missing_name_gives_validation_error(admin_request, notify_db_session): @@ -114,22 +110,21 @@ def test_post_update_organisation_updates_fields(admin_request, notify_db_sessio def test_post_update_organisation_raises_400_on_existing_org_name( - admin_request, notify_db_session, sample_organisation): + admin_request, sample_organisation): org = create_organisation() data = { 'name': sample_organisation.name, 'active': False } - with pytest.raises(expected_exception=InvalidRequest) as e: - admin_request.post( - 'organisation.update_organisation', - _data=data, - organisation_id=org.id, - _expected_status=400 - ) + response = admin_request.post( + 'organisation.update_organisation', + _data=data, + organisation_id=org.id, + _expected_status=400 + ) - assert 'Organisation name already exists' in str(e.value) + 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):