Added initial push handling for editors

Author: MarcelGeo

mergin/client.py

@@ -763,7 +763,7 @@ def set_project_access(self, project_path, access):
         :param project_path: project full name (<namespace>/<name>)
         :param access: dict <readersnames, editorsnames, writersnames, ownersnames> -> list of str username we want to give access to (editorsnames are only supported on servers at version 2024.4.0 or later)
         """
-        if "editorsnames" in access and not is_version_acceptable(self.server_version(), "2024.4"):
+        if "editorsnames" in access and not self.has_editor_support():
             raise NotImplementedError("Editors are only supported on servers at version 2024.4.0 or later")
 
         if not self._user_info:
@@ -790,7 +790,7 @@ def add_user_permissions_to_project(self, project_path, usernames, permission_le
         Editor permission_level is only supported on servers at version 2024.4.0 or later.
         """
         if permission_level not in ["owner", "reader", "writer", "editor"] or (
-            permission_level == "editor" and not is_version_acceptable(self.server_version(), "2024.4")
+            permission_level == "editor" and not self.has_editor_support()
         ):
             raise ClientError("Unsupported permission level")
 
@@ -1235,3 +1235,9 @@ def download_files(
         job = download_files_async(self, project_dir, file_paths, output_paths, version=version)
         pull_project_wait(job)
         download_files_finalize(job)
+
+    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")

mergin/client_push.py

@@ -18,6 +18,7 @@
 
 from .common import UPLOAD_CHUNK_SIZE, ClientError
 from .merginproject import MerginProject
+from .editor import EditorHandler
 
 
 class UploadJob:
@@ -91,12 +92,12 @@ def push_project_async(mc, directory):
     mp.log.info(f"--- start push {project_path}")
 
     try:
-        server_info = mc.project_info(project_path)
+        project_info = mc.project_info(project_path)
     except ClientError as err:
         mp.log.error("Error getting project info: " + str(err))
         mp.log.info("--- push aborted")
         raise
-    server_version = server_info["version"] if server_info["version"] else "v0"
+    server_version = project_info["version"] if project_info["version"] else "v0"
 
     mp.log.info(f"got project info: local version {local_version} / server version {server_version}")
 
@@ -132,6 +133,9 @@ def push_project_async(mc, directory):
     for f in changes["added"]:
         if mp.is_versioned_file(f["path"]):
             mp.copy_versioned_file_for_upload(f, tmp_dir.name)
+    
+    editorHandler = EditorHandler(mc, project_info)
+    changes = editorHandler.filter_changes(changes)
 
     if not sum(len(v) for v in changes.values()):
         mp.log.info(f"--- push {project_path} - nothing to do")

mergin/editor.py

@@ -0,0 +1,56 @@
+from itertools import filterfalse
+
+from .utils import is_mergin_config, is_qgis_file, is_versioned_file
+
+
+class EditorHandler:
+    def __init__(self, mergin_client, project_info: dict):
+        """
+        Initializes the editor instance with the provided Mergin client and project information.
+
+        Args:
+            mergin_client (MerginClient): The Mergin client instance.
+            project_info (dict): The project information dictionary.
+        """
+        self.mc = mergin_client
+        self.project_info = project_info
+        self.ROLE_NAME = "editor"
+
+    _disallowed_added_changes = lambda self, change: is_qgis_file(change["path"]) or is_mergin_config(change["path"])
+    _disallowed_updated_changes = (
+        lambda self, change: is_qgis_file(change["path"])
+        or is_mergin_config(change["path"])
+        or (is_versioned_file(change["path"]) and change.get("diff") is None)
+    )
+    _disallowed_removed_changes = (
+        lambda self, change: is_qgis_file(change["path"])
+        or is_mergin_config(change["path"])
+        or is_versioned_file(change["path"])
+    )
+
+    def is_enabled(self):
+        """
+        Returns True if the current user has editor access to the project, False otherwise.
+
+        The method checks if the server supports editor access, and if the current user's project role matches the expected role name for editors.
+        """
+        server_support_editor = self.mc.has_editor_support()
+        project_role = self.project_info.get("role")
+
+        return server_support_editor and project_role == self.ROLE_NAME
+
+    def _apply_editor_filters(self, changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
+        added = changes.get("added", [])
+        updated = changes.get("updated", [])
+        removed = changes.get("removed", [])
+
+        # filter out files that are not in the editor's list of allowed files
+        changes["added"] = list(filterfalse(self._disallowed_added_changes, added))
+        changes["updated"] = list(filterfalse(self._disallowed_updated_changes, updated))
+        changes["removed"] = list(filterfalse(self._disallowed_removed_changes, removed))
+        return changes
+
+    def filter_changes(self, changes: dict[str, list[dict]]) -> dict[str, list[dict]]:
+        if not self.is_enabled():
+            return changes
+        return self._apply_editor_filters(changes)

mergin/test/test_client.py

@@ -39,6 +39,7 @@
 )
 from ..merginproject import pygeodiff
 from ..report import create_report
+from ..editor import EditorHandler
 
 
 SERVER_URL = os.environ.get("TEST_MERGIN_URL")
@@ -113,7 +114,12 @@ def server_has_editor_support(mc, access):
     Returns:
         bool: True if the server has editor support, False otherwise.
     """
-    return "editorsnames" in access and is_version_acceptable(mc.server_version(), "2024.4")
+    return "editorsnames" in access and mc.has_editor_support()
+
+
+def test_client_instance(mc, mc2):
+    assert isinstance(mc, MerginClient)
+    assert isinstance(mc2, MerginClient)
 
 
 def test_login(mc):
@@ -1191,42 +1197,36 @@ def test_download_diffs(mc):
 
 def test_modify_project_permissions(mc):
     test_project = "test_project"
-    project = API_USER + "/" + test_project
+    test_project_fullname = API_USER + "/" + test_project
     project_dir = os.path.join(TMP_DIR, test_project)
     download_dir = os.path.join(TMP_DIR, "download", test_project)
 
-    cleanup(mc, project, [project_dir, download_dir])
+    cleanup(mc, test_project_fullname, [project_dir, download_dir])
     # prepare local project
     shutil.copytree(TEST_DATA_DIR, project_dir)
 
     # create remote project
-    mc.create_project_and_push(project, directory=project_dir)
-
-    # check basic metadata about created project
-    project_info = mc.project_info(project)
-    assert project_info["version"] == "v1"
-    assert project_info["name"] == test_project
-    assert project_info["namespace"] == API_USER
+    mc.create_project_and_push(test_project_fullname, directory=project_dir)
 
-    permissions = mc.project_user_permissions(project)
+    permissions = mc.project_user_permissions(test_project_fullname)
     assert permissions["owners"] == [API_USER]
     assert permissions["writers"] == [API_USER]
     assert permissions["readers"] == [API_USER]
     editor_support = server_has_editor_support(mc, permissions)
     if editor_support:
         assert permissions["editors"] == [API_USER]
 
-    mc.add_user_permissions_to_project(project, [API_USER2], "writer")
-    permissions = mc.project_user_permissions(project)
+    mc.add_user_permissions_to_project(test_project_fullname, [API_USER2], "writer")
+    permissions = mc.project_user_permissions(test_project_fullname)
     assert set(permissions["owners"]) == {API_USER}
     assert set(permissions["writers"]) == {API_USER, API_USER2}
     assert set(permissions["readers"]) == {API_USER, API_USER2}
     editor_support = server_has_editor_support(mc, permissions)
     if editor_support:
         assert set(permissions["editors"]) == {API_USER, API_USER2}
 
-    mc.remove_user_permissions_from_project(project, [API_USER2])
-    permissions = mc.project_user_permissions(project)
+    mc.remove_user_permissions_from_project(test_project_fullname, [API_USER2])
+    permissions = mc.project_user_permissions(test_project_fullname)
     assert permissions["owners"] == [API_USER]
     assert permissions["writers"] == [API_USER]
     assert permissions["readers"] == [API_USER]
@@ -2524,3 +2524,117 @@ def test_download_failure(mc):
     with open(job.failure_log_file, "r", encoding="utf-8") as f:
         content = f.read()
         assert "Traceback" in content
+
+
+# TODO: consider to use separate test_editor.py file for tests that require editor
+def test_editor_handler(mc: MerginClient):
+    """Test editor handler class and push with editor"""
+
+    project_info = {"role": "editor"}
+    # intilize handler
+    editor_handler = EditorHandler(mc, project_info)
+    if not mc.has_editor_support():
+        assert editor_handler.is_enabled() is False
+        return
+
+    # mock that user is editor
+    project_info["role"] = editor_handler.ROLE_NAME
+    assert editor_handler.is_enabled() is True
+
+    # unit test for EditorHandler methods
+    qgs_changeset = {
+        "added": [{"path": "/folder/project.new.Qgz"}],
+        "updated": [{"path": "/folder/project.updated.Qgs"}],
+        "removed": [{"path": "/folder/project.removed.qgs"}],
+    }
+    qgs_changeset = editor_handler.filter_changes(qgs_changeset)
+    assert sum(len(v) for v in qgs_changeset.values()) == 0
+
+    mergin_config_changeset = {
+        "added": [{"path": "/.mergin/mergin-config.json"}],
+        "updated": [{"path": "/.mergin/mergin-config.json"}],
+        "removed": [{"path": "/.mergin/mergin-config.json"}],
+    }
+    mergin_config_changeset = editor_handler.filter_changes(mergin_config_changeset)
+    assert sum(len(v) for v in mergin_config_changeset.values()) == 0
+
+    gpkg_changeset = {
+        "added": [{"path": "/.mergin/data.gpkg"}],
+        "updated": [{"path": "/.mergin/conflict-data.gpkg"}, {"path": "/.mergin/data.gpkg", "diff": {}}],
+        "removed": [{"path": "/.mergin/data.gpkg"}],
+    }
+    gpkg_changeset = editor_handler.filter_changes(gpkg_changeset)
+    assert sum(len(v) for v in gpkg_changeset.values()) == 2
+    assert gpkg_changeset["added"][0]["path"] == "/.mergin/data.gpkg"
+    assert gpkg_changeset["updated"][0]["path"] == "/.mergin/data.gpkg"
+
+
+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)
+    cleanup(mc, project, [project_dir])
+
+    # 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")
+    project_info = mc2.project_info(test_project_fullname)
+    editor_handler = EditorHandler(mc2, project_info)
+    assert project_info["role"] == editor_handler.ROLE_NAME
+    assert editor_handler.is_enabled() is True
+
+    # download empty project
+    mc2.download_project(test_project_fullname, project_dir)
+
+    # editor is starting to adding qgis files and "normal" file
+    qgs_file_name = "test.qgs"
+    txt_file_name = "test.txt"
+    gpkg_file_name = "base.gpkg"
+    files_to_push = [qgs_file_name, txt_file_name, gpkg_file_name]
+    for file in files_to_push:
+        shutil.copy(os.path.join(TEST_DATA_DIR, file), project_dir)
+    # it's possible to push allowed files if editor
+    mc2.push_project(project_dir)
+    project_info = mc2.project_info(test_project_fullname)
+    assert len(project_info.get("files")) == len(files_to_push) - 1  # ggs is not pushed
+    # find pushed files in server
+    assert any(file["path"] == qgs_file_name for file in project_info.get("files")) is False
+    assert any(file["path"] == txt_file_name for file in project_info.get("files")) is True
+    assert any(file["path"] == gpkg_file_name for file in project_info.get("files")) is True
+    pull_changes, push_changes, push_changes_summary = mc.project_status(project_dir)
+    assert not sum(len(v) for v in pull_changes.values())
+    assert sum(len(v) for v in push_changes.values()) == 1  
+    # ggs is still waiting to push
+    assert any(file["path"] == qgs_file_name for file in push_changes.get("added")) is True
+
+    # editor is trying to psuh row to gpkg file -> it's possible
+    shutil.copy(
+        os.path.join(TEST_DATA_DIR, "inserted_1_A.gpkg"),
+        os.path.join(project_dir, gpkg_file_name),
+    )
+    mc2.push_project(project_dir)
+    project_info = mc2.project_info(test_project_fullname)
+    pull_changes, push_changes, push_changes_summary = mc.project_status(project_dir)
+    assert any(file["path"] == gpkg_file_name for file in project_info.get("files")) is True
+    assert any(file["path"] == gpkg_file_name for file in push_changes.get("updated")) is False
+
+    # editor is trying to insert tables to gpkg file
+    shutil.copy(
+        os.path.join(TEST_DATA_DIR, "two_tables.gpkg"),
+        os.path.join(project_dir, gpkg_file_name),
+    )
+    mc2.push_project(project_dir)
+    pull_changes, push_changes, push_changes_summary = mc.project_status(project_dir)
+    assert not sum(len(v) for v in pull_changes.values())
+    # gpkg was filter by editor_handler in push_project, because new tables added
+    assert sum(len(v) for v in push_changes.values()) == 2 
+    # ggs and gpkg are still waiting to push
+    assert any(file["path"] == qgs_file_name for file in push_changes.get("added")) is True
+    assert any(file["path"] == gpkg_file_name for file in push_changes.get("updated")) is True
+

mergin/utils.py

@@ -255,3 +255,20 @@ def is_version_acceptable(version, min_version):
     major, minor = m.group(1), m.group(2)
 
     return major > min_major or (major == min_major and minor >= min_minor)
+
+def is_versioned_file(path: str) -> bool:
+    diff_extensions = [".gpkg", ".sqlite"]
+    f_extension = os.path.splitext(path)[1]
+    return f_extension.lower() in diff_extensions
+
+def is_qgis_file(path: str) -> bool:
+    """
+    Check if file is a QGIS project file.
+    """
+    f_extension = os.path.splitext(path)[1]
+    return f_extension.lower() in [".qgs", ".qgz"]
+
+def is_mergin_config(path: str) -> bool:
+    """Check if the given path is for file mergin-config.json"""
+    filename = os.path.basename(path).lower()
+    return filename == "mergin-config.json"