Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor validation gh#581 #583

Merged
merged 13 commits into from
Jun 17, 2024
2 changes: 1 addition & 1 deletion .github/workflows/packages.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: Build Mergin Plugin Packages
env:
MERGIN_CLIENT_VER: "0.9.0"
MERGIN_CLIENT_VER: "0.9.1"
GEODIFF_VER: "2.0.2"
PYTHON_VER: "38"
PLUGIN_NAME: Mergin
Expand Down
36 changes: 25 additions & 11 deletions Mergin/project_status_dialog.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand All @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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)],
}
tomasMizera marked this conversation as resolved.
Show resolved Hide resolved

# 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 = []
Expand All @@ -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))
Expand All @@ -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()
Expand All @@ -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,
)
Expand Down
16 changes: 11 additions & 5 deletions Mergin/projects_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

from .mergin.merginproject import MerginProject
from .project_status_dialog import ProjectStatusDialog
from .validation import MerginProjectValidator


class MerginProjectsManager(object):
Expand Down Expand Up @@ -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)}"
Expand Down Expand Up @@ -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):
Expand All @@ -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)
tomasMizera marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down
13 changes: 13 additions & 0 deletions Mergin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
64 changes: 56 additions & 8 deletions Mergin/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
QgsVectorDataProvider,
QgsExpression,
QgsRenderContext,
QgsProviderRegistry,
)

from .help import MerginHelp
Expand All @@ -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]')
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -427,6 +467,14 @@ 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 QGS project file. Ask the workspace admin to upgrade your permission or <a href='{url}'>reset QGS project file</a> to be able to sync data changes. This might involve deleting layers you created locally."
harminius marked this conversation as resolved.
Show resolved Hide resolved
tomasMizera marked this conversation as resolved.
Show resolved Hide resolved
elif warning_id == Warning.EDITOR_NON_DIFFABLE_CHANGE:
return f"You don't have permission to edit the layer schema. Ask the workspace admin to upgrade your permission or <a href='{url}'>reset the layer</a> to be able to sync changes in other layers."
harminius marked this conversation as resolved.
Show resolved Hide resolved
elif warning_id == Warning.EDITOR_JSON_CONFIG_CHANGE:
return f"You don't have permission to change the config file. <a href='{url}'>Reset the file</a> to be able to sync data changes."
harminius marked this conversation as resolved.
Show resolved Hide resolved
elif warning_id == Warning.EDITOR_DIFFBASED_FILE_REMOVED:
return f"You don't have permission to remove a versioned file. <a href='{url}'>Reset the file</a> o be able to sync changes in other layers."
harminius marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions docs/dev-docs.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading