Merge pull request #974 from alphagov/revert-969-ken-refactor-service_permissions-data_model

Revert "Refactored service permissisons data model"

app/models.py

@@ -3,7 +3,6 @@ import uuid
 import datetime
 from flask import url_for, current_app
 
-from sqlalchemy.ext.associationproxy import association_proxy
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.dialects.postgresql import (
     UUID,
@@ -145,9 +144,9 @@ class DVLAOrganisation(db.Model):
 
 
 INTERNATIONAL_SMS_TYPE = 'international_sms'
-INBOUND_SMS_TYPE = 'inbound_sms'
+INCOMING_SMS_TYPE = 'incoming_sms'
 
-SERVICE_PERMISSION_TYPES = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE]
+SERVICE_PERMISSION_TYPES = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INCOMING_SMS_TYPE]
 
 
 class ServicePermissionTypes(db.Model):
@@ -156,6 +155,18 @@ class ServicePermissionTypes(db.Model):
     name = db.Column(db.String(255), primary_key=True)
 
 
+class ServicePermission(db.Model):
+    __tablename__ = "service_permissions"
+
+    service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'),
+                           primary_key=True, index=True, nullable=False)
+    service = db.relationship('Service')
+    permission = db.Column(db.String(255), db.ForeignKey('service_permission_types.name'),
+                           index=True, primary_key=True, nullable=False)
+    created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False)
+    updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow)
+
+
 class Service(db.Model, Versioned):
     __tablename__ = 'services'
 
@@ -206,8 +217,7 @@ class Service(db.Model, Versioned):
         nullable=False,
         default=BRANDING_GOVUK
     )
-
+    permissions = db.relationship('ServicePermission')
-    association_proxy('permissions', 'service_permission_types')
 
     # This is only for backward compatibility and will be dropped when the columns are removed from the data model
     def set_permissions(self):
@@ -216,22 +226,6 @@ class Service(db.Model, Versioned):
             self.can_send_international_sms = INTERNATIONAL_SMS_TYPE in [p.permission for p in self.permissions]
 
 
-class ServicePermission(db.Model):
-    __tablename__ = "service_permissions"
-
-    service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'),
-                           primary_key=True, index=True, nullable=False)
-    permission = db.Column(db.String(255), db.ForeignKey('service_permission_types.name'),
-                           index=True, primary_key=True, nullable=False)
-    service = db.relationship("Service")
-    created_at = db.Column(db.DateTime, default=datetime.datetime.utcnow, nullable=False)
-
-    service_permission_types = db.relationship(Service, backref=db.backref("permissions"))
-
-    def __repr__(self):
-        return '<{} has service permission: {}>'.format(self.service_id, self.permission)
-
-
 MOBILE_TYPE = 'mobile'
 EMAIL_TYPE = 'email'
 

migrations/versions/0085_update_incoming_to_inbound.py

@@ -1,24 +0,0 @@
-"""empty message
-
-Revision ID: 0085_update_incoming_to_inbound
-Revises: 0084_add_job_stats
-Create Date: 2017-05-22 10:23:43.939050
-
-"""
-
-# revision identifiers, used by Alembic.
-revision = '0085_update_incoming_to_inbound'
-down_revision = '0084_add_job_stats'
-
-from alembic import op
-import sqlalchemy as sa
-from sqlalchemy.dialects import postgresql
-
-def upgrade():
-    op.drop_column('service_permissions', 'updated_at')
-    op.execute("UPDATE service_permission_types SET name='inbound_sms' WHERE name='incoming_sms'")
-
-
-def downgrade():
-    op.add_column('service_permissions', sa.Column('updated_at', postgresql.TIMESTAMP(), autoincrement=False, nullable=True))
-    op.execute("UPDATE service_permission_types SET name='incoming_sms' WHERE name='inbound_sms'")

tests/app/dao/test_service_permissions_dao.py

@@ -1,7 +1,7 @@
 import pytest
 
 from app.dao.service_permissions_dao import dao_fetch_service_permissions, dao_remove_service_permission
-from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INBOUND_SMS_TYPE
+from app.models import EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INCOMING_SMS_TYPE
 
 from tests.app.db import create_service_permission, create_service
 
@@ -34,11 +34,11 @@ def test_fetch_service_permissions_gets_service_permissions(service_without_perm
 
 def test_remove_service_permission(service_without_permissions):
     create_service_permission(service_id=service_without_permissions.id, permission=EMAIL_TYPE)
-    create_service_permission(service_id=service_without_permissions.id, permission=INBOUND_SMS_TYPE)
+    create_service_permission(service_id=service_without_permissions.id, permission=INCOMING_SMS_TYPE)
 
     dao_remove_service_permission(service_without_permissions.id, EMAIL_TYPE)
 
     permissions = dao_fetch_service_permissions(service_without_permissions.id)
     assert len(permissions) == 1
-    assert permissions[0].permission == INBOUND_SMS_TYPE
+    assert permissions[0].permission == INCOMING_SMS_TYPE
     assert permissions[0].service_id == service_without_permissions.id

tests/app/dao/test_services_dao.py

@@ -44,8 +44,6 @@ from app.models import (
     User,
     InvitedUser,
     Service,
-    ServicePermission,
-    ServicePermissionTypes,
     BRANDING_GOVUK,
     DVLA_ORG_HM_GOVERNMENT,
     KEY_TYPE_NORMAL,
@@ -54,8 +52,7 @@ from app.models import (
     EMAIL_TYPE,
     SMS_TYPE,
     LETTER_TYPE,
-    INTERNATIONAL_SMS_TYPE,
+    INTERNATIONAL_SMS_TYPE
-    SERVICE_PERMISSION_TYPES
 )
 
 from tests.app.db import create_user, create_service
@@ -289,14 +286,6 @@ def test_remove_permission_from_service_by_id_returns_service_with_correct_permi
     assert service.permissions[0].permission == EMAIL_TYPE
 
 
-def test_remove_service_does_not_remove_service_permission_types(sample_service):
-    delete_service_and_all_associated_db_objects(sample_service)
-
-    services = dao_fetch_all_services()
-    assert len(services) == 0
-    assert set([p.name for p in ServicePermissionTypes.query.all()]) == set(SERVICE_PERMISSION_TYPES)
-
-
 def test_create_service_by_id_adding_and_removing_letter_returns_service_without_letter(service_factory):
     service = service_factory.get('testing', email_from='testing')
 
@@ -403,9 +392,6 @@ def test_delete_service_and_associated_objects(notify_db,
                                                sample_invited_user,
                                                sample_permission,
                                                sample_provider_statistics):
-    # Default service permissions of Email and SMS
-    assert ServicePermission.query.count() == 2
-
     delete_service_and_all_associated_db_objects(sample_service)
     assert NotificationStatistics.query.count() == 0
     assert TemplateStatistics.query.count() == 0
@@ -422,7 +408,6 @@ def test_delete_service_and_associated_objects(notify_db,
     assert InvitedUser.query.count() == 0
     assert Service.query.count() == 0
     assert Service.get_history_model().query.count() == 0
-    assert ServicePermission.query.count() == 0
 
 
 def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sample_user):