From f5d3eed7e0dc0c0998eebaa0e5471c1b9ee706fa Mon Sep 17 00:00:00 2001 From: kentsanggds Date: Mon, 22 May 2017 16:05:31 +0100 Subject: [PATCH] Revert "Refactored service permissisons data model" --- app/models.py | 36 ++++++++----------- .../0085_update_incoming_to_inbound.py | 24 ------------- tests/app/dao/test_service_permissions_dao.py | 6 ++-- tests/app/dao/test_services_dao.py | 17 +-------- 4 files changed, 19 insertions(+), 64 deletions(-) delete mode 100644 migrations/versions/0085_update_incoming_to_inbound.py diff --git a/app/models.py b/app/models.py index 378c45b3e..5fff42027 100644 --- a/app/models.py +++ b/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 ) - - association_proxy('permissions', 'service_permission_types') + permissions = db.relationship('ServicePermission') # 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' diff --git a/migrations/versions/0085_update_incoming_to_inbound.py b/migrations/versions/0085_update_incoming_to_inbound.py deleted file mode 100644 index ae4388c27..000000000 --- a/migrations/versions/0085_update_incoming_to_inbound.py +++ /dev/null @@ -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'") diff --git a/tests/app/dao/test_service_permissions_dao.py b/tests/app/dao/test_service_permissions_dao.py index 50b5559f8..c41c891f4 100644 --- a/tests/app/dao/test_service_permissions_dao.py +++ b/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 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 0a682ad54..2dc6f199f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/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, - SERVICE_PERMISSION_TYPES + INTERNATIONAL_SMS_TYPE ) 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):