Issue 12678 fixing workflow roles permission issue #13526
Conversation
| .doesRoleHavePermission(action, PermissionAPI.PERMISSION_USE, | ||
| anyWhoCanViewContent)) { | ||
| if (APILocator.getPermissionAPI() | ||
| if (permissionAPI |
There was a problem hiding this comment.
| .doesRoleHavePermission(action, PermissionAPI.PERMISSION_USE, | ||
| anyWhoCanPublishContent)) { | ||
| if (APILocator.getPermissionAPI().doesUserHavePermission(permissionable, | ||
| if (permissionAPI.doesUserHavePermission(permissionable, |
There was a problem hiding this comment.
| .doesRoleHavePermission(action, PermissionAPI.PERMISSION_USE, | ||
| anyWhoCanEditContent)) { | ||
| if (APILocator.getPermissionAPI().doesUserHavePermission(permissionable, | ||
| if (permissionAPI.doesUserHavePermission(permissionable, |
There was a problem hiding this comment.
| .doesRoleHavePermission(action, PermissionAPI.PERMISSION_USE, | ||
| anyWhoCanEditPermisionsContent)) { | ||
| if (APILocator.getPermissionAPI().doesUserHavePermission(permissionable, | ||
| if (permissionAPI.doesUserHavePermission(permissionable, |
There was a problem hiding this comment.
| * @throws DotDataException | ||
| * @throws DotSecurityException | ||
| */ | ||
| public WorkflowAction findActionRespectingPermissions(String actionId, String stepId, Permissionable permissionable, User user) throws DotDataException, DotSecurityException; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Wouldn't be bad to add an integration test for this new API method
| * @throws DotDataException | ||
| * @throws DotSecurityException | ||
| */ | ||
| public WorkflowAction findActionRespectingPermissions(String id, Permissionable permissionable, User user) throws DotDataException, DotSecurityException; |
There was a problem hiding this comment.
|
SonarQube analysis reported 9 issues
Watch the comments in this conversation to review them. 3 extra issuesNote: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:
|
| } catch(Exception e){ | ||
|
|
||
|
|
There was a problem hiding this comment.
@oswaldogallango should we remove the try?, not empty catch should exist, if the try is required and no logic in the catch should be done please add a Log.error but most likely the try should be remove.
There was a problem hiding this comment.
@jgambarios the try and catch can not be removed because, if the user doesn't have permission to execute that action the method is going to throw an exception but you still want to continue doing the save, but if the user have permission you just execute the fireworkflow action according to the if that is below
|
If the user does not have permission to the action, then the save should fail. |
jgambarios
deleted the
issue-12678-fixing-workflow-roles-permission-issue
branch
No description provided.