-
Notifications
You must be signed in to change notification settings - Fork 7
Add support for moving workflow jobs #7279
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
Conversation
…doesn't have permission in the target container
| if (!_targetContainer.hasPermission(user, InsertPermission.class)) | ||
| Class<? extends Permission> permClass = InsertPermission.class; | ||
| if (schemaName != null && schemaName.equalsIgnoreCase("workflow") && queryName != null && queryName.equalsIgnoreCase("job")) | ||
| permClass = SampleWorkflowJobPermission.class; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this is a bit of a hack and am open to other ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is only called from the MoveRowsAction, there really isn't a better, workflow specific location for this code. One idea would be to pass in the permClass to this method so that it is the MoveRowsAction.validateForm which has this workflow specific code in it instead of the ContainerManager (but that is minor).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a bad idea, but only marginally better, so I'm leaving as is for now.
| if (!_targetContainer.hasPermission(user, InsertPermission.class)) | ||
| Class<? extends Permission> permClass = InsertPermission.class; | ||
| if (schemaName != null && schemaName.equalsIgnoreCase("workflow") && queryName != null && queryName.equalsIgnoreCase("job")) | ||
| permClass = SampleWorkflowJobPermission.class; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is only called from the MoveRowsAction, there really isn't a better, workflow specific location for this code. One idea would be to pass in the permClass to this method so that it is the MoveRowsAction.validateForm which has this workflow specific code in it instead of the ContainerManager (but that is minor).
Rationale
Users want to move jobs around as they work to reorganize their data when the organization or its usage of the Workflow feature set expands.
Related Pull Requests
Changes
getMoveTargetContainermethod to checkSampleWorkflowJobPermissionfor moving jobs and to throwUnauthorizedExceptioninstead of the generic error if permissions are not satisfied