From 054023aaafcd1600e95bdfe5b7cfffc6b4d41924 Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Wed, 12 Feb 2025 13:44:53 +0100
Subject: [PATCH 01/11] Typing fix for Python3.8 compatibility

---
 mergin/client.py | 2 +-
 mergin/editor.py | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index 4e33072a..14bcda17 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -17,7 +17,7 @@
 import typing
 import warnings
 
-from .common import ClientError, LoginError, InvalidProject, ErrorCode
+from .common import ClientError, LoginError
 from .merginproject import MerginProject
 from .client_pull import (
     download_file_finalize,
diff --git a/mergin/editor.py b/mergin/editor.py
index 9d0b5c86..237b0ea1 100644
--- a/mergin/editor.py
+++ b/mergin/editor.py
@@ -1,5 +1,5 @@
 from itertools import filterfalse
-from typing import Callable
+from typing import Callable, Dict, List
 
 from .utils import is_mergin_config, is_qgis_file, is_versioned_file
 
@@ -24,7 +24,7 @@ def is_editor_enabled(mc, project_info: dict) -> bool:
     return server_support and project_role == EDITOR_ROLE_NAME
 
 
-def _apply_editor_filters(changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
+def _apply_editor_filters(changes: Dict[str, List[dict]]) -> Dict[str, List[dict]]:
     """
     Applies editor-specific filters to the changes dictionary, removing any changes to files that are not in the editor's list of allowed files.
 
@@ -40,7 +40,7 @@ def _apply_editor_filters(changes: dict[str, list[dict]]) -> dict[str, list[dict
     return changes
 
 
-def filter_changes(mc, project_info: dict, changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
+def filter_changes(mc, project_info: dict, changes: Dict[str, List[dict]]) -> Dict[str, List[dict]]:
     """
     Filters the given changes dictionary based on the editor's enabled state.
 

From 2feb93c090f7cbb67f70ec6601ce8a5d723a98ab Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Thu, 13 Feb 2025 23:22:56 +0100
Subject: [PATCH 02/11] Add v2 endpoint for access management

---
 mergin/client.py           | 90 +++++++++++++++++++++++++++++++++++++-
 mergin/test/test_client.py | 71 ++++++++++++++++++++++++++----
 2 files changed, 152 insertions(+), 9 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index 14bcda17..d649a029 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -36,6 +36,7 @@
 from .version import __version__
 
 this_dir = os.path.dirname(os.path.realpath(__file__))
+json_headers = {"Content-Type": "application/json"}
 
 
 class TokenError(Exception):
@@ -244,6 +245,11 @@ def patch(self, path, data=None, headers={}):
         request = urllib.request.Request(url, data, headers, method="PATCH")
         return self._do_request(request)
 
+    def delete(self, path, data=None, headers={}):
+        url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
+        request = urllib.request.Request(url, data, headers, method="DELETE")
+        return self._do_request(request)
+
     def login(self, login, password):
         """
         Authenticate login credentials and store session token
@@ -1205,7 +1211,8 @@ def reset_local_changes(self, directory: str, files_to_reset: typing.List[str] =
             self.download_files(directory, files_download, version=current_version)
 
     def download_files(
-        self, project_dir: str, file_paths: typing.List[str], output_paths: typing.List[str] = None, version: str = None
+            self, project_dir: str, file_paths: typing.List[str], output_paths: typing.List[str] = None,
+            version: str = None
     ):
         """
         Download project files at specified version. Get the latest if no version specified.
@@ -1228,3 +1235,84 @@ def has_editor_support(self):
         Returns whether the server version is acceptable for editor support.
         """
         return is_version_acceptable(self.server_version(), "2024.4.0")
+
+    def create_user(self, email: str, password: str, workspace_id: int, workspace_role: str, notify_user: bool = False):
+        """
+        Create a new user in a workspace. The username is generated from the email address.
+        """
+        params = {
+            "email": email,
+            "password": password,
+            "workspace_id": workspace_id,
+            "role": workspace_role,
+            "notify_user": notify_user,
+        }
+        try:
+            self.post("v2/users", params, json_headers)
+        except ClientError as e:
+            e.extra = f"Email: {email}"
+            raise e
+
+    def get_workspace_member(self, workspace_id: int, user_id: int):
+        """
+        Get a workspace member detail
+        """
+        resp = self.get(f"v2/workspaces/{workspace_id}/members/{user_id}")
+        return json.load(resp)
+
+    def list_workspace_members(self, workspace_id: int):
+        """
+        Get a list of workspace members
+        """
+        resp = self.get(f"v2/workspaces/{workspace_id}/members")
+        return json.load(resp)
+
+    def update_workspace_member(self, workspace_id: int, user_id: int, workspace_role: str,
+                                reset_projects_roles: bool = False):
+        """
+        Update workspace role of a workspace member, optionally resets the projects role
+        """
+        params = {
+            "reset_projects_roles": reset_projects_roles,
+            "workspace_role": workspace_role,
+        }
+        resp = self.patch(f"v2/workspaces/{workspace_id}/members/{user_id}", params, json_headers)
+        return json.load(resp)
+
+    def remove_workspace_member(self, workspace_id: int, user_id: int):
+        """
+        Remove a user from workspace members
+        """
+        self.delete(f"v2/workspaces/{workspace_id}/members/{user_id}")
+
+    def list_project_collaborators(self, project_id: int):
+        """
+        Get a list of project collaborators
+        """
+        project_collaborators = self.get(f"v2/projects/{project_id}/collaborators")
+        return json.load(project_collaborators)
+
+    def add_project_collaborator(self, project_id: int, user: str, project_role: str):
+        """
+        Add a user to project collaborators and grant them a project role
+        """
+        params = {
+            "role": project_role,
+            "user": user
+        }
+        project_collaborator = self.post(f"v2/projects/{project_id}/collaborators", params, json_headers)
+        return json.load(project_collaborator)
+
+    def update_project_collaborator(self, project_id: int, user_id: int, project_role: str):
+        """
+        Update project role of the existing project collaborator
+        """
+        params = {"role": project_role}
+        project_collaborator = self.patch(f"v2/projects/{project_id}/collaborators/{user_id}", params, json_headers)
+        return json.load(project_collaborator)
+
+    def remove_project_collaborator(self, project_id: int, user_id: int):
+        """
+        Remove a user from project collaborators
+        """
+        self.delete(f"v2/projects/{project_id}/collaborators/{user_id}")
diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py
index f8934298..8c657282 100644
--- a/mergin/test/test_client.py
+++ b/mergin/test/test_client.py
@@ -1,6 +1,7 @@
 import json
 import logging
 import os
+import random
 import tempfile
 import subprocess
 import shutil
@@ -19,7 +20,6 @@
     decode_token_data,
     TokenError,
     ServerType,
-    ErrorCode,
 )
 from ..client_push import push_project_async, push_project_cancel
 from ..client_pull import (
@@ -39,7 +39,7 @@
 from ..merginproject import pygeodiff
 from ..report import create_report
 from ..editor import EDITOR_ROLE_NAME, filter_changes, is_editor_enabled
-
+from ..common import ErrorCode
 
 SERVER_URL = os.environ.get("TEST_MERGIN_URL")
 API_USER = os.environ.get("TEST_API_USERNAME")
@@ -355,8 +355,8 @@ def test_push_pull_changes(mc):
     assert os.path.exists(os.path.join(project_dir_2, "renamed.txt"))
     assert os.path.exists(os.path.join(project_dir_2, conflicted_copy_file_name(f_updated, API_USER, 1)))
     assert (
-        generate_checksum(os.path.join(project_dir_2, conflicted_copy_file_name(f_updated, API_USER, 1)))
-        == f_conflict_checksum
+            generate_checksum(os.path.join(project_dir_2, conflicted_copy_file_name(f_updated, API_USER, 1)))
+            == f_conflict_checksum
     )
     assert generate_checksum(os.path.join(project_dir_2, f_updated)) == f_remote_checksum
 
@@ -2459,8 +2459,8 @@ def test_project_rename(mc: MerginClient):
 
     # cannot rename with full project name
     with pytest.raises(
-        ClientError,
-        match="Project's new name should be without workspace specification",
+            ClientError,
+            match="Project's new name should be without workspace specification",
     ):
         mc.rename_project(project, "workspace" + "/" + test_project_renamed)
 
@@ -2711,8 +2711,8 @@ def test_error_projects_limit_hit(mcStorage: MerginClient):
         mcStorage.create_project_and_push(test_project_fullname, project_dir)
     assert e.value.server_code == ErrorCode.ProjectsLimitHit.value
     assert (
-        e.value.detail
-        == "Maximum number of projects is reached. Please upgrade your subscription to create new projects (ProjectsLimitHit)"
+            e.value.detail
+            == "Maximum number of projects is reached. Please upgrade your subscription to create new projects (ProjectsLimitHit)"
     )
     assert e.value.http_error == 422
     assert e.value.http_method == "POST"
@@ -2742,3 +2742,58 @@ def test_workspace_requests(mc2: MerginClient):
     assert service["plan"]["product_id"] == None
     assert service["plan"]["type"] == "custom"
     assert service["subscription"] == None
+
+
+def test_access_management(mc: MerginClient):
+    # create a user in a workspace
+    workspace_id = None
+    for workspace in mc.workspaces_list():
+        if workspace["name"] == mc.username():
+            workspace_id = workspace["id"]
+            break
+    email = "create_user" + str(random.randint(1000, 9999)) + "@client.py"
+    password = "Il0vemergin"
+    role = "writer"
+    mc.create_user(email, password, workspace_id, role)
+    workspace_members = mc.list_workspace_members(workspace_id)
+    new_user = next((m for m in workspace_members if m["email"] == email))
+    assert new_user
+    assert new_user["workspace_role"] == role
+    # test get workspace member
+    ws_member = mc.get_workspace_member(workspace_id, new_user["id"])
+    assert ws_member["email"] == email
+    assert ws_member["workspace_role"] == role
+    updated_role = "admin"
+    # test update workspace member
+    mc.update_workspace_member(workspace_id, new_user["id"], updated_role)
+    updated_user = mc.get_workspace_member(workspace_id, new_user["id"])
+    assert updated_user["workspace_role"] == updated_role
+    # test remove workspace member
+    mc.remove_workspace_member(workspace_id, new_user["id"])
+    workspace_members = mc.list_workspace_members(workspace_id)
+    assert not any(m["id"] == new_user["id"] for m in workspace_members)
+    # add project
+    test_project_name = "test_collaborators"
+    test_project_fullname = API_USER + "/" + test_project_name
+    project_dir = os.path.join(TMP_DIR, test_project_name, API_USER)
+    cleanup(mc, test_project_fullname, [project_dir])
+    mc.create_project(test_project_name)
+    project_info = get_project_info(mc, API_USER, test_project_name)
+    test_project_id = project_info["id"]
+    project_role = "reader"
+    # test add project collaborator
+    mc.add_project_collaborator(test_project_id, new_user["email"], project_role)
+    collaborators = mc.list_project_collaborators(test_project_id)
+    new_collaborator = next((c for c in collaborators if c["id"] == new_user["id"]))
+    assert new_collaborator
+    assert new_collaborator["project_role"] == project_role
+    updated_role = "owner"
+    # test update project collaborator
+    mc.update_project_collaborator(test_project_id, new_user["id"], updated_role)
+    collaborators = mc.list_project_collaborators(test_project_id)
+    updated_collaborator = next((c for c in collaborators if c["id"] == new_user["id"]))
+    assert updated_collaborator["project_role"] == updated_role
+    # test remove project collaborator
+    mc.remove_project_collaborator(test_project_id, new_user["id"])
+    collaborators = mc.list_project_collaborators(test_project_id)
+    assert not any(c["id"] == new_user["id"] for c in collaborators)

From e528c5ba526c5c2a69edf5d9944edeb1af6fbad9 Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Thu, 13 Feb 2025 23:43:16 +0100
Subject: [PATCH 03/11] black formatting

---
 mergin/client.py           | 13 +++++--------
 mergin/test/test_client.py | 12 ++++++------
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index d649a029..23ace62a 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -1211,8 +1211,7 @@ def reset_local_changes(self, directory: str, files_to_reset: typing.List[str] =
             self.download_files(directory, files_download, version=current_version)
 
     def download_files(
-            self, project_dir: str, file_paths: typing.List[str], output_paths: typing.List[str] = None,
-            version: str = None
+        self, project_dir: str, file_paths: typing.List[str], output_paths: typing.List[str] = None, version: str = None
     ):
         """
         Download project files at specified version. Get the latest if no version specified.
@@ -1267,8 +1266,9 @@ def list_workspace_members(self, workspace_id: int):
         resp = self.get(f"v2/workspaces/{workspace_id}/members")
         return json.load(resp)
 
-    def update_workspace_member(self, workspace_id: int, user_id: int, workspace_role: str,
-                                reset_projects_roles: bool = False):
+    def update_workspace_member(
+        self, workspace_id: int, user_id: int, workspace_role: str, reset_projects_roles: bool = False
+    ):
         """
         Update workspace role of a workspace member, optionally resets the projects role
         """
@@ -1296,10 +1296,7 @@ def add_project_collaborator(self, project_id: int, user: str, project_role: str
         """
         Add a user to project collaborators and grant them a project role
         """
-        params = {
-            "role": project_role,
-            "user": user
-        }
+        params = {"role": project_role, "user": user}
         project_collaborator = self.post(f"v2/projects/{project_id}/collaborators", params, json_headers)
         return json.load(project_collaborator)
 
diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py
index 8c657282..8da2fbd6 100644
--- a/mergin/test/test_client.py
+++ b/mergin/test/test_client.py
@@ -355,8 +355,8 @@ def test_push_pull_changes(mc):
     assert os.path.exists(os.path.join(project_dir_2, "renamed.txt"))
     assert os.path.exists(os.path.join(project_dir_2, conflicted_copy_file_name(f_updated, API_USER, 1)))
     assert (
-            generate_checksum(os.path.join(project_dir_2, conflicted_copy_file_name(f_updated, API_USER, 1)))
-            == f_conflict_checksum
+        generate_checksum(os.path.join(project_dir_2, conflicted_copy_file_name(f_updated, API_USER, 1)))
+        == f_conflict_checksum
     )
     assert generate_checksum(os.path.join(project_dir_2, f_updated)) == f_remote_checksum
 
@@ -2459,8 +2459,8 @@ def test_project_rename(mc: MerginClient):
 
     # cannot rename with full project name
     with pytest.raises(
-            ClientError,
-            match="Project's new name should be without workspace specification",
+        ClientError,
+        match="Project's new name should be without workspace specification",
     ):
         mc.rename_project(project, "workspace" + "/" + test_project_renamed)
 
@@ -2711,8 +2711,8 @@ def test_error_projects_limit_hit(mcStorage: MerginClient):
         mcStorage.create_project_and_push(test_project_fullname, project_dir)
     assert e.value.server_code == ErrorCode.ProjectsLimitHit.value
     assert (
-            e.value.detail
-            == "Maximum number of projects is reached. Please upgrade your subscription to create new projects (ProjectsLimitHit)"
+        e.value.detail
+        == "Maximum number of projects is reached. Please upgrade your subscription to create new projects (ProjectsLimitHit)"
     )
     assert e.value.http_error == 422
     assert e.value.http_method == "POST"

From c92a1458ae6a0c4dc41501bc11fa36f908771ce4 Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Fri, 14 Feb 2025 18:08:21 +0100
Subject: [PATCH 04/11] Address reviews - add project and workspace role enums
 - add optional username to create_user - remove redudant arguments from
 delete() method - add tests for failures - permission issue, duplicate call,
 editor limit hit

---
 mergin/client.py           | 38 +++++++++++--------
 mergin/common.py           | 24 ++++++++++++
 mergin/test/test_client.py | 75 ++++++++++++++++++++++++++------------
 3 files changed, 97 insertions(+), 40 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index 23ace62a..983a6c78 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -17,7 +17,7 @@
 import typing
 import warnings
 
-from .common import ClientError, LoginError
+from .common import ClientError, LoginError, WorkspaceRole, ProjectRole
 from .merginproject import MerginProject
 from .client_pull import (
     download_file_finalize,
@@ -245,9 +245,9 @@ def patch(self, path, data=None, headers={}):
         request = urllib.request.Request(url, data, headers, method="PATCH")
         return self._do_request(request)
 
-    def delete(self, path, data=None, headers={}):
+    def delete(self, path):
         url = urllib.parse.urljoin(self.url, urllib.parse.quote(path))
-        request = urllib.request.Request(url, data, headers, method="DELETE")
+        request = urllib.request.Request(url, method="DELETE")
         return self._do_request(request)
 
     def login(self, login, password):
@@ -1235,7 +1235,15 @@ def has_editor_support(self):
         """
         return is_version_acceptable(self.server_version(), "2024.4.0")
 
-    def create_user(self, email: str, password: str, workspace_id: int, workspace_role: str, notify_user: bool = False):
+    def create_user(
+        self,
+        email: str,
+        password: str,
+        workspace_id: int,
+        workspace_role: WorkspaceRole,
+        username: str = None,
+        notify_user: bool = False,
+    ):
         """
         Create a new user in a workspace. The username is generated from the email address.
         """
@@ -1243,14 +1251,12 @@ def create_user(self, email: str, password: str, workspace_id: int, workspace_ro
             "email": email,
             "password": password,
             "workspace_id": workspace_id,
-            "role": workspace_role,
+            "role": workspace_role.value,
             "notify_user": notify_user,
         }
-        try:
-            self.post("v2/users", params, json_headers)
-        except ClientError as e:
-            e.extra = f"Email: {email}"
-            raise e
+        if username:
+            params["username"] = username
+        self.post("v2/users", params, json_headers)
 
     def get_workspace_member(self, workspace_id: int, user_id: int):
         """
@@ -1267,14 +1273,14 @@ def list_workspace_members(self, workspace_id: int):
         return json.load(resp)
 
     def update_workspace_member(
-        self, workspace_id: int, user_id: int, workspace_role: str, reset_projects_roles: bool = False
+        self, workspace_id: int, user_id: int, workspace_role: WorkspaceRole, reset_projects_roles: bool = False
     ):
         """
         Update workspace role of a workspace member, optionally resets the projects role
         """
         params = {
             "reset_projects_roles": reset_projects_roles,
-            "workspace_role": workspace_role,
+            "workspace_role": workspace_role.value,
         }
         resp = self.patch(f"v2/workspaces/{workspace_id}/members/{user_id}", params, json_headers)
         return json.load(resp)
@@ -1292,19 +1298,19 @@ def list_project_collaborators(self, project_id: int):
         project_collaborators = self.get(f"v2/projects/{project_id}/collaborators")
         return json.load(project_collaborators)
 
-    def add_project_collaborator(self, project_id: int, user: str, project_role: str):
+    def add_project_collaborator(self, project_id: int, user: str, project_role: ProjectRole):
         """
         Add a user to project collaborators and grant them a project role
         """
-        params = {"role": project_role, "user": user}
+        params = {"role": project_role.value, "user": user}
         project_collaborator = self.post(f"v2/projects/{project_id}/collaborators", params, json_headers)
         return json.load(project_collaborator)
 
-    def update_project_collaborator(self, project_id: int, user_id: int, project_role: str):
+    def update_project_collaborator(self, project_id: int, user_id: int, project_role: ProjectRole):
         """
         Update project role of the existing project collaborator
         """
-        params = {"role": project_role}
+        params = {"role": project_role.value}
         project_collaborator = self.patch(f"v2/projects/{project_id}/collaborators/{user_id}", params, json_headers)
         return json.load(project_collaborator)
 
diff --git a/mergin/common.py b/mergin/common.py
index 4a10ee33..191c1753 100644
--- a/mergin/common.py
+++ b/mergin/common.py
@@ -65,3 +65,27 @@ class InvalidProject(Exception):
 
         import dateutil.parser
         from dateutil.tz import tzlocal
+
+
+class WorkspaceRole(Enum):
+    """
+    Workspace roles
+    """
+
+    GUEST = "guest"
+    READER = "reader"
+    EDITOR = "editor"
+    WRITER = "writer"
+    ADMIN = "admin"
+    OWNER = "owner"
+
+
+class ProjectRole(Enum):
+    """
+    Project roles
+    """
+
+    READER = "reader"
+    EDITOR = "editor"
+    WRITER = "writer"
+    OWNER = "owner"
diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py
index 8da2fbd6..5d0440cd 100644
--- a/mergin/test/test_client.py
+++ b/mergin/test/test_client.py
@@ -39,7 +39,7 @@
 from ..merginproject import pygeodiff
 from ..report import create_report
 from ..editor import EDITOR_ROLE_NAME, filter_changes, is_editor_enabled
-from ..common import ErrorCode
+from ..common import ErrorCode, WorkspaceRole, ProjectRole
 
 SERVER_URL = os.environ.get("TEST_MERGIN_URL")
 API_USER = os.environ.get("TEST_API_USERNAME")
@@ -51,6 +51,8 @@
 CHANGED_SCHEMA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "modified_schema")
 STORAGE_WORKSPACE = os.environ.get("TEST_STORAGE_WORKSPACE", "testpluginstorage")
 
+json_headers = {"Content-Type": "application/json"}
+
 
 def get_limit_overrides(storage: int):
     return {"storage": storage, "projects": 2, "api_allowed": True}
@@ -2744,34 +2746,37 @@ def test_workspace_requests(mc2: MerginClient):
     assert service["subscription"] == None
 
 
-def test_access_management(mc: MerginClient):
-    # create a user in a workspace
-    workspace_id = None
-    for workspace in mc.workspaces_list():
-        if workspace["name"] == mc.username():
-            workspace_id = workspace["id"]
-            break
+def test_access_management(mc: MerginClient, mc2: MerginClient):
+    # create a user in the workspace
+    workspace_id = next((w["id"] for w in mc.workspaces_list() if w["name"] == mc.username()))
     email = "create_user" + str(random.randint(1000, 9999)) + "@client.py"
     password = "Il0vemergin"
-    role = "writer"
-    mc.create_user(email, password, workspace_id, role)
+    ws_role = WorkspaceRole.WRITER
+    mc.create_user(email, password, workspace_id, ws_role)
     workspace_members = mc.list_workspace_members(workspace_id)
     new_user = next((m for m in workspace_members if m["email"] == email))
     assert new_user
-    assert new_user["workspace_role"] == role
-    # test get workspace member
+    assert new_user["workspace_role"] == ws_role.value
+    # get workspace member
     ws_member = mc.get_workspace_member(workspace_id, new_user["id"])
     assert ws_member["email"] == email
-    assert ws_member["workspace_role"] == role
-    updated_role = "admin"
-    # test update workspace member
+    assert ws_member["workspace_role"] == ws_role.value
+    updated_role = WorkspaceRole.ADMIN
+    # update workspace member
     mc.update_workspace_member(workspace_id, new_user["id"], updated_role)
     updated_user = mc.get_workspace_member(workspace_id, new_user["id"])
-    assert updated_user["workspace_role"] == updated_role
-    # test remove workspace member
+    assert updated_user["workspace_role"] == updated_role.value
+    # test permissions - a different client cannot update the role
+    with pytest.raises(ClientError, match=f"You do not have admin permissions to workspace"):
+        mc2.update_workspace_member(workspace_id, new_user["id"], ws_role)
+    # remove workspace member
     mc.remove_workspace_member(workspace_id, new_user["id"])
     workspace_members = mc.list_workspace_members(workspace_id)
     assert not any(m["id"] == new_user["id"] for m in workspace_members)
+    # duplicated call
+    with pytest.raises(ClientError) as exc_info:
+        mc.remove_workspace_member(workspace_id, new_user["id"])
+    assert exc_info.value.http_error == 404
     # add project
     test_project_name = "test_collaborators"
     test_project_fullname = API_USER + "/" + test_project_name
@@ -2780,20 +2785,42 @@ def test_access_management(mc: MerginClient):
     mc.create_project(test_project_name)
     project_info = get_project_info(mc, API_USER, test_project_name)
     test_project_id = project_info["id"]
-    project_role = "reader"
-    # test add project collaborator
+    project_role = ProjectRole.READER
+    # user must be added to project collaborators before updating project role
+    updated_role = ProjectRole.OWNER
+    with pytest.raises(ClientError) as exc_info2:
+        mc.update_project_collaborator(test_project_id, new_user["id"], updated_role)
+    assert exc_info2.value.http_error == 404
+    # add project collaborator
     mc.add_project_collaborator(test_project_id, new_user["email"], project_role)
     collaborators = mc.list_project_collaborators(test_project_id)
     new_collaborator = next((c for c in collaborators if c["id"] == new_user["id"]))
     assert new_collaborator
-    assert new_collaborator["project_role"] == project_role
-    updated_role = "owner"
-    # test update project collaborator
+    assert new_collaborator["project_role"] == project_role.value
+    # update project collaborator
     mc.update_project_collaborator(test_project_id, new_user["id"], updated_role)
     collaborators = mc.list_project_collaborators(test_project_id)
     updated_collaborator = next((c for c in collaborators if c["id"] == new_user["id"]))
-    assert updated_collaborator["project_role"] == updated_role
-    # test remove project collaborator
+    assert updated_collaborator["project_role"] == updated_role.value
+    # remove project collaborator
     mc.remove_project_collaborator(test_project_id, new_user["id"])
     collaborators = mc.list_project_collaborators(test_project_id)
     assert not any(c["id"] == new_user["id"] for c in collaborators)
+    # try to assign new editor when editors limit is reached
+    ws_usage = mc.workspace_usage(workspace_id)
+    editors_usage = ws_usage["editors"]["editors_count"] + ws_usage["editors"]["invitations_count"]
+    mc.patch(
+        f"/v1/tests/workspaces/{workspace_id}",
+        {"limits_override": {"editors": editors_usage}},
+        json_headers,
+    )
+    editor_role = ProjectRole.EDITOR
+    with pytest.raises(ClientError, match="Maximum number of editors in this workspace is reached."):
+        mc.add_project_collaborator(test_project_id, new_user["email"], editor_role)
+    # set limits to the original state
+    orig_projects_limit = ws_usage["projects"]["quota"]
+    mc.patch(
+        f"/v1/tests/workspaces/{workspace_id}",
+        {"limits_override": {"projects": orig_projects_limit}},
+        json_headers,
+    )

From cccddffad7c815d8bb1fbffbb52e757aa4d3fd6e Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Mon, 17 Feb 2025 16:54:45 +0100
Subject: [PATCH 05/11] Improve arguments description

---
 mergin/client.py           | 40 ++++++++++++++++++++++++++------------
 mergin/test/test_client.py |  5 ++++-
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index 983a6c78..ffbe9635 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -17,6 +17,8 @@
 import typing
 import warnings
 
+from typing import List
+
 from .common import ClientError, LoginError, WorkspaceRole, ProjectRole
 from .merginproject import MerginProject
 from .client_pull import (
@@ -1243,9 +1245,16 @@ def create_user(
         workspace_role: WorkspaceRole,
         username: str = None,
         notify_user: bool = False,
-    ):
+    ) -> dict:
         """
         Create a new user in a workspace. The username is generated from the email address.
+
+        param email: email of the new user - must be unique
+        param password: password - must meet the requirements
+        param workspace_id: id of the workspace user is created in
+        param workspace_role: workspace role of the user
+        param username: username - will be autogenerated from the email if not provided
+        param notify_user: flag for email notifications - confirmation email will be sent
         """
         params = {
             "email": email,
@@ -1256,16 +1265,17 @@ def create_user(
         }
         if username:
             params["username"] = username
-        self.post("v2/users", params, json_headers)
+        user_info = self.post("v2/users", params, json_headers)
+        return json.load(user_info)
 
-    def get_workspace_member(self, workspace_id: int, user_id: int):
+    def get_workspace_member(self, workspace_id: int, user_id: int) -> dict:
         """
         Get a workspace member detail
         """
         resp = self.get(f"v2/workspaces/{workspace_id}/members/{user_id}")
         return json.load(resp)
 
-    def list_workspace_members(self, workspace_id: int):
+    def list_workspace_members(self, workspace_id: int) -> List[dict]:
         """
         Get a list of workspace members
         """
@@ -1274,16 +1284,18 @@ def list_workspace_members(self, workspace_id: int):
 
     def update_workspace_member(
         self, workspace_id: int, user_id: int, workspace_role: WorkspaceRole, reset_projects_roles: bool = False
-    ):
+    ) -> dict:
         """
         Update workspace role of a workspace member, optionally resets the projects role
+
+        param reset_projects_roles: all project specific roles will be removed
         """
         params = {
             "reset_projects_roles": reset_projects_roles,
             "workspace_role": workspace_role.value,
         }
-        resp = self.patch(f"v2/workspaces/{workspace_id}/members/{user_id}", params, json_headers)
-        return json.load(resp)
+        workspace_member = self.patch(f"v2/workspaces/{workspace_id}/members/{user_id}", params, json_headers)
+        return json.load(workspace_member)
 
     def remove_workspace_member(self, workspace_id: int, user_id: int):
         """
@@ -1291,24 +1303,28 @@ def remove_workspace_member(self, workspace_id: int, user_id: int):
         """
         self.delete(f"v2/workspaces/{workspace_id}/members/{user_id}")
 
-    def list_project_collaborators(self, project_id: int):
+    def list_project_collaborators(self, project_id: int) -> List[dict]:
         """
         Get a list of project collaborators
         """
         project_collaborators = self.get(f"v2/projects/{project_id}/collaborators")
         return json.load(project_collaborators)
 
-    def add_project_collaborator(self, project_id: int, user: str, project_role: ProjectRole):
+    def add_project_collaborator(self, project_id: int, user: str, project_role: ProjectRole) -> dict:
         """
-        Add a user to project collaborators and grant them a project role
+        Add a user to project collaborators and grant them a project role.
+        Fails if user is already a member of the project.
+
+        param user: login (username or email) of the user
         """
         params = {"role": project_role.value, "user": user}
         project_collaborator = self.post(f"v2/projects/{project_id}/collaborators", params, json_headers)
         return json.load(project_collaborator)
 
-    def update_project_collaborator(self, project_id: int, user_id: int, project_role: ProjectRole):
+    def update_project_collaborator(self, project_id: int, user_id: int, project_role: ProjectRole) -> dict:
         """
-        Update project role of the existing project collaborator
+        Update project role of the existing project collaborator.
+        Fails if user is not a member of the project yet.
         """
         params = {"role": project_role.value}
         project_collaborator = self.patch(f"v2/projects/{project_id}/collaborators/{user_id}", params, json_headers)
diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py
index 5d0440cd..65c496e0 100644
--- a/mergin/test/test_client.py
+++ b/mergin/test/test_client.py
@@ -2752,7 +2752,10 @@ def test_access_management(mc: MerginClient, mc2: MerginClient):
     email = "create_user" + str(random.randint(1000, 9999)) + "@client.py"
     password = "Il0vemergin"
     ws_role = WorkspaceRole.WRITER
-    mc.create_user(email, password, workspace_id, ws_role)
+    user_info = mc.create_user(email, password, workspace_id, ws_role)
+    assert user_info["email"] == email
+    assert user_info["receive_notifications"] is False
+    # list workspace members
     workspace_members = mc.list_workspace_members(workspace_id)
     new_user = next((m for m in workspace_members if m["email"] == email))
     assert new_user

From 68b54050ead4e287d8b25aafae4cbc4cd9663784 Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Mon, 17 Feb 2025 16:55:30 +0100
Subject: [PATCH 06/11] Add deprecated coming soon warning

---
 mergin/client.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/mergin/client.py b/mergin/client.py
index ffbe9635..c16c4cc4 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -804,6 +804,12 @@ def add_user_permissions_to_project(self, project_path, usernames, permission_le
             if permission_level in ("writer", "owner", "editor", "reader"):
                 access.get("readersnames").append(name)
         self.set_project_access(project_path, access)
+        warnings.warn(
+            "This method will be deprecated in the next major release (1.0.0)"
+            "Use `add_project_collaborator` to create a project permission and "
+            "`update_project_collaborator` to change it instead.",
+            category=DeprecationWarning,
+        )
 
     def remove_user_permissions_from_project(self, project_path, usernames):
         """
@@ -823,6 +829,11 @@ def remove_user_permissions_from_project(self, project_path, usernames):
             if name in access.get("readersnames", []):
                 access.get("readersnames").remove(name)
         self.set_project_access(project_path, access)
+        warnings.warn(
+            "This method will be deprecated in the next major release (1.0.0)"
+            "Use `remove_project_collaborator` instead.",
+            category=DeprecationWarning,
+        )
 
     def project_user_permissions(self, project_path):
         """

From e1b806e07cb743fdc94a801ece37529ee7f5d4bf Mon Sep 17 00:00:00 2001
From: "marcel.kocisek" <marcel.kocisek@lutraconsulting.co.uk>
Date: Mon, 17 Feb 2025 09:58:36 +0100
Subject: [PATCH 07/11] Trying to fix coveralls

---
 .github/workflows/autotests.yml | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/autotests.yml b/.github/workflows/autotests.yml
index 50b5660a..fc658a58 100644
--- a/.github/workflows/autotests.yml
+++ b/.github/workflows/autotests.yml
@@ -28,10 +28,9 @@ jobs:
 
       - name: Run tests
         run: |
-          pytest --cov=mergin mergin/test/
+          pytest --cov=mergin --cov-report=lcov mergin/test/
 
       - name: Submit coverage to Coveralls
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        run: |
-          coveralls --service=github
+        uses: coverallsapp/github-action@v2
+        with:
+          format: lcov

From 93971bdabaa32e7f925597ea22fb583b0890f060 Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Mon, 17 Feb 2025 17:03:56 +0100
Subject: [PATCH 08/11] Use new method in editor test

---
 mergin/test/test_client.py | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py
index 65c496e0..d4f489c1 100644
--- a/mergin/test/test_client.py
+++ b/mergin/test/test_client.py
@@ -2599,17 +2599,17 @@ def test_editor_push(mc: MerginClient, mc2: MerginClient):
     """Test push with editor"""
     if not mc.has_editor_support():
         return
-    test_project = "test_editor_push"
-    test_project_fullname = API_USER + "/" + test_project
-    project = API_USER + "/" + test_project
-    project_dir = os.path.join(TMP_DIR, test_project)
-    project_dir2 = os.path.join(TMP_DIR, test_project + "_2")
-    cleanup(mc, project, [project_dir, project_dir2])
+    test_project_name = "test_editor_push"
+    test_project_fullname = API_USER + "/" + test_project_name
+    project_dir = os.path.join(TMP_DIR, test_project_name)
+    project_dir2 = os.path.join(TMP_DIR, test_project_name + "_2")
+    cleanup(mc, test_project_fullname, [project_dir, project_dir2])
 
     # create new (empty) project on server
     # TODO: return project_info from create project, don't use project_full name for project info, instead returned id of project
-    mc.create_project(test_project)
-    mc.add_user_permissions_to_project(project, [API_USER2], "editor")
+    mc.create_project(test_project_name)
+    project_info = get_project_info(mc, API_USER, test_project_name)
+    mc.add_project_collaborator(project_info["id"], mc2.username(), ProjectRole.EDITOR)
     # download empty project
     mc2.download_project(test_project_fullname, project_dir)
 
@@ -2651,12 +2651,11 @@ def test_editor_push(mc: MerginClient, mc2: MerginClient):
     # editor is trying to update qgis file
     with open(os.path.join(project_dir, qgs_file_name), "a") as f:
         f.write("Editor is here!")
-    project_info = mc2.project_info(test_project_fullname)
     pull_changes, push_changes, push_changes_summary = mc.project_status(project_dir)
     # ggs is still waiting to push
     assert any(file["path"] == qgs_file_name for file in push_changes.get("updated")) is True
 
-    # push as owner do cleanup local changes and preparation to conflicited copy simulate
+    # push as owner do cleanup local changes and preparation to conflicted copy simulate
     mc.push_project(project_dir)
 
     # simulate conflicting copy of qgis file

From 451449d5ad471bc3febc19e2c07d49bdf8a7ede4 Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Thu, 20 Feb 2025 11:36:07 +0100
Subject: [PATCH 09/11] Improve error handling - show custom details

---
 mergin/client.py           | 15 +++++++++++++--
 mergin/test/test_client.py |  9 +++++++--
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index c16c4cc4..0a547c38 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -210,9 +210,20 @@ def _do_request(self, request):
         except urllib.error.HTTPError as e:
             server_response = json.load(e)
 
-            # We first to try to get the value from the response otherwise we set a default value
-            err_detail = server_response.get("detail", e.read().decode("utf-8"))
             server_code = server_response.get("code", None)
+            # Try to get error detail
+            if isinstance(server_response, dict):
+                if "detail" in server_response:
+                    err_detail = server_response["detail"]
+                else:
+                    # Extract all field-specific errors and format them
+                    err_detail = "\n".join(
+                        f"{key}: {', '.join(map(str, value))}"
+                        for key, value in server_response.items()
+                        if isinstance(value, list)
+                    ) or str(server_response)  # Fallback to raw response if structure is unexpected
+            else:
+                err_detail = str(server_response)
 
             raise ClientError(
                 detail=err_detail,
diff --git a/mergin/test/test_client.py b/mergin/test/test_client.py
index d4f489c1..ba14e6bc 100644
--- a/mergin/test/test_client.py
+++ b/mergin/test/test_client.py
@@ -2746,11 +2746,16 @@ def test_workspace_requests(mc2: MerginClient):
 
 
 def test_access_management(mc: MerginClient, mc2: MerginClient):
-    # create a user in the workspace
+    # create a user in the workspace -
     workspace_id = next((w["id"] for w in mc.workspaces_list() if w["name"] == mc.username()))
+    ws_role = WorkspaceRole.WRITER
     email = "create_user" + str(random.randint(1000, 9999)) + "@client.py"
+    # returning meaningful error when requirements are not met
+    password = "1234"
+    with pytest.raises(ClientError, match=f"Passwords must be at least 8 characters long."):
+        mc.create_user(email, password, workspace_id, ws_role)
+    # strong password
     password = "Il0vemergin"
-    ws_role = WorkspaceRole.WRITER
     user_info = mc.create_user(email, password, workspace_id, ws_role)
     assert user_info["email"] == email
     assert user_info["receive_notifications"] is False

From 0e448f01a9834ce4c44af88c10f5316cce50a51e Mon Sep 17 00:00:00 2001
From: Herman Snevajs <herman.snevajs@lutraconsulting.co.uk>
Date: Thu, 20 Feb 2025 11:55:10 +0100
Subject: [PATCH 10/11] black

---
 mergin/client.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mergin/client.py b/mergin/client.py
index 0a547c38..297fc9ff 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -221,7 +221,9 @@ def _do_request(self, request):
                         f"{key}: {', '.join(map(str, value))}"
                         for key, value in server_response.items()
                         if isinstance(value, list)
-                    ) or str(server_response)  # Fallback to raw response if structure is unexpected
+                    ) or str(
+                        server_response
+                    )  # Fallback to raw response if structure is unexpected
             else:
                 err_detail = str(server_response)
 

From e7aed8eb8bf0522dc455bd7c4ac3d98f899066ab Mon Sep 17 00:00:00 2001
From: "marcel.kocisek" <marcel.kocisek@lutraconsulting.co.uk>
Date: Fri, 21 Feb 2025 12:04:37 +0100
Subject: [PATCH 11/11] Improve do request handling

---
 mergin/client.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mergin/client.py b/mergin/client.py
index 297fc9ff..86d0f818 100644
--- a/mergin/client.py
+++ b/mergin/client.py
@@ -210,12 +210,13 @@ def _do_request(self, request):
         except urllib.error.HTTPError as e:
             server_response = json.load(e)
 
-            server_code = server_response.get("code", None)
+            err_detail = None
+            server_code = None
             # Try to get error detail
             if isinstance(server_response, dict):
-                if "detail" in server_response:
-                    err_detail = server_response["detail"]
-                else:
+                server_code = server_response.get("code")
+                err_detail = server_response.get("detail")
+                if not err_detail:
                     # Extract all field-specific errors and format them
                     err_detail = "\n".join(
                         f"{key}: {', '.join(map(str, value))}"