diff --git a/app/dao/email_branding_dao.py b/app/dao/email_branding_dao.py index bcf4a93c8..87ccc61cf 100644 --- a/app/dao/email_branding_dao.py +++ b/app/dao/email_branding_dao.py @@ -23,5 +23,5 @@ def dao_create_email_branding(email_branding): @transactional def dao_update_email_branding(email_branding, **kwargs): for key, value in kwargs.items(): - setattr(email_branding, key, value) + setattr(email_branding, key, value or None) db.session.add(email_branding) diff --git a/app/email_branding/rest.py b/app/email_branding/rest.py index 0991c9d0b..990f1905e 100644 --- a/app/email_branding/rest.py +++ b/app/email_branding/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.email_branding_dao import ( dao_create_email_branding, @@ -18,6 +19,22 @@ email_branding_blueprint = Blueprint('email_branding', __name__) register_errors(email_branding_blueprint) +@email_branding_blueprint.errorhandler(IntegrityError) +def handle_integrity_error(exc): + """ + Handle integrity errors caused by the unique constraint on domain + """ + if 'domain' in str(exc): + return jsonify( + result='error', + message={'name': ["Duplicate domain '{}'".format( + exc.params.get('domain') + )]} + ), 400 + current_app.logger.exception(exc) + return jsonify(result='error', message="Internal server error"), 500 + + @email_branding_blueprint.route('', methods=['GET']) def get_email_branding_options(): email_branding_options = [o.serialize() for o in dao_get_email_branding_options()] diff --git a/app/models.py b/app/models.py index 39e7b2716..353b6e56c 100644 --- a/app/models.py +++ b/app/models.py @@ -207,7 +207,7 @@ class EmailBranding(db.Model): logo = db.Column(db.String(255), nullable=True) name = db.Column(db.String(255), nullable=True) text = db.Column(db.String(255), nullable=True) - domain = db.Column(db.Text, nullable=True) + domain = db.Column(db.Text, unique=True, nullable=True) brand_type = db.Column( db.String(255), db.ForeignKey('branding_type.name'), diff --git a/migrations/versions/0223_add_domain_constraint.py b/migrations/versions/0223_add_domain_constraint.py new file mode 100644 index 000000000..8d3f808dc --- /dev/null +++ b/migrations/versions/0223_add_domain_constraint.py @@ -0,0 +1,28 @@ +""" +Revision ID: 0223_add_domain_constraint +Revises: 0222_drop_service_branding +Create Date: 2018-08-24 13:36:49.346156 + """ +from alembic import op + + +revision = '0223_add_domain_constraint' +down_revision = '0222_drop_service_branding' + + +def upgrade(): + + op.execute(""" + update + email_branding + set + domain = null + where + domain = '' + """) + op.create_unique_constraint('uq_email_branding_domain', 'email_branding', ['domain']) + + +def downgrade(): + + op.drop_constraint('uq_email_branding_domain', 'email_branding') diff --git a/tests/app/dao/test_email_branding_dao.py b/tests/app/dao/test_email_branding_dao.py index fc150beca..5f741da3e 100644 --- a/tests/app/dao/test_email_branding_dao.py +++ b/tests/app/dao/test_email_branding_dao.py @@ -51,3 +51,15 @@ def test_update_email_branding(notify_db, notify_db_session): assert len(email_branding) == 1 assert email_branding[0].name == updated_name + + +def test_domain_cant_be_empty_string(notify_db, notify_db_session): + create_email_branding() + + email_branding = EmailBranding.query.all() + assert email_branding[0].domain is None + + dao_update_email_branding(email_branding[0], domain='') + + email_branding = EmailBranding.query.all() + assert email_branding[0].domain is None diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 03311514a..7c7954887 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -257,3 +257,30 @@ def test_update_email_branding_reject_invalid_brand_type(admin_request, notify_d ) assert response['errors'][0]['message'] == 'brand_type NOT A TYPE is not one of [org, both, org_banner]' + + +def test_400_for_duplicate_domain(admin_request, notify_db_session): + branding_1 = create_email_branding() + branding_2 = create_email_branding() + admin_request.post( + 'email_branding.update_email_branding', + _data={'domain': 'example.com'}, + email_branding_id=branding_1.id, + ) + + response = admin_request.post( + 'email_branding.update_email_branding', + _data={'domain': 'example.com'}, + email_branding_id=branding_2.id, + _expected_status=400, + ) + assert response['result'] == 'error' + assert response['message']['name'] == ["Duplicate domain 'example.com'"] + + response = admin_request.post( + 'email_branding.create_email_branding', + _data={'domain': 'example.com'}, + _expected_status=400, + ) + assert response['result'] == 'error' + assert response['message']['name'] == ["Duplicate domain 'example.com'"]