Editor validation gh#581 (#583)

* fix dev-docs.md

* validate project file change

* validate non-diffable edits

* mergin-config check

* check if a .gpkg was deleted

* Upgrade to mergin client 0.9.1

* Py-client v0.9.2

.github/workflows/packages.yml

@@ -1,6 +1,6 @@
 name: Build Mergin Plugin Packages
 env:
-  MERGIN_CLIENT_VER: "0.9.0"
+  MERGIN_CLIENT_VER: "0.9.2"
   GEODIFF_VER: "2.0.2"
   PYTHON_VER: "38"
   PLUGIN_NAME: Mergin

Mergin/project_status_dialog.py

@@ -1,5 +1,6 @@
 import os
 from itertools import groupby
+from urllib.parse import urlparse, parse_qs
 
 from qgis.PyQt import uic
 from qgis.PyQt.QtWidgets import (
@@ -16,7 +17,7 @@
 from qgis.core import Qgis, QgsApplication, QgsProject
 from qgis.utils import OverrideCursor
 from .diff_dialog import DiffViewerDialog
-from .validation import MultipleLayersWarning, warning_display_string, MerginProjectValidator
+from .validation import MultipleLayersWarning, warning_display_string, MerginProjectValidator, SingleLayerWarning
 from .utils import is_versioned_file, icon_path, unsaved_project_check, UnsavedChangesStrategy
 from .repair import fix_datum_shift_grids
 
@@ -43,10 +44,14 @@ def __init__(
         push_changes_summary,
         has_write_permissions,
         mergin_project=None,
+        project_permission=None,
         parent=None,
     ):
         QDialog.__init__(self, parent)
         self.ui = uic.loadUi(ui_file, self)
+        self.project_permission = project_permission
+        self.push_changes = push_changes
+        self.file_to_reset = None
 
         with OverrideCursor(Qt.WaitCursor):
             QgsGui.instance().enableAutoGeometryRestore(self)
@@ -175,15 +180,16 @@ def show_validation_results(self, results):
         html = []
 
         # separate MultipleLayersWarning and SingleLayerWarning items
-        groups = dict()
-        for k, v in groupby(results, key=lambda x: "multi" if isinstance(x, MultipleLayersWarning) else "single"):
-            groups[k] = list(v)
+        groups = {
+            "single": [item for item in results if isinstance(item, SingleLayerWarning)],
+            "multi": [item for item in results if isinstance(item, MultipleLayersWarning)],
+        }
 
         # first add MultipleLayersWarnings. They are displayed using warning
         # string as a title and list of affected layers/items
         if "multi" in groups:
             for w in groups["multi"]:
-                issue = warning_display_string(w.id)
+                issue = warning_display_string(w.id, w.url)
                 html.append(f"<h3>{issue}</h3>")
                 if w.items:
                     items = []
@@ -202,7 +208,7 @@ def show_validation_results(self, results):
                 html.append(f"<h3>{map_layers[lid].name()}</h3>")
                 items = []
                 for w in layers[lid]:
-                    items.append(f"<li>{warning_display_string(w.warning)}</li>")
+                    items.append(f"<li>{warning_display_string(w.warning, w.url)}</li>")
                 html.append(f"<ul>{''.join(items)}</ul>")
 
         self.txtWarnings.setHtml("".join(html))
@@ -223,17 +229,20 @@ def show_changes(self):
         dlg_diff_viewer.exec_()
 
     def link_clicked(self, url):
-        function_name = url.toString()
-        if function_name == "#fix_datum_shift_grids":
+        parsed_url = urlparse(url.toString())
+        if parsed_url.path == "fix_datum_shift_grids":
             msg = fix_datum_shift_grids(self.mp)
             if msg is not None:
                 self.ui.messageBar.pushMessage("Mergin", f"Failed to fix issue: {msg}", Qgis.Warning)
                 return
+        if parsed_url.path == "reset_file":
+            query_parameters = parse_qs(parsed_url.query)
+            self.reset_local_changes(query_parameters["layer"][0])
 
         self.validate_project()
 
     def validate_project(self):
-        validator = MerginProjectValidator(self.mp)
+        validator = MerginProjectValidator(self.mp, self.push_changes, self.project_permission)
         results = validator.run_checks()
         if not results:
             self.ui.lblWarnings.hide()
@@ -243,11 +252,16 @@ def validate_project(self):
             self.show_validation_results(results)
             self.btn_sync.setStyleSheet("background-color: #ffc800")
 
-    def reset_local_changes(self):
+    def reset_local_changes(self, file_to_reset=None):
+        if file_to_reset:
+            self.file_to_reset = file_to_reset
+            text = f"Local changes to file '{file_to_reset}' will be discarded. Do you want to proceed?"
+        else:
+            text = "All changes in your project directory will be reverted. Do you want to proceed?"
         btn_reply = QMessageBox.question(
             None,
             "Reset changes",
-            "All changes in your project directory will be reverted. Do you want to proceed?",
+            text,
             QMessageBox.Yes | QMessageBox.No,
             QMessageBox.Yes,
         )

Mergin/projects_manager.py

@@ -27,7 +27,6 @@
 
 from .mergin.merginproject import MerginProject
 from .project_status_dialog import ProjectStatusDialog
-from .validation import MerginProjectValidator
 
 
 class MerginProjectsManager(object):
@@ -183,16 +182,17 @@ def project_status(self, project_dir):
                 push_changes_summary,
                 self.mc.has_writing_permissions(project_name),
                 mp,
+                self.mc.project_info(project_name)["role"],
             )
             # Sync button in the status dialog returns QDialog.Accepted
-            # and Close button retuns QDialog::Rejected, so it dialog was
+            # and Close button returns QDialog::Rejected, so if dialog was
             # accepted we start sync
             return_value = dlg.exec_()
 
             if return_value == ProjectStatusDialog.Accepted:
                 self.sync_project(project_dir)
             elif return_value == ProjectStatusDialog.RESET_CHANGES:
-                self.reset_local_changes(project_dir)
+                self.reset_local_changes(project_dir, dlg.file_to_reset)
 
         except (URLError, ClientError, InvalidProject) as e:
             msg = f"Failed to get status for project {project_name}:\n\n{str(e)}"
@@ -229,7 +229,7 @@ def check_project_server(self, project_dir, inform_user=True):
             QMessageBox.critical(None, "Mergin Maps", info)
         return False
 
-    def reset_local_changes(self, project_dir: str):
+    def reset_local_changes(self, project_dir: str, files_to_reset=None):
         if not project_dir:
             return
         if not self.check_project_server(project_dir):
@@ -241,7 +241,13 @@ def reset_local_changes(self, project_dir: str):
             QgsProject.instance().clear()
 
         try:
-            self.mc.reset_local_changes(project_dir)
+            self.mc.reset_local_changes(project_dir, files_to_reset)
+            if files_to_reset:
+                msg = f"File {files_to_reset} was successfully reset"
+            else:
+                msg = "Project local changes were successfully reset"
+            QMessageBox.information(None, "Project reset local changes", msg, QMessageBox.Close)
+
         except Exception as e:
             msg = f"Failed to reset local changes:\n\n{str(e)}"
             QMessageBox.critical(None, "Project reset local changes", msg, QMessageBox.Close)

Mergin/utils.py

@@ -1491,3 +1491,16 @@ def set_tracking_layer_flags(layer):
     """
     layer.setReadOnly(False)
     layer.setFlags(QgsMapLayer.LayerFlag(QgsMapLayer.Identifiable + QgsMapLayer.Searchable + QgsMapLayer.Removable))
+
+
+def get_layer_by_path(path):
+    """
+    Returns layer object for project layer that matches the path
+    """
+    layers = QgsProject.instance().mapLayers()
+    for layer in layers.values():
+        _, layer_path = os.path.split(layer.source())
+        # file path may contain layer name next to the file name (e.g. 'Survey_lines.gpkg|layername=lines')
+        safe_file_path = layer_path.split("|")
+        if safe_file_path[0] == path:
+            return layer

Mergin/validation.py

@@ -9,7 +9,6 @@
     QgsVectorDataProvider,
     QgsExpression,
     QgsRenderContext,
-    QgsProviderRegistry,
 )
 
 from .help import MerginHelp
@@ -22,6 +21,8 @@
     project_grids_directory,
     QGIS_DB_PROVIDERS,
     QGIS_NET_PROVIDERS,
+    is_versioned_file,
+    get_layer_by_path,
 )
 
 INVALID_CHARS = re.compile('[\\\/\(\)\[\]\{\}"\n\r]')
@@ -52,6 +53,10 @@ class Warning(Enum):
     MERGIN_SNAPPING_NOT_ENABLED = 21
     MISSING_DATUM_SHIFT_GRID = 22
     SVG_NOT_EMBEDDED = 23
+    EDITOR_PROJECT_FILE_CHANGE = 24
+    EDITOR_NON_DIFFABLE_CHANGE = 25
+    EDITOR_JSON_CONFIG_CHANGE = 26
+    EDITOR_DIFFBASED_FILE_REMOVED = 27
 
 
 class MultipleLayersWarning:
@@ -62,23 +67,25 @@ class MultipleLayersWarning:
     layers.
     """
 
-    def __init__(self, warning_id):
+    def __init__(self, warning_id, url=""):
         self.id = warning_id
         self.items = list()
+        self.url = url
 
 
 class SingleLayerWarning:
     """Class for warning which is associated with single layer."""
 
-    def __init__(self, layer_id, warning):
+    def __init__(self, layer_id, warning, url=None):
         self.layer_id = layer_id
         self.warning = warning
+        self.url = url
 
 
 class MerginProjectValidator(object):
     """Class for checking Mergin project validity and fixing the problems, if possible."""
 
-    def __init__(self, mergin_project=None):
+    def __init__(self, mergin_project=None, changes=None, project_permission=None):
         self.mp = mergin_project
         self.layers = None  # {layer_id: map layer}
         self.editable = None  # list of editable layers ids
@@ -88,6 +95,8 @@ def __init__(self, mergin_project=None):
         self.qgis_proj = None
         self.qgis_proj_path = None
         self.qgis_proj_dir = None
+        self.changes = changes
+        self.project_permission = project_permission
 
     def run_checks(self):
         if self.mp is None:
@@ -112,6 +121,7 @@ def run_checks(self):
         self.check_snapping()
         self.check_datum_shift_grids()
         self.check_svgs_embedded()
+        self.check_editor_perms()
 
         return self.issues
 
@@ -380,9 +390,39 @@ def check_svgs_embedded(self):
                     self.issues.append(SingleLayerWarning(lid, Warning.SVG_NOT_EMBEDDED))
                     break
 
-
-def warning_display_string(warning_id):
-    """Returns a display string for a corresponing warning"""
+    def check_editor_perms(self):
+        if self.project_permission != "editor":
+            return
+        # editor cannot change specific files - QGS project file, mergin-config.json file (e.g. selective sync changes)
+        for category in self.changes:
+            for file in self.changes[category]:
+                path = file["path"]
+                if path.lower().endswith((".qgs", ".qgz")):
+                    url = f"reset_file?layer={path}"
+                    self.issues.append(MultipleLayersWarning(Warning.EDITOR_PROJECT_FILE_CHANGE, url))
+                elif path.lower().endswith("mergin-config.json"):
+                    url = f"reset_file?layer={path}"
+                    self.issues.append(MultipleLayersWarning(Warning.EDITOR_JSON_CONFIG_CHANGE, url))
+        # editor cannot do non diff-based change (e.g. schema change)
+        for file in self.changes["updated"]:
+            path = file["path"]
+            if is_versioned_file(path) and "diff" not in file:
+                layer = get_layer_by_path(path)
+                if layer:
+                    url = f"reset_file?layer={path}"
+                    self.issues.append(SingleLayerWarning(layer.id(), Warning.EDITOR_NON_DIFFABLE_CHANGE, url))
+        # editor cannot delete a versioned file (e.g. '*.gpkg')
+        for file in self.changes["removed"]:
+            path = file["path"]
+            if is_versioned_file(path):
+                layer = get_layer_by_path(path)
+                if layer:
+                    url = f"reset_file?layer={path}"
+                    self.issues.append(SingleLayerWarning(layer.id(), Warning.EDITOR_DIFFBASED_FILE_REMOVED, url))
+
+
+def warning_display_string(warning_id, url=None):
+    """Returns a display string for a corresponding warning"""
     help_mgr = MerginHelp()
     if warning_id == Warning.PROJ_NOT_LOADED:
         return "The QGIS project is not loaded. Open it to allow validation"
@@ -427,6 +467,18 @@ def warning_display_string(warning_id):
     elif warning_id == Warning.MERGIN_SNAPPING_NOT_ENABLED:
         return "Snapping is currently enabled in this QGIS project, but not enabled in Mergin Maps Input"
     elif warning_id == Warning.MISSING_DATUM_SHIFT_GRID:
-        return "Required datum shift grid is missing, reprojection may not work correctly. <a href='#fix_datum_shift_grids'>Fix the issue.</a>"
+        return "Required datum shift grid is missing, reprojection may not work correctly. <a href='fix_datum_shift_grids'>Fix the issue.</a>"
     elif warning_id == Warning.SVG_NOT_EMBEDDED:
         return "SVGs used for layer styling are not embedded in the project file, as a result those symbols won't be displayed in Mergin Maps Input"
+    elif warning_id == Warning.EDITOR_PROJECT_FILE_CHANGE:
+        return (
+            f"You don't have permission to edit the QGIS project file. Your changes to this file will not be sent to the server. "
+            f"Ask the workspace admin to upgrade your permission if you want your changes sent to the server. "
+            f"You can also <a href='{url}'>reset this QGIS project file</a> to the server version."
+        )
+    elif warning_id == Warning.EDITOR_NON_DIFFABLE_CHANGE:
+        return f"You don't have permission to edit layer fields and properties. Ask the workspace admin to upgrade your permission or <a href='{url}'>reset the layer</a> to be able to sync changes."
+    elif warning_id == Warning.EDITOR_JSON_CONFIG_CHANGE:
+        return f"You don't have permission to change the configuration of this project. <a href='{url}'>Reset the configuration</a> to be able to sync data changes."
+    elif warning_id == Warning.EDITOR_DIFFBASED_FILE_REMOVED:
+        return f"You don't have permission to remove this layer. <a href='{url}'>Reset the layer</a> to be able to sync changes."

docs/dev-docs.md

@@ -1,15 +1,15 @@
 
 # Developer's documentation
 ## Development/Testing
-Download python [client](https://github.com/MerginMaps/mergin-py-client) and
+Download python [client](https://github.com/MerginMaps/mergin-py-client), install deps and
 link to qgis plugin:
 ```
     ln -s <path-to-py-client>/mergin/ <path-to-mergin-qgis-plugin>/Mergin/mergin
 ```
 
 Now link the plugin to your QGIS profile python, e.g. for MacOS
 ```  
-    ln -s <path-to-py-client>/Mergin/ <path-to-QGIS-user-folder>/QGIS3/profiles/default/python/plugins/Mergin
+    ln -s <path-to-mergin-qgis-plugin>/Mergin/ <path-to-QGIS-user-folder>/QGIS3/profiles/default/python/plugins/Mergin
 ```
 
 ## Production