Merge pull request #992 from alphagov/ken-refactor-service-schema

Refactor schema to remove redundant bits

app/schemas.py

@@ -184,39 +184,7 @@ class ServiceSchema(BaseSchema):
     override_flag = False
 
     def service_permissions(self, service):
-        permissions = []
+        return [p.permission for p in 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)
-
-        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
-                    }
-                    permissions.append(permission)
-                elif flag is False and notify_type in str_permissions:
-                    permission = {
-                        "service_id": service.id,
-                        "permission": notify_type
-                    }
-                    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:
         model = models.Service
@@ -254,14 +222,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_letters'] = LETTER_TYPE in str_permissions
-                in_data['can_send_international_sms'] = INTERNATIONAL_SMS_TYPE in [p.permission for p in 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 +247,7 @@ class ServiceSchema(BaseSchema):
                 deprecate_override_flags()
             else:
                 deprecate_convert_flags_to_permissions()
-
+            in_data['permissions'] = 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
 
     def set_override_flag(self, flag):
         self.override_flag = flag

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(
+    permissions = ServicePermission.query.filter_by(service_id=service_with_no_permissions.id).all()
-        '/service/{}'.format(service_with_no_permissions.id),
-        headers=[auth_header]
-    )
-    result = json.loads(resp.get_data(as_text=True))
 
     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):