feat(ui): add shared UpgradeToProDialog component - #321
Conversation
Refactored and centralized Pro upgrade dialog into a reusable shared component. Replaced individual dialog instances across PlanManagement and pricing plans with the new component for better code reuse and consistency.
WalkthroughThe upgrade-to-Pro subscription flow has been refactored to use a new shared Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PlanManagement/PricingPlans
participant UpgradeToProDialog
participant API
participant Stripe
User->>PlanManagement/PricingPlans: Click "Upgrade to Pro"
PlanManagement/PricingPlans->>UpgradeToProDialog: Open dialog
User->>UpgradeToProDialog: Click "Upgrade" button
UpgradeToProDialog->>API: Request Stripe Checkout session
API-->>UpgradeToProDialog: Return Stripe Checkout URL
UpgradeToProDialog->>Stripe: Redirect user to Stripe Checkout
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/ui/src/components/shared/upgrade-to-pro-dialog.tsx (1)
74-76: Consider extracting hardcoded price to a constantThe Pro plan price of "$50/month" is hardcoded in multiple places. This could lead to inconsistencies if the pricing changes.
Extract the price to a constant at the top of the file:
+const PRO_PLAN_PRICE = "$50/month"; + interface UpgradeToProDialogProps { children: React.ReactNode; onSuccess?: () => void; }Then update the usage:
<DialogDescription> Unlock provider keys and get full access to all features for - $50/month. + {PRO_PLAN_PRICE}. </DialogDescription>{loading || createSubscriptionMutation.isPending ? "Redirecting to checkout..." - : "Upgrade for $50/month now"} + : `Upgrade for ${PRO_PLAN_PRICE} now`}Also applies to: 107-109
apps/ui/src/components/landing/pricing-plans.tsx (1)
70-71: Clarify loading state management between componentsThe component manages its own loading state (line 71, 340-343) while the
UpgradeToProDialogalso has internal loading state. This creates two separate loading indicators that could be confusing - the local loading state won't be set when using the dialog.Consider either:
- Remove the local loading state management for the Pro plan since it's handled by the dialog
- Pass a loading callback to the dialog to synchronize states
For option 1, you could simplify the loading check:
const isLoading = loading === plan.name.toLowerCase() || (plan.name === "Pro" && - (loading === "cancel" || loading === "resume")); + subscriptionStatus?.plan === "pro" && + (loading === "cancel" || loading === "resume"));Also applies to: 340-343, 413-415
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/ui/src/components/billing/plan-management.tsx(2 hunks)apps/ui/src/components/landing/pricing-plans.tsx(2 hunks)apps/ui/src/components/shared/upgrade-to-pro-dialog.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/ui/src/components/billing/plan-management.tsx (1)
apps/ui/src/components/shared/upgrade-to-pro-dialog.tsx (1)
UpgradeToProDialog(21-40)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e / run
🔇 Additional comments (3)
apps/ui/src/components/billing/plan-management.tsx (1)
187-189: Clean integration of the shared dialog componentThe replacement of the inline dialog with the centralized
UpgradeToProDialogcomponent is well implemented. The button remains as the trigger element, maintaining the existing UI while improving code reusability.apps/ui/src/components/landing/pricing-plans.tsx (2)
405-406: Good implementation of the success callbackThe
onSuccesscallback properly refreshes the subscription status after a successful upgrade, ensuring the UI stays in sync with the backend state.
60-101:Details
❌ Incorrect review comment
Remove dead code to improve maintainability
The
handleCreateProSubscriptionfunction (lines 60-101) appears to be dead code now that the Pro upgrade flow uses theUpgradeToProDialogcomponent for authenticated users. This function duplicates the logic in the shared dialog and could lead to confusion.Let me verify if this function is still being used:
If the verification confirms it's only called in line 190 (which won't be reached for authenticated users due to the conditional rendering), consider removing this function and updating the
handlePlanSelectionlogic.Also applies to: 190-190, 402-419
🏁 Script executed:
#!/bin/bash # Check if handleCreateProSubscription is called anywhere else rg -A 5 "handleCreateProSubscription"Length of output: 875
I’d like to see the surrounding logic for both the subscription handler invocation and the plan‐selection flow. Let’s print context around those definitions and call sites:
#!/bin/bash # Show context around handleCreateProSubscription invocation rg -n -C5 "handleCreateProSubscription" apps/ui/src/components/landing/pricing-plans.tsx echo # Show the handlePlanSelection definition & immediate logic rg -n -C5 "const handlePlanSelection" apps/ui/src/components/landing/pricing-plans.tsx
Let’s inspect the call sites and surrounding logic to determine if this is truly dead:
🏁 Script executed:
#!/bin/bash # Show definition & invocation context for handleCreateProSubscription rg -n -C5 "handleCreateProSubscription" apps/ui/src/components/landing/pricing-plans.tsx echo # Show the plan‐selection handler to see how this is wired up rg -n -C5 "const handlePlanSelection" apps/ui/src/components/landing/pricing-plans.tsx echo # Verify if the new UpgradeToProDialog is used in this file rg -n -C5 "UpgradeToProDialog" apps/ui/src/components/landing/pricing-plans.tsxLength of output: 2408
handleCreateProSubscription is still in use
The
handleCreateProSubscriptionfunction (defined at line 60) is invoked in yourhandlePlanSelectionswitch (case "Pro" at line 190) to redirect unauthenticated users to signup or kick off the checkout flow. SinceUpgradeToProDialogonly replaces the flow for authenticated users, this helper remains necessary.• Definition: apps/ui/src/components/landing/pricing-plans.tsx line 60
• Invocation: same file,handlePlanSelectioncase "Pro" at line 190No action required here.
Likely an incorrect or invalid review comment.
| toast({ | ||
| title: "Upgrade Failed", | ||
| description: `Failed to create checkout session. Please try again. Error: ${error}`, | ||
| variant: "destructive", | ||
| }); | ||
| setLoading(false); |
There was a problem hiding this comment.
Improve error handling for better user experience
The error object is directly interpolated into the error message string, which could result in [object Object] being displayed if the error is not a simple string.
Apply this diff to properly handle different error types:
} catch (error) {
toast({
title: "Upgrade Failed",
- description: `Failed to create checkout session. Please try again. Error: ${error}`,
+ description: error instanceof Error
+ ? `Failed to create checkout session: ${error.message}`
+ : "Failed to create checkout session. Please try again.",
variant: "destructive",
});
setLoading(false);📝 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.
| toast({ | |
| title: "Upgrade Failed", | |
| description: `Failed to create checkout session. Please try again. Error: ${error}`, | |
| variant: "destructive", | |
| }); | |
| setLoading(false); | |
| } catch (error) { | |
| toast({ | |
| title: "Upgrade Failed", | |
| description: error instanceof Error | |
| ? `Failed to create checkout session: ${error.message}` | |
| : "Failed to create checkout session. Please try again.", | |
| variant: "destructive", | |
| }); | |
| setLoading(false); | |
| } |
🤖 Prompt for AI Agents
In apps/ui/src/components/shared/upgrade-to-pro-dialog.tsx around lines 60 to
65, the error object is directly interpolated into the toast description, which
can display as [object Object] if the error is not a string. To fix this, check
if the error is an instance of Error and use its message property; otherwise,
convert the error to a string safely before including it in the toast
description. This ensures the user sees a meaningful error message.
| const handleUpgrade = async () => { | ||
| setLoading(true); | ||
|
|
||
| try { | ||
| const { checkoutUrl } = await createSubscriptionMutation.mutateAsync({}); | ||
|
|
||
| // Redirect to Stripe Checkout | ||
| window.location.href = checkoutUrl; | ||
| } catch (error) { | ||
| toast({ | ||
| title: "Upgrade Failed", | ||
| description: `Failed to create checkout session. Please try again. Error: ${error}`, | ||
| variant: "destructive", | ||
| }); | ||
| setLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent potential race conditions in mutation handling
While the button is disabled during loading, there's no explicit check to prevent multiple concurrent mutation calls if the button is rapidly clicked or if the component state updates unexpectedly.
Add a guard to prevent concurrent mutations:
const handleUpgrade = async () => {
+ if (loading || createSubscriptionMutation.isPending) {
+ return;
+ }
+
setLoading(true);
try {📝 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.
| const handleUpgrade = async () => { | |
| setLoading(true); | |
| try { | |
| const { checkoutUrl } = await createSubscriptionMutation.mutateAsync({}); | |
| // Redirect to Stripe Checkout | |
| window.location.href = checkoutUrl; | |
| } catch (error) { | |
| toast({ | |
| title: "Upgrade Failed", | |
| description: `Failed to create checkout session. Please try again. Error: ${error}`, | |
| variant: "destructive", | |
| }); | |
| setLoading(false); | |
| } | |
| }; | |
| const handleUpgrade = async () => { | |
| if (loading || createSubscriptionMutation.isPending) { | |
| return; | |
| } | |
| setLoading(true); | |
| try { | |
| const { checkoutUrl } = await createSubscriptionMutation.mutateAsync({}); | |
| // Redirect to Stripe Checkout | |
| window.location.href = checkoutUrl; | |
| } catch (error) { | |
| toast({ | |
| title: "Upgrade Failed", | |
| description: `Failed to create checkout session. Please try again. Error: ${error}`, | |
| variant: "destructive", | |
| }); | |
| setLoading(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/ui/src/components/shared/upgrade-to-pro-dialog.tsx around lines 51 to
67, the handleUpgrade function lacks a guard against multiple concurrent
mutation calls, which can occur if the button is clicked rapidly. Add a check at
the start of handleUpgrade to return early if loading is already true,
preventing multiple simultaneous executions of the mutation and avoiding race
conditions.
| export function UpgradeToProDialog({ | ||
| children, | ||
| onSuccess, | ||
| }: UpgradeToProDialogProps) { | ||
| const [open, setOpen] = useState(false); | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogTrigger asChild>{children}</DialogTrigger> | ||
| <DialogContent> | ||
| <UpgradeDialogContent | ||
| onSuccess={() => { | ||
| setOpen(false); | ||
| onSuccess?.(); | ||
| }} | ||
| /> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider calling onSuccess after successful subscription creation
The onSuccess callback is only triggered when the dialog is closed manually, not after a successful subscription creation. Since the component redirects to Stripe immediately after a successful mutation, the onSuccess callback won't be executed in the typical success flow. This could lead to stale data in the parent component.
Consider storing the onSuccess callback and executing it after a successful mutation, possibly with a delay before redirect:
function UpgradeDialogContent({ onSuccess }: { onSuccess: () => void }) {
const { toast } = useToast();
const [loading, setLoading] = useState(false);
const createSubscriptionMutation = $api.useMutation(
"post",
"/subscriptions/create-pro-subscription",
);
const handleUpgrade = async () => {
setLoading(true);
try {
const { checkoutUrl } = await createSubscriptionMutation.mutateAsync({});
+ // Call onSuccess before redirecting
+ onSuccess();
+
// Redirect to Stripe Checkout
window.location.href = checkoutUrl;
} catch (error) {🤖 Prompt for AI Agents
In apps/ui/src/components/shared/upgrade-to-pro-dialog.tsx around lines 21 to
40, the onSuccess callback is currently only called when the dialog closes
manually, but not after a successful subscription creation which triggers an
immediate redirect to Stripe. To fix this, modify the UpgradeDialogContent
component or its mutation handler to accept and store the onSuccess callback,
then invoke it right after the subscription is successfully created, potentially
adding a short delay before redirecting to Stripe to ensure onSuccess completes
and parent data stays fresh.
1 participant
Refactored and centralized Pro upgrade dialog into a reusable shared component. Replaced individual dialog instances across PlanManagement and pricing plans with the new component for better code reuse and consistency.
Summary by CodeRabbit
New Features
Refactor