refactor(group): migrate group creation to membership package#1601
Conversation
group.Create now delegates SpiceDB wiring (org<->group hierarchy + creator-as-owner) to membership.OnGroupCreated. The three helpers (addAsOrgMember, addOrgToGroup, addOwner) are removed from the group service. MembershipService is injected via setter to break the circular init order with the membership service (same pattern as organization and serviceuser). group.Create's behavior is unchanged at the SpiceDB layer; this is a pure refactor that consolidates the writes through the membership package so the rollback/compensation logic added in #1596 applies to group creation as well. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
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 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
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 |
Manual Testing Results - Group Creation MigrationTested group creation flow after migration to Test Environment
Happy Path Tests
Unhappy Path Tests
Integration with PR 1596
Summary
The migration to |
Coverage Report for CI Build 25777007518Coverage decreased (-0.07%) to 42.291%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
Summary
Second PR in the group-membership migration (parent: #1478).
group.Createno longer wires SpiceDB relations directly — it delegates the org↔group hierarchy and the creator-as-owner add tomembership.OnGroupCreated, which was introduced in #1596.Changes
core/group/service.go:MembershipServiceinterface withOnGroupCreated.group.Service.Createnow does:repository.Create+membership.OnGroupCreated. Drops the three direct-relation helpers (addAsOrgMember,addOrgToGroup,addOwner).MembershipServicevia a setter (SetMembershipService) to break the circular init order with the membership service — same pattern asorganization.Serviceandserviceuser.Service.cmd/serve.go: callsgroupService.SetMembershipService(membershipService)after both services are constructed, alongside the existing org/serviceuser setter wiring.core/group/service_test.go:TestService_Createmocks the membership call instead of asserting on direct policy/relation writes. Adds a propagation-error case.core/group/mocks/membership_service.go.Why this is safe
OnGroupCreated(added in feat(membership): add group membership management #1596) creates exactly the same three relations + policy that the oldgroup.Service.Createdid. A bonus: the rollback/compensation paths added during feat(membership): add group membership management #1596's review now apply to group creation too.AddGroupUsersand the legacyAddMember/AddUsers/addMemberPolicyhelpers are untouched in this PR — they'll be migrated in the next PR (Centralize membership management into a dedicated package to fix leaky relations #1478 follow-up 3/3).Test plan
go build ./...go test ./core/group/... ./core/membership/... ./internal/api/v1beta1connect/...gofmt -lclean