From 5d5047fb13673903849ed7fc281d08abab6cd131 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 4 Sep 2018 10:57:47 +0100
Subject: [PATCH] Add unique constraint to email branding domains
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Does two things:

1. Revert "Revert "Add unique constraint to email branding domain""

This reverts commit af9cb30ef30fe3859507a5747a65255b7466af4b.

2. Don’t allow empty string in email branding domain

Columns with multiple `null`s can have a uniqueness constraint. Columns
with multiple empty string values are not considered unique.

This commit:
- removes any duplicate empty string values
- casts empty strings to null string any time these columns are updated

---

Squashed into this single commits because these two things are not
atomic as individual commits.
---
 app/dao/email_branding_dao.py                 |  2 +-
 app/models.py                                 |  2 +-
 .../versions/0223_add_domain_constraint.py    | 28 +++++++++++++++++++
 tests/app/dao/test_email_branding_dao.py      | 12 ++++++++
 4 files changed, 42 insertions(+), 2 deletions(-)
 create mode 100644 migrations/versions/0223_add_domain_constraint.py

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/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