feat(frontier): add AddOrganizationMembers RPC#469
Conversation
|
The latest Buf updates on your PR. Results from workflow Validate / validate (pull_request).
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a new RPC method Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@raystack/frontier/v1beta1/frontier.proto`:
- Around line 1616-1620: OrgMemberResult is ambiguous because requests are keyed
by {user_id, role_id} but the response only returns user_id; update the response
shape to include the role identifier so results can be correlated to the
original request entries. Modify the OrgMemberResult message (e.g., add a string
role_id field or an index field) and ensure any code that constructs
OrgMemberResult (handlers that populate OrgMemberResult) sets this new field so
clients can reliably map success/error back to the specific {user_id, role_id}
pair.
- Around line 1602-1605: The AddOrganizationMembersRequest message's members
field currently allows empty and unbounded lists—add Buf validation constraints
to enforce a minimum of 1 item and a bounded maximum; update the members field
(in the AddOrganizationMembersRequest message) to include
(buf.validate.field).repeated.min_items = 1 and set
(buf.validate.field).repeated.max_items = 10 (or change to 20 if the team agrees
on a larger batch size) so it matches other bulk operation patterns—confirm the
final max_items with the team and apply that value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e72736a-3b52-49de-ba1f-b774f5f06c1b
📒 Files selected for processing (1)
raystack/frontier/v1beta1/frontier.proto
| message AddOrganizationMembersRequest { | ||
| string org_id = 1 [(buf.validate.field).string.uuid = true]; | ||
| repeated OrgMemberEntry members = 2; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --type=proto '\(buf\.validate\.field\)\.repeated\s*=\s*\{' raystack/frontier/v1beta1/frontier.proto -C2Repository: raystack/proton
Length of output: 88
🏁 Script executed:
# Read the actual code at the specified lines
cat -n raystack/frontier/v1beta1/frontier.proto | sed -n '1600,1615p'Repository: raystack/proton
Length of output: 601
🏁 Script executed:
# Search for repeated field validation patterns in proto files (no -t filter)
rg -n 'repeated.*\(buf\.validate\.field\)' raystack/frontier/v1beta1/frontier.proto -A2Repository: raystack/proton
Length of output: 750
🏁 Script executed:
# Search for max_items and min_items patterns in the proto file
rg -n '(min_items|max_items)' raystack/frontier/v1beta1/frontier.proto -B2 -A2Repository: raystack/proton
Length of output: 1923
🏁 Script executed:
# Look for similar bulk operation patterns (e.g., AddOrganization* or *MembersRequest)
rg -n 'repeated.*Entry|repeated.*Request' raystack/frontier/v1beta1/frontier.proto | head -30Repository: raystack/proton
Length of output: 582
Add cardinality constraints to members field.
Line 1604 currently allows empty and unbounded member lists. The codebase enforces min_items: 1 on all similar bulk operations (e.g., BatchCheckPermissionRequest, PreferenceRequestBody patterns). Add the same constraint here.
For max_items, existing patterns show 20 (permission checks) or 10 (other operations). Confirm the appropriate max batch size with the team, then apply the constraint.
Proposed contract update
message AddOrganizationMembersRequest {
string org_id = 1 [(buf.validate.field).string.uuid = true];
- repeated OrgMemberEntry members = 2;
+ repeated OrgMemberEntry members = 2 [(buf.validate.field).repeated = {
+ min_items: 1
+ max_items: <confirm with team>
+ }];
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@raystack/frontier/v1beta1/frontier.proto` around lines 1602 - 1605, The
AddOrganizationMembersRequest message's members field currently allows empty and
unbounded lists—add Buf validation constraints to enforce a minimum of 1 item
and a bounded maximum; update the members field (in the
AddOrganizationMembersRequest message) to include
(buf.validate.field).repeated.min_items = 1 and set
(buf.validate.field).repeated.max_items = 10 (or change to 20 if the team agrees
on a larger batch size) so it matches other bulk operation patterns—confirm the
final max_items with the team and apply that value.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
raystack/frontier/v1beta1/admin.proto (2)
1094-1096: Clarify whetherresultspreserves request order.Clients handling partial failures will be more robust if the ordering contract is explicit in the proto docs.
✏️ Proposed contract clarification
message AddOrganizationMembersResponse { + // Results correspond to AddOrganizationMembersRequest.members in the same order. repeated OrgMemberResult results = 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@raystack/frontier/v1beta1/admin.proto` around lines 1094 - 1096, Update the proto docs for AddOrganizationMembersResponse to explicitly state the ordering contract for the repeated field results: clarify whether the repeated OrgMemberResult results = 1 preserves the order of the incoming AddOrganizationMembersRequest entries (e.g., "results are in the same order as the corresponding requests") or that order is unspecified; add a comment above the message or the results field referencing OrgMemberResult and describing how clients should correlate partial failures to requests (e.g., by index matching or by including an identifier) so consumers know how to map results to original requests.
1098-1103: Prefer machine-readable error classification alongsideerrortext.Relying only on free-form
errorstrings is brittle for clients; adding an enum error code improves compatibility and retry/UX logic.🧩 Proposed API refinement
message OrgMemberResult { string user_id = 1; string role_id = 2; bool success = 3; string error = 4; + OrgMemberErrorCode error_code = 5; } + +enum OrgMemberErrorCode { + ORG_MEMBER_ERROR_CODE_UNSPECIFIED = 0; + ORG_MEMBER_ERROR_CODE_USER_NOT_FOUND = 1; + ORG_MEMBER_ERROR_CODE_ROLE_NOT_FOUND = 2; + ORG_MEMBER_ERROR_CODE_ALREADY_MEMBER = 3; + ORG_MEMBER_ERROR_CODE_PERMISSION_DENIED = 4; + ORG_MEMBER_ERROR_CODE_INTERNAL = 5; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@raystack/frontier/v1beta1/admin.proto` around lines 1098 - 1103, The OrgMemberResult message currently exposes only a free-form error string; add a machine-readable enum error code to support structured error handling by defining a new enum (e.g., OrgMemberError or OrgMemberResult_ErrorCode) in the same proto and adding a field to OrgMemberResult such as "OrgMemberError error_code = 5;" (or similar name) while keeping the existing "string error = 4" for human-readable text; update any RPCs/handlers that populate OrgMemberResult (e.g., where OrgMemberResult is constructed/returned) to set both the enum code and the error string and regenerate protobuf artifacts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@raystack/frontier/v1beta1/admin.proto`:
- Around line 1094-1096: Update the proto docs for
AddOrganizationMembersResponse to explicitly state the ordering contract for the
repeated field results: clarify whether the repeated OrgMemberResult results = 1
preserves the order of the incoming AddOrganizationMembersRequest entries (e.g.,
"results are in the same order as the corresponding requests") or that order is
unspecified; add a comment above the message or the results field referencing
OrgMemberResult and describing how clients should correlate partial failures to
requests (e.g., by index matching or by including an identifier) so consumers
know how to map results to original requests.
- Around line 1098-1103: The OrgMemberResult message currently exposes only a
free-form error string; add a machine-readable enum error code to support
structured error handling by defining a new enum (e.g., OrgMemberError or
OrgMemberResult_ErrorCode) in the same proto and adding a field to
OrgMemberResult such as "OrgMemberError error_code = 5;" (or similar name) while
keeping the existing "string error = 4" for human-readable text; update any
RPCs/handlers that populate OrgMemberResult (e.g., where OrgMemberResult is
constructed/returned) to set both the enum code and the error string and
regenerate protobuf artifacts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1198d7a1-d1c4-495e-a453-c33d4cd9beff
📒 Files selected for processing (1)
raystack/frontier/v1beta1/admin.proto
…xplicit roles
AddOrganizationUsers lacks a role parameter and hardcodes the default
viewer role. AddOrganizationMembers accepts a list of {user_id, role_id}
pairs so callers can specify the role at add time. Returns per-member
success/error results for partial failure handling.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This is a superadmin-only operation. Move from FrontierService to AdminService where authorization checks IsSuperUser. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add min_items=1 validation on members field so empty requests are rejected
- Add role_id to OrgMemberResult so clients can correlate results back
to specific {user_id, role_id} request entries
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Accidentally removed by replace_all in previous commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
e7f1e5d to
b79b72b
Compare
Summary
AddOrganizationMembersRPC that accepts a list of{user_id, role_id}pairs with UUID validation{user_id, success, error}results for partial failure handlingAddOrganizationUserswhich lacks a role parameter and hardcodes the default viewer roleExample
Request
{ "org_id": "a9a62b93-ae6f-4aba-9c32-73281c49da73", "members": [ { "user_id": "998d7ebe-cd2c-4f0c-a560-7dd2a7d1a62c", "role_id": "e57e1ba4-21fd-43a4-8aca-aa560afb32cf" }, { "user_id": "6a51c542-7ca9-4b23-8709-74145013d919", "role_id": "afe94f10-1508-4379-88b1-2c328cb2b769" } ] }Response
{ "results": [ { "user_id": "998d7ebe-cd2c-4f0c-a560-7dd2a7d1a62c", "success": true, "error": "" }, { "user_id": "6a51c542-7ca9-4b23-8709-74145013d919", "success": false, "error": "principal is already a member of this resource" } ] }🤖 Generated with Claude Code