diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index f1c66126a..ab4e71b56 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -31,7 +31,9 @@ from app.models import ( TEMPLATE_TYPES, JobStatistics, SMS_TYPE, - EMAIL_TYPE + EMAIL_TYPE, + INTERNATIONAL_SMS_TYPE, + LETTER_TYPE ) from app.service.statistics import format_monthly_template_notification_stats from app.statsd_decorators import statsd @@ -136,10 +138,28 @@ def dao_create_service(service, user, service_id=None, service_permissions=[SMS_ service.active = True service.research_mode = False - for permission in service_permissions: - service_permission = ServicePermission(service_id=service.id, permission=permission) - service.permissions.append(service_permission) + def process_deprecated_service_permissions(): + for permission in service_permissions: + service_permission = ServicePermission(service_id=service.id, permission=permission) + service.permissions.append(service_permission) + if permission == INTERNATIONAL_SMS_TYPE: + service.can_send_international_sms = True + if permission == LETTER_TYPE: + service.can_send_letters = True + + def sync_flags(flag, notify_type): + if flag and notify_type not in service_permissions: + service_permission = ServicePermission(service_id=service.id, permission=notify_type) + service.permissions.append(service_permission) + elif flag is False and notify_type in service_permissions: + service_permission = ServicePermission(service_id=service.id, permission=notify_type) + service.permissions.remove(service_permission) + + sync_flags(service.can_send_international_sms, INTERNATIONAL_SMS_TYPE) + sync_flags(service.can_send_letters, LETTER_TYPE) + + process_deprecated_service_permissions() db.session.add(service) diff --git a/app/schemas.py b/app/schemas.py index dd9e0d0ae..d9d71c991 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -25,7 +25,7 @@ from notifications_utils.recipients import ( from app import ma from app import models -from app.models import ServicePermission, INTERNATIONAL_SMS_TYPE, SMS_TYPE +from app.models import ServicePermission, INTERNATIONAL_SMS_TYPE, SMS_TYPE, LETTER_TYPE from app.dao.permissions_dao import permission_dao from app.dao.service_permissions_dao import dao_fetch_service_permissions from app.utils import get_template_instance @@ -184,6 +184,7 @@ class ServiceSchema(BaseSchema): def service_permissions(self, service): permissions = [] + str_permissions = [] perms = dao_fetch_service_permissions(service.id) for p in perms: @@ -192,6 +193,28 @@ class ServiceSchema(BaseSchema): "permission": p.permission } permissions.append(permission) + str_permissions.append(p.permission) + + def process_deprecated_permission_flags(): + def sync_flags(flag, notify_type): + if flag and notify_type not in str_permissions: + permission = { + "service_id": service.id, + "permission": notify_type + } + permissions.append(permission) + elif flag is False and notify_type in str_permissions: + permission = { + "service_id": service.id, + "permission": notify_type + } + permissions.remove(permission) + + sync_flags(service.can_send_international_sms, INTERNATIONAL_SMS_TYPE) + sync_flags(service.can_send_letters, LETTER_TYPE) + + process_deprecated_permission_flags() + return permissions class Meta: @@ -232,7 +255,7 @@ class ServiceSchema(BaseSchema): def format_permissions_for_data_model(self, in_data): if isinstance(in_data, dict) and 'permissions' in in_data: permissions = [] - for p in in_data.get('permissions'): + for p in in_data['permissions']: permission = models.ServicePermission(service_id=in_data["id"], permission=p) permissions.append(permission) in_data['permissions'] = permissions @@ -240,7 +263,7 @@ class ServiceSchema(BaseSchema): @post_dump def format_permissions_as_string_array(self, in_data): if isinstance(in_data, dict) and 'permissions' in in_data: - in_data['permissions'] = [p.get('permission') for p in in_data.get('permissions')] + in_data['permissions'] = [p.get("permission") for p in in_data['permissions']] return in_data diff --git a/tests/app/conftest.py b/tests/app/conftest.py index b7ca092ba..a95228426 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -24,7 +24,7 @@ from app.models import ( NotificationStatistics, ServiceWhitelist, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, - MOBILE_TYPE, EMAIL_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED, ScheduledNotification) + MOBILE_TYPE, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED, ScheduledNotification) from app.dao.users_dao import (create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template @@ -124,7 +124,8 @@ def sample_service( restricted=False, limit=1000, email_from=None, - can_send_international_sms=False + can_send_international_sms=False, + permissions=[SMS_TYPE, EMAIL_TYPE] ): if user is None: user = create_user() @@ -142,7 +143,7 @@ def sample_service( service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) - dao_create_service(service, user) + dao_create_service(service, user, service_permissions=permissions) else: if user not in service.users: dao_add_user_to_service(service, user) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 223ffa033..dbb00615a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -21,6 +21,7 @@ from tests.app.conftest import ( sample_notification_with_job ) from app.models import ( + ServicePermission, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE ) @@ -157,7 +158,7 @@ def test_get_service_list_has_default_permissions(client, service_factory): assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) assert len(json_resp['data']) == 3 - assert all([set(json['permissions']) & set([EMAIL_TYPE, SMS_TYPE]) for json in json_resp['data']]) + assert all([set(json['permissions']) == set([EMAIL_TYPE, SMS_TYPE]) for json in json_resp['data']]) def test_get_service_by_id_has_default_permissions(client, sample_service): @@ -490,18 +491,41 @@ def test_update_service_flags_will_add_service_permissions(client, sample_servic assert result['data']['research_mode'] is True assert result['data']['can_send_letters'] is True assert result['data']['can_send_international_sms'] is True - assert all(set(result['data']['permissions']) & set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE])) + assert set(result['data']['permissions']) == set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) + + +def test_update_service_flags_will_remove_service_permissions(client, notify_db, notify_db_session): + auth_header = create_authorization_header() + + service = create_service(notify_db, notify_db_session, can_send_international_sms=True) + + assert service.can_send_international_sms is True + assert set([p.permission for p in service.permissions]) == set([SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE]) + + data = { + 'can_send_international_sms': False, + } + + resp = client.post( + '/service/{}'.format(service.id), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), auth_header] + ) + result = json.loads(resp.get_data(as_text=True)) + + assert resp.status_code == 200 + assert result['data']['can_send_international_sms'] is False + assert set(result['data']['permissions']) == set([SMS_TYPE, EMAIL_TYPE]) def test_update_permissions_can_add_service_permissions(client, sample_service): - from app.models import ServicePermission auth_header = create_authorization_header() data = { 'research_mode': True, - 'can_send_letters': True, 'can_send_international_sms': True, - 'permissions': [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE] + 'can_send_letters': True, + 'permissions': [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE] } resp = client.post( @@ -515,7 +539,7 @@ def test_update_permissions_can_add_service_permissions(client, sample_service): assert result['data']['research_mode'] is True assert result['data']['can_send_letters'] is True assert result['data']['can_send_international_sms'] is True - assert all(set(result['data']['permissions']) & set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE])) + assert set(result['data']['permissions']) == set([SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) def test_update_permissions_with_an_invalid_permission_will_raise_error(client, sample_service): @@ -523,9 +547,6 @@ def test_update_permissions_with_an_invalid_permission_will_raise_error(client, invalid_permission = 'invalid_permission' data = { - 'research_mode': True, - 'can_send_letters': True, - 'can_send_international_sms': True, 'permissions': [EMAIL_TYPE, SMS_TYPE, invalid_permission] } @@ -579,40 +600,6 @@ def test_update_permissions_with_international_sms_without_sms_permissions_will_ assert "International SMS must have SMS enabled" == result['message'] -def test_update_service_flags_can_remove_service_permissions(client, sample_service): - auth_header = create_authorization_header() - initial_data = { - 'research_mode': True, - 'can_send_letters': True, - 'can_send_international_sms': True, - 'permissions': [EMAIL_TYPE, SMS_TYPE] - } - - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(initial_data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - - remove_permission_data = { - 'can_send_letters': True, - 'can_send_international_sms': False, - 'permissions': [EMAIL_TYPE] - } - - resp = client.post( - '/service/{}'.format(sample_service.id), - data=json.dumps(remove_permission_data), - headers=[('Content-Type', 'application/json'), auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) - - assert resp.status_code == 200 - assert result['data']['can_send_letters'] is True - assert result['data']['can_send_international_sms'] is False - assert result['data']['permissions'] == [EMAIL_TYPE] - - def test_update_service_research_mode_throws_validation_error(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: