[Refactor] [Fix] Remove default prod creation#1350
Open
Conversation
With the new bulldozer rework we dont support default products anymore. Users are encouraged to currently manually handle granting products to their end users.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR removes the legacy "include-by-default" pricing sentinel: it migrates stored snapshots to explicit price objects, removes sentinel support from schemas/types/validation, rejects new writes using the sentinel, and updates backend logic, dashboard UI, tests, docs, and tooling to use explicit price objects. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API Server
participant Config as Config Validator
participant DB as Database
participant Migr as Offline Migrator
Client->>API: PUT/PATCH config with product.prices = "include-by-default"
API->>Config: parseAndValidateConfig()
Config->>Config: findIncludeByDefaultPath() → detects sentinel
Config-->>API: throw BadRequest (reject write)
Note right of Migr: Offline migration process
Migr->>DB: scan snapshots for prices == "include-by-default"
DB-->>Migr: rows with legacy sentinel
Migr->>DB: rewrite product JSON → prices: {} and COALESCE includedItems
DB-->>Migr: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
…nel from product snapshots and update related code to enforce new pricing structure. This includes SQL migration scripts and adjustments across various files to ensure compatibility with the new pricing model, enhancing data integrity and preventing errors in downstream processing.
…data.ts` to streamline data generation and improve maintainability.
Collaborator
|
@greptile-ai review |
Contributor
Greptile SummaryThis PR removes the deprecated Confidence Score: 5/5Safe to merge — all remaining findings are P2 style/consistency suggestions that do not affect correctness. The migration is narrowly scoped (~5 affected rows), the sentinel is fully removed from schema, write path, and UI. Previously-raised P1 concerns (server-granted sub guard, competing-sub fallthrough) are addressed in this revision. Only P2 issues remain. apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx — uses alert() for validation instead of inline error state. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Config Override Write] --> B{findIncludeByDefaultPath}
B -->|found| C[400 Bad Request - use explicit price]
B -->|not found| D[migrateConfigOverride - rewrite sentinel to empty]
D --> E[Validate via pricesSchema]
E --> F[Store normalised config]
G[SQL Migration] --> H{prices is include-by-default?}
H -->|yes| I[SET prices to empty, coalesce includedItems]
H -->|no| J[Pass through unchanged]
K[Switch Endpoint] --> L{existingSub found?}
L -->|yes| M{stripeSubscriptionId present?}
M -->|no| N[400 - server-granted, use admin tooling]
M -->|yes| O[Update Stripe subscription]
L -->|no| P{fromIsFreePlan?}
P -->|no| Q[400 - no active subscription]
P -->|yes| R{competingSub in product line?}
R -->|yes| S[400 - switch from that product instead]
R -->|no| T[Create new Stripe subscription]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx
Line: 328-332
Comment:
**`alert()` for validation inconsistent with rest of the codebase**
The validation guard in `handleSave` uses the native `alert()` dialog, but every other product form in this PR (`edit/page-client.tsx`, `new/page-client.tsx`) accumulates errors into a `setErrors` state object and renders them as inline `<Typography>` messages. The `alert()` blocks the thread, cannot be styled, and gives the user no visual context about where the problem is.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx
Line: 168-187
Comment:
**Dot-split key traversal implicit assumption on product IDs**
`findIncludeByDefaultPath` calls `key.split(".")` when walking object keys, then checks `path.length === 4`. The logic is consistent with the existing `mapProperty` implementation and passes all tested representations (fully nested, dot-notation at root, mixed). However, it silently depends on product IDs never containing dots — if `userSpecifiedIdSchema` ever relaxes that constraint, this validator and its `migrateConfigOverride` counterpart would silently miss paths containing dot-containing product IDs. Worth a clarifying comment.
How can I resolve this? If you propose a fix, please make it concise.Reviews (5): Last reviewed commit: "fix(payments): enhance subscription swit..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/dashboard/src/components/payments/product-dialog.tsx (1)
40-45:⚠️ Potential issue | 🟡 MinorDefensive check for null/undefined value in test.
If
valueis evernull/undefined(e.g., before form defaults kick in, or when.defined()fails and yup still runs subsequent tests),Object.keys(value)throws aTypeErrorthat surfaces as an unhelpful error rather than the intended validation message.Proposed guard
- prices: pricesSchema.defined().label("Prices").test("at-least-one-price", (value, context) => { - if (Object.keys(value).length === 0) { + prices: pricesSchema.defined().label("Prices").test("at-least-one-price", (value, context) => { + if (value == null || Object.keys(value).length === 0) { return context.createError({ message: "At least one price is required" }); } return true; }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/payments/product-dialog.tsx` around lines 40 - 45, The prices test on pricesSchema ("at-least-one-price") calls Object.keys(value) without guarding for null/undefined, causing a TypeError; update the test to first check if value is null or undefined and return context.createError(...) (or false) with the same "At least one price is required" message when missing, otherwise continue to check Object.keys(value).length === 0 and return true when valid; ensure the test uses the same test name ("at-least-one-price") and operates on the value safely.apps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.ts (1)
208-243:⚠️ Potential issue | 🟡 MinorSeed ownership of the source plan before testing switch blocking.
After replacing
include-by-default, signing up no longer makes the user ownplanA/freePlan. These switch tests should first grant or purchase the source plan withblockNewPurchasesdisabled, then enable the block and assert the switch is rejected; otherwise the new$0 plancase is only testing a generic blocked switch request.Also applies to: 259-310
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.ts` around lines 208 - 243, The test calls Auth.fastSignUp and then attempts a product switch from planA to planB without ensuring the user actually owns planA; update the test to first grant or create ownership of the source product (e.g., simulate a purchase or call the ownership/grant helper) for the new user so they own "planA"/freePlan, then enable blockNewPurchases on planA and assert the switch via niceBackendFetch is rejected; apply the same fix to the sibling test block (lines referenced 259-310) so both tests create ownership before toggling blockNewPurchases and asserting blocked switches.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsx (1)
163-176:⚠️ Potential issue | 🟠 MajorValidate
pricesbefore saving the dialog product.Now that
{}no longer maps to the legacy free/default sentinel, the scratch flow can save a product with no prices. The full new-product page already rejects this state; this dialog should do the same beforeonSave.Proposed fix
+ const validatePricing = () => { + const newErrors: Record<string, string> = {}; + if (Object.keys(prices).length === 0) { + newErrors.prices = "At least one price is required"; + } + return newErrors; + }; + const handleSave = async () => { + const pricingErrors = validatePricing(); + if (Object.keys(pricingErrors).length > 0) { + setErrors(pricingErrors); + setCurrentStep(2); + return; + } + const product: Product = { displayName, customerType, productLineId: productLineId || undefined, isAddOnTo: isAddOn ? Object.fromEntries(isAddOnTo.map(id => [id, true])) : false, stackable, prices, includedItems, serverOnly, freeTrial, };<div className="border rounded-lg"> <ListSection title="Prices"> <PricingSection prices={prices} - onPricesChange={setPrices} + onPricesChange={(newPrices) => { + setPrices(newPrices); + if (errors.prices && Object.keys(newPrices).length > 0) { + setErrors(prev => { + const newErrors = { ...prev }; + delete newErrors.prices; + return newErrors; + }); + } + }} variant="dialog" /> + {errors.prices ? ( + <Typography type="label" className="text-destructive text-xs px-3 pb-3 block"> + {errors.prices} + </Typography> + ) : null} </ListSection> </div>Also applies to: 608-615
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsx around lines 163 - 176, The save path in handleSave constructs a Product and calls onSave without validating prices, allowing a product with no prices to be saved; add a guard in handleSave that checks prices has at least one price entry (e.g., Object.keys/prices.length > 0 or prices.some) and if empty prevent saving by returning early and surface a validation error (set a local error state or call the existing validation UI) instead of calling onSave(productId, product); apply the same check to the other save branch that builds and submits a Product object so both save flows reject an empty prices set.apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts (1)
118-136:⚠️ Potential issue | 🟠 MajorDon’t let the OTP guard block the source free plan.
Line 118 classifies any owned product in the same product line without an active subscription as an OTP. For the new empty-price/free-plan path, the source product itself can have no subscription row, so switching from that free plan is blocked before Line 136 can fall through to subscription creation.
🐛 Proposed fix
const fromPriceEntries = typedEntries(fromProduct.prices); const fromHasIntervalPrice = fromPriceEntries.some(([, price]) => price.interval); + const fromIsFreePlan = fromPriceEntries.length === 0 + || fromPriceEntries.every(([, p]) => !p.USD || Number(p.USD) === 0); // A product with non-interval prices is a one-time purchase and can't be switched. // A product with no prices at all (e.g. auto-migrated from the legacy `include-by-default` // sentinel, or an intentionally free product) is treated as a free plan the customer may // upgrade away from. @@ const hasOtpInProductLine = Object.entries(ownedProducts).some( ([productId, p]) => p.productLineId === fromProduct.productLineId && p.quantity > 0 + && !(fromIsFreePlan && productId === body.from_product_id) && !activeSubProductIds.has(productId) ); @@ - const fromIsFreePlan = fromPriceEntries.length === 0 - || fromPriceEntries.every(([, p]) => !p.USD || Number(p.USD) === 0); if (!existingSub && !fromIsFreePlan) { throw new StatusError(400, "This subscription cannot be switched."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts around lines 118 - 136, The OTP guard (hasOtpInProductLine) is incorrectly treating the source product as an OTP and can block switching from a free-source product; update the predicate used to compute hasOtpInProductLine to ignore the source product itself (compare productId against the source id — e.g. body.from_product_id or fromProduct.id) so only other owned products in the same product line with quantity>0 and not in activeSubProductIds are considered; leave the rest of the logic (existingSub, fromIsFreePlan) unchanged.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx (1)
846-876:⚠️ Potential issue | 🟠 MajorPrevent saving an empty
pricesmap from the details page.Line 847 and Line 867 can leave
pendingChanges.pricesas{}, andhandleSavepersists that value without validation. This bypasses the new/edit form requirement for at least one explicit price and can create products that are neither explicitly free nor purchasable.🐛 Proposed fix
-import { DEFAULT_INTERVAL_UNITS, generateUniqueId, intervalLabel, shortIntervalLabel, type Price, type Product } from "../utils"; +import { DEFAULT_INTERVAL_UNITS, createFreePrice, generateUniqueId, intervalLabel, shortIntervalLabel, type Price, type Product } from "../utils"; @@ const handleSave = async () => { + if (pendingChanges.prices !== undefined && Object.keys(pendingChanges.prices).length === 0) { + alert("Add at least one price before saving this product."); + return; + } + const configUpdate: Record<string, any> = {}; @@ const handleMakeFree = () => { - const newPriceId = generateUniqueId('price'); - onPricesChange({ - [newPriceId]: { USD: '0', serverOnly: false }, - }); + onPricesChange(createFreePrice()); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx around lines 846 - 876, The code allows persisting an empty prices map; fix by (1) stopping handleMakePaid from calling onPricesChange({}) — instead just call openAddDialog() (or create+set a new draft price) so you don't save an empty map, and (2) add validation in handleSave (or the component's save flow) that inspects priceEntries / hasNoPrices / isFree and rejects saving when hasNoPrices is true (or requires at least one explicit price or a free price), returning an error UI state instead of persisting an empty object; update references to handleMakePaid, handleSave, priceEntries, hasNoPrices, and isFree accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/payments/purchases/validate-code/route.ts`:
- Line 83: The isSubscribable check uses an untyped callback and a boolean
truthiness check; update the callback to use the proper PriceObject type from
inlineProductSchema (e.g., annotate the parameter as PriceObject) instead of
any, and replace the truthy check with explicit null/undefined checks like p !=
null && p.interval != null when computing isSubscribable from product.prices to
avoid looser semantics and eliminate any.
---
Outside diff comments:
In
`@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts:
- Around line 118-136: The OTP guard (hasOtpInProductLine) is incorrectly
treating the source product as an OTP and can block switching from a free-source
product; update the predicate used to compute hasOtpInProductLine to ignore the
source product itself (compare productId against the source id — e.g.
body.from_product_id or fromProduct.id) so only other owned products in the same
product line with quantity>0 and not in activeSubProductIds are considered;
leave the rest of the logic (existingSub, fromIsFreePlan) unchanged.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx:
- Around line 846-876: The code allows persisting an empty prices map; fix by
(1) stopping handleMakePaid from calling onPricesChange({}) — instead just call
openAddDialog() (or create+set a new draft price) so you don't save an empty
map, and (2) add validation in handleSave (or the component's save flow) that
inspects priceEntries / hasNoPrices / isFree and rejects saving when hasNoPrices
is true (or requires at least one explicit price or a free price), returning an
error UI state instead of persisting an empty object; update references to
handleMakePaid, handleSave, priceEntries, hasNoPrices, and isFree accordingly.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsx:
- Around line 163-176: The save path in handleSave constructs a Product and
calls onSave without validating prices, allowing a product with no prices to be
saved; add a guard in handleSave that checks prices has at least one price entry
(e.g., Object.keys/prices.length > 0 or prices.some) and if empty prevent saving
by returning early and surface a validation error (set a local error state or
call the existing validation UI) instead of calling onSave(productId, product);
apply the same check to the other save branch that builds and submits a Product
object so both save flows reject an empty prices set.
In `@apps/dashboard/src/components/payments/product-dialog.tsx`:
- Around line 40-45: The prices test on pricesSchema ("at-least-one-price")
calls Object.keys(value) without guarding for null/undefined, causing a
TypeError; update the test to first check if value is null or undefined and
return context.createError(...) (or false) with the same "At least one price is
required" message when missing, otherwise continue to check
Object.keys(value).length === 0 and return true when valid; ensure the test uses
the same test name ("at-least-one-price") and operates on the value safely.
In
`@apps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.ts`:
- Around line 208-243: The test calls Auth.fastSignUp and then attempts a
product switch from planA to planB without ensuring the user actually owns
planA; update the test to first grant or create ownership of the source product
(e.g., simulate a purchase or call the ownership/grant helper) for the new user
so they own "planA"/freePlan, then enable blockNewPurchases on planA and assert
the switch via niceBackendFetch is rejected; apply the same fix to the sibling
test block (lines referenced 259-310) so both tests create ownership before
toggling blockNewPurchases and asserting blocked switches.
🪄 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: df20c2a4-0e06-4c87-a92d-23e901f0fa62
📒 Files selected for processing (33)
apps/backend/prisma/migrations/20260421000000_drop_include_by_default_snapshots/migration.sqlapps/backend/scripts/verify-data-integrity/payments-verifier.tsapps/backend/src/app/api/latest/internal/config/override/[level]/route.tsxapps/backend/src/app/api/latest/internal/payments/transactions/route.tsxapps/backend/src/app/api/latest/internal/payments/transactions/transaction-builder.tsapps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/route.tsapps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.tsapps/backend/src/app/api/latest/payments/purchases/validate-code/route.tsapps/backend/src/app/api/latest/teams/crud.tsxapps/backend/src/lib/payments.tsxapps/backend/src/lib/payments/schema/__tests__/dual-write.test.tsapps/backend/src/lib/payments/schema/types.tsapps/backend/src/lib/seed-dummy-data.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/new/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-list-view.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-card-preview.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-price-row.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.tsapps/dashboard/src/components/data-table/transaction-table.tsxapps/dashboard/src/components/payments/product-dialog.tsxapps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.tsapps/e2e/tests/backend/endpoints/api/v1/payments/switch-plans.test.tsapps/internal-tool/scripts/spacetime-publish.mjsdocs-mintlify/guides/apps/payments/overview.mdxdocs-mintlify/snippets/payments-concepts.jsxpackages/stack-shared/src/config/schema-fuzzer.test.tspackages/stack-shared/src/config/schema.tspackages/stack-shared/src/schema-fields.ts
💤 Files with no reviewable changes (5)
- docs-mintlify/guides/apps/payments/overview.mdx
- apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/route.ts
- apps/backend/src/lib/seed-dummy-data.ts
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-list-view.tsx
- apps/backend/scripts/verify-data-integrity/payments-verifier.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the legacy "include-by-default" product pricing mode across shared schemas, backend payments flows, dashboard UI, and docs, while adding migrations/guards so legacy stored configs and snapshots keep loading without accepting new writes using the sentinel.
Changes:
- Replaces the
"include-by-default"union with an explicitprices: Record<priceId, priceConfig>schema and updates dashboard editors accordingly. - Adds config-override migration + write-time rejection for the deprecated sentinel, and migrates historical product snapshots in DB JSON columns.
- Updates backend switching/purchase logic and test suites to reflect explicit $0 pricing instead of default-included products.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/stack-shared/src/schema-fields.ts | Removes include-by-default sentinel from shared product pricing schema (prices are always a record). |
| packages/stack-shared/src/config/schema.ts | Migrates legacy sentinel to {} in overrides; updates sanitization + tests around mapping. |
| packages/stack-shared/src/config/schema-fuzzer.test.ts | Updates fuzz inputs to stop generating include-by-default prices. |
| docs-mintlify/snippets/payments-concepts.jsx | Updates docs snippet to remove include-by-default usage. |
| docs-mintlify/guides/apps/payments/overview.mdx | Removes include-by-default from payments overview docs. |
| apps/internal-tool/scripts/spacetime-publish.mjs | Refactors local Spacetime publish server URL computation. |
| apps/e2e/tests/backend/endpoints/api/v1/payments/switch-plans.test.ts | Updates switching tests and adds coverage for rejecting deprecated sentinel writes. |
| apps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.ts | Updates tests to use explicit prices; adds scenario for blocking upgrades from $0 plans. |
| apps/dashboard/src/components/payments/product-dialog.tsx | Updates dashboard form validation and removes include-by-default UI controls. |
| apps/dashboard/src/components/data-table/transaction-table.tsx | Removes include-by-default handling in product price display logic. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/utils.ts | Removes include-by-default helpers; adds createFreePrice() utility. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-price-row.tsx | Simplifies price row props and removes include-by-default toggles. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsx | Removes free-by-default state; always manages explicit prices map. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-card-preview.tsx | Uses product.prices directly (no sentinel conversion). |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/pricing-section.tsx | Removes include-by-default UI; “Make free” now creates a $0 price entry. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-product-lines-view.tsx | Removes sentinel handling in prompts/sorting and price rendering. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/page-client-list-view.tsx | Removes sentinel handling in formatting and sorting. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/new/page-client.tsx | Removes free-by-default flow; requires explicit prices and uses createFreePrice(). |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx | Removes include-by-default transitions; “Make free” now creates an explicit $0 price. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsx | Removes free-by-default flow; adds alert for legacy products with empty prices. |
| apps/backend/src/lib/seed-dummy-data.ts | Removes dummy legacy include-by-default subscription/invoice data. |
| apps/backend/src/lib/payments/schema/types.ts | Updates ProductSnapshot.prices type to always be a record. |
| apps/backend/src/lib/payments/schema/tests/dual-write.test.ts | Updates tests to use {} instead of include-by-default sentinel in snapshots. |
| apps/backend/src/lib/payments.tsx | Removes sentinel branching; selected price is always resolved from record. |
| apps/backend/src/app/api/latest/teams/crud.tsx | Removes sentinel handling when deriving internal free-plan subscription interval. |
| apps/backend/src/app/api/latest/payments/purchases/validate-code/route.ts | Updates subscribable detection to no longer special-case include-by-default. |
| apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts | Reinterprets empty prices as free-plan upgrade path; removes sentinel logic. |
| apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/route.ts | Removes filter that excluded include-by-default products. |
| apps/backend/src/app/api/latest/internal/payments/transactions/transaction-builder.ts | Removes sentinel handling when resolving selected price from product snapshot. |
| apps/backend/src/app/api/latest/internal/payments/transactions/route.tsx | Removes legacy include-by-default snapshot fallback; now expects normalized snapshots. |
| apps/backend/src/app/api/latest/internal/config/override/[level]/route.tsx | Rejects config writes that contain deprecated include-by-default sentinel at product prices path. |
| apps/backend/scripts/verify-data-integrity/payments-verifier.ts | Removes default-product verification logic tied to include-by-default. |
| apps/backend/prisma/migrations/20260421000000_drop_include_by_default_snapshots/migration.sql | Migrates historical JSON snapshots to replace sentinel with {} and ensure includedItems exists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Added validation to ensure at least one price is provided when saving a product. - Enhanced error handling to clear price-related errors when prices are updated. - Updated price validation schema to allow for null values. - Removed unnecessary price reset logic in the product prices section. These changes aim to improve user experience by enforcing price requirements and providing clearer feedback in the product dialog.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx (2)
345-347:⚠️ Potential issue | 🟠 MajorBlock saving a product with zero prices.
Deleting the last price now sends
{}upstream, andhandleSavepersists it unchanged. That bypasses the “at least one price when saving a product” requirement and can leave this page creating invalid price configs.🛡️ Proposed fix: validate before saving pending prices
const handleSave = async () => { const configUpdate: Record<string, any> = {}; + + if (pendingChanges.prices !== undefined && typedEntries(pendingChanges.prices).length === 0) { + window.alert("Products must have at least one price. Add a price option or make the product free before saving."); + return; + } // Apply product changesAs per coding guidelines, “For blocking alerts and errors, never use
toast, as they are easily missed by the user. Instead, use alerts”.Also applies to: 846-848
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx around lines 345 - 347, The save path currently writes an empty object for prices (configUpdate[`payments.products.${productId}.prices`]) when the last price is deleted, which allows handleSave to persist an invalid product config; update the save logic to validate pendingChanges.prices before assigning it: if pendingChanges.prices is empty or would result in zero prices, do not set configUpdate and instead prevent the save by throwing/returning early and show a blocking alert component (not toast) with a clear message (e.g., "A product must have at least one price") so the UI blocks until the user adds a price; apply the same validation/alert behavior for the other occurrence around lines 846-848 where prices are written.
830-841:⚠️ Potential issue | 🟠 MajorMake paid currently keeps the free checkout option.
From a free product,
handleMakePaidopens the add-price dialog, thenhandleSavePricemerges the new paid price into the existing{ USD: '0' }price set. The product ends up with both free and paid prices, so it is still purchasable for free.🐛 Proposed fix: replace prices when converting free → paid
function ProductPricesSection({ productId, prices, onPricesChange, inline = false }: ProductPricesSectionProps) { const [editingPrice, setEditingPrice] = useState<EditingPrice | null>(null); const [isAddingPrice, setIsAddingPrice] = useState(false); + const [replacePricesOnSave, setReplacePricesOnSave] = useState(false); const handleSavePrice = (editing: EditingPrice) => { const newPrice = editingPriceToPrice(editing); - const updatedPrices = { - ...prices, - [editing.priceId]: newPrice, - }; + const updatedPrices = replacePricesOnSave + ? { [editing.priceId]: newPrice } + : { + ...prices, + [editing.priceId]: newPrice, + }; onPricesChange(updatedPrices); setEditingPrice(null); setIsAddingPrice(false); + setReplacePricesOnSave(false); };const handleMakePaid = () => { + setReplacePricesOnSave(true); openAddDialog(); };onOpenChange={(open) => { if (!open) { setEditingPrice(null); setIsAddingPrice(false); + setReplacePricesOnSave(false); } }}Also applies to: 866-868, 993-997
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx around lines 830 - 841, handleSavePrice currently merges the new price into the existing prices object, which preserves a free price when converting a product to paid; update handleSavePrice (and the analogous logic used by handleMakePaid and the other save paths) to detect a free-only price set (e.g., all existing price values are '0' or the object equals { USD: '0' }) and, when creating a paid price, replace the entire prices object with a new single-entry map built from editingPriceToPrice(editing) instead of merging, then call onPricesChange with that replaced prices object.apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts (1)
199-258:⚠️ Potential issue | 🟡 MinorThe old free-plan subscription is never deactivated when switching to a paid product.
When a customer on a free plan (which has a subscription row with
stripeSubscriptionId: null) switches to a paid product, the code creates a new subscription for the paid product but does not cancel or mark the old free-plan subscription as inactive. This leaves two active subscription rows for the same customer in the same product line.The code correctly handles paid-to-paid switches by updating the existing subscription (line 207). However, the free-to-paid path (lines 259–317) only creates a new row without addressing the prior subscription. The old free-plan subscription should be explicitly cancelled when the new paid subscription is created, to maintain consistency and avoid orphaned rows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts around lines 199 - 258, The free-to-paid path currently creates a new paid subscription but never deactivates the prior free-plan row (the existingSub with stripeSubscriptionId === null). After you create the new Stripe subscription and persist the new Prisma subscription record (the else branch handling new paid creation), add a Prisma update for the existingSub.id to mark it inactive/canceled (e.g., set status to "canceled" or an equivalent field and set cancelAtPeriodEnd/currentPeriodEnd appropriately) and call bulldozerWriteSubscription(prisma, updatedOldSub) to dual-write the change; ensure you reference existingSub.id and the newly created subscription record names used in this file so the old free row is explicitly closed.
🧹 Nitpick comments (1)
apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts (1)
129-139: Prefer explicit null check forp.USD.
!p.USDmixesundefinedand the empty string together; per the repo guideline favoring explicit null/undefinedness checks over boolean checks, this reads more clearly asp.USD == null. TheNumber(p.USD) === 0guard already handles"0"/"0.00".♻️ Suggested tweak
- const fromIsFreePlan = fromPriceEntries.length === 0 - || fromPriceEntries.every(([, p]) => !p.USD || Number(p.USD) === 0); + const fromIsFreePlan = fromPriceEntries.length === 0 + || fromPriceEntries.every(([, p]) => p.USD == null || Number(p.USD) === 0);As per coding guidelines: "Unless very clearly equivalent from types, prefer explicit null/undefinedness checks over boolean checks, eg.
foo == nullinstead of!foo".Also worth confirming: the
fromIsFreePlanheuristic only considersUSD, so a product priced only in a non-USD currency would be classified as "free" here. Given the existing USD-only assumption across the payments code, this is consistent — just flagging in case multi-currency support lands.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts around lines 129 - 139, Update the fromIsFreePlan predicate to use an explicit null/undefined check for p.USD instead of a boolean negation: change the condition that currently reads "!p.USD || Number(p.USD) === 0" to use "p.USD == null || Number(p.USD) === 0" so the intent and types are clear; this touches the fromIsFreePlan computation that iterates over fromPriceEntries and references p.USD.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/payments/purchases/validate-code/route.ts`:
- Around line 83-84: Remove the stale sentinel check comparing productPrices to
"include-by-default": the inlineProductSchema.prices is now a Record<string,
PriceObject>, so update the isSubscribable logic in route.ts to stop checking
productPrices !== "include-by-default" and instead determine subscribability
directly from the price objects (use Object.values(product.prices).some(...) on
the existing product/prices variables). Keep references to the same variables
(productPrices or product.prices and isSubscribable) so the change is minimal
and type-correct.
---
Outside diff comments:
In
`@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts:
- Around line 199-258: The free-to-paid path currently creates a new paid
subscription but never deactivates the prior free-plan row (the existingSub with
stripeSubscriptionId === null). After you create the new Stripe subscription and
persist the new Prisma subscription record (the else branch handling new paid
creation), add a Prisma update for the existingSub.id to mark it
inactive/canceled (e.g., set status to "canceled" or an equivalent field and set
cancelAtPeriodEnd/currentPeriodEnd appropriately) and call
bulldozerWriteSubscription(prisma, updatedOldSub) to dual-write the change;
ensure you reference existingSub.id and the newly created subscription record
names used in this file so the old free row is explicitly closed.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx:
- Around line 345-347: The save path currently writes an empty object for prices
(configUpdate[`payments.products.${productId}.prices`]) when the last price is
deleted, which allows handleSave to persist an invalid product config; update
the save logic to validate pendingChanges.prices before assigning it: if
pendingChanges.prices is empty or would result in zero prices, do not set
configUpdate and instead prevent the save by throwing/returning early and show a
blocking alert component (not toast) with a clear message (e.g., "A product must
have at least one price") so the UI blocks until the user adds a price; apply
the same validation/alert behavior for the other occurrence around lines 846-848
where prices are written.
- Around line 830-841: handleSavePrice currently merges the new price into the
existing prices object, which preserves a free price when converting a product
to paid; update handleSavePrice (and the analogous logic used by handleMakePaid
and the other save paths) to detect a free-only price set (e.g., all existing
price values are '0' or the object equals { USD: '0' }) and, when creating a
paid price, replace the entire prices object with a new single-entry map built
from editingPriceToPrice(editing) instead of merging, then call onPricesChange
with that replaced prices object.
---
Nitpick comments:
In
`@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts:
- Around line 129-139: Update the fromIsFreePlan predicate to use an explicit
null/undefined check for p.USD instead of a boolean negation: change the
condition that currently reads "!p.USD || Number(p.USD) === 0" to use "p.USD ==
null || Number(p.USD) === 0" so the intent and types are clear; this touches the
fromIsFreePlan computation that iterates over fromPriceEntries and references
p.USD.
🪄 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: db3535a8-4706-4d0f-8909-e3d7a9a2627d
📒 Files selected for processing (5)
apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.tsapps/backend/src/app/api/latest/payments/purchases/validate-code/route.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsxapps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.ts
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/product-dialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/e2e/tests/backend/endpoints/api/v1/payments/block-new-purchases.test.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
The sentinel was removed from the type system in b322d07 but the comparison left behind as dead code broke next build under strict TS.
Comment on lines
+132
to
139
| const existingSub = Object.values(subMap).find( | ||
| s => s.productId === body.from_product_id && isActiveSubscription(s) | ||
| ) ?? null; | ||
| const fromIsFreePlan = fromPriceEntries.length === 0 | ||
| || fromPriceEntries.every(([, p]) => !p.USD || Number(p.USD) === 0); | ||
| if (!existingSub && !fromIsFreePlan) { | ||
| throw new StatusError(400, "This subscription cannot be switched."); | ||
| } |
Contributor
There was a problem hiding this comment.
The old guard if (existingSub && !existingSub.stripeSubscriptionId) throw 400 has been removed. When a customer is server-granted a paid product (creating an active DB subscription with no stripeSubscriptionId), calling the switch endpoint now falls through to the else branch (lines 258–316), which creates a new Stripe subscription for to_product_id — but never cancels or updates the original server-granted subscription for from_product_id. The customer ends up owning both products simultaneously.
This is confirmed by the updated block-new-purchases.test.ts, which now server-grants planA (USD 1000, interval 1 month) before the switch call. The test only verifies that blockNewPurchases stops the request before Stripe is reached; if that flag were false, the path above would leave planA and paidPlan both active for the same user.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts
Line: 132-139
Comment:
**Server-granted paid subscription not cancelled on switch**
The old guard `if (existingSub && !existingSub.stripeSubscriptionId) throw 400` has been removed. When a customer is server-granted a paid product (creating an active DB subscription with no `stripeSubscriptionId`), calling the switch endpoint now falls through to the `else` branch (lines 258–316), which creates a new Stripe subscription for `to_product_id` — but never cancels or updates the original server-granted subscription for `from_product_id`. The customer ends up owning both products simultaneously.
This is confirmed by the updated `block-new-purchases.test.ts`, which now server-grants `planA` (USD 1000, interval 1 month) before the switch call. The test only verifies that `blockNewPurchases` stops the request before Stripe is reached; if that flag were `false`, the path above would leave `planA` and `paidPlan` both active for the same user.
How can I resolve this? If you propose a fix, please make it concise.…tripe ID Enhanced the subscription switching logic to include a check that prevents switching if the existing subscription does not have a Stripe subscription ID. This ensures that only valid subscriptions can be switched, improving error handling and user feedback.
…ching logic - Added validation to ensure a product has at least one price before saving, improving user feedback. - Updated subscription switching logic to clarify that server-granted subscriptions without a Stripe ID cannot be switched, enhancing error handling.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.ts (1)
118-137:⚠️ Potential issue | 🟠 MajorRequire ownership/replacement for the free-plan switch path.
With default products removed,
fromIsFreePlanshould not imply the customer ownsfrom_product_id. As written, a client can switch from any no-price/zero-price product in the same line without an active subscription; if that source product was manually granted, this also creates the paid subscription without revoking/replacing the grant.Please verify the intended migration behavior, then either require an owned source grant before this branch and revoke/replace it, or reject manually granted free products through this endpoint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts around lines 118 - 137, The free-plan branch currently allows switching from any zero-price product without verifying ownership; update the logic where existingSub and fromIsFreePlan are checked to require the customer actually owns the source product (check ownedProducts[body.from_product_id]?.quantity > 0 and ensure it is not an active subscription via subMap/activeSubProductIds) before proceeding; if the source is a manually-granted free product either reject the request with a StatusError(400) or, preferred, revoke/replace that grant before creating the paid subscription by calling your revoke/replace routine (e.g., revokeManualGrant or whatever grant-revocation path exists) so the manual grant is removed/marked replaced when creating the new paid subscription.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx:
- Around line 868-870: The current isFree logic collapses any single USD "0"
price into the free-product view and loses interval/trial/serverOnly metadata;
update the isFree computation so it only returns true for a canonical free price
(e.g., check an explicit marker on the price like
price[1].metadata?.canonicalFree || price[1].type === 'free') and/or ensure you
exclude prices that have interval, interval_count, trial_period_days, or
serverOnly flags from being treated as the canonical free view; keep all other
zero-dollar prices rendered normally with their edit/delete controls and
metadata.
- Around line 325-330: The save handler (handleSave) currently only rejects when
pendingChanges.prices exists but is an empty object, which lets an already-empty
product be saved when other fields change; update the validation to compute the
effective price set (use localPrices merged/overridden by pendingChanges.prices
if present) and block saving when that effective set is empty. Specifically, in
handleSave, replace the current check of pendingChanges.prices with a check that
inspects localPrices together with pendingChanges.prices (or derives the
resulting price keys) and show the existing alert if the resulting effective
price list has length 0.
---
Outside diff comments:
In
`@apps/backend/src/app/api/latest/payments/products/`[customer_type]/[customer_id]/switch/route.ts:
- Around line 118-137: The free-plan branch currently allows switching from any
zero-price product without verifying ownership; update the logic where
existingSub and fromIsFreePlan are checked to require the customer actually owns
the source product (check ownedProducts[body.from_product_id]?.quantity > 0 and
ensure it is not an active subscription via subMap/activeSubProductIds) before
proceeding; if the source is a manually-granted free product either reject the
request with a StatusError(400) or, preferred, revoke/replace that grant before
creating the paid subscription by calling your revoke/replace routine (e.g.,
revokeManualGrant or whatever grant-revocation path exists) so the manual grant
is removed/marked replaced when creating the new paid subscription.
🪄 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: 71b9f83f-b743-4ebb-9564-067fde00a15b
📒 Files selected for processing (2)
apps/backend/src/app/api/latest/payments/products/[customer_type]/[customer_id]/switch/route.tsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/[productId]/page-client.tsx
- Added validation to prevent switching from a free product if an active subscription exists in the same product line, improving error handling. - Updated product price validation to ensure at least one price is present before saving, enhancing user feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With the new bulldozer rework we dont support default products anymore. Users are encouraged to currently manually handle granting products to their end users.
Summary by CodeRabbit
Bug Fixes
Chores
Tests