Code Review Bench PR #26829 - fix: add server-side redirect for users with pending invites on onboarding page

[ketkarameya]

Code Review Bench PR calcom#26829 - fix: add server-side redirect for users with pending invites on onboarding page

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#2

fix: add server-side redirect for users with pending invites on onboarding page

---

Summary by Gitar

This PR fixes a UX issue where users signing up with team invite tokens were unnecessarily shown payment prompts during onboarding:

• Added server-side redirect logic in /onboarding/getting-started page to detect users with pending team invites and redirect them directly to personal onboarding settings, skipping the plan selection page
• Extracted reusable repository method hasPendingInviteByUserId() as a static method in MembershipRepository to check for unaccepted memberships, replacing direct Prisma queries
• Refactored onboarding utilities to use the new repository method for consistency across the codebase
• Added comprehensive integration tests (164 lines) covering edge cases: pending-only invites, accepted-only invites, mixed states, and no memberships
• Improved code organization by reorganizing imports alphabetically and following repository pattern conventions

[gitar-bot-staging]

🚨 Bug: hasPendingInviteByUserId queries accepted: true instead of false

The new hasPendingInviteByUserId method is supposed to check if a user has pending (unaccepted) invites, but the Prisma query filters for accepted: true instead of accepted: false. This means the method returns true when a user has accepted memberships and false when they actually have pending invites — the exact opposite of the intended behavior.

Impact: Users with pending team invites will NOT be redirected to the personal onboarding page (the entire point of this PR). Users who have already accepted team memberships WILL be incorrectly redirected away from the getting-started page. This completely inverts the business logic.

The original code in onboardingUtils.ts (before this PR) correctly queried accepted: false. The refactoring introduced this inversion.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	accepted: true,
	const pendingInvite = await prisma.membership.findFirst({
	where: {
	userId,
	accepted: false,
	},
	select: {
	id: true,
	},
	});

• Apply suggested fix

[gitar-bot-staging]

🚨 Bug: Logic changed from && to ||, breaking onboarding redirect

The original condition was if (pendingInvite && onboardingV3Enabled) — meaning users are redirected to personal onboarding only when they have a pending invite AND onboarding-v3 is enabled. The refactored code changes this to if (hasPendingInvite || onboardingV3Enabled), which means:

1. All users with onboardingV3Enabled will be redirected to /onboarding/personal/settings regardless of invite status, bypassing the getting-started flow entirely.
2. Users with pending invites will be redirected even when onboarding-v3 is disabled.
3. Line 73 is now dead code: return onboardingV3Enabled ? "/onboarding/getting-started" : "/getting-started" can never evaluate onboardingV3Enabled as true because that case is already caught by the || condition on line 69.

This is a significant behavioral regression that changes the onboarding flow for all users, not just those with pending invites.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	if (hasPendingInvite || onboardingV3Enabled) {
	if (hasPendingInvite && onboardingV3Enabled) {

• Apply suggested fix

[gitar-bot-staging]

⚠️ Bug: Test silently passes without validating core behavior

The "mixed invites" test at line 119 wraps the pending membership creation in if (team2) and then asserts expect(result).toBe(team2 ? true : false). If no second team exists in the database, the test never creates a pending invite and simply asserts false — making it a duplicate of the "no pending invites" test.

This means the test can pass in environments where only one team exists without ever testing the "both accepted and pending invites" scenario. A test that silently degrades to testing a different scenario undermines confidence in the test suite.

The test should either:

1. Create a second team explicitly (like beforeAll does for the first team), or
2. Fail explicitly if the required precondition (team2 exists) is not met.

_{Was this helpful? React with 👍 / 👎}

• Apply suggested fix

[gitar-bot-staging]

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}

[gitar-bot-staging]

Code Review 🚫 Blocked 0 resolved / 3 findings

Two critical logic bugs: the hasPendingInviteByUserId query checks accepted: true instead of false (inverted logic), and the condition in onboardingUtils.ts was changed from && to || which breaks the onboarding redirect for all users. Both bugs together mean this PR achieves the opposite of its stated intent.

🚨 Bug: hasPendingInviteByUserId queries accepted: true instead of false

📄 packages/features/membership/repositories/MembershipRepository.ts:588

The new hasPendingInviteByUserId method is supposed to check if a user has pending (unaccepted) invites, but the Prisma query filters for accepted: true instead of accepted: false. This means the method returns true when a user has accepted memberships and false when they actually have pending invites — the exact opposite of the intended behavior.

Impact: Users with pending team invites will NOT be redirected to the personal onboarding page (the entire point of this PR). Users who have already accepted team memberships WILL be incorrectly redirected away from the getting-started page. This completely inverts the business logic.

The original code in onboardingUtils.ts (before this PR) correctly queried accepted: false. The refactoring introduced this inversion.

Suggested fix

    const pendingInvite = await prisma.membership.findFirst({
      where: {
        userId,
        accepted: false,
      },
      select: {
        id: true,
      },
    });

🚨 Bug: Logic changed from && to ||, breaking onboarding redirect

📄 packages/features/auth/lib/onboardingUtils.ts:69

The original condition was if (pendingInvite && onboardingV3Enabled) — meaning users are redirected to personal onboarding only when they have a pending invite AND onboarding-v3 is enabled. The refactored code changes this to if (hasPendingInvite || onboardingV3Enabled), which means:

1. All users with onboardingV3Enabled will be redirected to /onboarding/personal/settings regardless of invite status, bypassing the getting-started flow entirely.
2. Users with pending invites will be redirected even when onboarding-v3 is disabled.
3. Line 73 is now dead code: return onboardingV3Enabled ? "/onboarding/getting-started" : "/getting-started" can never evaluate onboardingV3Enabled as true because that case is already caught by the || condition on line 69.

This is a significant behavioral regression that changes the onboarding flow for all users, not just those with pending invites.

Suggested fix

  if (hasPendingInvite && onboardingV3Enabled) {

⚠️ Bug: Test silently passes without validating core behavior

📄 packages/features/membership/repositories/MembershipRepository.integration-test.ts:144

The "mixed invites" test at line 119 wraps the pending membership creation in if (team2) and then asserts expect(result).toBe(team2 ? true : false). If no second team exists in the database, the test never creates a pending invite and simply asserts false — making it a duplicate of the "no pending invites" test.

This means the test can pass in environments where only one team exists without ever testing the "both accepted and pending invites" scenario. A test that silently degrades to testing a different scenario undermines confidence in the test suite.

The test should either:

1. Create a second team explicitly (like beforeAll does for the first team), or
2. Fail explicitly if the required precondition (team2 exists) is not met.

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: Appended technical summary section to PR description covering server-side redirect logic, repository method extraction, refactoring, and integration tests

_{5 rules not applicable. Show all rules by commenting gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[kayagokalp]

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 3/4 = 75.0%)

ID	Severity	Title	Found?
GT-2-001	Critical	hasPendingInviteByUserId checks accepted: true instead of fa	Yes
GT-2-002	Critical	Logic changed from AND to OR, breaking onboarding redirect f	Yes
GT-2-003	Important	Test silently degrades when second team doesn't exist	Yes
GT-2-004	Minor	No error handling for database call in render path	No

Extra Gitar Findings

File	Line	Valid Bug?	Notes
(none)	-	-	-

Summary

• Recall: 75.0% (3/4)
• Precision: 100.0% (3/3)
• Precision counts ground truth matches + valid extra findings as true positives