-
Notifications
You must be signed in to change notification settings - Fork 490
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
6290 dataset role mgmt api #6622
Conversation
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.
@sekmiller and I agree that we should also have "revoke role" functionality. He thinks "list" is already there. CRUD, basically.
I also found a tiny bit of English that should go in a bundle.
} | ||
DataverseRole theRole = rolesSvc.findBuiltinRoleByAlias("admin"); | ||
if (theRole == null) { | ||
return error(Status.BAD_REQUEST, "Can't find role named '" + ra.getRole() + "' in dataverse " + dataset.getOwner()); |
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 think the new-ish rule is that when we touch APIs we should put the English in a bundle.
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.
Added revoke and api messages to the Bundle.
@sekmiller Need GET after -X in list assignments example. |
@sekmiller Discussed with @scolapasta and @djbrooke , please add inherited as well as directly assigned roles and indicate which is which. Also can't get role assignment endpoint to work using sample file. Throws error:
For the record, I see the same error with the dataverse version of this endpoint. Is the example file incorrect? |
Thanks @kcondon for the discussion and @sekmiller for taking another pass at this! |
What this PR does / why we need it:
Allows admins to assign roles to users at the dataset level
Which issue(s) this PR closes:
Closes #6290 Dataset Role Management API
Special notes for your reviewer:
None
Suggestions on how to test this:
Does this PR introduce a user interface change?:
None
Is there a release notes update needed for this change?:
No
Additional documentation: