Integrated dodo subscription in ui, opens checkout, portal and displays credits used#374
Integrated dodo subscription in ui, opens checkout, portal and displays credits used#374shmbhvi101 wants to merge 3 commits into
Conversation
WalkthroughReplaces link-based subscription CTAs with button-driven checkout/portal flows, switches subscription state to raw API fields, adds a payment success page with invoice/portal handling, and consolidates credit/subscription fetching into a single polled credit-balance API. Loading and portal states added; CTA helpers and plan UI updated. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as Subscription UI
participant Minor as MinorService
participant Payment as Payment Service
participant Portal as Portal Service
participant Success as Payment Success Page
User->>UI: Click plan card or "Manage Subscription"
alt checkout flow
UI->>Minor: createCheckoutSession(userId, planType)
Minor->>Payment: GET /checkout-session?user_id&plan_type
Payment-->>Minor: { checkoutUrl }
Minor-->>UI: checkoutUrl
UI->>User: Redirect to checkout
else manage / invoice
UI->>Minor: getCustomerPortal(userId)
Minor->>Portal: GET /customer-portal
Portal-->>Minor: { portalUrl }
Minor-->>UI: portalUrl
UI->>User: Open portal in new tab
end
Payment->>Success: Redirect with subscription_id & status
Success->>Minor: fetchUserSubscription(userId) & fetchCreditBalance(userId)
Minor-->>Success: subscription & credit data
User->>Success: Click "Download Invoice"
Success->>Minor: getCustomerPortal(userId)
Minor->>Portal: GET /customer-portal
Portal-->>Minor: { portalUrl }
Minor-->>Success: portalUrl
Success->>User: Open portal / download invoice
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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📝 Generate docstrings
🧪 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 |
|
Cloud Run service deployed: https://pr-feat-dodo-payments-cj6r7x3fpa-uc.a.run.app |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/Layouts/Sidebar.tsx (1)
97-106:⚠️ Potential issue | 🟡 MinorPotential null reference when accessing
userproperties.At line 99,
user.emailis accessed without optional chaining. WhilehandleTrackis only called within a rendered component (implying user should exist), theuserobject fromuseAuthContext()can be null.Add optional chaining for safety
const handleTrack = () => { formbricksApp.track("report-btn", { - email: user.email, - name: user.displayName, + email: user?.email, + name: user?.displayName, hiddenFields: { - user_id: user.uid, - user_name: user.displayName, + user_id: user?.uid, + user_name: user?.displayName, }, }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/Sidebar.tsx` around lines 97 - 106, The handleTrack function reads properties off user without null checks; update handleTrack (the function that calls formbricksApp.track) to guard against a null user by either returning early if user is falsy or using optional chaining when accessing user.email, user.displayName, and user.uid so you never dereference a null/undefined user from useAuthContext(); ensure the payload omits or substitutes safe defaults for missing fields before calling formbricksApp.track.
🧹 Nitpick comments (4)
app/(main)/user-subscription/page.tsx (1)
73-81: Consider adding type safety for the API response.The
SubscriptionDatainterface is defined locally butMinorService.fetchUserSubscriptionreturns an untypedresponse.data. For better type safety, consider exporting a response type fromminorService.tssimilar toCreditBalanceResponse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(main)/user-subscription/page.tsx around lines 73 - 81, Export a typed response for the subscription API and use it where the data is consumed: add a SubscriptionData (or SubscriptionResponse) type in minorService.ts (mirroring the local SubscriptionData shape) and export it, update MinorService.fetchUserSubscription to return that typed response (instead of untyped response.data), and import and use that exported type in user-subscription/page.tsx to replace the local interface or annotate the fetched value so consumers (e.g., any code referencing SubscriptionData) get compile-time safety similar to CreditBalanceResponse.app/payment/success/page.tsx (3)
18-33: Good null safety, but consider improving error UX.The early return on
!user?.uidproperly guards against null user. However, usingalert()for error feedback (line 29) provides a jarring user experience.Consider using a toast notification or inline message
+ import { useState } from "react"; // ... existing imports + const [errorMessage, setErrorMessage] = useState<string | null>(null); const handleOpenPortal = async () => { if (!user?.uid) return; setPortalLoading(true); + setErrorMessage(null); try { const data = await MinorService.getCustomerPortal(user.uid); if (data.url) { window.open(data.url, "_blank"); } } catch { - alert("Your invoice will be sent to your email shortly. You can also access it from the Manage Subscription page once your account is fully activated."); + setErrorMessage("Your invoice will be sent to your email shortly. You can also access it from the Manage Subscription page once your account is fully activated."); } finally { setPortalLoading(false); } };Then render the error message in the UI instead of using an alert.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` around lines 18 - 33, The handler handleOpenPortal uses alert() for errors; replace that with a non-blocking UI notification: catch errors from MinorService.getCustomerPortal and instead call your app's toast API (e.g., showToast or useToast) or set a local error state (e.g., portalError) and render an inline message in the page; keep using setPortalLoading(true/false) around the async work and ensure user?.uid early-return remains. Use the identifiers handleOpenPortal, MinorService.getCustomerPortal, setPortalLoading, and alert to find and update the code.
60-74: Subscription details displayed from URL params without backend verification.The
statusandsubscriptionIdare read directly from URL query parameters and displayed to the user. While this is likely fine for a success page redirect from the payment provider, consider that these values could be manipulated. For sensitive displays, fetching the actual subscription status from your backend would be more reliable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` around lines 60 - 74, The page is blindly trusting URL query params (status and subscriptionId) and rendering them; update the payment success flow in app/payment/success/page.tsx to verify these values by fetching the authoritative subscription record from your backend: use the subscriptionId from the URL only to call your server API (e.g., an endpoint like /api/subscriptions/[id] or an internal server-side fetch in the page component), validate the returned status and details, and render those backend-provided fields (falling back to a safe default if the fetch fails); ensure you reference and replace current uses of status and subscriptionId in the component render logic so only verified data is shown.
77-85: Button label may mislead users.The button says "Download Invoice" but actually opens the customer portal in a new tab. The portal may contain invoice access, but users might expect a direct PDF download.
Consider a more accurate label
- {portalLoading ? "Opening..." : "Download Invoice"} + {portalLoading ? "Opening..." : "View Invoice & Billing"}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` around lines 77 - 85, The button label "Download Invoice" is misleading because the onClick handler handleOpenPortal opens the customer portal (not a direct PDF download); update the UI in the component that renders this button (the element using handleOpenPortal and portalLoading) to use a clearer label such as "Open Customer Portal" or "Open Billing Portal" and adjust any aria-label or title attributes accordingly so screen-reader text matches the new action; ensure the conditional text still shows the loading state (portalLoading ? "Opening..." : "Open Customer Portal") and keep the existing icons and disabled logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(main)/user-subscription/page.tsx:
- Around line 96-119: The success and error branches set inconsistent defaults
for is_active (success uses data.is_active ?? false, error sets is_active:
true); update the error-path setSubscription call so is_active matches the
success default (set is_active: false) to ensure consistent behavior when the
API omits that field; adjust only the is_active value in the error branch inside
the same setSubscription call so both paths use the same default semantics.
- Around line 388-400: The loading indicator logic wrongly uses a single
portalLoading flag for all actions, causing non-current cards to show
"Loading..." whenever portalLoading is true; add a separate checkout/loading
state (e.g., checkoutLoading) and update handlers (handleManagePlan and
handleUpgradeToPro) to set/clear their respective loading flags, then change the
button logic in handleCardAction rendering (functions isButtonDisabled,
getButtonText and the button's onClick branch) to consider both portalLoading
and checkoutLoading and only show "Loading..." for the card whose action is
currently in progress (compare plan.key to currentPlanKey or the active loading
target) so other cards don't display the wrong state.
In `@components/Layouts/Sidebar.tsx`:
- Around line 90-95: The project contains an orphaned method fetchUserUsage in
minorService.ts which is no longer used; remove the fetchUserUsage function and
any exported references to it, and ensure that callers (if any) are using
fetchCreditBalance instead; update the minorService.ts exports to only expose
active functions (including fetchCreditBalance) and run a quick grep to confirm
there are no remaining references to fetchUserUsage before committing.
---
Outside diff comments:
In `@components/Layouts/Sidebar.tsx`:
- Around line 97-106: The handleTrack function reads properties off user without
null checks; update handleTrack (the function that calls formbricksApp.track) to
guard against a null user by either returning early if user is falsy or using
optional chaining when accessing user.email, user.displayName, and user.uid so
you never dereference a null/undefined user from useAuthContext(); ensure the
payload omits or substitutes safe defaults for missing fields before calling
formbricksApp.track.
---
Nitpick comments:
In `@app/`(main)/user-subscription/page.tsx:
- Around line 73-81: Export a typed response for the subscription API and use it
where the data is consumed: add a SubscriptionData (or SubscriptionResponse)
type in minorService.ts (mirroring the local SubscriptionData shape) and export
it, update MinorService.fetchUserSubscription to return that typed response
(instead of untyped response.data), and import and use that exported type in
user-subscription/page.tsx to replace the local interface or annotate the
fetched value so consumers (e.g., any code referencing SubscriptionData) get
compile-time safety similar to CreditBalanceResponse.
In `@app/payment/success/page.tsx`:
- Around line 18-33: The handler handleOpenPortal uses alert() for errors;
replace that with a non-blocking UI notification: catch errors from
MinorService.getCustomerPortal and instead call your app's toast API (e.g.,
showToast or useToast) or set a local error state (e.g., portalError) and render
an inline message in the page; keep using setPortalLoading(true/false) around
the async work and ensure user?.uid early-return remains. Use the identifiers
handleOpenPortal, MinorService.getCustomerPortal, setPortalLoading, and alert to
find and update the code.
- Around line 60-74: The page is blindly trusting URL query params (status and
subscriptionId) and rendering them; update the payment success flow in
app/payment/success/page.tsx to verify these values by fetching the
authoritative subscription record from your backend: use the subscriptionId from
the URL only to call your server API (e.g., an endpoint like
/api/subscriptions/[id] or an internal server-side fetch in the page component),
validate the returned status and details, and render those backend-provided
fields (falling back to a safe default if the fetch fails); ensure you reference
and replace current uses of status and subscriptionId in the component render
logic so only verified data is shown.
- Around line 77-85: The button label "Download Invoice" is misleading because
the onClick handler handleOpenPortal opens the customer portal (not a direct PDF
download); update the UI in the component that renders this button (the element
using handleOpenPortal and portalLoading) to use a clearer label such as "Open
Customer Portal" or "Open Billing Portal" and adjust any aria-label or title
attributes accordingly so screen-reader text matches the new action; ensure the
conditional text still shows the loading state (portalLoading ? "Opening..." :
"Open Customer Portal") and keep the existing icons and disabled logic intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20603906-9cee-44fb-87b4-e40e84fa1b49
📒 Files selected for processing (4)
app/(main)/user-subscription/page.tsxapp/payment/success/page.tsxcomponents/Layouts/Sidebar.tsxservices/minorService.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/payment/success/page.tsx (2)
15-15: Type the component withFCto match repo TSX convention.This component is currently untyped at declaration. Use
FCtyping for consistency with the project rule.💡 Suggested fix
-import { useState, useEffect } from "react"; +import { useState, useEffect, type FC } from "react"; @@ -export default function PaymentSuccessPage() { +const PaymentSuccessPage: FC = () => { @@ -} +}; + +export default PaymentSuccessPage;As per coding guidelines, "Use functional components with TypeScript FC types".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` at line 15, The PaymentSuccessPage component is declared without a TypeScript functional component type; update its declaration to use the FC type from React (import React and/or FC as needed) and type the component as FC: change the function declaration for PaymentSuccessPage to a typed functional component using FC (e.g., const PaymentSuccessPage: FC = () => { ... } or export default const PaymentSuccessPage: FC = () => { ... }), ensuring any props are typed if present and that React/FC is imported.
97-97: Use nullish coalescing for status fallback.
||can treat valid falsy values as missing. Prefer??for predictable fallback semantics here.💡 Suggested fix
- {status || subscription?.status || "active"} + {status ?? subscription?.status ?? "active"}As per coding guidelines, "Use Next.js error boundaries, optional chaining, and nullish coalescing for error handling".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` at line 97, The fallback uses logical OR which treats valid falsy values as missing; update the expression that renders the status (the values status and subscription?.status) to use nullish coalescing (??) instead of || so only null or undefined fall back to "active" and keep the optional chaining on subscription?.status intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/payment/success/page.tsx`:
- Around line 27-46: The effect's early return when user?.uid is falsy leaves
loading true and the async fetchSubscriptionDetails may call set state after
unmount; update fetchSubscriptionDetails (and the surrounding useEffect) to
always call setLoading(false) when skipping work (so loading won't remain true),
and guard any post-await setSubscription, setCreditBalance, and setLoading with
a mounted flag or AbortController check to avoid stale updates after unmount;
specifically modify the useEffect/fetchSubscriptionDetails flow (the functions
named useEffect and fetchSubscriptionDetails and the state setters
setSubscription, setCreditBalance, setLoading) to set loading false on early
exit and to only invoke state setters when the component is still mounted.
- Around line 38-40: The catch block currently only does console.error and lets
the UI fall back to defaults; add a component-level error state (e.g.,
useState<string | null> fetchError) in the page component, replace the catch
body to setFetchError(error.message || String(error)) (you can still
console.error for devs), and update the render logic to conditionally show a
visible error state/UI when fetchError is non-null instead of showing the
"Individual - Pro" defaults; mirror the portal-request alert approach used
elsewhere so users see that subscription/credit fetch failed.
- Around line 53-55: The window.open call that uses data.url should be hardened:
first validate that data.url is a safe absolute URL with an allowed protocol
(e.g., starts with "https://") and reject or sanitize any other schemes before
using it, then open it with security parameters to prevent tabnabbing by using
target "_blank" together with rel="noopener,noreferrer" (or set window.open with
the third argument specifying "noopener,noreferrer"), and add defensive null
checks around data.url; apply the same validation and rel/noopener change to the
analogous window.open usage in the user-subscription page so both places
validate protocol and use noopener,noreferrer.
---
Nitpick comments:
In `@app/payment/success/page.tsx`:
- Line 15: The PaymentSuccessPage component is declared without a TypeScript
functional component type; update its declaration to use the FC type from React
(import React and/or FC as needed) and type the component as FC: change the
function declaration for PaymentSuccessPage to a typed functional component
using FC (e.g., const PaymentSuccessPage: FC = () => { ... } or export default
const PaymentSuccessPage: FC = () => { ... }), ensuring any props are typed if
present and that React/FC is imported.
- Line 97: The fallback uses logical OR which treats valid falsy values as
missing; update the expression that renders the status (the values status and
subscription?.status) to use nullish coalescing (??) instead of || so only null
or undefined fall back to "active" and keep the optional chaining on
subscription?.status intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3e6fbf7-6fa4-45ed-8fb6-da9748894408
📒 Files selected for processing (1)
app/payment/success/page.tsx
|
Cloud Run service deployed: https://pr-feat-dodo-payments-cj6r7x3fpa-uc.a.run.app |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/payment/success/page.tsx (2)
44-46:⚠️ Potential issue | 🟠 MajorSurface a visible error state when the subscription/credit fetch fails.
The catch block only logs to console; users see fallback values ("Individual - Pro") without knowing the fetch failed. Add an error state and display it in the UI so users understand their actual subscription data may not have loaded.
💡 Suggested fix
+ const [fetchError, setFetchError] = useState<string | null>(null); + // ... in fetchSubscriptionDetails ... } catch (error) { console.error("Error fetching subscription details:", error); + if (mounted) { + setFetchError("Failed to load subscription details. Please refresh the page."); + } } finally {Then conditionally render the error in the JSX:
{fetchError && ( <div className="bg-red-50 border border-red-200 text-red-700 px-4 py-3 rounded-lg mb-6"> {fetchError} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` around lines 44 - 46, The catch block in the async fetch (inside the try/catch around fetching subscription/credits) only logs to console; add a component state (e.g., fetchError via useState) and set it in the catch to a user-facing message, update the fetching function (the async that currently logs "Error fetching subscription details:") to set fetchError when an exception occurs, and then conditionally render that error message in the page JSX (above the subscription display) so users see a visible error banner when subscription/credit fetch fails.
64-67:⚠️ Potential issue | 🟠 MajorHarden
window.openusage for portal URLs.The current implementation lacks security parameters and URL validation. Add
noopener,noreferrerto prevent tabnabbing, and use optional chaining for safer access.💡 Suggested fix
const data = await MinorService.getCustomerPortal(user.uid); - if (data.url) { - window.open(data.url, "_blank"); + if (data?.url) { + try { + const portalUrl = new URL(data.url); + if (portalUrl.protocol === "https:") { + window.open(portalUrl.toString(), "_blank", "noopener,noreferrer"); + } + } catch { + console.error("Invalid portal URL"); + } }Based on learnings: Use optional chaining for error handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` around lines 64 - 67, The call to window.open in the success page should be hardened: use optional chaining when accessing the portal URL returned by MinorService.getCustomerPortal (e.g., data?.url) and validate it is a non-empty http(s) URL before opening; when calling window.open, include the rel/security parameters "noopener,noreferrer" (e.g., window.open(validUrl, "_blank", "noopener,noreferrer")). Locate the call to MinorService.getCustomerPortal and the usage of data.url and change to a guarded/validated flow that only calls window.open with the validated URL and the noopener,noreferrer flags.
🧹 Nitpick comments (1)
app/payment/success/page.tsx (1)
86-98: Consider extracting repeated plan-type formatting logic.The capitalization logic (
type.charAt(0).toUpperCase() + type.slice(1)) is duplicated on lines 86 and 97. A small helper would reduce repetition and improve readability.♻️ Suggested helper
+const formatPlanType = (type: string | undefined): string => { + if (!type) return "Pro"; + return type.charAt(0).toUpperCase() + type.slice(1); +}; + export default function PaymentSuccessPage() { // ... existing code ... // Then use in JSX: - You're on {subscription?.type === "pro" ? "Pro" : subscription?.type ? subscription.type.charAt(0).toUpperCase() + subscription.type.slice(1) : "Pro"}! + You're on {formatPlanType(subscription?.type)}! - {loading ? "Loading..." : subscription?.type ? `Individual - ${subscription.type.charAt(0).toUpperCase() + subscription.type.slice(1)}` : "Individual - Pro"} + {loading ? "Loading..." : `Individual - ${formatPlanType(subscription?.type)}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/payment/success/page.tsx` around lines 86 - 98, Extract the repeated capitalization logic into a small helper like formatPlanType(type) and use it wherever subscription.type is formatted (the two inline expressions inside the JSX that produce "You're on ..." and the "Individual - ..." label). Implement formatPlanType to return "Pro" for falsy input, otherwise return type.charAt(0).toUpperCase()+type.slice(1); place it in the same module (above the component) or import it, and replace the ternary expressions with calls to formatPlanType(subscription?.type) while keeping the existing loading checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/payment/success/page.tsx`:
- Around line 44-46: The catch block in the async fetch (inside the try/catch
around fetching subscription/credits) only logs to console; add a component
state (e.g., fetchError via useState) and set it in the catch to a user-facing
message, update the fetching function (the async that currently logs "Error
fetching subscription details:") to set fetchError when an exception occurs, and
then conditionally render that error message in the page JSX (above the
subscription display) so users see a visible error banner when
subscription/credit fetch fails.
- Around line 64-67: The call to window.open in the success page should be
hardened: use optional chaining when accessing the portal URL returned by
MinorService.getCustomerPortal (e.g., data?.url) and validate it is a non-empty
http(s) URL before opening; when calling window.open, include the rel/security
parameters "noopener,noreferrer" (e.g., window.open(validUrl, "_blank",
"noopener,noreferrer")). Locate the call to MinorService.getCustomerPortal and
the usage of data.url and change to a guarded/validated flow that only calls
window.open with the validated URL and the noopener,noreferrer flags.
---
Nitpick comments:
In `@app/payment/success/page.tsx`:
- Around line 86-98: Extract the repeated capitalization logic into a small
helper like formatPlanType(type) and use it wherever subscription.type is
formatted (the two inline expressions inside the JSX that produce "You're on
..." and the "Individual - ..." label). Implement formatPlanType to return "Pro"
for falsy input, otherwise return type.charAt(0).toUpperCase()+type.slice(1);
place it in the same module (above the component) or import it, and replace the
ternary expressions with calls to formatPlanType(subscription?.type) while
keeping the existing loading checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc92ae7b-7107-4b25-bc0b-f4dc462763b7
📒 Files selected for processing (1)
app/payment/success/page.tsx
|
Cloud Run service deployed: https://pr-feat-dodo-payments-cj6r7x3fpa-uc.a.run.app |
Summary by CodeRabbit
New Features
Improvements