Skip to content

refactor(group): remove AddGroupUsers handler + authz entry#1614

Merged
whoAbhishekSah merged 3 commits into
mainfrom
refactor/remove-add-group-users-handler
May 15, 2026
Merged

refactor(group): remove AddGroupUsers handler + authz entry#1614
whoAbhishekSah merged 3 commits into
mainfrom
refactor/remove-add-group-users-handler

Conversation

@whoAbhishekSah
Copy link
Copy Markdown
Member

@whoAbhishekSah whoAbhishekSah commented May 14, 2026

SDK no longer uses AddGroupUsers (#1609) and the AddGroupUsers had become a thin loop over SetGroupMemberRole (#1608). Drops the handler, its test, the authz interceptor entry, and the now-unused proto types.

RemoveGroupUser stays — no replacement RPC.

Stacked on raystack/proton#488; PROTON_COMMIT will be re-pinned to the merge commit before this merges.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment May 15, 2026 7:56am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Review Change Stack

Warning

Rate limit exceeded

@whoAbhishekSah has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 12 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ca48e592-33a5-4e8f-b868-707a2fe7950a

📥 Commits

Reviewing files that changed from the base of the PR and between 5786739 and f357bb9.

⛔ Files ignored due to path filters (4)
  • proto/v1beta1/admin.pb.go is excluded by !**/*.pb.go, !proto/**
  • proto/v1beta1/frontier.pb.go is excluded by !**/*.pb.go, !proto/**
  • proto/v1beta1/frontierv1beta1connect/frontier.connect.go is excluded by !proto/**
  • proto/v1beta1/models.pb.go is excluded by !**/*.pb.go, !proto/**
📒 Files selected for processing (5)
  • Makefile
  • internal/api/v1beta1connect/group.go
  • internal/api/v1beta1connect/group_test.go
  • pkg/server/connect_interceptors/authorization.go
  • test/e2e/regression/api_test.go
📝 Walkthrough

Walkthrough

This PR removes the AddGroupUsers RPC endpoint from the Frontier API and updates e2e tests to call SetGroupMemberRole instead. The Connect handler, authorization validation entry, and unit test are deleted. E2E regression tests now fetch the group member role during setup and use it when assigning group membership.

Changes

API Handler and Authorization Removal

Layer / File(s) Summary
AddGroupUsers handler, authorization validation, and unit test removal
internal/api/v1beta1connect/group.go, pkg/server/connect_interceptors/authorization.go, internal/api/v1beta1connect/group_test.go
The ConnectHandler.AddGroupUsers method is removed from the group handler; its authorization validation map entry is removed; and the comprehensive TestConnectHandler_AddGroupUsers table-driven test covering error and success paths is removed.

E2E Test Migration

Layer / File(s) Summary
E2E test suite updates for SetGroupMemberRole
test/e2e/regression/api_test.go
Test suite gains a groupMemberRole field; SetupSuite queries enabled roles in the group namespace to find and store the group member role ID; five test cases across group access, membership management, and user deletion scenarios replace AddGroupUsers calls with SetGroupMemberRole, providing GroupId, OrgId, PrincipalId, PrincipalType, and RoleId set to the stored group member role ID.

Build Tooling

Layer / File(s) Summary
Proton archive version update
Makefile
PROTON_COMMIT is updated to a new git revision, changing which Proton archive version the proto target downloads for protobuf generation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • raystack/frontier#1608: This PR removes the AddGroupUsers handler and updates e2e tests to use SetGroupMemberRole, building directly on the prior migration of group user assignment behavior to the membership service.
  • raystack/frontier#1609: The backend removal of AddGroupUsers and authorization updates align with the SDK UI layer refactoring from addGroupUsers to setGroupMemberRole.
  • raystack/frontier#1436: Both PRs update Makefile PROTON_COMMIT to regenerate protobufs with a different Proton revision.

Suggested reviewers

  • rohilsurana
  • rsbh
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coveralls
Copy link
Copy Markdown

coveralls commented May 14, 2026

Coverage Report for CI Build 25907017306

Coverage decreased (-0.04%) to 42.232%

Details

  • Coverage decreased (-0.04%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 37694
Covered Lines: 15919
Line Coverage: 42.23%
Coverage Strength: 11.87 hits per line

💛 - Coveralls

whoAbhishekSah and others added 3 commits May 15, 2026 13:25
The AddGroupUsers RPC was removed from proton in raystack/proton#488.
With the SDK fully off this RPC since #1609 and the handler now just a
thin loop over SetGroupMemberRole, drop the handler, its test, and the
authorization-interceptor entry. Regenerated proto stubs reflect the
removal.

PROTON_COMMIT bumped to the proton branch SHA; will be re-pinned to the
merge commit once that PR lands.

RemoveGroupUser stays — no replacement RPC for group member removal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI failed on the AddGroupUsers removal because the e2e regression tests
still called the now-deleted RPC. Migrates the 5 call sites to
SetGroupMemberRole with the app_group_member role, resolved once in
SetupSuite alongside the existing org-role lookups.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Picks up raystack/proton#488 from main now that it's merged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@whoAbhishekSah whoAbhishekSah force-pushed the refactor/remove-add-group-users-handler branch from 5786739 to f357bb9 Compare May 15, 2026 07:55
@whoAbhishekSah whoAbhishekSah merged commit 5e7ebe5 into main May 15, 2026
8 checks passed
@whoAbhishekSah whoAbhishekSah deleted the refactor/remove-add-group-users-handler branch May 15, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants