Roles refactor: Fix depracated update project access endpoint

server/mergin/sync/public_api.yaml

@@ -232,6 +232,7 @@ paths:
       summary: Update an existing project
       description: Updates 'public' flag and access list for project
       operationId: update_project
+      deprecated: true
       requestBody:
         description: Data to be updated
         required: true

server/mergin/sync/public_api_controller.py

@@ -45,6 +45,7 @@
     FileHistory,
     ProjectFilePath,
     ProjectUser,
+    ProjectRole,
 )
 from .files import (
     UploadChanges,
@@ -95,14 +96,19 @@
 def parse_project_access_update_request(access: Dict) -> Dict:
     """Parse raw project access update request and filter out invalid entries.
     New access can be specified either by list of usernames or ids -> convert only to ids fur further processing.
+    Converted lists are flattened, e.g. user id is unique within all keys. Bear in mind roles keys are optional,
+    if missing, it means that we do not want to do any changes there.
+
+    Deprecated. Used only in legacy PUT /v1/project endpoint for project access replacement.
 
     :Example:
 
         >>> parse_project_access_update_request({"writersnames": ["john"], "readersnames": ["john, jack, bob.inactive"]})
-        {"writers": [1], "readers": [1,2], "invalid_usernames": ["bob.inactive"], "invalid_ids":[]}
+        {"ProjectRole.WRITER": [1], "ProjectRole.READER": [2], "invalid_usernames": ["bob.inactive"], "invalid_ids":[]}
         >>> parse_project_access_update_request({"writers": [1], "readers": [1,2,3]})
-        {"writers": [1], "readers": [1,2], "invalid_usernames": [], "invalid_ids":[3]"}
+        {"ProjectRole.WRITER": [1], "ProjectRole.READER": [2], "invalid_usernames": [], "invalid_ids":[3]"}
     """
+    resp = {}
     parsed_access = {}
     names = set(
         access.get("ownersnames", [])
@@ -139,9 +145,23 @@ def parse_project_access_update_request(access: Dict) -> Dict:
         # use legacy option
         elif key in access:
             parsed_access[key] = [id for id in access.get(key) if id in valid_ids]
-    parsed_access["invalid_usernames"] = list(names.difference(valid_usernames))
-    parsed_access["invalid_ids"] = list(ids.difference(valid_ids))
-    return parsed_access
+
+    # remove 'inheritance', prepare final map for direct assignments
+    processed_ids = []
+    for key in ("owners", "writers", "editors", "readers"):
+        # we might not want to modify all roles
+        if key not in parsed_access:
+            continue
+        role = ProjectRole(key[:-1])
+        resp[role] = []
+        for user_id in parsed_access.get(key):
+            if user_id not in processed_ids:
+                resp[role].append(user_id)
+                processed_ids.append(user_id)
+
+    resp["invalid_usernames"] = list(names.difference(valid_usernames))
+    resp["invalid_ids"] = list(ids.difference(valid_ids))
+    return resp
 
 
 @auth_required

server/mergin/sync/workspace.py

@@ -14,6 +14,7 @@
     ProjectAccessDetail,
     ProjectVersion,
     ProjectUser,
+    ProjectRole,
 )
 from .permissions import projects_query, ProjectPermissions
 from .public_api_controller import parse_project_access_update_request
@@ -276,16 +277,32 @@ def update_project_members(
         """Update project members doing bulk access update"""
         error = None
         parsed_access = parse_project_access_update_request(access)
-        # FIXME
-        id_diffs = set()
-        # id_diffs = project.access.bulk_update(parsed_access)
+        id_diffs = []
+        for role in list(ProjectRole.__reversed__()):
+            # we might not want to modify all roles
+            if role not in parsed_access:
+                continue
+
+            for user_id in parsed_access.get(role):
+                if project.get_role(user_id) != role:
+                    project.set_role(user_id, role)
+                    id_diffs.append(user_id)
+
+            # make sure we do not have other user ids than in the list at this role
+            for user in project.project_users:
+                if ProjectRole(
+                    user.role
+                ) == role and user.user_id not in parsed_access.get(role):
+                    project.unset_role(user.user_id)
+                    id_diffs.append(user.user_id)
+
         db.session.add(project)
         db.session.commit()
         if parsed_access.get("invalid_usernames") or parsed_access.get("invalid_ids"):
             error = UpdateProjectAccessError(
                 parsed_access["invalid_usernames"], parsed_access["invalid_ids"]
             )
-        return id_diffs, error
+        return set(id_diffs), error
 
     @staticmethod
     def access_requests_query():

server/mergin/tests/test_project_controller.py

@@ -36,6 +36,7 @@
     ProjectFilePath,
 )
 from ..sync.files import ChangesSchema
+from ..sync.permissions import projects_query
 from ..sync.schemas import ProjectListSchema, ProjectSchema
 from ..sync.utils import generate_checksum, is_versioned_file
 from ..auth.models import User, UserProfile
@@ -628,6 +629,7 @@ def test_update_project(client):
     project = Project.query.filter_by(
         name=test_project, workspace_id=test_workspace_id
     ).first()
+    creator = User.query.get(project.creator_id)
     # need for private project
     project.public = False
     db.session.add(project)
@@ -641,16 +643,7 @@ def test_update_project(client):
     db.session.commit()
 
     # add tests user as reader to project
-    data = {
-        "access": {
-            "readers": [
-                u.id
-                for u in project.project_users
-                if u.role == ProjectRole.READER.value
-            ]
-            + [test_user.id]
-        }
-    }
+    data = {"access": {"readers": [test_user.id]}}
     resp = client.put(
         "/v1/project/{}/{}".format(test_workspace_name, test_project),
         data=json.dumps(data),
@@ -660,20 +653,15 @@ def test_update_project(client):
     assert project.get_role(test_user.id) is ProjectRole.READER
 
     # add tests user as writer to project
-    current_writers = [
-        u.id for u in project.project_users if u.role == ProjectRole.WRITER.value
-    ]
-    writers = [
-        u.username for u in User.query.filter(User.id.in_(current_writers)).all()
-    ]
-    data = {"access": {"writersnames": writers + [test_user.username]}}
+    data = {"access": {"writersnames": [test_user.username]}}
     resp = client.put(
         "/v1/project/{}/{}".format(test_workspace_name, test_project),
         data=json.dumps(data),
         headers=json_headers,
     )
     assert resp.status_code == 200
     assert project.get_role(test_user.id) is ProjectRole.WRITER
+    assert project.get_role(creator.id) is ProjectRole.OWNER
 
     # try to remove project creator from owners
     data = {"access": {"owners": [test_user.id]}}
@@ -683,15 +671,10 @@ def test_update_project(client):
         headers=json_headers,
     )
     assert resp.status_code == 200
+    assert not project.get_role(creator.id)
 
     # try to add non-existing user
-    current_readers = [
-        u.id for u in project.project_users if u.role == ProjectRole.READER.value
-    ]
-    readers = [
-        user.username for user in User.query.filter(User.id.in_(current_readers)).all()
-    ]
-    data = {"access": {"readersnames": readers + ["not-found-user"]}}
+    data = {"access": {"readersnames": ["not-found-user"]}}
     resp = client.put(
         f"/v1/project/{test_workspace_name}/{test_project}",
         data=json.dumps(data),
@@ -703,18 +686,16 @@ def test_update_project(client):
     assert resp.json["invalid_usernames"] == ["not-found-user"]
 
     # try to add non-existing user plus make some valid update -> only partial success
-    current_readers = [
-        u.id for u in project.project_users if u.role == ProjectRole.READER.value
-    ]
-    readers = [
-        user.username for user in User.query.filter(User.id.in_(current_readers)).all()
+    current_users = [u.user_id for u in project.project_users]
+    usernames = [
+        user.username for user in User.query.filter(User.id.in_(current_users)).all()
     ]
     data = {
         "access": {
-            "readersnames": readers + ["not-found-user"],
-            "editorsnames": readers,
-            "writersnames": readers,
-            "ownersnames": readers,
+            "readersnames": ["not-found-user"],
+            "editorsnames": usernames + [creator.username],
+            "writersnames": usernames + [creator.username],
+            "ownersnames": usernames,
         }
     }
     resp = client.put(
@@ -724,6 +705,8 @@ def test_update_project(client):
     )
     assert resp.status_code == 207
     assert resp.json["code"] == "UpdateProjectAccessError"
+    assert project.get_role(test_user.id) is ProjectRole.OWNER
+    assert project.get_role(creator.id) is ProjectRole.WRITER
 
     # login as a new project owner and check permissions
     login(client, test_user.username, "tester")