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

Only accept changes from work packages if they pass the filter #5

Open
wonder-sk opened this issue Mar 5, 2021 · 4 comments
Open
Labels
enhancement New feature or request

Comments

@wonder-sk
Copy link
Contributor

Currently we are accepting all changes from work packages. We should probably test whether inserted/updated rows from a work package would pass the filtering. If not, we would reject such changes. Any rejected changes would need to be stored somewhere in the master Mergin project. There is a risk with rejections that the invalid edits could be unintended (e.g. the "team" column value would be NULL instead of the expected). Another option would be that instead of rejecting such edits, we would modify such insert/update statements to pass the filtering.

@wonder-sk wonder-sk added the enhancement New feature or request label Mar 5, 2021
@dracic
Copy link
Contributor

dracic commented Apr 23, 2021

I suggest "team" column should never be populated manually - i plan to disable that in all forms, and if for some reason projects are made in a way that user can enter some value (right or wrong), this value should be updated on every sync with the main project. I don't expect my team members to even know which team they belong to or what their team id is.

@dracic
Copy link
Contributor

dracic commented Apr 23, 2021

So:

The main project ("Survey") contains all data, while the work package projects ("Survey Team A", "Survey Team B") only contain partial data.

So from this definition unwanted changes are impossible.

@dracic
Copy link
Contributor

dracic commented Apr 23, 2021

Now I see in wp.py:

289        elif isinstance(wp_value, list):
290                values_str = ",".join(["?"] * len(wp_value))

So scenario A:

Team A - Area A and/or Department A
Team B - Area B and/or Department B
making changes to same data (trees, shrubs, plants and mushrooms for example) in their department
Possible issue: Team A can make an entry that is spatially in area B. You can have NULL values in team columns for new records or wrong values if it is up to users to enter that value. If you have @mergin_wp_value variable somehow added to wp, than you can make it as a default value for that column. You can use some function from other filtered data to get that value.

Check - if isinstance(wp_value, (str, int, float)) - always UPDATE tables and set filter column to wp_value when syncing back

Additional spatial filter would be usefull...

Scenario B:

Team A - Area A
Team B - Same area A
Team A is recording/editing trees and shrubs, and Team B is recording plants and mushrooms on the same area
Possible issue: you have to check if team A has some plants or mushrooms in new edits and discard them when syncing to main base. That filter-column would perhaps be some sort of relation widget. That relation widget should be filtered somehow. It doesn't make sense that team A has an option to record mushrooms. You have to make that somehow in QGIS project or you have to filter relation table. If you filter relation table with extra "team column" and make it as NOT NULL in QGIS you cannot have unwanted changes, and you cannot have NULL values.

Check - isinstance(wp_value, list) - just discard data that doesn't have correct values or have NULL values. Delete it and don't collect it anywhere.

Other checks should be on us who do our projects. I think you should keep it simple.

@wonder-sk
Copy link
Contributor Author

Yeah, if there is only single value allowed for a WP, the default behavior could be to automatically set that value (if not set already).

Things get a bit more tricky if there are multiple values allowed - discarding such rows would be an option, or possibly setting the filter column to some special value, so that project admin is aware of the fact there are some issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants