Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds two Authorization methods to list role assignments (by resourceId and by organizationId/resourceTypeSlug/externalId), extends RoleAssignment types/serializer to include Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 3/5 reviews remaining, refill in 13 minutes and 7 seconds. Comment |
Greptile SummaryThis PR adds two new Confidence Score: 4/5Safe to merge once the open prior-thread issues (nullable organizationMembershipId typing, unencoded path parameters) are resolved or consciously accepted. No new P0/P1 issues were found beyond what existing review threads already flagged. The implementation is consistent with existing patterns and is well-tested. Score capped at 4 due to the unresolved P1-level concerns in previous threads. src/authorization/interfaces/role-assignment.interface.ts (organizationMembershipId nullability) and src/authorization/authorization.ts (unencoded path parameters) Important Files Changed
|
| /** The ID of the organization membership the role is assigned to. */ | ||
| organizationMembershipId: string; |
There was a problem hiding this comment.
organizationMembershipId typed as required but may be nullable
organizationMembershipId is declared as a non-optional string, but the new resource-level listing endpoints return assignments across all org memberships. If the API can ever return a null organization_membership_id (e.g., for roles granted at an org level without a specific membership, or for legacy records), callers reading assignment.organizationMembershipId will get undefined at runtime while TypeScript guarantees it's a string. Consider typing it as string | null to match the JSON wire type accurately.
| /** The ID of the organization membership the role is assigned to. */ | |
| organizationMembershipId: string; | |
| /** The ID of the organization membership the role is assigned to. */ | |
| organizationMembershipId: string | null; |
| export interface RoleAssignmentResponse { | ||
| object: 'role_assignment'; | ||
| id: string; | ||
| organization_membership_id: string; |
There was a problem hiding this comment.
Response interface also typed as required
RoleAssignmentResponse.organization_membership_id is declared as a non-optional string. If the API omits or nulls the field on any existing or future record, the deserializer will silently propagate undefined while TypeScript believes the field is always a string. This should stay in sync with whatever optionality is chosen for organizationMembershipId on RoleAssignment.
| organization_membership_id: string; | |
| organization_membership_id: string | null; |
| ), | ||
| (params) => |
There was a problem hiding this comment.
externalId is not URL-encoded in the path
externalId is interpolated directly into the URL path. If a caller passes a value containing /, ?, #, or other URL-special characters (e.g. "docs/2024", "proj #5"), the resulting URL will be malformed and the request will hit the wrong endpoint or fail. The same pattern exists in getOrganizationResource and other methods, but this new endpoint exposes the same pre-existing gap. Consider wrapping each path segment with encodeURIComponent.
| ), | |
| (params) => | |
| const endpoint = `/authorization/organizations/${encodeURIComponent(organizationId)}/resources/${encodeURIComponent(resourceTypeSlug)}/${encodeURIComponent(externalId)}/role_assignments`; |
Description
Adds two new methods on
Authorizationfor listing every role assignment granted on a resource.GET /authorization/resources/:resource_id/role_assignmentsGET /authorization/organizations/:organization_id/resources/:resource_type_slug/:external_id/role_assignmentsBoth return the same shape as
listOrganizationMembershipRoleAssignments. TheRoleAssignmentresponse object now also includesorganizationMembershipIdso callers can identify the membership a given assignment belongs to without an extra lookup.Changes
Authorization.listRoleAssignmentsForResource(options)— lists role assignments by internalresourceId, supports pagination.Authorization.listResourceRoleAssignments(options)— lists role assignments byorganizationId+resourceTypeSlug+externalId, supports pagination.organizationMembershipIdtoRoleAssignment(andorganization_membership_idtoRoleAssignmentResponse) and updateddeserializeRoleAssignmentto map it.ListRoleAssignmentsForResourceOptionsandListRoleAssignmentsForResourceByExternalIdOptionsinterfaces.role-assignment.jsonandlist-role-assignments.jsonfixtures with the new field.Summary by CodeRabbit
New Features
Tests