From 5f73dfdbe9d11d4cea643d5267156a91dd60a179 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Tue, 18 Sep 2018 17:41:25 +0100
Subject: [PATCH] Added constraints to services.postage: not nullable and can
 only containt "first" or "second"

---
 app/dao/services_dao.py                       |  2 --
 app/models.py                                 |  4 ++-
 .../versions/0227_postage_constraints.py      | 33 +++++++++++++++++++
 tests/app/dao/test_services_dao.py            | 22 +++++++------
 4 files changed, 48 insertions(+), 13 deletions(-)
 create mode 100644 migrations/versions/0227_postage_constraints.py

diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py
index fa77b7cd1..40918f7e7 100644
--- a/app/dao/services_dao.py
+++ b/app/dao/services_dao.py
@@ -167,7 +167,6 @@ def dao_create_service(service, user, service_id=None, service_permissions=None)
     service.active = True
     service.research_mode = False
     service.crown = service.organisation_type == 'central'
-    service.postage = 'second'
 
     for permission in service_permissions:
         service_permission = ServicePermission(service_id=service.id, permission=permission)
@@ -181,7 +180,6 @@ def dao_create_service(service, user, service_id=None, service_permissions=None)
 @transactional
 @version_class(Service)
 def dao_update_service(service):
-    service.postage = service.postage or 'second'
     db.session.add(service)
 
 
diff --git a/app/models.py b/app/models.py
index a160b0a33..b61809f89 100644
--- a/app/models.py
+++ b/app/models.py
@@ -353,7 +353,9 @@ class Service(db.Model, Versioned):
     crown = db.Column(db.Boolean, index=False, nullable=False, default=True)
     rate_limit = db.Column(db.Integer, index=False, nullable=False, default=3000)
     contact_link = db.Column(db.String(255), nullable=True, unique=False)
-    postage = db.Column(db.String(255), index=False, nullable=True)
+    postage = db.Column(db.String(255), index=False, nullable=False, default='second')
+
+    CheckConstraint("'postage' in ('first', 'second')")
 
     organisation = db.relationship(
         'Organisation',
diff --git a/migrations/versions/0227_postage_constraints.py b/migrations/versions/0227_postage_constraints.py
new file mode 100644
index 000000000..f0eb861be
--- /dev/null
+++ b/migrations/versions/0227_postage_constraints.py
@@ -0,0 +1,33 @@
+"""
+Revision ID: 0227_postage_constraints
+Revises: 0226_service_postage
+Create Date: 2018-09-13 16:23:59.168877
+"""
+from alembic import op
+import sqlalchemy as sa
+
+revision = '0227_postage_constraints'
+down_revision = '0226_service_postage'
+
+
+def upgrade():
+    op.execute("""
+            update
+                services
+            set
+                postage = 'second'
+        """)
+
+    op.create_check_constraint(
+        'ck_services_postage',
+        'services',
+        "postage in ('second', 'first')"
+    )
+    op.alter_column('services', 'postage', nullable=False)
+
+
+def downgrade():
+    op.drop_constraint('ck_services_postage', 'services')
+    op.alter_column('services', 'postage',
+                    existing_type=sa.VARCHAR(length=255),
+                    nullable=True)
diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py
index 041085813..6ea7123e3 100644
--- a/tests/app/dao/test_services_dao.py
+++ b/tests/app/dao/test_services_dao.py
@@ -3,7 +3,7 @@ from datetime import datetime, timedelta
 
 import pytest
 from freezegun import freeze_time
-from sqlalchemy.exc import IntegrityError
+from sqlalchemy.exc import IntegrityError, SQLAlchemyError
 from sqlalchemy.orm.exc import FlushError, NoResultFound
 
 from app import db
@@ -437,7 +437,7 @@ def test_update_service_permission_creates_a_history_record_with_current_data(no
     assert Service.get_history_model().query.filter_by(name='service_name').all()[2].version == 3
 
 
-def test_update_service_set_postage_to_default(notify_db_session):
+def test_service_postage_constraint_on_create(notify_db_session):
     user = create_user()
     assert Service.query.count() == 0
     assert Service.get_history_model().query.count() == 0
@@ -445,16 +445,18 @@ def test_update_service_set_postage_to_default(notify_db_session):
                       email_from="email_from",
                       message_limit=1000,
                       restricted=False,
-                      created_by=user)
-    dao_create_service(service, user)
+                      created_by=user,
+                      postage='third')
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        dao_create_service(service, user)
 
+
+def test_service_postage_constraint_on_update(notify_db_session):
+    create_service()
     service_from_db = Service.query.first()
-    service_from_db.postage = None
-    dao_update_service(service_from_db)
-    service_with_update = Service.query.first()
-    assert service_with_update.postage == 'second'
-    service_history_with_update = Service.get_history_model().query.filter_by(version=2).one()
-    assert service_history_with_update.postage == 'second'
+    service_from_db.postage = 'third'
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        dao_update_service(service_from_db)
 
 
 def test_create_service_and_history_is_transactional(notify_db_session):