From 733c16b2bbf20247947565c11594e685326da1f5 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 16 May 2017 12:33:27 +0100 Subject: [PATCH] Update to strip down DAO and clarify tests --- app/dao/service_permissions_dao.py | 13 ++---- tests/app/conftest.py | 8 ++-- tests/app/dao/test_service_permissions_dao.py | 42 +++++++------------ tests/app/db.py | 4 +- 4 files changed, 23 insertions(+), 44 deletions(-) diff --git a/app/dao/service_permissions_dao.py b/app/dao/service_permissions_dao.py index 26b1f6f9e..5c38ad6a2 100644 --- a/app/dao/service_permissions_dao.py +++ b/app/dao/service_permissions_dao.py @@ -1,8 +1,6 @@ -from sqlalchemy import exc - from app import db from app.dao.dao_utils import transactional -from app.models import Service, ServicePermission, SERVICE_PERMISSION_TYPES +from app.models import ServicePermission, SERVICE_PERMISSION_TYPES def dao_fetch_service_permissions(service_id): @@ -11,16 +9,13 @@ def dao_fetch_service_permissions(service_id): @transactional -def dao_add_and_commit_service_permission(service_id, permission): - if permission not in SERVICE_PERMISSION_TYPES: - raise ValueError("'{}' not of service permission type: {}".format(permission, SERVICE_PERMISSION_TYPES)) - +def dao_create_service_permission(service_id, permission): service_permission = ServicePermission(service_id=service_id, permission=permission) db.session.add(service_permission) -def dao_remove_service_permission(service_id, permission=None): +def dao_remove_service_permission(service_id, permission): return ServicePermission.query.filter( ServicePermission.service_id == service_id, - ServicePermission.permission == permission if permission else None).delete() + ServicePermission.permission == permission).delete() diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 00b4604b1..c10c053cb 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -666,11 +666,9 @@ def sample_permission(notify_db, @pytest.fixture(scope='function') -def sample_user_service_permission(notify_db, - notify_db_session, - service=None, - user=None, - permission="manage_settings"): +def sample_user_service_permission( + notify_db, notify_db_session, service=None, user=None, permission="manage_settings" +): if user is None: user = create_user() if service is None: diff --git a/tests/app/dao/test_service_permissions_dao.py b/tests/app/dao/test_service_permissions_dao.py index cf3768306..248dd6ae6 100644 --- a/tests/app/dao/test_service_permissions_dao.py +++ b/tests/app/dao/test_service_permissions_dao.py @@ -5,51 +5,37 @@ from app.dao.service_permissions_dao import ( from app.models import ( EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE, INCOMING_SMS_TYPE, SERVICE_PERMISSION_TYPES) -from tests.app.db import create_service_permission, create_service +from tests.app.db import create_service_permission def test_create_service_permission(sample_service): - service_permission_type = SMS_TYPE - service_permission = create_service_permission( - service_id=sample_service.id, permission=service_permission_type) + service_id=sample_service.id, permission=SMS_TYPE) assert len(service_permission) == 1 - assert all(sp.service_id == sample_service.id for sp in service_permission) - assert all(sp.permission in service_permission_type for sp in service_permission) + assert service_permission[0].service_id == sample_service.id + assert service_permission[0].permission == SMS_TYPE def test_fetch_service_permissions_gets_service_permissions(sample_service): - service_permission_types = [LETTER_TYPE, EMAIL_TYPE, SMS_TYPE] - for spt in service_permission_types: - create_service_permission(service_id=sample_service.id, permission=spt) + create_service_permission(service_id=sample_service.id, permission=LETTER_TYPE) + create_service_permission(service_id=sample_service.id, permission=INTERNATIONAL_SMS_TYPE) + create_service_permission(service_id=sample_service.id, permission=SMS_TYPE) + service_permissions = dao_fetch_service_permissions(sample_service.id) - assert len(service_permissions) == len(service_permission_types) + assert len(service_permissions) == 3 assert all(sp.service_id == sample_service.id for sp in service_permissions) - assert all(sp.permission in service_permission_types for sp in service_permissions) - - -def test_create_invalid_service_permissions_raises_error(sample_service): - service_permission_type = 'invalid' - - with pytest.raises(ValueError) as e: - create_service_permission(service_id=sample_service.id, permission=service_permission_type) - - assert "'invalid' not of service permission type: {}".format(str(SERVICE_PERMISSION_TYPES)) in str(e.value) + assert all(sp.permission in [LETTER_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE] for sp in service_permissions) def test_remove_service_permission(sample_service): - service_permission_types_to_create = [EMAIL_TYPE, INCOMING_SMS_TYPE] - service_permission_type_to_remove = EMAIL_TYPE - service_permission_type_remaining = INCOMING_SMS_TYPE + create_service_permission(service_id=sample_service.id, permission=EMAIL_TYPE) + create_service_permission(service_id=sample_service.id, permission=INCOMING_SMS_TYPE) - for spt in service_permission_types_to_create: - create_service_permission(service_id=sample_service.id, permission=spt) - - dao_remove_service_permission(sample_service.id, service_permission_type_to_remove) + dao_remove_service_permission(sample_service.id, EMAIL_TYPE) permissions = dao_fetch_service_permissions(sample_service.id) assert len(permissions) == 1 - assert permissions[0].permission == service_permission_type_remaining + assert permissions[0].permission == INCOMING_SMS_TYPE assert permissions[0].service_id == sample_service.id diff --git a/tests/app/db.py b/tests/app/db.py index e4cded3cc..228d49ec4 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -8,7 +8,7 @@ from app.dao.users_dao import save_model_user from app.dao.notifications_dao import dao_create_notification from app.dao.templates_dao import dao_create_template from app.dao.services_dao import dao_create_service -from app.dao.service_permissions_dao import dao_add_and_commit_service_permission +from app.dao.service_permissions_dao import dao_create_service_permission def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk"): @@ -146,7 +146,7 @@ def create_job(template, def create_service_permission(service_id, permission=EMAIL_TYPE): - dao_add_and_commit_service_permission( + dao_create_service_permission( service_id if service_id else create_service().id, permission) service_permissions = ServicePermission.query.all()