From 56e9faab2e62e01ead46eea078b2cd335d8fc449 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 26 May 2017 15:27:49 +0100 Subject: [PATCH 1/3] Refactor schema --- app/schemas.py | 40 ++++++++-------------------------------- 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index ff4f83a95..09301868e 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -184,31 +184,13 @@ class ServiceSchema(BaseSchema): override_flag = False def service_permissions(self, service): - permissions = [] - str_permissions = [] - - perms = dao_fetch_service_permissions(service.id) - for p in perms: - permission = { - "service_id": service.id, - "permission": p.permission - } - permissions.append(permission) - str_permissions.append(p.permission) + permissions = [p.permission for p in service.permissions] def deprecate_convert_flags_to_permissions(): - def convert_flags(flag, notify_type): - if flag and notify_type not in str_permissions: - permission = { - "service_id": service.id, - "permission": notify_type - } + def convert_flags(flag, permission): + if flag and permission not in permissions: permissions.append(permission) - elif flag is False and notify_type in str_permissions: - permission = { - "service_id": service.id, - "permission": notify_type - } + elif flag is False and permission in permissions: permissions.remove(permission) convert_flags(service.can_send_international_sms, INTERNATIONAL_SMS_TYPE) @@ -254,14 +236,13 @@ class ServiceSchema(BaseSchema): if isinstance(in_data, dict) and 'permissions' in in_data: str_permissions = in_data['permissions'] permissions = [] - for p in in_data['permissions']: + for p in str_permissions: permission = ServicePermission(service_id=in_data["id"], permission=p) permissions.append(permission) - in_data['permissions'] = permissions def deprecate_override_flags(): - in_data['can_send_letters'] = LETTER_TYPE in [p.permission for p in permissions] - in_data['can_send_international_sms'] = INTERNATIONAL_SMS_TYPE in [p.permission for p in permissions] + in_data['can_send_letters'] = LETTER_TYPE in str_permissions + in_data['can_send_international_sms'] = INTERNATIONAL_SMS_TYPE in str_permissions def deprecate_convert_flags_to_permissions(): def convert_flags(flag, notify_type): @@ -280,12 +261,7 @@ class ServiceSchema(BaseSchema): deprecate_override_flags() else: deprecate_convert_flags_to_permissions() - - @post_dump - def format_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['permissions']] - return in_data + in_data['permissions'] = permissions def set_override_flag(self, flag): self.override_flag = flag From 18b8382d6e0cc6037766d96239043f5b12bee4f5 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 26 May 2017 17:17:15 +0100 Subject: [PATCH 2/3] Refactor schema and improve tests --- app/schemas.py | 12 ------------ tests/app/service/test_rest.py | 12 +++++------- 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 09301868e..a89bbcd9f 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -186,18 +186,6 @@ class ServiceSchema(BaseSchema): def service_permissions(self, service): permissions = [p.permission for p in service.permissions] - def deprecate_convert_flags_to_permissions(): - def convert_flags(flag, permission): - if flag and permission not in permissions: - permissions.append(permission) - elif flag is False and permission in permissions: - permissions.remove(permission) - - convert_flags(service.can_send_international_sms, INTERNATIONAL_SMS_TYPE) - convert_flags(service.can_send_letters, LETTER_TYPE) - - deprecate_convert_flags_to_permissions() - return permissions class Meta: diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 7b5fda823..46adbba1b 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -519,7 +519,9 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db, assert resp.status_code == 200 assert result['data']['can_send_international_sms'] is False - assert set(result['data']['permissions']) == set([SMS_TYPE, EMAIL_TYPE]) + + permissions = ServicePermission.query.filter_by(service_id=service.id).all() + assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE]) def test_update_permissions_will_override_permission_flags(client, service_with_no_permissions): @@ -583,14 +585,10 @@ def test_add_service_permission_will_add_permission(client, service_with_no_perm headers=[('Content-Type', 'application/json'), auth_header] ) - resp = client.get( - '/service/{}'.format(service_with_no_permissions.id), - headers=[auth_header] - ) - result = json.loads(resp.get_data(as_text=True)) + permissions = ServicePermission.query.filter_by(service_id=service_with_no_permissions.id).all() assert resp.status_code == 200 - assert result['data']['permissions'] == [permission_to_add] + assert [p.permission for p in permissions] == [permission_to_add] def test_update_permissions_with_an_invalid_permission_will_raise_error(client, sample_service): From 112c6735930801375d45a927a66eaf82d44d9a32 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 26 May 2017 17:23:01 +0100 Subject: [PATCH 3/3] Removed a few lines from schema --- app/schemas.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index a89bbcd9f..b8d3ee7af 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -184,9 +184,7 @@ class ServiceSchema(BaseSchema): override_flag = False def service_permissions(self, service): - permissions = [p.permission for p in service.permissions] - - return permissions + return [p.permission for p in service.permissions] class Meta: model = models.Service