refactor(frontier): remove deprecated fields from list member request messages#483
refactor(frontier): remove deprecated fields from list member request messages#483rohilsurana wants to merge 4 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR removes role/filter fields from several Frontier protobuf request messages and reserves their tag numbers. Changes:
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
The latest Buf updates on your PR. Results from workflow Validate / validate (pull_request).
|
There was a problem hiding this comment.
Pull request overview
Removes the deprecated permission_filter field from ListOrganizationUsersRequest in the Frontier API proto and reserves its field number to prevent reuse, aligning clients toward role_filters.
Changes:
- Removed deprecated
permission_filterfromListOrganizationUsersRequest. - Reserved protobuf field number
2to avoid tag reuse. - Keeps
role_filtersas the supported filtering mechanism for organization user listing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
raystack/frontier/v1beta1/frontier.proto (1)
1605-1605: Also reserve the removed field names, not just the tag numbers.When removing protobuf fields, it is a best practice to reserve both the field tag numbers and field names. Reserving only the numbers prevents tag reuse but still allows field names to be reintroduced with a new tag, creating confusion in text-format representations (TextProto/JSON). The same applies to the parallel changes in
ListProjectUsersRequest(Line 1868),ListProjectServiceUsersRequest(Line 1883),ListProjectGroupsRequest(Line 1898), andListGroupUsersRequest(Line 2128).Suggested change for Line 1605 (apply the analogous edit to each of the other four messages)
- reserved 2, 3; + reserved 2, 3; + reserved "permission_filter", "with_roles";For the other messages that only dropped
with_roles:- reserved 3; + reserved 3; + reserved "with_roles";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@raystack/frontier/v1beta1/frontier.proto` at line 1605, The proto messages that removed the with_roles field reserve only the tag numbers (e.g., the line containing "reserved 2, 3;") but must also reserve the removed field names to prevent reuse; update the reserved declarations in the affected messages (e.g., the message where you saw "reserved 2, 3;" and the messages ListProjectUsersRequest, ListProjectServiceUsersRequest, ListProjectGroupsRequest, and ListGroupUsersRequest) to include the string names of the removed fields (for example include "with_roles" in the reserved clause) so both tag numbers and field names are reserved.
🤖 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/frontier.proto`:
- Line 1605: The proto messages that removed the with_roles field reserve only
the tag numbers (e.g., the line containing "reserved 2, 3;") but must also
reserve the removed field names to prevent reuse; update the reserved
declarations in the affected messages (e.g., the message where you saw "reserved
2, 3;" and the messages ListProjectUsersRequest, ListProjectServiceUsersRequest,
ListProjectGroupsRequest, and ListGroupUsersRequest) to include the string names
of the removed fields (for example include "with_roles" in the reserved clause)
so both tag numbers and field names are reserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e586aed-ea12-4538-8fca-3ca9cb343563
📒 Files selected for processing (1)
raystack/frontier/v1beta1/frontier.proto
…nizationUsersRequest
…ion in ListOrganizationUsersRequest
ad2fbaf to
6de0e10
Compare
Summary
permission_filterfield fromListOrganizationUsersRequestandListProjectUsersRequest, reserve field idswith_rolesfield fromListOrganizationUsersRequest,ListProjectUsersRequest,ListProjectServiceUsersRequest,ListProjectGroupsRequest, andListGroupUsersRequest, reserve field idsTest plan