[Fix]: Payments App Sundry Fixes#1455
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds free ($0) subscription handling: models free prices as monthly recurring, enforces billing-interval and Stripe one-time minimums, returns empty JSON (no client_secret) for free purchases, updates dashboard UI and editor validation, adds shared Stripe limits, updates OpenAPI, and adds e2e tests. ChangesFree Subscription Feature
Sequence DiagramsequenceDiagram
participant Client
participant PurchaseSessionRoute
participant StripeAPI
participant PurchaseCodeStore
Client->>PurchaseSessionRoute: POST /payments/purchases/purchase-session (price)
PurchaseSessionRoute->>PurchaseSessionRoute: Validate price, interval, one-time min
alt Free price (USD === 0)
PurchaseSessionRoute->>PurchaseCodeStore: Revoke purchase verification code
PurchaseSessionRoute->>Client: Return {}
else Paid price
PurchaseSessionRoute->>StripeAPI: Create/update subscription/payment
StripeAPI-->>PurchaseSessionRoute: Respond (may include client_secret)
PurchaseSessionRoute->>PurchaseCodeStore: Revoke purchase verification code
PurchaseSessionRoute->>Client: Return { client_secret }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Stripe doesn't produce a PaymentIntent for $0 invoices, so the purchase-session route 500'd on "No client secret returned from Stripe for subscription" for every free product. Stripe itself happily activates $0 subs synchronously (status=active, invoice=paid) — the only thing broken was our post-call read of latest_invoice.confirmation_secret. For $0 prices we now skip the client-secret read on both the create and switch-within-product-line paths and return an empty body, signalling the client to skip mounting Stripe Elements. $0 one-time prices are rejected up front with a 400 so they never reach Stripe's amount_too_small. - response: client_secret is now optional; OpenAPI description tightened so we don't leak webhook names or DB internals on the public surface - new e2e tests for $0 recurring (create), $0 one-time (rejection), and paid -> free switch in the same product line
There was a problem hiding this comment.
Pull request overview
This PR improves the payments purchase flow and dashboard UX around “free” products by modeling free tiers as $0 recurring subscriptions (instead of $0 one-time purchases), and by documenting/handling cases where no Stripe client secret is produced.
Changes:
- Backend: make
client_secretoptional inpurchase-sessionresponses; return{}for $0 subscriptions and reject $0 one-time prices with a clear 400. - Dashboard: improve price display (always indicates recurring vs one-time), update “Make free” to create a $0 monthly recurring price, and improve “Make free” UX with tooltips and better free-tier visibility.
- Tests/docs: add e2e coverage for $0 subscription and $0 one-time rejection; update OpenAPI schemas to reflect optional
client_secret.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs-mintlify/openapi/server.json | Marks client_secret as optional and clarifies when it’s omitted. |
| docs-mintlify/openapi/client.json | Same OpenAPI clarification for client schema. |
| docs-mintlify/openapi/admin.json | Same OpenAPI clarification for admin schema. |
| apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts | Adds e2e coverage for $0 subscription flow and $0 one-time rejection. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.ts | Adjusts free-tier creation to be recurring; improves price display text. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsx | Improves free-tier UI and “Make free” tooltips; surfaces underlying free price row. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/included-item-dialog.tsx | Fixes dialog state reset/sync across opens; disables item selection tooltip while editing. |
| apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsx | Makes client_secret optional; adds $0 safeguards and a $0 subscription carve-out. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR adds robust support for $0 (free) recurring subscriptions and enforces Stripe's per-currency one-time minimum ($0.50 USD) at both the server and dashboard levels. It also fixes the included-item dialog retaining stale values when reopened.
Confidence Score: 5/5Safe to merge; all new paths are well-validated and changes are well-scoped to the payments feature. The backend validation is symmetric across both the conflict-update and new-subscription branches. Price amount validation fires before any Stripe call, and the $0 recurring fast-path correctly skips client_secret extraction. The shared stripe-limits module prevents frontend/backend drift. The two findings are both non-blocking documentation and UX gaps. utils.ts has a displaced JSDoc comment worth cleaning up. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant API as purchase-session API
participant Stripe
participant Webhook as invoice.paid webhook
Client->>API: POST /purchase-session
API->>API: Validate price (finite, non-negative)
alt $0 + no interval
API-->>Client: 400 Free products must have a billing interval
else "one-time 0 < amount < $0.50"
API-->>Client: 400 One-time prices must be at least $0.50
else $0 recurring subscription
API->>Stripe: "subscriptions.create/update (unit_amount=0)"
Stripe-->>API: "sub (status=active, no confirmation_secret)"
API->>API: revokeCode
API-->>Client: "200 {}"
Stripe->>Webhook: invoice.paid
else paid recurring subscription
API->>Stripe: subscriptions.create/update
Stripe-->>API: sub + confirmation_secret
API->>API: revokeCode
API-->>Client: "200 { client_secret }"
else "one-time payment >= $0.50"
API->>Stripe: paymentIntents.create
Stripe-->>API: client_secret
API->>API: revokeCode
API-->>Client: "200 { client_secret }"
end
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.ts:167-173
The JSDoc that was written for `isFreePrices` is now orphaned above `getPriceCheckoutError`. When the two functions were reordered, the docblock ended up documenting the wrong function. `isFreePrices` is left without any JSDoc while `getPriceCheckoutError` has two stacked comments — the first of which describes a completely different function.
```suggestion
* We accept both `'0'` and `'0.00'` for backward-compatibility with rows written
* before we standardized on `createFreePrice()` (which emits `'0.00'`). All three
* product pages (list, edit, create) call this so the "Free" indicator and the
* "Make free" / "Make paid" toggles stay in sync.
*/
export function isFreePrices(prices: PricesObject): boolean {
const entries = Object.values(prices);
if (entries.length !== 1) return false;
const [price] = entries;
return (price.USD === '0' || price.USD === '0.00')
&& !price.freeTrial
&& !price.serverOnly;
}
/**
* Returns a human-readable error string if this price would be rejected by
```
### Issue 2 of 2
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsx:437
**Free card hides checkout errors for legacy $0 one-time prices**
`isFreePrices` now returns `true` for both new-style `{ USD: '0.00', interval: [1,'month'] }` prices and legacy `{ USD: '0', serverOnly: false }` (no interval) prices, since `!price.interval` was removed from the check. When a legacy price matches, this branch renders the "Free" card — which never calls `getPriceCheckoutError` — so the admin sees no inline warning. The `ProductsWithInvalidPricesAlert` on the list page will still flag it, but on the product edit/create form there is no visual indication that the saved price will be rejected at checkout.
Consider passing `checkoutError` to the free-card branch the same way the price-list rows do, e.g. rendering a `WarningIcon` next to "Free" when `getPriceCheckoutError(freePrice)` is non-null.
Reviews (4): Last reviewed commit: "fix: add warning for users with invalid ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts (2)
1158-1166: 🏗️ Heavy liftDon't leave the Stripe-backed update branch review-only.
This note makes it explicit that the free-switch case through
stripe.subscriptions.update(...)is still untested. Since this PR changed both the create and update free-subscription branches, add a narrower mocked route-level test for the Stripe-backed conflict path so regressions do not depend on flaky webhook e2e coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts` around lines 1158 - 1166, The Stripe-backed update branch (stripe.subscriptions.update) is untested by the current e2e note; add a narrow mocked route-level test in purchase-session.test.ts that simulates the Stripe-backed conflict path exercised in route.tsx: mock stripe.subscriptions.update to return an active Stripe-backed subscription (or throw the same conflict response shape), call the endpoint route that triggers the update branch, and assert the same patch/response behavior as the DB-only cancel branch (status and payload expectations). Ensure the test isolates only the route-level logic (avoid flaky webhook flows), uses the same identifiers as the existing create-path test, and restores/clears the Stripe mock after the test.
1048-1098: ⚡ Quick winUse inline snapshots for these new happy-path responses.
These two tests split assertions across
statusandbody, which loses the file's establishedNiceResponsesnapshot style.Suggested change
- expect(response.status).toBe(200); - expect(response.body).toEqual({}); + expect(response).toMatchInlineSnapshot(` + NiceResponse { + "status": 200, + "body": {}, + "headers": Headers { <some fields may have been hidden> }, + } + `);- expect(switchRes.status).toBe(200); - expect(switchRes.body).toEqual({}); + expect(switchRes).toMatchInlineSnapshot(` + NiceResponse { + "status": 200, + "body": {}, + "headers": Headers { <some fields may have been hidden> }, + } + `);As per coding guidelines, "
**/*.test.{ts,tsx}: When writing tests, prefer.toMatchInlineSnapshotover other selectors in Vitest tests."Also applies to: 1154-1228
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts` around lines 1048 - 1098, The test "creates a $0 recurring subscription without requiring a payment intent" (and the similar test around lines 1154-1228) splits assertions into expect(response.status).toBe(200) and expect(response.body).toEqual({}), breaking the file's NiceResponse inline-snapshot style; change these to a single inline snapshot assertion using Vitest's toMatchInlineSnapshot on the full response object returned by niceBackendFetch (e.g., replace the two assertions with expect(response).toMatchInlineSnapshot(/* update snapshot here */)), and do the same for the corresponding test at 1154-1228 so both follow the established NiceResponse snapshot pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsx`:
- Around line 82-88: The current check only rejects exactly-zero prices via
isFreePrice; update the validation around selectedPrice (e.g., where isFreePrice
is computed and before calling paymentIntents.create) to also reject one-time
amounts less than $0.50 by converting Number(selectedPrice.USD) and, if
selectedPrice.interval is falsy and amount < 0.5, throw a StatusError(400,
"One-time prices must be at least $0.50") so sub-$0.50 one-time prices are
rejected with a stable 400 before contacting Stripe.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsx:
- Around line 154-156: The isFree branch assumes exactly one backing price but
only checks for zero; update the guard to assert the invariant that
Object.entries(prices) has length === 1 and throw (e.g., via throwErr) if not,
then destructure [freePriceId, freePrice] from the single entry; specifically
validate the invariant before using isFree and the prices object (referencing
isFree, prices, and throwErr) so multiple-price cases fail fast instead of
silently using the first entry.
In `@docs-mintlify/openapi/client.json`:
- Around line 6175-6177: The description change in the generated OpenAPI
artifact must not be edited directly; instead update the source schema metadata
that produces that property (the model/schema that defines the Stripe client
secret description in your Yup/OpenAPI generator), commit the change, and then
run the “Regenerate OpenAPI schemas” workflow to produce a new
docs-mintlify/openapi/*.json; ensure you modify the schema file or generator
function that defines the client secret property (the schema field for the
Stripe client secret) so the updated description is applied during regeneration.
---
Nitpick comments:
In `@apps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.ts`:
- Around line 1158-1166: The Stripe-backed update branch
(stripe.subscriptions.update) is untested by the current e2e note; add a narrow
mocked route-level test in purchase-session.test.ts that simulates the
Stripe-backed conflict path exercised in route.tsx: mock
stripe.subscriptions.update to return an active Stripe-backed subscription (or
throw the same conflict response shape), call the endpoint route that triggers
the update branch, and assert the same patch/response behavior as the DB-only
cancel branch (status and payload expectations). Ensure the test isolates only
the route-level logic (avoid flaky webhook flows), uses the same identifiers as
the existing create-path test, and restores/clears the Stripe mock after the
test.
- Around line 1048-1098: The test "creates a $0 recurring subscription without
requiring a payment intent" (and the similar test around lines 1154-1228) splits
assertions into expect(response.status).toBe(200) and
expect(response.body).toEqual({}), breaking the file's NiceResponse
inline-snapshot style; change these to a single inline snapshot assertion using
Vitest's toMatchInlineSnapshot on the full response object returned by
niceBackendFetch (e.g., replace the two assertions with
expect(response).toMatchInlineSnapshot(/* update snapshot here */)), and do the
same for the corresponding test at 1154-1228 so both follow the established
NiceResponse snapshot pattern.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83a9c0d8-177c-4541-9855-c8d56332431b
📒 Files selected for processing (8)
apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/included-item-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.tsapps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.tsdocs-mintlify/openapi/admin.jsondocs-mintlify/openapi/client.jsondocs-mintlify/openapi/server.json
|
@greptileai please do a full rereview |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsx`:
- Around line 95-99: selectedPrice.USD may be NaN or negative which bypasses the
one-time price guard; update the validation around priceAmount (derived from
selectedPrice.USD) to first assert Number.isFinite(priceAmount) and priceAmount
>= 0 and throw a StatusError(400, ...) for invalid values before calling
getStripeOneTimeMinAmount('USD') and comparing against stripeOneTimeMin; adjust
the logic in the same block that currently uses priceAmount, selectedPrice and
getStripeOneTimeMinAmount so invalid or non-finite USD values return a clear 400
route error rather than allowing Stripe errors downstream.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c214cfc4-853b-4cc0-8a7c-8813a757ad82
📒 Files selected for processing (6)
apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/included-item-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/price-edit-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsxapps/e2e/tests/backend/endpoints/api/v1/payments/purchase-session.test.tspackages/stack-shared/src/payments/stripe-limits.ts
|
@greptileai please rereview |
|
@greptileai please rereview |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.ts (1)
172-202: ⚡ Quick winMove the free-price JSDoc so it documents the right function.
Line 172 introduces
getPriceCheckoutError, but theisFreePricesdoc block above it is now orphaned. Please move that block directly aboveisFreePricesto avoid misleading maintenance.Suggested cleanup
-/** - * Returns true if `prices` represents a "free" product: exactly one price entry - * whose USD amount is `'0'` or `'0.00'` and which has no free-trial or - * server-only flag set (either would change the semantics meaningfully). - * - * An interval IS allowed (and expected, post-update) because a free product is - * modelled as a $0 recurring subscription — see `createFreePrice()` for the - * rationale. - * - * We accept both `'0'` and `'0.00'` for backward-compatibility with rows written - * before we standardized on `createFreePrice()` (which emits `'0.00'`). All three - * product pages (list, edit, create) call this so the "Free" indicator and the - * "Make free" / "Make paid" toggles stay in sync. - */ /** * Returns a human-readable error string if this price would be rejected by * Stripe at checkout time, or `null` if it's valid. Used both at price-edit @@ export function getPriceCheckoutError(price: Price): string | null { @@ } +/** + * Returns true if `prices` represents a "free" product: exactly one price entry + * whose USD amount is `'0'` or `'0.00'` and which has no free-trial or + * server-only flag set (either would change the semantics meaningfully). + * + * An interval IS allowed (and expected, post-update) because a free product is + * modelled as a $0 recurring subscription — see `createFreePrice()` for the + * rationale. + * + * We accept both `'0'` and `'0.00'` for backward-compatibility with rows written + * before we standardized on `createFreePrice()` (which emits `'0.00'`). All three + * product pages (list, edit, create) call this so the "Free" indicator and the + * "Make free" / "Make paid" toggles stay in sync. + */ export function isFreePrices(prices: PricesObject): boolean {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/utils.ts around lines 172 - 202, The JSDoc describing free-price behavior is currently placed above getPriceCheckoutError and should be moved to document isFreePrices instead; find the orphaned JSDoc (the block that talks about $0/recurring vs one-time free prices) and relocate it directly above the isFreePrices function declaration so it documents isFreePrices rather than getPriceCheckoutError, leaving getPriceCheckoutError with only its own Stripe validation JSDoc.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/utils.ts:
- Around line 172-202: The JSDoc describing free-price behavior is currently
placed above getPriceCheckoutError and should be moved to document isFreePrices
instead; find the orphaned JSDoc (the block that talks about $0/recurring vs
one-time free prices) and relocate it directly above the isFreePrices function
declaration so it documents isFreePrices rather than getPriceCheckoutError,
leaving getPriceCheckoutError with only its own Stripe validation JSDoc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4155b72d-6b37-4d68-812e-ee2487f93c01
📒 Files selected for processing (5)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-list-view.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/price-edit-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-price-row.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.ts
Summary of Changes
You can now edit items on a product view.
The "Make free" button is less obtuse, and it clearly tells you what it's going to do.
Additionally, we found out while working on this PR that you cannot create a
paymentIntenton stripe that is < 0.5$. So, you can't create an OTP for a "free" product. We add safeguards to protect against that.Also, 0 dollar subscriptions don't create a subscription invoice. Additionally, the old code relied on being able to fetch the stripe client secret, which would be null for a 0 dollar subscription so we create a carve out.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests