Merge pull request #2081 from alphagov/add-domain-constraint-2

Add domain constraint 2

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)

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()]

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'),

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')

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

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'"]