[cuegui] Limit user actions on alien jobs #1488
Conversation
Check user permissions before allowing a user to "eat, kill, retry" jobs that they do not own. Allow an override in "File -> Enable Job Interaction". This gets saved to the config file and is disabled by default.
USER_CONFIRM_RESTART and USER_INTERACTION_PERMISSIONS are only used internally on a single class each, there's no point in making them global gui constants.
|
The permission logic should collect all the blocked actions before creating the warning dialog when interacting with multiple items at the same time.
DiegoTavares
force-pushed
the
limit_user_interaction
branch
8 times, most recently
from
fd74119 to
b696b43
Compare
DiegoTavares
force-pushed
the
limit_user_interaction
branch
from
b696b43 to
a61b939
Compare
DiegoTavares
requested review from
bcipriano and
ramonfigueiredo
as code owners
|
Roula's contribution to this PR happened while she was still at SPI. Even though she has signed the CLA, the check will fail as the email assigned to this commit is no longer associated with her GitHub account. |
| @@ -454,9 +475,26 @@ def __revertLayout(self): | |||
| result = QtWidgets.QMessageBox.question( | |||
| self, | |||
| "Restart required ", | |||
| "You must restart for this action to take effect, close window?: ", | |||
| MainWindow.USER_CONFIRM_RESTART, | |||
There was a problem hiding this comment.
Suggestion: use cuegui.Constants.USER_CONFIRM_RESTART instead of MainWindow.USER_CONFIRM_RESTART.
Constants.py
USER_CONFIRM_RESTART = "You must restart for this action to take effect, close window?: "| @@ -46,6 +47,9 @@ | |||
| class MainWindow(QtWidgets.QMainWindow): | |||
| """The main window of the application. Multiple windows may exist.""" | |||
|
|
|||
| # Message to be displayed when a change requires an application restart | |||
| USER_CONFIRM_RESTART = "You must restart for this action to take effect, close window?: " | |||
There was a problem hiding this comment.
Suggestion: move use USER_CONFIRM_RESTART = "You must restart for this action to take effect, close window?: " to cuegui/cuegui/Constants.py
| @@ -80,6 +80,11 @@ class AbstractActions(object): | |||
|
|
|||
| __iconCache = {} | |||
|
|
|||
| # Template for permission alert messages | |||
| USER_INTERACTION_PERMISSIONS = "You do not have permissions to {0} owned by {1}" \ | |||
There was a problem hiding this comment.
Suggestion:
Move USER_INTERACTION_PERMISSIONS to cuegui/Constansts.py
| if blocked_job_owners: | ||
| cuegui.Utils.showErrorMessageBox( | ||
| AbstractActions.USER_INTERACTION_PERMISSIONS.format( | ||
| "eat dead for some of the selected jobs", |
There was a problem hiding this comment.
eat dead frames:
"eat dead for some of the selected jobs" -> "eat dead frames for some of the selected jobs"
| if blocked_job_owners: | ||
| cuegui.Utils.showErrorMessageBox( | ||
| AbstractActions.USER_INTERACTION_PERMISSIONS.format( | ||
| "retry dead for some of the selected jobs", |
There was a problem hiding this comment.
retry dead frames:
"retry dead for some of the selected jobs" => "retry dead frames for some of the selected jobs"
| @@ -872,3 +873,21 @@ def byteConversion(amount, btype): | |||
| for _ in range(n): | |||
| _bytes *= 1024 | |||
| return _bytes | |||
|
|
|||
|
|
|||
| def isPermissible(jobObject): | |||
There was a problem hiding this comment.
Suggestion:
Use snake_case for Python variables (PEP = Python Enhancement Proposals):
jobObject -> job_object
hasPermissions -> has_permissions
currentUser -> current_user
DiegoTavares
merged commit 195a003
into
AcademySoftwareFoundation:master
Rationale
Users shouldn't be allowed by default to execute destructive actions on jobs they don't own. For "superusers" there should be a simple way to toggle this behavior off. This preference should be saved on the user profile.
Implementation details
This feature checks user permissions before allowing a user to "eat, kill, retry" jobs that they do not own.
The behavior can be overwritten at: "File -> Enable Job Interaction". This gets saved to the config file and is disabled by
default.