MerginMaps / qgis-plugin

QGIS plugin for managing Mergin Maps projects
GNU General Public License v3.0
36 stars 13 forks source link

Issues with Editor mode #581

Closed jozef-budac closed 5 months ago

jozef-budac commented 5 months ago

There is one problem with the editor role in the plugin. The Editor should be able to create and edit features in the project. The QGIS sometimes updates the project during feature editing, probably in case, the project is from an older QGIS version or some similar problem. It happens when I open the attribute table of the layer.

For example, when you open the attribute table of the Survey layer from project https://app.dev.merginmaps.com/projects/tme1/client/tree on QGIS 3.34.6, it changes the project file.

The editor should not be able to change the project file, so the result is, that the Editor can't modify a feature. In the zip files, you can see, what changes are made in the project file.

files.zip

This should be addressed for the Editor role to work well.

jozef-budac commented 5 months ago

A similar problem appeared when I updated the project from previous steps when I tried to to the conflict file.

1) in mobile app: update a field in a specific feature and synchronize 2) in plugin: update the same filed in the same feature and hit synchronize

Again, with the geopackage, also the project file is updated.

wonder-sk commented 5 months ago

...from discussion with Tomas:

Things "editor" is not allowed:

  1. add/update/delete .qgs/.qgz project file

  2. add/update/delete mergin-config.json

  3. gpkg not updated through diff

  4. gpkg deleted


try sync -> client detects problems (in project status dialog)


CLIENT

PLUGIN

tomasMizera commented 5 months ago

12/6/'24 -> awaits review and testing, first internal, then from the testing team

tomasMizera commented 5 months ago

Hi @MarcelGeo and @wonder-sk, I was playing around with the editor permission in the plugin and found some interesting behavior. I thought that we wanted to ignore changes only on QGIS project files during sync. However, I am experiencing that we ignore all files with unsupported editor changes (removal of .gpkg, non-diff based update,...). Is this intended? It does not reflect the originally specified behavior. Was there an update to the logic?

Example: I deleted a GeoPackage from the disk and also removed it from the layers list in the layers browser. When I hit sync, I see status information that the GeoPackage was removed and also that the QGIS file was updated (correct ✔). However, when I hit sync again, the sync process finishes without any issue (no 403 unauthorized sync error ⛔️). When I check the history of the project, I can see that no version was created as there were no files for the sync (both changes were skipped).

Is this intended? I find it quite user-unfriendly to be ignoring all changes.

See https://github.com/MerginMaps/python-api-client/blob/2d72fc07bb292cca0bb5fb88a63aeb121356686b/mergin/editor.py#L73

MarcelGeo commented 5 months ago

Hi @MarcelGeo and @wonder-sk, I was playing around with the editor permission in the plugin and found some interesting behavior. I thought that we wanted to ignore changes only on QGIS project files during sync. However, I am experiencing that we ignore all files with unsupported editor changes (removal of .gpkg, non-diff based update,...). Is this intended? It does not reflect the originally specified behavior. Was there an update to the logic?

Example: I deleted a GeoPackage from the disk and also removed it from the layers list in the layers browser. When I hit sync, I see status information that the GeoPackage was removed and also that the QGIS file was updated (correct ✔). However, when I hit sync again, the sync process finishes without any issue (no 403 unauthorized sync error ⛔️). When I check the history of the project, I can see that no version was created as there were no files for the sync (both changes were skipped).

Is this intended? I find it quite user-unfriendly to be ignoring all changes.

See https://github.com/MerginMaps/python-api-client/blob/2d72fc07bb292cca0bb5fb88a63aeb121356686b/mergin/editor.py#L73

  1. We implemented all rules for editor into python client, see PR and comments 🙂: https://github.com/MerginMaps/python-api-client/pull/207#issuecomment-2143857830 . If there is any problem with this rules, we can make patch for client . As definition was not complete and there are warnings in plugin about not applicable / ignored changes.
  2. I don't understand example 🙂
tomasMizera commented 5 months ago

As agreed on a call: