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

Check for gislock and other issues when deleting/renaming/editing mapset and location in catalog #904

Conversation

lindakarlovska
Copy link
Contributor

Currently we don't have mechanism in data catalog to check for gislock and if mapset belongs to different user, so that we prevent user from actions including deleting or renaming mapset, and editing layers. The same applies to location deleting/renaming, it needs to check whether all mapsets can be edited.

@lindakarlovska lindakarlovska marked this pull request as draft August 12, 2020 15:53
@wenzeslaus wenzeslaus added this to PRs in progress in New GUI Startup via automation Aug 13, 2020
@petrasovaa petrasovaa added gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related labels Aug 13, 2020
lindakladivova added 3 commits August 13, 2020 09:00
…nd check_locations_interactively. Rename and delete function edited in order to manage those checking functions.
@lindakarlovska lindakarlovska marked this pull request as ready for review August 14, 2020 14:46
@lindakarlovska lindakarlovska self-assigned this Aug 14, 2020
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/guiutils.py Outdated Show resolved Hide resolved
return messages


def get_reasons_mapset_not_removable(grassdb, location, mapset, check_permanent):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we wanted "get_reason_mapset_not_removable", currently the implementation reports 1 reason because you have elif there and not if for every case. We could also report multiple reasons (e.g. mapset can be locked and PERMANENT), but I would keep single reason. Therefore the name should reflect that. It's a detail, but let's get it right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it should return message as string (or None), not list to keep it all consistent. Update the documentation.

Comment on lines 233 to 236
Queue().put(
(maps_dict,
_("Failed to read mapsets from location <{location}>.").format(
location=location_path)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't just blindly copy code. This is handling communication from parallelized calls of getLocationTree function, remove it.

dlg = wx.MessageDialog(
parent=guiparent,
message=_(
"Cannot rename selected mapset for the following reason:\n\n"
"Cannot rename the mapset for the following reason:\n\n"
Copy link
Contributor

@petrasovaa petrasovaa Aug 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably could say cannot rename mapset <mapset> for ...

…ssages and docstrings made more comprehensive.
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in good shape now, but I realized we should probably add the same checks for deleting grass database. Should be straightforward, use GetListOfLocations to get the locations and check if db is current and check if the locations can be removed. GetListOfLocations should be moved to the grassdb library, but let's not do that in this PR.

lindakladivova added 2 commits August 18, 2020 01:24
@petrasovaa petrasovaa merged commit 9b88f3f into OSGeo:master Aug 19, 2020
New GUI Startup automation moved this from PRs in progress to Done Aug 19, 2020
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Feat] Check for gislock and other issues when deleting/renaming/editing mapset and location in catalog
3 participants