-
Notifications
You must be signed in to change notification settings - Fork 33
Add permissions to Role
#382
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
The API response for Roles now includes the permissions associated with that role. However, this has not been exposed in the ruby API. This commit fixes that problem.
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.
PR Summary
Adds permissions attribute to the Role class to expose permissions data from the WorkOS API response.
- Added
permissionstolib/workos/role.rbwith empty array fallback for backward compatibility - Updated
lib/workos/organizations.rbdocumentation forlist_organization_rolesto reflect new permissions field - Added comprehensive test coverage in
spec/lib/workos/role_spec.rbfor permissions handling - Updated VCR cassettes in
spec/support/fixtures/vcr_cassettes/organization/list_organization_roles.ymlto include permissions data - Added test cases in
spec/lib/workos/organizations_spec.rbfor proper serialization and initialization
5 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile
| roles = described_class.list_organization_roles(organization_id: 'org_01JEXP6Z3X7HE4CB6WQSH9ZAFE') | ||
|
|
||
| billing_role = roles.data.find { |role| role.slug == 'billing' } | ||
| serialized = billing_role.to_json |
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.
logic: .to_json returns a JSON string, but test expects a hash. Should be testing against billing_role.as_json or parsing the JSON string first
| serialized = billing_role.to_json | |
| serialized = billing_role.as_json |
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.
to_json returns a hash. We do things a little different around here. 🤠
nholden
left a comment
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.
This is great, thank you! Appreciate the thorough tests.
| roles = described_class.list_organization_roles(organization_id: 'org_01JEXP6Z3X7HE4CB6WQSH9ZAFE') | ||
|
|
||
| billing_role = roles.data.find { |role| role.slug == 'billing' } | ||
| serialized = billing_role.to_json |
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.
to_json returns a hash. We do things a little different around here. 🤠
Description
The API response for Roles now includes the permissions associated with that role. However, this has not been exposed in the ruby API. This commit fixes that problem.
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.