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

add do get role api #42

Merged
merged 2 commits into from Aug 26, 2018

Conversation

Projects
None yet
2 participants
@runzexia
Copy link
Member

commented Aug 8, 2018

add api to get role's permission & pattern

@oleg-nenashev oleg-nenashev self-requested a review Aug 8, 2018

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Aug 8, 2018

will try to review and release it this week

@oleg-nenashev
Copy link
Member

left a comment

Thanks for the proposal! It needs a fix in the comparison logic, ready to go after that IMO

permissionsMap.put(permission.getId(),permission.getEnabled());
}
responseJson.put("permissionIds",permissionsMap);
if (type.equals(RoleBasedAuthorizationStrategy.PROJECT)){

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev Aug 11, 2018

Member

It also applies to agents and other types. I would rather check for != RoleBasedAuthorizationStrategy.GLOBAL

*
* @param type (globalRoles, projectRoles, slaveRoles)
* @param roleName name of role (single, no list)
* @throws IOException

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev Aug 11, 2018

Member

Better to add some documentation so there is no Javadoc warnings

@runzexia

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2018

@runzexia

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2018

@oleg-nenashev Could you please help review this again? Any comments is appreciated. Thanks!

@oleg-nenashev oleg-nenashev merged commit 56f48f0 into jenkinsci:master Aug 26, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Aug 27, 2018

I will release the new version soon, do not want to make it wait for long

@runzexia

This comment has been minimized.

Copy link
Member Author

commented Aug 27, 2018

Thanks! @oleg-nenashev

@runzexia runzexia deleted the runzexia:add-doGetRole branch Sep 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.