Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as AddAccount Client
participant AuthURL as OAuth Auth-URL Endpoint
participant Cookie as Cookie Storage
participant Callback as OAuth Callback
participant Validation as Validation Logic
participant Linking as Account Linking
Client->>AuthURL: GET /api/{provider}/linking/auth-url<br/>(userId only)
AuthURL->>AuthURL: generateState(userId)
AuthURL->>AuthURL: getAuthUrl({ userId })
AuthURL->>Cookie: Set OAuth state cookie
AuthURL-->>Client: Return auth URL
Client->>Callback: OAuth provider redirects to callback<br/>with code & state
Callback->>Validation: validateOAuthCallback(state, code)
Validation->>Validation: Decode state (userId, nonce)
Validation-->>Callback: { targetUserId, code }
Callback->>Linking: handleAccountLinking({ targetUserId, code })
alt Account exists for same user
Linking-->>Callback: { type: "continue_create" }
else Account exists for different user
Linking-->>Callback: { type: "merge", sourceUserId, sourceAccountId }
else No existing account
Linking-->>Callback: { type: "continue_create" }
end
sequenceDiagram
participant Store as Premium Data Store
participant Card as PremiumCard Component
participant UI as UI Render
Store->>Card: premium data (may be undefined)
Card->>Card: hasNoSubscription check
alt Premium data exists
Card->>Card: Extract renewal date
Card->>UI: Render expired card (Reactivate)
else Premium data missing
Card->>Card: Determine if new user<br/>(no subscription data)
alt New user
Card->>UI: Render upgrade message<br/>(Upgrade to Premium)
else Existing user
Card->>UI: Render reactivate message<br/>(Reactivate)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/PremiumCard.tsx (1)
46-70: Suggest extracting duplicate logic into a helper function.The logic for determining if a user has no subscription appears twice (lines 51-53 and lines 60-64). Additionally, similar logic is repeated at lines 116-120 for
isNewUser.Consider extracting this into a helper function:
+const hasNoSubscriptionHistory = (premium: PremiumData | null | undefined) => { + if (!premium) return true; + return ( + !premium.stripeSubscriptionStatus && + !premium.stripeSubscriptionId && + !premium.lemonSqueezySubscriptionId + ); +}; + const getSubscriptionMessage = () => { const UPGRADE_MESSAGE = { title: "Upgrade to Premium", description: "Upgrade to Premium to enable your AI email assistant.", }; - if (!premium) { + if (hasNoSubscriptionHistory(premium)) { return UPGRADE_MESSAGE; } const status = premium.stripeSubscriptionStatus; const hasLemonSqueezyExpired = lemonSqueezyRenewsAt && lemonSqueezyRenewsAt < new Date(); - // Check if user never had a subscription - const hasNoSubscription = - !status && - !premium.stripeSubscriptionId && - !premium.lemonSqueezySubscriptionId; - - if (!premium || hasNoSubscription) { - return { - title: "Upgrade to Premium", - description: "Upgrade to Premium to enable your AI email assistant.", - }; - }Then reuse the helper for
isNewUserat line 116:-const isNewUser = - !premium || - (!premium.stripeSubscriptionStatus && - !premium.stripeSubscriptionId && - !premium.lemonSqueezySubscriptionId); +const isNewUser = hasNoSubscriptionHistory(premium);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/app/(app)/accounts/AddAccount.tsx(2 hunks)apps/web/app/(app)/accounts/page.tsx(2 hunks)apps/web/app/(landing)/components/page.tsx(1 hunks)apps/web/app/api/google/linking/auth-url/route.ts(2 hunks)apps/web/app/api/google/linking/callback/route.ts(1 hunks)apps/web/app/api/outlook/linking/auth-url/route.ts(2 hunks)apps/web/app/api/outlook/linking/callback/route.ts(1 hunks)apps/web/components/PremiumCard.tsx(5 hunks)apps/web/components/ReferralDialog.tsx(4 hunks)apps/web/components/SideNav.tsx(2 hunks)apps/web/components/VideoCard.tsx(1 hunks)apps/web/utils/oauth/account-linking.test.ts(1 hunks)apps/web/utils/oauth/account-linking.ts(2 hunks)apps/web/utils/oauth/callback-validation.test.ts(0 hunks)apps/web/utils/oauth/callback-validation.ts(0 hunks)version.txt(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web/utils/oauth/callback-validation.test.ts
- apps/web/utils/oauth/callback-validation.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
📚 Learning: 2025-07-08T13:14:07.449Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 537
File: apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx:30-34
Timestamp: 2025-07-08T13:14:07.449Z
Learning: The clean onboarding page in apps/web/app/(app)/[emailAccountId]/clean/onboarding/page.tsx is intentionally Gmail-specific and should show an error for non-Google email accounts rather than attempting to support multiple providers.
Applied to files:
apps/web/utils/oauth/account-linking.test.tsapps/web/app/(app)/accounts/AddAccount.tsxapps/web/app/(app)/accounts/page.tsxapps/web/utils/oauth/account-linking.ts
📚 Learning: 2025-06-05T09:49:12.168Z
Learnt from: elie222
Repo: elie222/inbox-zero PR: 485
File: apps/web/app/(landing)/login/page.tsx:41-43
Timestamp: 2025-06-05T09:49:12.168Z
Learning: In Next.js App Router, components that use the `useSearchParams` hook require a Suspense boundary to handle the asynchronous nature of search parameter access. The Suspense wrapper is necessary and should not be removed when a component uses useSearchParams.
Applied to files:
apps/web/app/(app)/accounts/page.tsx
📚 Learning: 2024-08-23T11:37:26.779Z
Learnt from: aryanprince
Repo: elie222/inbox-zero PR: 210
File: apps/web/app/(app)/stats/NewsletterModal.tsx:2-4
Timestamp: 2024-08-23T11:37:26.779Z
Learning: `MoreDropdown` is a React component and `useUnsubscribeButton` is a custom React hook, and they should not be imported using `import type`.
Applied to files:
apps/web/app/(app)/accounts/page.tsx
🧬 Code graph analysis (5)
apps/web/app/api/google/linking/auth-url/route.ts (2)
apps/web/utils/gmail/client.ts (1)
getLinkingOAuth2Client(40-46)apps/web/utils/oauth/state.ts (1)
generateOAuthState(10-18)
apps/web/app/api/outlook/linking/auth-url/route.ts (1)
apps/web/utils/oauth/state.ts (1)
generateOAuthState(10-18)
apps/web/app/(app)/accounts/AddAccount.tsx (1)
apps/web/components/Typography.tsx (1)
TypographyP(129-129)
apps/web/components/SideNav.tsx (1)
apps/web/components/PremiumCard.tsx (1)
PremiumCard(213-232)
apps/web/components/PremiumCard.tsx (1)
apps/web/utils/premium/index.ts (1)
isPremium(14-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: test
🔇 Additional comments (24)
version.txt (1)
1-1: LGTM!Version bump is appropriate for the changes in this PR.
apps/web/components/VideoCard.tsx (1)
109-109: LGTM!The responsive behavior appropriately hides the video thumbnail on small screens while keeping the "Watch Video" button accessible, improving mobile UX.
apps/web/components/ReferralDialog.tsx (3)
31-31: LGTM!Adding scrollable overflow ensures the referral dialog content remains accessible on smaller viewports.
99-108: LGTM!Responsive typography and spacing adjustments improve readability across different screen sizes.
121-127: LGTM!The overflow-x-auto and break-all handling ensures long referral URLs display correctly without breaking the layout.
apps/web/app/(app)/accounts/AddAccount.tsx (2)
22-25: LGTM!The simplification from POST with action parameter to GET aligns with the PR objective to remove action-based OAuth flows. The cleaner API surface reduces complexity.
86-87: Minor text improvement.Changing "each additional account" to "each account" is clearer and more accurate, as all accounts are billed, not just additional ones.
apps/web/components/SideNav.tsx (1)
60-60: LGTM!Clean refactor updating to the renamed PremiumCard component.
Also applies to: 235-235
apps/web/app/(landing)/components/page.tsx (1)
41-41: LGTM!Import path correctly updated to reference the renamed PremiumCard module.
apps/web/app/(app)/accounts/page.tsx (2)
6-6: LGTM!Cleanup of unused useState import after MergeConfirmationDialog removal.
38-38: Note the responsive layout change.The grid now displays:
- 1 column until lg breakpoint (1024px)
- 2 columns from lg to xl (1024-1280px)
- 3 columns at xl+ (1280px+)
Previously it went multi-column earlier at md (768px). This may provide better spacing on tablets but delays the multi-column view.
apps/web/app/api/outlook/linking/callback/route.ts (1)
36-36: Verification confirmed - changes are consistent across both OAuth callback routes.The Google callback route at line 35 shows the same destructuring pattern:
const { targetUserId, code } = validation;with the action parameter removed. ThehandleAccountLinkingcall also does not include the action parameter, confirming that both the Outlook and Google linking callbacks have been updated consistently with the PR objective.apps/web/app/api/google/linking/auth-url/route.ts (1)
13-26: LGTM! Clean simplification of OAuth state generation.The removal of the
actionparameter streamlines the authorization flow. The state now derives solely fromuserId, which simplifies the OAuth flow and reduces complexity.apps/web/app/api/google/linking/callback/route.ts (2)
33-33: LGTM! Correctly updated to remove action from validation.The destructuring now matches the updated
validateOAuthCallbackreturn type, which no longer includes theactionfield.
93-102: LGTM! Simplified account linking flow.The
handleAccountLinkingcall no longer passes theactionparameter, relying instead on the function's internal logic to determine the appropriate flow (merge vs. create). The handling of the result types (redirect,continue_create, and merge) remains comprehensive.apps/web/app/api/outlook/linking/auth-url/route.ts (2)
12-19: LGTM! Consistent simplification with Google OAuth flow.The removal of the
actionparameter aligns with the Google linking flow, providing consistency across OAuth providers. State generation is now streamlined to derive fromuserIdonly.
27-31: Good addition: OAuth state cookie is now properly set.This ensures the state is available for validation during the callback, matching the pattern used in the Google linking flow.
apps/web/utils/oauth/account-linking.test.ts (1)
75-92: LGTM! Test correctly reflects new direct merge behavior.The test has been appropriately updated to expect a direct merge result (
type: "merge") instead of a redirect to a confirmation flow. This aligns with the simplified account linking logic that no longer requires action-based branching.apps/web/components/PremiumCard.tsx (3)
32-41: LGTM! Robust null handling for premium data.The use of optional chaining ensures the component gracefully handles missing premium data without throwing errors.
116-123: LGTM! Clear differentiation between new and existing users.The logic correctly determines whether to show an "Upgrade" or "Reactivate" flow based on subscription history, improving the user experience.
213-213: No remaining references to old component name detected.The search for
PremiumExpiredCardfound no direct matches in the codebase. The search results showPremiumExpiredCardContent—a separate component—which appears to be an internal sub-component and is distinct from the renamed public exportPremiumCard. This indicates all references to the old component name have been successfully updated or removed.apps/web/utils/oauth/account-linking.ts (3)
6-15: Public API change: Removed action parameter from AccountLinkingParams.The interface has been simplified by removing the
action: "auto" | "merge_confirmed"field. This is a breaking change that affects all callers ofhandleAccountLinking.
46-65: Excellent addition: Prevents duplicate email account creation.This new logic checks if an email account already exists for a different user before attempting to create a new one, preventing conflicts. The error message "account_already_exists_use_merge" clearly guides the user to use the merge flow instead.
86-96: LGTM! Simplified and more direct merge flow.The function now proceeds directly to merge when an account exists for a different user, removing the need for action-based branching. The logging clearly documents the merge decision.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 16 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="apps/web/app/(app)/accounts/page.tsx">
<violation number="1" location="apps/web/app/(app)/accounts/page.tsx:6">
Removing the merge confirmation dialog removes the entire confirm-merge flow that reacts to the ?confirm_merge/provider/email query params and issues the merge_confirmed redirect, so users can no longer finalize an account merge.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| import Link from "next/link"; | ||
| import { Trash2, ArrowRight, BotIcon } from "lucide-react"; | ||
| import { useEffect, useState } from "react"; | ||
| import { useEffect } from "react"; |
There was a problem hiding this comment.
Removing the merge confirmation dialog removes the entire confirm-merge flow that reacts to the ?confirm_merge/provider/email query params and issues the merge_confirmed redirect, so users can no longer finalize an account merge.
Prompt for AI agents
Address the following comment on apps/web/app/(app)/accounts/page.tsx at line 6:
<comment>Removing the merge confirmation dialog removes the entire confirm-merge flow that reacts to the ?confirm_merge/provider/email query params and issues the merge_confirmed redirect, so users can no longer finalize an account merge.</comment>
<file context>
@@ -3,7 +3,7 @@
import Link from "next/link";
import { Trash2, ArrowRight, BotIcon } from "lucide-react";
-import { useEffect, useState } from "react";
+import { useEffect } from "react";
import { useSearchParams, useRouter, usePathname } from "next/navigation";
import { ConfirmDialog } from "@/components/ConfirmDialog";
</file context>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores