From 54d801979caf0bb0c25711b2b29e4df3e586b416 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Tue, 16 May 2017 10:57:57 +0100 Subject: [PATCH] Refactored to handle single service permission --- app/dao/service_permissions_dao.py | 27 ++----- tests/app/dao/test_service_permissions_dao.py | 71 ++++++------------- tests/app/db.py | 9 ++- 3 files changed, 32 insertions(+), 75 deletions(-) diff --git a/app/dao/service_permissions_dao.py b/app/dao/service_permissions_dao.py index 7ea9eb0d3..26b1f6f9e 100644 --- a/app/dao/service_permissions_dao.py +++ b/app/dao/service_permissions_dao.py @@ -10,29 +10,14 @@ def dao_fetch_service_permissions(service_id): ServicePermission.service_id == service_id).all() -def make_service_permissions_list(service_id, permissions): - arr = [] - for permission in permissions: - if permission not in SERVICE_PERMISSION_TYPES: - raise ValueError("'{}' not of service permission type: {}".format(permission, SERVICE_PERMISSION_TYPES)) - - service_permission = ServicePermission(service_id=service_id, permission=permission) - arr.append(service_permission) - - return arr - - @transactional -def dao_add_and_commit_service_permissions(service_id, permissions): - service_permissions = make_service_permissions_list(service_id, permissions) +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)) - try: - db.session.add_all(service_permissions) - db.session.commit() - except exc.IntegrityError as e: - if "duplicate key value violates unique constraint" in str(e.orig): - raise ValueError(e.orig) - raise + service_permission = ServicePermission(service_id=service_id, permission=permission) + + db.session.add(service_permission) def dao_remove_service_permission(service_id, permission=None): diff --git a/tests/app/dao/test_service_permissions_dao.py b/tests/app/dao/test_service_permissions_dao.py index b6130f0da..cf3768306 100644 --- a/tests/app/dao/test_service_permissions_dao.py +++ b/tests/app/dao/test_service_permissions_dao.py @@ -5,61 +5,47 @@ 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_permissions, create_service +from tests.app.db import create_service_permission, create_service -def test_create_service_permissions(sample_service): - service_permission_types = [SMS_TYPE, INTERNATIONAL_SMS_TYPE] +def test_create_service_permission(sample_service): + service_permission_type = SMS_TYPE - service_permissions = create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types) + service_permission = create_service_permission( + service_id=sample_service.id, permission=service_permission_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) + + +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) + service_permissions = dao_fetch_service_permissions(sample_service.id) assert len(service_permissions) == len(service_permission_types) 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_fetch_service_permissions_gets_service_permissions(sample_service): - service_permission_types = [LETTER_TYPE, EMAIL_TYPE, SMS_TYPE] - create_service_permissions(service_id=sample_service.id, permissions=service_permission_types) - service_permissions = dao_fetch_service_permissions(sample_service.id) - - assert len(service_permission_types) == len(service_permission_types) - 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_add_service_permissions_to_existing_permissions(sample_service): - service_permission_types_1 = [EMAIL_TYPE, INCOMING_SMS_TYPE] - service_permission_types_2 = [LETTER_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE] - - create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types_1) - create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types_2) - - permissions = dao_fetch_service_permissions(sample_service.id) - - assert len(permissions) == len(service_permission_types_1 + service_permission_types_2) - - def test_create_invalid_service_permissions_raises_error(sample_service): - service_permission_types = ['invalid'] + service_permission_type = 'invalid' with pytest.raises(ValueError) as e: - service_permissions = create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types) + create_service_permission(service_id=sample_service.id, permission=service_permission_type) - assert "'invalid' not of service permission type: " + str(SERVICE_PERMISSION_TYPES) in str(e.value) + assert "'invalid' not of service permission type: {}".format(str(SERVICE_PERMISSION_TYPES)) in str(e.value) def test_remove_service_permission(sample_service): - service_permission_types = [EMAIL_TYPE, INCOMING_SMS_TYPE] + service_permission_types_to_create = [EMAIL_TYPE, INCOMING_SMS_TYPE] service_permission_type_to_remove = EMAIL_TYPE service_permission_type_remaining = INCOMING_SMS_TYPE - service_permissions = create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types) + 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) @@ -67,16 +53,3 @@ def test_remove_service_permission(sample_service): assert len(permissions) == 1 assert permissions[0].permission == service_permission_type_remaining assert permissions[0].service_id == sample_service.id - - -def test_adding_duplicate_service_id_permission_raises_value_error(sample_service): - service_permission_types = [EMAIL_TYPE, INCOMING_SMS_TYPE] - service_permission_types_with_duplicate_email_type = [LETTER_TYPE, EMAIL_TYPE] - - with pytest.raises(ValueError) as e: - create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types) - create_service_permissions( - service_id=sample_service.id, permissions=service_permission_types_with_duplicate_email_type) - - assert "duplicate key value violates unique constraint \"service_permissions_pkey\"" in str(e.value) diff --git a/tests/app/db.py b/tests/app/db.py index c193fd4a9..e4cded3cc 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -8,8 +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_permissions, make_service_permissions_list) +from app.dao.service_permissions_dao import dao_add_and_commit_service_permission def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk"): @@ -146,9 +145,9 @@ def create_job(template, return job -def create_service_permissions(service_id, permissions=[EMAIL_TYPE, LETTER_TYPE]): - dao_add_and_commit_service_permissions( - service_id if service_id else create_service().id, permissions) +def create_service_permission(service_id, permission=EMAIL_TYPE): + dao_add_and_commit_service_permission( + service_id if service_id else create_service().id, permission) service_permissions = ServicePermission.query.all()