Skip to content

fix: User can add a credit card and select their tier#4128

Merged
perkinsjr merged 69 commits intomainfrom
eng-2126-a-user-who-wants-to-pay-unkey-can-add-a-credit-card-and
Nov 12, 2025
Merged

fix: User can add a credit card and select their tier#4128
perkinsjr merged 69 commits intomainfrom
eng-2126-a-user-who-wants-to-pay-unkey-can-add-a-credit-card-and

Conversation

@MichaelUnkey
Copy link
Collaborator

routes not actions.

What does this PR do?

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Flow for billing looks nice and works as expected
  1. Adding a credit card should then have a modal to select a plan before returning to billing page
  2. Upgrading or Downgrading a plan should also show a modal and work as expected
  3. Canceling a plan should work as expected
  4. Re-Enabling plan should work as expected
  5. Billing page cards should update when plan is changed
  6. No client or server errors

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

@linear
Copy link

linear bot commented Oct 21, 2025

@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2025

⚠️ No Changeset found

Latest commit: 6818e31

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
dashboard Ready Ready Preview Comment Nov 12, 2025 3:21pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
engineering Ignored Ignored Preview Nov 12, 2025 3:21pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Caution

Review failed

The head commit changed during the review from 3c262b4 to 07e0902.

📝 Walkthrough

Walkthrough

Moves billing and post-checkout logic from server to client TRPC-driven components; adds multiple Stripe TRPC endpoints and utilities; introduces new billing UI components (plan modal, current plan, cancel/resume/status, usage) and a shared Stripe client; updates related UI/layout and cache invalidation.

Changes

Cohort / File(s) Summary
Billing page & client
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx, apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx
Converted server-rendered billing to client components; client queries trpc.stripe.getBillingInfo, added loading/error states, workspace navigation, and drives rendering via new billing components.
Billing UI components
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx, .../current-plan-card.tsx, .../cancel-plan.tsx, .../cancel-alert.tsx, .../subscription-status.tsx, .../free-tier-alert.tsx, .../usage.tsx, .../shell.tsx
Added PlanSelectionModal, CurrentPlanCard, CancelPlan, CancelAlert, SubscriptionStatus, FreeTierAlert; refactored Usage to accept quota and fetch usage via TRPC (exports ProgressCircle); removed Shell title prop and title rendering.
Success (checkout return) flow
apps/dashboard/app/success/page.tsx, apps/dashboard/app/success/client.tsx
Reworked success flow to client-side staged processing: read session_id, call TRPC endpoints (getCheckoutSession, getBillingInfo, getCustomer, getSetupIntent), update customer/workspace, detect first-time users and optionally show PlanSelectionModal.
TRPC — Stripe endpoints
apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts, .../getProducts.ts, .../getCheckoutSession.ts, .../getCustomer.ts, .../getSetupIntent.ts, .../updateCustomer.ts, .../createSubscription.ts, .../updateWorkspace.ts, .../cancelSubscription.ts, .../uncancelSubscription.ts, .../updateSubscription.ts
Added getBillingInfo, getProducts, getCheckoutSession, getCustomer, getSetupIntent, updateCustomer, updateWorkspaceStripeCustomer; migrated Stripe client usage to getStripeClient(), added schemas, rate-limits, error mapping, and cache invalidation/clear calls.
TRPC — workspace & utils
apps/dashboard/lib/trpc/routers/workspace/getById.ts, apps/dashboard/lib/trpc/routers/utils/stripe.ts
Added getWorkspaceById and mapProduct/handleStripeError utilities for validating/normalizing Stripe product, price, and quota metadata.
Billing usage router minor
apps/dashboard/lib/trpc/routers/billing/query-usage/index.ts
Minor return-object mapping change (explicit key/value mapping for billableRatelimits and billableVerifications).
TRPC router index
apps/dashboard/lib/trpc/routers/index.ts
Exposed new Stripe endpoints and workspace.getById on the public router.
Stripe client provider
apps/dashboard/lib/stripe.ts
Added singleton getStripeClient() provider for shared Stripe client initialization and error on missing config.
UI dialog container
internal/ui/src/components/dialog/dialog-container.tsx
DialogContainer props extended with optional showCloseWarning?: boolean and onAttemptClose?: () => void, passed through to DialogContent.
Layout tweak & NotFound nav
apps/dashboard/app/(app)/[workspaceSlug]/settings/general/page.tsx, apps/dashboard/app/(app)/[...not-found]/page.tsx
Removed workspace header and adjusted spacing; replaced Link navigation with programmatic router.push("/") in NotFound.
Onboarding hook & cache clears
apps/dashboard/app/new/hooks/use-workspace-step.tsx, apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts
Added additional workspace/stripe cache clears (clearWorkspaceCache) in onboarding and after subscription creation.
PageLoading component
apps/dashboard/components/dashboard/page-loading.tsx
Added new PageLoading component exported for reuse.
Tooling small edits
tools/artillery/create-keys.ts
Commented-out CreateKeyError interface and minor log string tweak.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant BillingPage
    participant BillingClient
    participant TRPC as trpc.stripe.getBillingInfo
    participant StripeAPI as Stripe
    participant PlanModal

    User->>BillingPage: navigate to /settings/billing
    BillingPage->>BillingClient: mount
    BillingClient->>TRPC: query getBillingInfo
    TRPC->>StripeAPI: fetch products + subscription
    StripeAPI-->>TRPC: billing data
    TRPC-->>BillingClient: billingInfo
    BillingClient->>BillingClient: render CurrentPlanCard / Usage / actions
    User->>BillingClient: click "Change Plan"
    BillingClient->>PlanModal: open
    User->>PlanModal: select plan → confirm
    PlanModal->>trpc.stripe.createSubscription: mutate
    trpc.stripe.createSubscription->>StripeAPI: create/update subscription
    StripeAPI-->>trpc.stripe.createSubscription: success
    trpc.stripe.createSubscription-->>PlanModal: success (invalidate/clear caches)
    PlanModal->>BillingClient: close modal
    BillingClient->>TRPC: refetch getBillingInfo
    TRPC-->>BillingClient: updated billingInfo
Loading
sequenceDiagram
    actor User
    participant CheckoutReturn
    participant SuccessPage
    participant TRPC as trpc.stripe
    participant SuccessClient
    participant PlanModal

    User->>CheckoutReturn: return from Stripe (session_id)
    CheckoutReturn->>SuccessPage: mount with session_id
    SuccessPage->>TRPC.getCheckoutSession: getCheckoutSession(session_id)
    TRPC-->>SuccessPage: session data
    SuccessPage->>TRPC.getBillingInfo: getBillingInfo
    alt first-time user
        SuccessPage->>TRPC.getProducts: getProducts
        TRPC-->>SuccessPage: products
        SuccessPage->>SuccessClient: render with showPlanSelection
        SuccessClient->>PlanModal: open
    else returning user
        SuccessPage->>SuccessClient: render redirect to billing
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas to focus:

  • trpc.stripe.getBillingInfo and mapProduct validation/parsing of default_price and quota metadata.
  • PlanSelectionModal create/update subscription flows, hydration guards, and mutation error handling.
  • Success page/client staged flow and sequencing of TRPC calls/updates.
  • Cache invalidation: interplay of invalidate vs clearWorkspaceCache across procedures.
  • Usage/ProgressCircle percent calculations and guards for zero/negative quotas.

Possibly related PRs

Suggested labels

Feature, Dashboard, UI, :joystick: 300 points

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main enhancement: enabling users to add a credit card and select a subscription tier.
Description check ✅ Passed The description includes the required checklist (marked complete), testing instructions, and change type selection, but lacks the issue number and provides minimal summary detail.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@MichaelUnkey MichaelUnkey marked this pull request as ready for review October 21, 2025 14:49
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (15)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/subscription-status.tsx (1)

6-11: Remove or use unused props.

trialUntil and workspaceId are unused; drop them or wire them into UI.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1)

17-18: Unify spelling: “canceled” vs “cancelled”.

Toast uses “cancelled”. For US English, prefer “canceled”.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (2)

96-103: Unreachable “free” selection branch.

selectedProductId is never set to "free", so this path never executes. Either add a Free plan option to products or remove this branch.

-    if (selectedProductId === "free" && !isChangingPlan) {
-      // For free tier on initial selection, just close modal and redirect
-      setIsLoading(false);
-      handleOpenChange(false);
-      toast.success("Staying on Free plan - you can upgrade anytime!");
-      router.push(`/${workspaceSlug}/settings/billing`);
-      return;
-    }
+    // If you intend to allow selecting Free explicitly, include a "free" product in `products`
+    // and set selectedProductId accordingly; otherwise remove this branch.

72-87: Consider refreshing route after plan change for parity.

createSubscription refreshes and navigates; updateSubscription only invalidates. Add router.refresh() (and/or navigate) to ensure SSR sections update consistently.

   const updateSubscription = trpc.stripe.updateSubscription.useMutation({
     onSuccess: async () => {
       setIsLoading(false);
 
       toast.success("Plan changed successfully!");
       await trpcUtils.stripe.getBillingInfo.invalidate();
       await trpcUtils.billing.queryUsage.invalidate();
       await trpcUtils.workspace.getCurrent.invalidate();
       // await trpcUtils.workspace.getCurrent.refetch();
       handleOpenChange(false);
+      router.refresh();
     },
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/current-plan-card.tsx (3)

38-40: Prefer locale-aware currency formatting

Hardcoding $${number} will misformat for non‑US locales and cents. Use Intl or a shared formatter.

- <span className="font-medium text-gray-12">${currentProduct.dollar}/mo</span>
+ <span className="font-medium text-gray-12">
+   {new Intl.NumberFormat(undefined, { style: "currency", currency: "USD", maximumFractionDigits: 2 })
+     .format(currentProduct.dollar)}
+   /mo
+ </span>

Also applies to: 62-63


51-53: Avoid hard-coded free tier numbers

150,000 and $0 should come from config or getBillingInfo to keep a single source of truth.


16-19: Micro‑nit: inline the handler

useCallback adds little here; you can pass onChangePlan directly to Button.

- const handleChangePlan = useCallback(() => {
-   onChangePlan?.();
- }, [onChangePlan]);
...
- onClick={handleChangePlan}
+ onClick={onChangePlan}
apps/dashboard/app/success/page.tsx (1)

84-86: First‑time flow logic is solid; consider centralizing product fetch via TRPC

To keep “routes not actions” and single data path, you could reuse stripe.getBillingInfo on the server for products (same mapping/validation), avoiding a second Stripe client in this route.

Also applies to: 108-129

apps/dashboard/app/success/client.tsx (2)

37-49: Avoid blank screen during redirects; show a lightweight placeholder

While router.push runs, render a minimal “Redirecting…” to reduce perceived flicker.

-  return <></>;
+  return <div className="sr-only">Redirecting…</div>

Also applies to: 61-61


19-31: Naming consistency

Consider renaming workSpaceSlug → workspaceSlug for consistency with other props/paths.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx (2)

47-50: Layout consistency: place WorkspaceNavbar outside Shell in legacy branch.

Other branches render before ; the legacy branch nests it inside Shell. Align for consistent spacing and DOM order.

Also applies to: 73-97


56-69: Read-only inputs used for display — add readOnly or switch to text.

Passing value without onChange can trigger React warnings and is semantically incorrect for display-only fields.

-  <Input value={formatNumber(verifications)} />
+  <Input value={formatNumber(verifications)} readOnly />
...
-  <Input value={formatNumber(ratelimits)} />
+  <Input value={formatNumber(ratelimits)} readOnly />

Please confirm @unkey/ui Input doesn’t enforce onChange when value is provided.

apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (2)

17-24: Subscription status is a free-form string — restrict to known Stripe statuses.

Prevents accidental typos and aligns with UI switch-cases.

-    status: z.string(),
+    status: z.enum([
+      "trialing",
+      "active",
+      "past_due",
+      "canceled",
+      "unpaid",
+      "incomplete",
+      "incomplete_expired",
+      "paused",
+    ]),

73-77: Optional: reuse a single Stripe client instance.

Creating a client per request adds overhead. Cache at module scope.

-    const stripe = new Stripe(e.STRIPE_SECRET_KEY, {
+    const stripe = new Stripe(e.STRIPE_SECRET_KEY, {
       apiVersion: "2023-10-16",
       typescript: true,
     });

Move to top-level:

let stripeSingleton: Stripe | null = null;
function getStripe(e: { STRIPE_SECRET_KEY: string }) {
  if (!stripeSingleton) {
    stripeSingleton = new Stripe(e.STRIPE_SECRET_KEY, { apiVersion: "2023-10-16", typescript: true });
  }
  return stripeSingleton;
}

Then use getStripe(e).

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (1)

93-109: UX polish: plan change modal and portal cards.

  • When no customer exists, consider opening PlanSelectionModal after checkout success; ensure this is coordinated with the “Success” page logic.
  • Minor: wrapping Link inside Button can affect semantics. Prefer Link as the outer element or use Button asChild if supported by @unkey/ui.

Does @unkey/ui Button support an asChild prop to render an anchor for better semantics?

Also applies to: 126-146

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c764837 and 019d89f.

📒 Files selected for processing (14)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/current-plan-card.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/free-tier-alert.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/subscription-status.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/usage.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx (4 hunks)
  • apps/dashboard/app/success/client.tsx (1 hunks)
  • apps/dashboard/app/success/page.tsx (3 hunks)
  • apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (1 hunks)
  • internal/ui/src/components/dialog/dialog-container.tsx (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx (1)
apps/dashboard/lib/trpc/routers/index.ts (1)
  • router (130-345)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/current-plan-card.tsx (2)
internal/ui/src/components/settings-card.tsx (1)
  • SettingCard (57-57)
internal/ui/src/components/buttons/button.tsx (1)
  • Button (439-439)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (3)
apps/dashboard/lib/trpc/routers/index.ts (1)
  • router (130-345)
apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts (1)
  • cancelSubscription (6-51)
apps/dashboard/components/dashboard/confirm.tsx (1)
  • Confirm (16-55)
apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (2)
apps/dashboard/lib/trpc/trpc.ts (3)
  • t (8-8)
  • requireWorkspace (23-36)
  • withRatelimit (122-138)
apps/dashboard/lib/env.ts (1)
  • stripeEnv (80-80)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/free-tier-alert.tsx (1)
internal/ui/src/components/empty.tsx (1)
  • Empty (74-74)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (3)
apps/dashboard/lib/trpc/routers/index.ts (1)
  • router (130-345)
apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (1)
  • createSubscription (9-122)
apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts (1)
  • updateSubscription (9-156)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (1)
  • Client (29-150)
apps/dashboard/app/success/page.tsx (1)
apps/dashboard/app/success/client.tsx (1)
  • SuccessClient (33-62)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (7)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (1)
  • PlanSelectionModal (27-280)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/subscription-status.tsx (1)
  • SubscriptionStatus (6-40)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx (1)
  • CancelAlert (7-54)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/free-tier-alert.tsx (1)
  • FreeTierAlert (6-20)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/usage.tsx (1)
  • Usage (6-32)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/current-plan-card.tsx (1)
  • CurrentPlanCard (15-67)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1)
  • CancelPlan (7-46)
apps/dashboard/app/success/client.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (1)
  • PlanSelectionModal (27-280)
⏰ 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). (3)
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test Packages / Test
  • GitHub Check: Build / Build
🔇 Additional comments (8)
internal/ui/src/components/dialog/dialog-container.tsx (3)

22-23: LGTM! Props are well-typed and clearly named.

The new optional props follow React conventions and have descriptive names that clearly communicate their intent.


36-37: LGTM! Defaults are sensible.

The default value of false for showCloseWarning provides safe opt-in behavior, and leaving onAttemptClose without a default is appropriate for an optional callback.


54-55: Props are correctly accepted by DialogContent.

The verification confirms that DialogContent component signature (lines 38-39 in internal/ui/src/components/dialog/dialog.tsx) explicitly defines both showCloseWarning?: boolean and onAttemptClose?: () => void. The props passed in dialog-container.tsx lines 54-55 match the component's type signature exactly, with no TypeScript errors or runtime issues.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1)

5-5: Original review comment is incorrect.

The import from "./confirmation" is valid. A confirmation.tsx file exists in the same directory and exports the Confirm component. No changes needed.

Likely an incorrect or invalid review comment.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx (1)

33-35: The unit conversion is already handled correctly. At the data source (getBillingInfo.ts line 107), Stripe's cancel_at value in seconds is multiplied by 1000 to convert to milliseconds, so props.cancelAt is already in milliseconds—the same unit as Date.now(). The subtraction props.cancelAt - Date.now() is therefore correct, and new Date(props.cancelAt) receives the proper millisecond value.

No code changes are needed for the unit handling concern raised in the review comment.

Likely an incorrect or invalid review comment.

apps/dashboard/lib/trpc/routers/index.ts (1)

119-119: getBillingInfo properly enforces auth and caching; concerns verified as resolved.

The procedure correctly:

  • Enforces .use(requireWorkspace) middleware, matching all other Stripe routes
  • Is defined as .query(), making it idempotent and safe for client-side caching
  • Has proper cache invalidation in onSuccess callbacks across all plan-change mutations (createSubscription, updateSubscription, cancelSubscription, uncancelSubscription)
apps/dashboard/app/success/page.tsx (1)

111-118: No changes needed - coercion already implemented in Zod schema

The environment variable STRIPE_PRODUCT_IDS_PRO is a comma-separated string in the environment, but the Zod schema at apps/dashboard/lib/env.ts:75 already transforms it to string[] via .split(","). The function stripeEnv() returns the parsed and transformed object, so by the time e.STRIPE_PRODUCT_IDS_PRO is used at line 114 of apps/dashboard/app/success/page.tsx, it's already an array. The code is correct as-is.

Likely an incorrect or invalid review comment.

apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (1)

78-99: I need to verify the second concern about the status: "canceled" filter for subscriptions.list().

No issues found—both concerns verified.

The code is correct:

  • e.STRIPE_PRODUCT_IDS_PRO is properly typed as string[] (Stripe expects ids as string[]), confirmed by the env schema transforming the comma-separated string via .split(",").
  • status: "canceled" is a valid filter for subscriptions.list().

ogzhanolguncu

This comment was marked as resolved.

Copy link
Contributor

@ogzhanolguncu ogzhanolguncu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are a couple of places where we can improve the code. Btw, new dialog works and looks great.

@perkinsjr perkinsjr marked this pull request as draft October 23, 2025 18:40
@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2025

Thank you for following the naming conventions for pull request titles! 🙏

…-card-and' of https://github.com/unkeyed/unkey into eng-2126-a-user-who-wants-to-pay-unkey-can-add-a-credit-card-and
…-a-user-who-wants-to-pay-unkey-can-add-a-credit-card-and
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts (1)

28-32: CRITICAL: Workspace-scoped customer access control is missing.

The handler accepts arbitrary input.customerId without validating it against the workspace. Currently, ctx is not destructured and no workspace validation exists. While requireWorkspace middleware ensures a workspace context is present, it does not verify that the requested customer belongs to the current workspace. This allows any authenticated user to access any Stripe customer's data.

Add workspace validation by destructuring ctx and comparing input.customerId against ctx.workspace.stripeCustomerId before attempting to retrieve the customer.

apps/dashboard/lib/trpc/routers/stripe/getSetupIntent.ts (1)

25-51: CRITICAL: Missing tenant authorization – setup intent must belong to workspace.

The endpoint retrieves a setup intent by ID without verifying it belongs to the current workspace's Stripe customer. An attacker can supply any setupIntentId and leak client_secret and payment_method from other tenants.

Apply this diff to enforce tenant binding:

- .query(async ({ input }) => {
+ .query(async ({ input, ctx }) => {
     const stripe = getStripeClient();

     try {
       const setupIntent = await stripe.setupIntents.retrieve(input.setupIntentId);
+
+      // Authorization: ensure setup intent belongs to this workspace's customer
+      const siCustomer =
+        typeof setupIntent.customer === "string"
+          ? setupIntent.customer
+          : setupIntent.customer?.id;
+
+      if (ctx.workspace.stripeCustomerId && siCustomer !== ctx.workspace.stripeCustomerId) {
+        throw new TRPCError({
+          code: "FORBIDDEN",
+          message: "Setup intent does not belong to this workspace",
+        });
+      }

Also, safely extract the payment method ID without .toString():

       return {
         id: setupIntent.id,
         client_secret: setupIntent.client_secret,
-        payment_method: paymentMethodId,
+        payment_method: (() => {
+          const pm = setupIntent.payment_method;
+          return typeof pm === "string" ? pm : pm?.id ?? null;
+        })(),
         status: setupIntent.status,
         usage: setupIntent.usage,
       };
apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (2)

47-68: Performance issue: Remove await inside Promise.all array.

Lines 57 and 61 use await inside the Promise.all array, which serializes the requests instead of running them concurrently. This defeats the purpose of Promise.all.

Apply this diff:

     const [products, subscription, hasPreviousSubscriptions] = await Promise.all([
       stripe.products
         .list({
           active: true,
           ids: e.STRIPE_PRODUCT_IDS_PRO,
           limit: 100,
           expand: ["data.default_price"],
         })
         .then((res) => res.data.map(mapProduct).sort((a, b) => a.dollar - b.dollar)),
       ctx.workspace.stripeSubscriptionId
-        ? await stripe.subscriptions.retrieve(ctx.workspace.stripeSubscriptionId)
-        : undefined,
+        ? stripe.subscriptions.retrieve(ctx.workspace.stripeSubscriptionId)
+        : Promise.resolve(undefined),

       ctx.workspace.stripeCustomerId
-        ? await stripe.subscriptions
+        ? stripe.subscriptions
             .list({
               customer: ctx.workspace.stripeCustomerId,
               status: "canceled",
             })
             .then((res) => res.data.length > 0)
-        : false,
+        : Promise.resolve(false),
     ]);

70-81: Fix unsafe .toString() on union type for currentProductId.

Line 80 uses .toString() on plan.product, which could be an expanded object, resulting in "[object Object]". Derive from price.product instead and handle the union type safely.

Apply this diff:

       currentProductId: (() => {
         const item = subscription?.items.data.at(0);
-        const prod = item?.plan.product;
+        const prod = item?.price?.product ?? item?.plan.product;
         return typeof prod === "string" ? prod : prod?.id ?? undefined;
       })(),
🧹 Nitpick comments (4)
apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (1)

19-26: Remove redundant Stripe configuration check.

The stripeEnv() check at lines 20-26 is redundant because getStripeClient() (line 19) already validates Stripe configuration and throws TRPCError with INTERNAL_SERVER_ERROR if not configured.

Apply this diff to remove the redundant check:

   .mutation(async ({ ctx, input }) => {
     const stripe = getStripeClient();
-    const e = stripeEnv();
-    if (!e) {
-      throw new TRPCError({
-        code: "INTERNAL_SERVER_ERROR",
-        message: "Stripe is not set up",
-      });
-    }

     const product = await stripe.products.retrieve(input.productId);
apps/dashboard/app/success/client.tsx (1)

37-49: Consider handling modal dismissal for complete navigation coverage.

The current effect only handles the initial redirect. If a user dismisses the modal via ESC or backdrop click without triggering the skip handler, they remain on the /success route with the modal hidden. While the modal's handleSkip navigates (line 116 in plan-selection-modal.tsx), direct dismissal may not.

Add an effect to ensure navigation when modal closes:

   }, [router, workSpaceSlug, showPlanSelection, products]);
+
+  useEffect(() => {
+    if (!showModal && showPlanSelection && workSpaceSlug) {
+      // Navigate to billing when modal is closed
+      router.push(`/${workSpaceSlug}/settings/billing`);
+    }
+  }, [showModal, showPlanSelection, workSpaceSlug, router]);
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (2)

90-90: Consider adding a comment explaining the quota fallback.

The fallback to MAX_QUOTA when currentProduct is undefined correctly provides the free tier quota (150k requests). While the logic is sound, a brief inline comment would help future maintainers understand why the fallback is used.

-        <Usage quota={currentProduct?.quotas?.requestsPerMonth ?? MAX_QUOTA} />
+        {/* Free tier users (currentProduct undefined) fall back to MAX_QUOTA (150k) */}
+        <Usage quota={currentProduct?.quotas?.requestsPerMonth ?? MAX_QUOTA} />

92-150: Consider consolidating the two stripeCustomerId conditionals.

The code checks workspace.stripeCustomerId at both lines 92-107 and 129-150, which creates two separate conditional blocks for the same condition. Combining them would improve readability and reduce redundancy.

         <CancelAlert cancelAt={subscription?.cancelAt} />
         {isFreeTier ? <FreeTierAlert /> : null}
         <Usage quota={currentProduct?.quotas?.requestsPerMonth ?? MAX_QUOTA} />
 
         {workspace.stripeCustomerId ? (
           <>
             <CurrentPlanCard
               currentProduct={currentProduct}
               onChangePlan={() => setShowPlanModal(true)}
             />
 
             <PlanSelectionModal
               isOpen={showPlanModal}
               onOpenChange={setShowPlanModal}
               products={products}
               currentProductId={currentProductId}
               workspaceSlug={workspace.slug}
               isChangingPlan={Boolean(subscription)}
             />
+
+            <SettingCard
+              title="Billing Portal"
+              border="both"
+              description="Manage Payment methods and see your invoices."
+              className="w-full"
+              contentWidth="w-full lg:w-[320px]"
+            >
+              <div className="w-full flex h-full items-center justify-end gap-4">
+                <Button
+                  variant="outline"
+                  size="lg"
+                  aria-label="Open billing portal"
+                  onClick={() => {
+                    router.push(`/${workspace.slug}/settings/billing/stripe/portal`);
+                  }}
+                >
+                  Open Portal
+                </Button>
+              </div>
+            </SettingCard>
           </>
         ) : (
           <SettingCard
             title="Add payment method"
             border="both"
             description="Before upgrading, you need to add a payment method."
             className="sm:w-full text-wrap w-full"
             contentWidth="w-full"
           >
             <div className="flex justify-end w-full">
               <Button
                 variant="primary"
                 aria-label="Add payment method"
                 onClick={() => {
                   router.push(`/${workspace.slug}/settings/billing/stripe/checkout`);
                 }}
               >
                 Add payment method
               </Button>
             </div>
           </SettingCard>
         )}
-        {workspace.stripeCustomerId ? (
-          <SettingCard
-            title="Billing Portal"
-            border="both"
-            description="Manage Payment methods and see your invoices."
-            className="w-full"
-            contentWidth="w-full lg:w-[320px]"
-          >
-            <div className="w-full flex h-full items-center justify-end gap-4">
-              <Button
-                variant="outline"
-                size="lg"
-                aria-label="Open billing portal"
-                onClick={() => {
-                  router.push(`/${workspace.slug}/settings/billing/stripe/portal`);
-                }}
-              >
-                Open Portal
-              </Button>
-            </div>
-          </SettingCard>
-        ) : null}
 
         {allowCancel ? <CancelPlan /> : null}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67b2f59 and 0a37c2b.

📒 Files selected for processing (23)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx (4 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx (2 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/portal/page.tsx (2 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/general/page.tsx (1 hunks)
  • apps/dashboard/app/api/webhooks/stripe/route.ts (2 hunks)
  • apps/dashboard/app/success/client.tsx (1 hunks)
  • apps/dashboard/app/success/page.tsx (1 hunks)
  • apps/dashboard/components/dashboard/page-loading.tsx (1 hunks)
  • apps/dashboard/lib/stripe.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts (2 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (3 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/getCheckoutSession.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/getProducts.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/getSetupIntent.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/uncancelSubscription.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/updateCustomer.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts (2 hunks)
  • apps/dashboard/lib/trpc/routers/stripe/updateWorkspace.ts (1 hunks)
  • apps/dashboard/lib/trpc/routers/utils/stripe.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • apps/dashboard/lib/trpc/routers/stripe/updateWorkspace.ts
  • apps/dashboard/lib/trpc/routers/stripe/updateCustomer.ts
  • apps/dashboard/lib/trpc/routers/utils/stripe.ts
  • apps/dashboard/lib/trpc/routers/stripe/getCheckoutSession.ts
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/general/page.tsx
  • apps/dashboard/lib/trpc/routers/stripe/getProducts.ts
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.
📚 Learning: 2025-10-30T13:22:15.569Z
Learnt from: MichaelUnkey
Repo: unkeyed/unkey PR: 4128
File: apps/dashboard/lib/trpc/routers/stripe/updateCustomer.ts:30-33
Timestamp: 2025-10-30T13:22:15.569Z
Learning: The codebase uses Stripe API version "2023-10-16" across Stripe client initializations and cannot upgrade to newer versions at this time.

Applied to files:

  • apps/dashboard/lib/stripe.ts
  • apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts
  • apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts
  • apps/dashboard/app/api/webhooks/stripe/route.ts
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/portal/page.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx
📚 Learning: 2025-06-02T11:08:56.397Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3292
File: apps/dashboard/lib/vault.ts:80-97
Timestamp: 2025-06-02T11:08:56.397Z
Learning: The vault.ts file in apps/dashboard/lib/vault.ts is a duplicate of the vault package from the `api` directory and should be kept consistent with that original implementation.

Applied to files:

  • apps/dashboard/lib/stripe.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts
  • apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: When querying or updating namespaces in the Unkey dashboard, always scope the operations to the current workspace using `eq(table.workspaceId, ctx.workspace.id)` to prevent cross-workspace access.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts
📚 Learning: 2025-09-15T20:45:05.696Z
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 3952
File: go/apps/ctrl/services/routing/service.go:69-91
Timestamp: 2025-09-15T20:45:05.696Z
Learning: In Unkey's routing service, gateway lookups should be workspace-scoped using FindGatewayByHostnameAndWorkspace instead of hostname-only queries to prevent cross-tenant access issues.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts
📚 Learning: 2025-01-07T19:55:33.055Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2792
File: apps/dashboard/app/(app)/settings/user/update-user-email.tsx:76-78
Timestamp: 2025-01-07T19:55:33.055Z
Learning: In the Unkey codebase, the Empty component can be used as a container for loading states, as demonstrated in the UpdateUserEmail component where it wraps the Loading component.

Applied to files:

  • apps/dashboard/components/dashboard/page-loading.tsx
📚 Learning: 2025-06-24T13:29:10.129Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3401
File: apps/dashboard/app/(app)/logs/filters.query-params.ts:10-0
Timestamp: 2025-06-24T13:29:10.129Z
Learning: The `buildQueryParams` function in `apps/dashboard/app/(app)/logs/filters.query-params.ts` calls `useFilters()` hook inside it, but this is valid because the function is only called from within other React hooks, maintaining the Rules of Hooks compliance.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx
📚 Learning: 2025-08-25T13:46:08.303Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx:1-1
Timestamp: 2025-08-25T13:46:08.303Z
Learning: The NamespaceListDateTime component in apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-datetime/index.tsx is intentionally designed to use the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than the namespace list hook, as clarified by ogzhanolguncu. This coupling is by design, not an architectural issue.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx
📚 Learning: 2025-08-25T13:46:34.441Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3834
File: apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx:4-4
Timestamp: 2025-08-25T13:46:34.441Z
Learning: The namespace list refresh component (apps/dashboard/app/(app)/ratelimits/_components/controls/components/namespace-list-refresh.tsx) intentionally uses the overview hook (useFilters from @/app/(app)/ratelimits/[namespaceId]/_overview/hooks/use-filters) rather than a namespace-specific hook. This cross-coupling between namespace list components and overview hooks is an architectural design decision.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx
📚 Learning: 2025-04-22T11:48:39.670Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/ratelimit-setup.tsx:36-47
Timestamp: 2025-04-22T11:48:39.670Z
Learning: The Unkey dashboard's form validation for numeric values like rate limits is handled through the Zod schema validation (with `.positive()` validators and additional checks in `superRefine`), rather than HTML input attributes like `min`.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts
📚 Learning: 2024-12-05T13:27:55.555Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2707
File: apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts:63-63
Timestamp: 2024-12-05T13:27:55.555Z
Learning: In `apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts`, when determining the maximum number of rate limit overrides (`max`), the intentional use of `const max = hasWorkspaceAccess("ratelimitOverrides", namespace.workspace) || 5;` allows `max` to fall back to `5` when `hasWorkspaceAccess` returns `0` or `false`. This fallback behavior is expected and intended in the codebase.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts
📚 Learning: 2024-10-23T12:05:31.121Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The `cloudflareRatelimiter` type definition in `apps/api/src/pkg/env.ts` should not have its interface changed; it should keep the `limit` method returning `Promise<{ success: boolean }>` without additional error properties.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts
📚 Learning: 2024-10-08T15:33:04.290Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.

Applied to files:

  • apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts
  • apps/dashboard/lib/trpc/routers/stripe/getSetupIntent.ts
  • apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts
  • apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts
  • apps/dashboard/app/success/page.tsx
  • apps/dashboard/app/api/webhooks/stripe/route.ts
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/portal/page.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx
  • apps/dashboard/app/success/client.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.

Applied to files:

  • apps/dashboard/app/success/page.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx
  • apps/dashboard/app/success/client.tsx
📚 Learning: 2025-07-28T20:38:53.244Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx:322-341
Timestamp: 2025-07-28T20:38:53.244Z
Learning: In apps/dashboard/app/(app)/projects/[projectId]/diff/[...compare]/components/client.tsx, mcstepp prefers to keep hardcoded endpoint logic in the getDiffType function during POC phases for demonstrating diff functionality, rather than implementing a generic diff algorithm. This follows the pattern of keeping simplified implementations for demonstration purposes.

Applied to files:

  • apps/dashboard/app/success/page.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx
📚 Learning: 2025-04-30T15:25:33.917Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3210
File: apps/dashboard/app/new/page.tsx:3-3
Timestamp: 2025-04-30T15:25:33.917Z
Learning: There are two different `getAuth` functions in the Unkey codebase with different purposes:
1. `@/lib/auth/get-auth` - Base function without redirects, used in special cases on the dashboard where redirect control is needed (like `/new` page) and within tRPC context
2. `@/lib/auth` - Helper function with redirects, used in most dashboard cases (approximately 98%)

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/portal/page.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx
📚 Learning: 2024-12-03T14:07:45.173Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 2143
File: apps/dashboard/components/ui/group-button.tsx:21-31
Timestamp: 2024-12-03T14:07:45.173Z
Learning: In the `ButtonGroup` component (`apps/dashboard/components/ui/group-button.tsx`), avoid suggesting the use of `role="group"` in ARIA attributes.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx
📚 Learning: 2024-10-23T16:25:33.113Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/terms-stepper-mobile.tsx:16-20
Timestamp: 2024-10-23T16:25:33.113Z
Learning: In the `apps/www/components/glossary/terms-stepper-mobile.tsx` file, avoid suggesting to extract the term navigation logic into a custom hook, as the user prefers to keep the component straightforward.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx
  • apps/dashboard/app/success/client.tsx
📚 Learning: 2025-09-23T17:39:59.820Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:88-97
Timestamp: 2025-09-23T17:39:59.820Z
Learning: The useWorkspaceNavigation hook in the Unkey dashboard guarantees that a workspace exists. If no workspace is found, the hook redirects the user to create a new workspace. Users cannot be logged in without a workspace, and new users must create one to continue. Therefore, workspace will never be null when using this hook.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx
  • apps/dashboard/app/success/client.tsx
📚 Learning: 2025-09-22T18:44:56.279Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 4009
File: apps/dashboard/app/(app)/[workspace]/apis/[apiId]/_overview/components/table/components/log-details/index.tsx:4-5
Timestamp: 2025-09-22T18:44:56.279Z
Learning: In the Unkey dashboard, the workspace hook (useWorkspace) provides security validation by checking database access and user authorization to the workspace, with 10-minute caching for performance. Using URL params (useParams) for workspace slug would bypass this security validation and allow unauthorized access attempts. Always use the workspace hook for workspace-scoped navigation and handle loading states properly rather than switching to URL parameters.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx
  • apps/dashboard/app/success/client.tsx
📚 Learning: 2024-10-25T23:53:41.716Z
Learnt from: Srayash
Repo: unkeyed/unkey PR: 2568
File: apps/dashboard/app/auth/sign-up/oauth-signup.tsx:25-25
Timestamp: 2024-10-25T23:53:41.716Z
Learning: In the React component `OAuthSignUp` (`apps/dashboard/app/auth/sign-up/oauth-signup.tsx`), adding a `useEffect` cleanup function to reset the `isLoading` state causes a "something went wrong" popup to appear before redirecting when a user clicks on signup.

Applied to files:

  • apps/dashboard/app/success/client.tsx
📚 Learning: 2025-05-15T16:26:08.666Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3242
File: apps/dashboard/app/(app)/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:50-65
Timestamp: 2025-05-15T16:26:08.666Z
Learning: In the Unkey dashboard, Next.js router (router.push) should be used for client-side navigation instead of window.location.href to preserve client state and enable smoother transitions between pages.

Applied to files:

  • apps/dashboard/app/success/client.tsx
🧬 Code graph analysis (16)
apps/dashboard/lib/stripe.ts (1)
apps/dashboard/lib/env.ts (1)
  • stripeEnv (80-80)
apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts (1)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts (3)
apps/dashboard/lib/trpc/trpc.ts (3)
  • t (8-8)
  • requireWorkspace (23-36)
  • withRatelimit (122-138)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/lib/trpc/routers/utils/stripe.ts (1)
  • handleStripeError (49-77)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx (2)
apps/dashboard/components/dashboard/page-loading.tsx (1)
  • PageLoading (26-26)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (1)
  • Client (20-156)
apps/dashboard/lib/trpc/routers/stripe/getBillingInfo.ts (4)
apps/dashboard/lib/trpc/trpc.ts (3)
  • t (8-8)
  • requireWorkspace (23-36)
  • withRatelimit (122-138)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/lib/env.ts (1)
  • stripeEnv (80-80)
apps/dashboard/lib/trpc/routers/utils/stripe.ts (1)
  • mapProduct (4-47)
apps/dashboard/lib/trpc/routers/stripe/getSetupIntent.ts (3)
apps/dashboard/lib/trpc/trpc.ts (3)
  • t (8-8)
  • requireWorkspace (23-36)
  • withRatelimit (122-138)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/lib/trpc/routers/utils/stripe.ts (1)
  • handleStripeError (49-77)
apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts (1)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (1)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/app/success/page.tsx (1)
apps/dashboard/app/success/client.tsx (1)
  • SuccessClient (33-63)
apps/dashboard/app/api/webhooks/stripe/route.ts (1)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/portal/page.tsx (1)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx (1)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (8)
apps/dashboard/lib/trpc/routers/index.ts (1)
  • router (137-359)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/shell.tsx (1)
  • Shell (4-12)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/subscription-status.tsx (1)
  • SubscriptionStatus (6-35)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx (1)
  • CancelAlert (7-60)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/free-tier-alert.tsx (1)
  • FreeTierAlert (6-25)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/usage.tsx (1)
  • Usage (6-106)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/current-plan-card.tsx (1)
  • CurrentPlanCard (15-41)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1)
  • CancelPlan (7-46)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (3)
apps/dashboard/lib/trpc/routers/index.ts (1)
  • router (137-359)
apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (1)
  • createSubscription (10-122)
apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts (1)
  • updateSubscription (8-144)
apps/dashboard/app/success/client.tsx (1)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (1)
  • PlanSelectionModal (27-266)
apps/dashboard/lib/trpc/routers/stripe/uncancelSubscription.ts (2)
apps/dashboard/lib/trpc/trpc.ts (3)
  • t (8-8)
  • requireUser (10-21)
  • requireWorkspace (23-36)
apps/dashboard/lib/stripe.ts (1)
  • getStripeClient (12-31)
⏰ 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). (4)
  • GitHub Check: Test Dashboard / Test Dashboard
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
  • GitHub Check: Test Packages / Test
🔇 Additional comments (17)
apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts (1)

2-2: LGTM! Clean refactor to centralized Stripe client.

The migration from direct Stripe instantiation to getStripeClient() is consistent with the broader refactor across Stripe endpoints. Error handling is now centralized in the client provider.

Also applies to: 24-24

apps/dashboard/app/api/webhooks/stripe/route.ts (1)

5-6: LGTM! Webhook route successfully migrated to centralized client.

The refactor maintains the necessary stripeEnv() check for webhook secret validation while using the shared Stripe client instance. Webhook event handling logic remains unchanged.

Also applies to: 24-24

apps/dashboard/lib/stripe.ts (1)

1-31: LGTM! Well-structured Stripe client singleton.

The lazy initialization pattern is appropriate for a Stripe client. The API version "2023-10-16" is correctly maintained per existing constraints. Error handling via TRPCError ensures consistent error responses across all Stripe operations.

apps/dashboard/lib/trpc/routers/stripe/uncancelSubscription.ts (1)

1-1: LGTM! Consistent refactor to centralized client.

The migration simplifies the code by removing redundant Stripe initialization logic while preserving all precondition checks and business logic.

Also applies to: 8-8

apps/dashboard/lib/trpc/routers/stripe/updateSubscription.ts (1)

3-3: LGTM! Subscription update flow correctly migrated.

The centralized client usage maintains all existing validation, error handling, and cache invalidation logic.

Also applies to: 18-18

apps/dashboard/lib/trpc/routers/stripe/createSubscription.ts (1)

120-121: LGTM! Cache invalidation strategy is comprehensive.

Adding clearWorkspaceCache alongside invalidateWorkspaceCache ensures both the database-backed and tRPC-layer workspace caches are refreshed after subscription creation.

apps/dashboard/lib/trpc/routers/stripe/getCustomer.ts (2)

41-51: LGTM! Robust handling of payment method shapes.

The code correctly handles both string and expanded object forms of default_payment_method, preventing "[object Object]" serialization issues.


63-79: LGTM! Comprehensive error handling.

The error handling correctly preserves TRPCError instances, maps Stripe-specific errors via the shared handleStripeError utility, and provides a sensible fallback for unknown errors.

apps/dashboard/components/dashboard/page-loading.tsx (1)

1-26: LGTM! Clean and reusable loading component.

The component provides sensible defaults and flexible customization options. The centered layout with minimum height ensures a good user experience during loading states.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/checkout/page.tsx (1)

20-32: LGTM – Centralized Stripe client improves maintainability.

The migration to getStripeClient() with proper error handling is clean and aligns with the broader refactoring pattern across the PR.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/stripe/portal/page.tsx (1)

25-37: LGTM – Consistent refactoring pattern.

Same improvement as the checkout page—centralized client with appropriate error handling.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/page.tsx (1)

18-34: LGTM – Rules of Hooks violation fixed correctly.

The query hook is now called unconditionally with the enabled option to gate execution, which properly resolves the conditional hook call that past reviews flagged.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/plan-selection-modal.tsx (2)

91-109: LGTM – Mutation error handling is appropriate.

The mutations are not wrapped in try-catch, but this is acceptable since both have onError handlers (lines 72-75, 85-88) that manage errors and loading state. The pattern is clean and follows TRPC best practices.


55-62: Revalidation pattern confirmed—no hard refresh needed.

The revalidateData callback properly invalidates all active queries in the billing UI:

  • getBillingInfo is directly subscribed by the Client component and triggers parent re-render
  • queryUsage is directly subscribed by the Usage component and updates in-place
  • CurrentPlanCard and SubscriptionStatus receive updated data via parent props when Client re-renders
  • CancelAlert receives cancelAt via props and updates accordingly

All consuming components are connected to the invalidation chain, either through direct query subscriptions or parent re-renders. The pattern should eliminate the "hard refresh" requirement as intended.

apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/client.tsx (3)

37-49: LGTM: Loading skeleton provides good UX.

The skeleton UI with bg-grayA-3 (Radix alpha color) appropriately mirrors the structure of the actual billing cards and provides visual feedback during data fetch.


66-76: LGTM: Derived state logic is correct.

The state derivation correctly handles subscription statuses:

  • allowUpdate permits changes only for active/trialing subscriptions
  • isFreeTier properly treats trialing subscriptions as non-free (past issue resolved)
  • allowCancel prevents redundant cancel actions
  • currentProduct is only resolved for updateable subscriptions

117-125: LGTM: Button navigation properly refactored.

The buttons now correctly use onClick with router.push() instead of nesting <Link> elements, which resolves the previous accessibility concern. The addition of aria-label attributes further improves accessibility.

Also applies to: 138-147

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
apps/dashboard/app/success/page.tsx (5)

24-46: Suspense boundary doesn't align with useEffect data fetching.

The component is wrapped in Suspense (line 284) but uses useEffect for async operations instead of throwing promises or using React.use(). Suspense only catches during initial render, not during effect-driven fetches. This means users will see the loading state managed by local state rather than the Suspense fallback during data fetching.

Consider using TRPC's useQuery hooks directly in the component body, which properly integrate with Suspense, or accept that the Suspense boundary primarily handles searchParams hydration.


251-256: Mutation functions in dependencies could trigger re-runs.

Including updateCustomerMutation.mutateAsync and updateWorkspaceStripeCustomerMutation.mutateAsync in the dependency array could cause the effect to re-execute if TRPC creates new mutation instances (though typically these are stable). If the effect does re-run, the entire Stripe session processing repeats, potentially causing duplicate API calls.

Wrap the mutation functions with useCallback or restructure to define processStripeSession outside the effect and pass only the needed parameters:

+  const processStripeSession = useCallback(async () => {
+    // Move the entire async function here, using updateCustomerMutation.mutateAsync directly
+    // instead of passing it as a parameter
+  }, [sessionId, trpcUtils, updateCustomerMutation.mutateAsync, updateWorkspaceStripeCustomerMutation.mutateAsync]);
+
   useEffect(() => {
     let isMounted = true;
     if (!sessionId) {
       setProcessedData({});
       setLoading(false);
       return;
     }
-    const processStripeSession = async (...) => { ... };
-    processStripeSession(
-      updateCustomerMutation.mutateAsync,
-      updateWorkspaceStripeCustomerMutation.mutateAsync,
-    );
+    processStripeSession();
     return () => {
       isMounted = false;
     };
-  }, [sessionId, trpcUtils, updateCustomerMutation.mutateAsync, updateWorkspaceStripeCustomerMutation.mutateAsync]);
+  }, [processStripeSession]);

181-226: Add null-guard for billingInfo structure.

Line 189 accesses billingInfo.hasPreviousSubscriptions without checking if billingInfo is null or if the property exists. While the outer try-catch provides a fallback, explicitly guarding against null makes the intent clearer and prevents potential TypeScript issues:

   const billingInfo = await trpcUtils.stripe.getBillingInfo.fetch();
   
   if (!isMounted) {
     return;
   }
   
-  const isFirstTimeUser = !billingInfo.hasPreviousSubscriptions;
+  const isFirstTimeUser = !billingInfo?.hasPreviousSubscriptions;

Additionally, consider validating that products is non-empty before setting showPlanSelection: true (line 202), as an empty array would show a broken modal.


104-120: Parallelize independent customer and setup intent fetches.

The getCustomer and getSetupIntent calls are independent and currently fetch sequentially. Using Promise.all would reduce total processing time:

-  // Get customer details
-  const customer = await trpcUtils.stripe.getCustomer.fetch({
-    customerId: sessionResponse.customer,
-  });
-
-  if (!isMounted) {
-    return;
-  }
-
-  // Get setup intent details
-  const setupIntent = await trpcUtils.stripe.getSetupIntent.fetch({
-    setupIntentId: sessionResponse.setup_intent,
-  });
+  // Get customer and setup intent details in parallel
+  const [customer, setupIntent] = await Promise.all([
+    trpcUtils.stripe.getCustomer.fetch({
+      customerId: sessionResponse.customer,
+    }),
+    trpcUtils.stripe.getSetupIntent.fetch({
+      setupIntentId: sessionResponse.setup_intent,
+    }),
+  ]);
   
   if (!isMounted) {
     return;
   }

142-155: Review error message for sensitive data before showing to user.

Line 152 displays errorMessage directly to users. If the Stripe API error contains PII (customer IDs, email, payment details), it will leak. Consider sanitizing or using a generic message:

-  setError(`Failed to set up payment method: ${errorMessage}`);
+  setError("Failed to set up payment method. Please contact support.");

Then rely on the console.error (line 144) for debugging details.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a37c2b and fa2c115.

📒 Files selected for processing (3)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx (1 hunks)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1 hunks)
  • apps/dashboard/app/success/page.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-alert.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.
📚 Learning: 2024-10-08T15:33:04.290Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx
  • apps/dashboard/app/success/page.tsx
📚 Learning: 2024-10-25T23:53:41.716Z
Learnt from: Srayash
Repo: unkeyed/unkey PR: 2568
File: apps/dashboard/app/auth/sign-up/oauth-signup.tsx:25-25
Timestamp: 2024-10-25T23:53:41.716Z
Learning: In the React component `OAuthSignUp` (`apps/dashboard/app/auth/sign-up/oauth-signup.tsx`), adding a `useEffect` cleanup function to reset the `isLoading` state causes a "something went wrong" popup to appear before redirecting when a user clicks on signup.

Applied to files:

  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.

Applied to files:

  • apps/dashboard/app/success/page.tsx
🧬 Code graph analysis (2)
apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (3)
apps/dashboard/lib/trpc/routers/index.ts (1)
  • router (137-359)
apps/dashboard/lib/trpc/routers/stripe/cancelSubscription.ts (1)
  • cancelSubscription (5-42)
apps/dashboard/components/dashboard/confirm.tsx (1)
  • Confirm (16-55)
apps/dashboard/app/success/page.tsx (1)
apps/dashboard/app/success/client.tsx (1)
  • SuccessClient (33-63)
⏰ 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). (6)
  • GitHub Check: Build / Build
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Dashboard / Test Dashboard
  • GitHub Check: Test Packages / Test
  • GitHub Check: autofix
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/dashboard/app/success/page.tsx (3)

1-22: LGTM: Clean type definition and imports.

The ProcessedData type clearly models the three outcomes (no data, workspace only, or full plan selection flow).


273-288: LGTM: Clean separation between data processing and UI navigation.

The render logic correctly delegates to SuccessClient after processing completes, with appropriate loading and error states.


167-169: The race condition concern is unfounded—cache invalidations properly complete before navigation.

The code structure already prevents the race condition. Cache invalidations (lines 167-169) are awaited, blocking further execution until they complete. Only after invalidations finish does the code fetch billing info and products, then call setProcessedData. The SuccessClient navigation via router.push() occurs after setProcessedData, ensuring cache is already invalidated when the billing page mounts and queries for fresh data.

No action required.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/dashboard/app/success/page.tsx (1)

229-243: Verify stability of mutateAsync in dependency array.

Including updateCustomerMutation.mutateAsync and updateWorkspaceStripeCustomerMutation.mutateAsync in the dependency array (lines 241-242) may cause the effect to re-run if these functions aren't stable references. If they change on each render, the same sessionId could be processed multiple times, leading to redundant API calls.

Option 1 (preferred): Call mutations directly inside processStripeSession instead of passing them as parameters:

-    const processStripeSession = async (
-      updateCustomerFn: typeof updateCustomerMutation.mutateAsync,
-      updateWorkspaceFn: typeof updateWorkspaceStripeCustomerMutation.mutateAsync,
-    ) => {
+    const processStripeSession = async () => {
       try {
         // ... existing code ...
         
-        await updateCustomerFn({
+        await updateCustomerMutation.mutateAsync({
           customerId: customer.id,
           paymentMethod: setupIntent.payment_method,
         });
         
         // ... existing code ...
         
-        await updateWorkspaceFn({
+        await updateWorkspaceStripeCustomerMutation.mutateAsync({
           stripeCustomerId: customer.id,
         });
         
         // ... rest of code
       }
     };
     
-    processStripeSession(
-      updateCustomerMutation.mutateAsync,
-      updateWorkspaceStripeCustomerMutation.mutateAsync,
-    );
+    processStripeSession();

Then remove the mutation functions from the dependency array:

   }, [
     sessionId,
     trpcUtils,
-    updateCustomerMutation.mutateAsync,
-    updateWorkspaceStripeCustomerMutation.mutateAsync,
   ]);

Option 2: If mutateAsync is confirmed stable in your TRPC version, document this assumption with a comment.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa2c115 and 42c8118.

📒 Files selected for processing (3)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx (1 hunks)
  • apps/dashboard/app/success/page.tsx (1 hunks)
  • tools/artillery/create-keys.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/dashboard/app/(app)/[workspaceSlug]/settings/billing/components/cancel-plan.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.
📚 Learning: 2024-10-08T15:33:04.290Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 2120
File: apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx:19-19
Timestamp: 2024-10-08T15:33:04.290Z
Learning: In the `StripeSuccess` function, TypeScript's type-checking of the `new_plan` parameter ensures that only "free", "pro", or undefined values are accepted, so additional runtime validation is not necessary.

Applied to files:

  • apps/dashboard/app/success/page.tsx
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.

Applied to files:

  • apps/dashboard/app/success/page.tsx
🧬 Code graph analysis (1)
apps/dashboard/app/success/page.tsx (2)
apps/dashboard/components/dashboard/page-loading.tsx (1)
  • PageLoading (26-26)
apps/dashboard/app/success/client.tsx (1)
  • SuccessClient (33-63)
⏰ 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). (4)
  • GitHub Check: Test Dashboard / Test Dashboard
  • GitHub Check: Test Packages / Test
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
🔇 Additional comments (5)
tools/artillery/create-keys.ts (1)

43-48: Confirmed safe to remove; clarify if changes are intentional to this file.

Verification confirms the CreateKeyError interface is not referenced anywhere in the codebase—it appears only in the commented-out section in this file. Removing dead code is appropriate. However, these changes to an artillery load-testing utility appear unrelated to the PR's stated focus on billing flow and credit card/subscription features. Confirm whether these modifications were intentional or should be removed from this PR.

apps/dashboard/app/success/page.tsx (4)

1-22: LGTM! Clean setup and type definition.

The client directive, imports, and internal ProcessedData type are well-structured. The type properly models the UI state needed to drive the success flow and plan selection.


38-227: Excellent fixes addressing previous critical concerns.

The processing logic properly addresses the two main issues from previous reviews:

  1. Cache invalidation (lines 167-169): Now invalidates workspace, stripe, and billing caches after updating the workspace, which should resolve the "hard refresh" issue reported in PR objectives.

  2. Eliminated redundant fetch (lines 193-202): Uses billingInfo.products directly instead of calling getProducts again, avoiding duplicate network requests.

The isMounted guard pattern throughout prevents memory leaks and race conditions. Error handling is comprehensive with proper fallbacks.


245-267: LGTM! Proper state handling.

The conditional rendering correctly handles all three states (loading, error, success) with appropriate UI components and messaging. The "Processing payment..." message accurately describes the operation being performed.


269-275: LGTM! Proper Suspense boundary.

The Suspense wrapper provides a clean loading boundary for the async SuccessContent component with an appropriate fallback.

Copy link
Collaborator

@chronark chronark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a lockfile change without dependency changes anywhere else, is that intentional?

Copy link
Contributor

@ogzhanolguncu ogzhanolguncu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@graphite-app
Copy link

graphite-app bot commented Nov 12, 2025

Cartoon gif. An illustration of Borat giving us two thumbs up. His eyebrows rise and fall, and his hands move slightly closer and further from us. Text, 'Very nice.' (Added via Giphy)

@graphite-app
Copy link

graphite-app bot commented Nov 12, 2025

Graphite Automations

"Post a GIF when PR approved" took an action on this PR • (11/12/25)

1 gif was posted to this PR based on Andreas Thomas's automation.

@perkinsjr perkinsjr merged commit 2acd819 into main Nov 12, 2025
20 checks passed
@perkinsjr perkinsjr deleted the eng-2126-a-user-who-wants-to-pay-unkey-can-add-a-credit-card-and branch November 12, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants