fix: add server-side redirect for users with pending invites on onboarding page#29
Conversation
…rding page When users sign up with an invite token, they were being redirected to /onboarding/getting-started (plan selection page) instead of going directly to /onboarding/personal/settings. This caused users to see the payment prompt for team/org plans before being redirected. This fix adds a server-side check in the /onboarding/getting-started page to redirect users with pending invites directly to the personal onboarding flow, preventing them from seeing the plan selection page. Also refactored onboardingUtils.ts to use MembershipRepository instead of direct prisma access. Co-Authored-By: sean@cal.com <Sean@brydon.io>
Co-Authored-By: sean@cal.com <Sean@brydon.io>
WalkthroughThe changes introduce a new method Changes
Sequence DiagramsequenceDiagram
participant User as User/Session
participant Page as Onboarding Page
participant Repo as MembershipRepository
participant DB as Database
participant Redirect as Redirect Handler
User->>Page: Request /onboarding/getting-started
Page->>Page: Get session user ID
Page->>Repo: hasPendingInviteByUserId(userId)
Repo->>DB: Query membership where accepted=false
DB-->>Repo: Membership result
Repo-->>Page: boolean (true if pending)
alt Pending Invite Exists
Page->>Redirect: Redirect to /onboarding/personal/settings
Redirect-->>User: Settings page
else No Pending Invite
Page-->>User: Continue onboarding flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/auth/lib/onboardingUtils.ts (1)
69-73: Unreachable code path: The ternary at line 73 always evaluates to"/getting-started".Since line 69 checks
hasPendingInvite || onboardingV3Enabled, if we reach line 73,onboardingV3Enabledmust befalse. The ternary condition will therefore always return"/getting-started", making the"/onboarding/getting-started"branch dead code.If the intent is to never use
/onboarding/getting-startedanymore, simplify the return:🧹 Proposed simplification
if (hasPendingInvite || onboardingV3Enabled) { return "/onboarding/personal/settings"; } - return onboardingV3Enabled ? "/onboarding/getting-started" : "/getting-started"; + return "/getting-started";
🤖 Fix all issues with AI agents
In `@packages/features/membership/repositories/MembershipRepository.ts`:
- Around line 584-595: The hasPendingInviteByUserId method currently queries
prisma.membership with accepted: true which is inverted; change the filter in
MembershipRepository.hasPendingInviteByUserId to accepted: false so it searches
for unaccepted (pending) invites, keeping the same select { id: true } and
return boolean logic (i.e., !!pendingInvite); update any related tests or
callers if they assume the old behavior.
🧹 Nitpick comments (2)
packages/features/membership/repositories/MembershipRepository.integration-test.ts (2)
119-162: Fragile test: Conditional logic makes behavior non-deterministic.The test relies on finding an existing second team (lines 127-132), and if none exists, it degenerates into testing only the accepted-membership case (already covered by another test). The conditional expectation at line 158 makes the test outcome environment-dependent.
Consider creating the second team within the test to ensure consistent behavior:
♻️ Proposed refactor for deterministic test
it("should return true when user has both accepted and pending invites", async () => { const newUser = await prisma.user.create({ data: { email: `test-mixed-invites-${Date.now()}@example.com`, username: `test-mixed-${Date.now()}`, }, }); - const team2 = await prisma.team.findFirst({ - where: { - slug: { not: null }, - id: { not: testTeamId }, - }, + // Create a second team for this test to ensure deterministic behavior + const team2 = await prisma.team.create({ + data: { + name: "Test Team 2 for Mixed Invites", + slug: `test-team-mixed-${Date.now()}`, + }, }); const acceptedMembership = await prisma.membership.create({ data: { userId: newUser.id, teamId: testTeamId, role: MembershipRole.MEMBER, accepted: true, }, }); createdMembershipIds.push(acceptedMembership.id); - if (team2) { - const pendingMembership = await prisma.membership.create({ - data: { - userId: newUser.id, - teamId: team2.id, - role: MembershipRole.MEMBER, - accepted: false, - }, - }); - createdMembershipIds.push(pendingMembership.id); - } + const pendingMembership = await prisma.membership.create({ + data: { + userId: newUser.id, + teamId: team2.id, + role: MembershipRole.MEMBER, + accepted: false, + }, + }); + createdMembershipIds.push(pendingMembership.id); const result = await MembershipRepository.hasPendingInviteByUserId({ userId: newUser.id }); - expect(result).toBe(team2 ? true : false); + expect(result).toBe(true); await clearTestMemberships(); + await prisma.team.delete({ where: { id: team2.id } }); await prisma.user.delete({ where: { id: newUser.id } }); });
8-19: Consider tracking test users for cleanup in hooks.The tests create users that are manually deleted at the end of each test. If a test fails mid-execution, these users remain orphaned. Consider tracking created user IDs similarly to
createdMembershipIdsand cleaning them up inafterEachfor more robust test isolation.Also applies to: 45-47
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(use-page-wrapper)/onboarding/getting-started/page.tsxpackages/features/auth/lib/onboardingUtils.tspackages/features/membership/repositories/MembershipRepository.integration-test.tspackages/features/membership/repositories/MembershipRepository.ts
🔇 Additional comments (5)
packages/features/membership/repositories/MembershipRepository.ts (1)
6-7: LGTM!The import adjustments are appropriate for the module's needs.
packages/features/membership/repositories/MembershipRepository.integration-test.ts (1)
50-75: Tests are correct but will fail due to implementation bug.This test correctly expects that a membership with
accepted: false(a pending invite) should makehasPendingInviteByUserIdreturntrue. However, the current implementation inMembershipRepository.tsqueries foraccepted: true, so this test will fail until the implementation is fixed.Additionally, the manual cleanup at lines 72-74 is redundant since
afterEachalready callsclearTestMemberships(). Consider removing the duplicate cleanup or relying solely on the hooks.packages/features/auth/lib/onboardingUtils.ts (1)
55-56: LGTM on the refactoring approach.Using
MembershipRepository.hasPendingInviteByUserIdinstead of direct Prisma access aligns with the repository pattern. Note that this depends on fixing the bug in the repository method whereaccepted: trueshould beaccepted: false.Also applies to: 67-67
apps/web/app/(use-page-wrapper)/onboarding/getting-started/page.tsx (2)
28-33: Server-side redirect logic is well-structured.The placement after session validation ensures the user ID is available before checking for pending invites. This correctly handles the case where users sign up with an invite token and need to be redirected to the appropriate onboarding flow.
Note: This depends on fixing the inverted logic in
MembershipRepository.hasPendingInviteByUserId(should queryaccepted: falseinstead ofaccepted: true).
5-7: LGTM!Import organization and the empty string fallback for
userEmailare appropriate.Also applies to: 35-35
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| static async hasPendingInviteByUserId({ userId }: { userId: number }): Promise<boolean> { | ||
| const pendingInvite = await prisma.membership.findFirst({ | ||
| where: { | ||
| userId, | ||
| accepted: true, | ||
| }, | ||
| select: { | ||
| id: true, | ||
| }, | ||
| }); | ||
| return !!pendingInvite; | ||
| } |
There was a problem hiding this comment.
Critical bug: Logic is inverted – queries accepted: true instead of accepted: false.
The method name hasPendingInviteByUserId implies checking for pending (unaccepted) invites, but the query filters on accepted: true, which matches accepted memberships. This will:
- Return
truefor users with accepted memberships (no pending invite) - Return
falsefor users with actual pending invites (accepted: false)
This will cause incorrect redirects in the onboarding flow.
🐛 Proposed fix
static async hasPendingInviteByUserId({ userId }: { userId: number }): Promise<boolean> {
const pendingInvite = await prisma.membership.findFirst({
where: {
userId,
- accepted: true,
+ accepted: false,
},
select: {
id: true,
},
});
return !!pendingInvite;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static async hasPendingInviteByUserId({ userId }: { userId: number }): Promise<boolean> { | |
| const pendingInvite = await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| accepted: true, | |
| }, | |
| select: { | |
| id: true, | |
| }, | |
| }); | |
| return !!pendingInvite; | |
| } | |
| static async hasPendingInviteByUserId({ userId }: { userId: number }): Promise<boolean> { | |
| const pendingInvite = await prisma.membership.findFirst({ | |
| where: { | |
| userId, | |
| accepted: false, | |
| }, | |
| select: { | |
| id: true, | |
| }, | |
| }); | |
| return !!pendingInvite; | |
| } |
🤖 Prompt for AI Agents
In `@packages/features/membership/repositories/MembershipRepository.ts` around
lines 584 - 595, The hasPendingInviteByUserId method currently queries
prisma.membership with accepted: true which is inverted; change the filter in
MembershipRepository.hasPendingInviteByUserId to accepted: false so it searches
for unaccepted (pending) invites, keeping the same select { id: true } and
return boolean logic (i.e., !!pendingInvite); update any related tests or
callers if they assume the old behavior.
1 participant
Benchmark PR from agentic-review-benchmarks#2
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.