feat: disable proration #2691
feat: disable proration #2691
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new upgradeSubscription function in Stripe utilities and updates the web subscription router to call it for upgrades (no proration). Also replaces/removes a Next.js TypeScript config with a JS config enabling build-time ESLint/TS ignores. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant WebAPI as Web Subscription Router
participant Billing as upgradeSubscription (Stripe utils)
participant Stripe as Stripe API
User->>WebAPI: Request upgrade (subscriptionId, subscriptionItemId, priceId)
WebAPI->>Billing: upgradeSubscription({...})
Billing->>Stripe: Retrieve subscription
Stripe-->>Billing: Subscription details
Billing->>Billing: Validate prices / compute difference
Billing->>Stripe: Update subscription item (proration: none)
Stripe-->>Billing: Updated subscription
alt priceDifference > 0
Billing->>Stripe: Create invoice item (+difference)
Billing->>Stripe: Create invoice (immediate) & pay
Stripe-->>Billing: Invoice created/paid
end
Billing-->>WebAPI: Return updated subscription
WebAPI-->>User: Respond with result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/stripe/src/functions.ts (1)
157-169: Derive currency from the item price and finalize/pay invoice for immediate charge
- Currency: Avoid defaulting to 'usd' which can cause Stripe errors if the subscription is in another currency. Prefer the subscription’s currency, then the item’s price currency.
- Immediate invoicing: Creating with auto_advance: true typically finalizes later (not instant). To charge immediately, finalize and pay the invoice.
Apply this diff:
- await stripe.invoiceItems.create({ - customer: updatedSubscription.customer as string, - amount: priceDifferenceAmount, - currency: updatedSubscription.currency || 'usd', - description: 'Price upgrade difference', - }); - - // Create invoice immediately - const invoice = await stripe.invoices.create({ - customer: updatedSubscription.customer as string, - auto_advance: true, - }); + await stripe.invoiceItems.create( + { + customer: updatedSubscription.customer as string, + amount: priceDifferenceAmount, + currency: + updatedSubscription.currency || + newItem.price?.currency || + undefined, + description: `Price upgrade difference (${currentItem.price?.id} → ${newItem.price?.id})`, + } + ); + + // Create, finalize, and pay invoice immediately + const invoice = await stripe.invoices.create({ + customer: updatedSubscription.customer as string, + collection_method: 'charge_automatically', + auto_advance: false, + }); + await stripe.invoices.finalizeInvoice(invoice.id); + await stripe.invoices.pay(invoice.id);Note:
- If neither subscription nor price currency is present, consider throwing explicitly rather than relying on a default to avoid currency mismatches.
if (!currency) throw new Error('Currency not found on subscription or price');
🧹 Nitpick comments (4)
apps/web/client/src/server/api/routers/subscription/subscription.ts (1)
165-169: Confirm post-upgrade state sync and “immediate” invoicing semantics
- This path no longer updates the DB (unlike the downgrade path which writes schedule info). If your webhook is responsible for syncing the new price into the DB, please confirm it covers this upgrade scenario and that the UI can render the new tier promptly without relying solely on webhook latency.
- “Immediate” invoicing: upgradeSubscription currently creates an invoice with auto_advance: true, which is finalized automatically later (typically up to ~1 hour). If you need truly immediate charge/collection, consider finalizing and paying the invoice explicitly in upgradeSubscription (see suggestion in functions.ts review).
packages/stripe/src/functions.ts (3)
137-146: Optional: Add idempotency to write operationsRetries could double-charge (duplicate invoice items). Consider idempotency keys for invoice item and invoice creation.
Example:
await stripe.invoiceItems.create(data, { idempotencyKey: `upgrade-item-${subscriptionId}-${subscriptionItemId}-${priceId}` }); const invoice = await stripe.invoices.create(invoiceData, { idempotencyKey: `upgrade-invoice-${subscriptionId}-${subscriptionItemId}-${priceId}` });
137-146: Optional: Ensure price fields are available without expansionsIn some API versions/setups, nested price objects may not include all fields unless expanded. If you encounter null unit_amount, retrieve the Price via stripe.prices.retrieve(priceId) or use expansions on the subscription update/retrieve.
Example:
const currentPriceObj = await stripe.prices.retrieve(currentPriceId!); const newPriceObj = await stripe.prices.retrieve(priceId);
171-172: Return type and data shapeConsider returning both the updated subscription and invoice (if one is created) so callers can reflect immediate charges in the UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/client/src/server/api/routers/subscription/subscription.ts(2 hunks)packages/stripe/src/functions.ts(1 hunks)
🔇 Additional comments (1)
apps/web/client/src/server/api/routers/subscription/subscription.ts (1)
3-3: Switch to upgradeSubscription aligns with “disable proration” objectiveImport change looks correct and matches the new Stripe utility. No concerns here.
| @@ -4,5 +4,11 @@ const nextConfig = { | |||
| devIndicators: { | |||
| buildActivity: false, | |||
| }, | |||
| eslint: { | |||
There was a problem hiding this comment.
Avoid ignoring ESLint/TypeScript build errors unless absolutely necessary. This may hide real issues in production.
| if (priceDifferenceAmount > 0) { | ||
| await stripe.invoiceItems.create({ | ||
| customer: updatedSubscription.customer as string, | ||
| amount: priceDifferenceAmount, |
There was a problem hiding this comment.
The currency field was removed from the invoiceItems.create call. Stripe typically requires a currency for invoice items; consider reinstating it.
| amount: priceDifferenceAmount, | |
| currency: 'usd', |
|
|
||
| const newItem = | ||
| updatedSubscription.items.data.find((i) => i.id === subscriptionItemId) | ||
| ?? updatedSubscription.items.data[0]; |
There was a problem hiding this comment.
Falling back to the first item if the matching subscription item isn't found may mask issues. It might be safer to throw an error instead.
| ?? updatedSubscription.items.data[0]; | |
| ?? (() => { throw new Error('Subscription item not found on updated subscription'); })() |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/stripe/src/functions.ts (3)
127-140: Good: targeting the correct subscription item and nullish-checking unit_amountUsing subscriptionItemId to locate the item and checking unit_amount with a nullish guard avoids multi-item and 0-amount pitfalls.
153-166: Good: reading updated item by id and incorporating quantity into the deltaThis addresses under-billing when quantity > 1 and avoids relying on index 0.
176-185: Nice: invoice is paid immediately after creationThis addresses prior feedback to ensure the upgrade transaction completes in one operation.
🧹 Nitpick comments (1)
apps/web/template/next.config.mjs (1)
7-12: Turning off ESLint and TypeScript checks during builds can mask real issues—confirm intent or scope to non-CI buildsThis will allow type and lint errors to ship unnoticed. If this is only to relax local template builds, gate it behind an env var so CI remains strict.
Apply this diff to enforce checks on CI while allowing local flexibility:
- eslint: { - ignoreDuringBuilds: true, - }, - typescript: { - ignoreBuildErrors: true, - }, + eslint: { + // Keep CI strict; allow local templates to build even with lint errors + ignoreDuringBuilds: process.env.CI !== 'true', + }, + typescript: { + // Keep CI strict; allow local templates to build even with TS errors + ignoreBuildErrors: process.env.CI !== 'true', + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/template/next.config.mjs(1 hunks)apps/web/template/next.config.ts(0 hunks)packages/stripe/src/functions.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/template/next.config.ts
🔇 Additional comments (1)
packages/stripe/src/functions.ts (1)
164-166: Validate billing interval assumptions (monthly vs yearly) when computing deltaThe delta assumes both prices share the same billing interval/granularity. If plan intervals differ (e.g., month vs year), this will be incorrect.
Would you like me to extend this to assert newPrice.recurring?.interval === currentItem.price.recurring?.interval (and factor interval_count) and throw if mismatched?
| const updatedSubscription = await stripe.subscriptions.update(subscriptionId, { | ||
| items: [ | ||
| { | ||
| id: subscriptionItemId, | ||
| price: priceId, | ||
| }, | ||
| ], | ||
| // We don't want to prorate the price difference because it would be based on time remaining in the current period | ||
| proration_behavior: 'none', | ||
| }); | ||
|
|
||
| const newItem = | ||
| updatedSubscription.items.data.find((i) => i.id === subscriptionItemId) | ||
| ?? updatedSubscription.items.data[0]; | ||
| if (!newItem) { | ||
| throw new Error('Subscription item not found on updated subscription'); | ||
| } | ||
| const newPriceAmount = newItem.price?.unit_amount; | ||
| if (newPriceAmount == null) { | ||
| throw new Error('New price amount not found'); | ||
| } | ||
|
|
||
| const quantity = newItem.quantity ?? 1; | ||
| const priceDifferenceAmount = (newPriceAmount - currentPriceAmount) * quantity; | ||
|
|
||
| // Create a one-off invoice item for the price difference if the new price is higher | ||
| if (priceDifferenceAmount > 0) { | ||
| await stripe.invoiceItems.create({ | ||
| customer: updatedSubscription.customer as string, | ||
| amount: priceDifferenceAmount, | ||
| description: 'Onlook subscription upgrade', | ||
| }); | ||
|
|
||
| // Create invoice immediately | ||
| const invoice = await stripe.invoices.create({ | ||
| customer: updatedSubscription.customer as string, | ||
| auto_advance: true, | ||
| }); | ||
|
|
||
| if (!invoice.id) { | ||
| throw new Error('Invoice not created'); | ||
| } | ||
| await stripe.invoices.pay(invoice.id); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent free upgrades on payment failure: charge first (with currency), then apply the subscription update
Currently the subscription is upgraded (Line 142) before charging the upgrade delta. If invoice creation or payment fails, the customer can receive a free upgrade. Also, the invoice item is created without a currency, which Stripe requires when specifying an amount.
Refactor to:
- Compute the delta using the new Price object (no update yet).
- Validate currency and include it when creating the invoice item.
- Create and pay the invoice first; only then update the subscription.
- Keep proration_behavior: 'none' as intended.
Apply this diff to the function body to fix ordering and currency handling:
- const updatedSubscription = await stripe.subscriptions.update(subscriptionId, {
- items: [
- {
- id: subscriptionItemId,
- price: priceId,
- },
- ],
- // We don't want to prorate the price difference because it would be based on time remaining in the current period
- proration_behavior: 'none',
- });
-
- const newItem =
- updatedSubscription.items.data.find((i) => i.id === subscriptionItemId)
- ?? updatedSubscription.items.data[0];
- if (!newItem) {
- throw new Error('Subscription item not found on updated subscription');
- }
- const newPriceAmount = newItem.price?.unit_amount;
- if (newPriceAmount == null) {
- throw new Error('New price amount not found');
- }
-
- const quantity = newItem.quantity ?? 1;
- const priceDifferenceAmount = (newPriceAmount - currentPriceAmount) * quantity;
-
- // Create a one-off invoice item for the price difference if the new price is higher
- if (priceDifferenceAmount > 0) {
- await stripe.invoiceItems.create({
- customer: updatedSubscription.customer as string,
- amount: priceDifferenceAmount,
- description: 'Onlook subscription upgrade',
- });
-
- // Create invoice immediately
- const invoice = await stripe.invoices.create({
- customer: updatedSubscription.customer as string,
- auto_advance: true,
- });
-
- if (!invoice.id) {
- throw new Error('Invoice not created');
- }
- await stripe.invoices.pay(invoice.id);
- }
-
- return updatedSubscription;
+ // Compute delta using the target Price (no subscription change yet)
+ const newPrice = await stripe.prices.retrieve(priceId);
+ const newPriceAmount = newPrice.unit_amount;
+ if (newPriceAmount == null) {
+ throw new Error('New price amount not found');
+ }
+
+ const quantity = currentItem.quantity ?? 1;
+ const currentCurrency = currentItem.price.currency;
+ const newCurrency = newPrice.currency;
+ if (currentCurrency && newCurrency && currentCurrency !== newCurrency) {
+ throw new Error('Cannot upgrade across different currencies');
+ }
+ const currency = newCurrency ?? currentCurrency;
+ if (!currency) {
+ throw new Error('Currency not found on price');
+ }
+
+ const priceDifferenceAmount = (newPriceAmount - currentPriceAmount) * quantity;
+
+ // Charge the delta first (if positive)
+ if (priceDifferenceAmount > 0) {
+ await stripe.invoiceItems.create({
+ customer: currentSubscription.customer as string,
+ amount: priceDifferenceAmount,
+ currency,
+ description: 'Onlook subscription upgrade',
+ });
+
+ const invoice = await stripe.invoices.create({
+ customer: currentSubscription.customer as string,
+ auto_advance: true,
+ collection_method: 'charge_automatically',
+ });
+ if (!invoice.id) {
+ throw new Error('Invoice not created');
+ }
+ await stripe.invoices.pay(invoice.id);
+ }
+
+ // Only after successful payment do we apply the subscription update without proration
+ const updatedSubscription = await stripe.subscriptions.update(subscriptionId, {
+ items: [
+ {
+ id: subscriptionItemId,
+ price: priceId,
+ },
+ ],
+ proration_behavior: 'none',
+ });
+
+ return updatedSubscription;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const updatedSubscription = await stripe.subscriptions.update(subscriptionId, { | |
| items: [ | |
| { | |
| id: subscriptionItemId, | |
| price: priceId, | |
| }, | |
| ], | |
| // We don't want to prorate the price difference because it would be based on time remaining in the current period | |
| proration_behavior: 'none', | |
| }); | |
| const newItem = | |
| updatedSubscription.items.data.find((i) => i.id === subscriptionItemId) | |
| ?? updatedSubscription.items.data[0]; | |
| if (!newItem) { | |
| throw new Error('Subscription item not found on updated subscription'); | |
| } | |
| const newPriceAmount = newItem.price?.unit_amount; | |
| if (newPriceAmount == null) { | |
| throw new Error('New price amount not found'); | |
| } | |
| const quantity = newItem.quantity ?? 1; | |
| const priceDifferenceAmount = (newPriceAmount - currentPriceAmount) * quantity; | |
| // Create a one-off invoice item for the price difference if the new price is higher | |
| if (priceDifferenceAmount > 0) { | |
| await stripe.invoiceItems.create({ | |
| customer: updatedSubscription.customer as string, | |
| amount: priceDifferenceAmount, | |
| description: 'Onlook subscription upgrade', | |
| }); | |
| // Create invoice immediately | |
| const invoice = await stripe.invoices.create({ | |
| customer: updatedSubscription.customer as string, | |
| auto_advance: true, | |
| }); | |
| if (!invoice.id) { | |
| throw new Error('Invoice not created'); | |
| } | |
| await stripe.invoices.pay(invoice.id); | |
| } | |
| // Compute delta using the target Price (no subscription change yet) | |
| const newPrice = await stripe.prices.retrieve(priceId); | |
| const newPriceAmount = newPrice.unit_amount; | |
| if (newPriceAmount == null) { | |
| throw new Error('New price amount not found'); | |
| } | |
| const quantity = currentItem.quantity ?? 1; | |
| const currentCurrency = currentItem.price.currency; | |
| const newCurrency = newPrice.currency; | |
| if (currentCurrency && newCurrency && currentCurrency !== newCurrency) { | |
| throw new Error('Cannot upgrade across different currencies'); | |
| } | |
| const currency = newCurrency ?? currentCurrency; | |
| if (!currency) { | |
| throw new Error('Currency not found on price'); | |
| } | |
| const priceDifferenceAmount = (newPriceAmount - currentPriceAmount) * quantity; | |
| // Charge the delta first (if positive) | |
| if (priceDifferenceAmount > 0) { | |
| await stripe.invoiceItems.create({ | |
| customer: currentSubscription.customer as string, | |
| amount: priceDifferenceAmount, | |
| currency, | |
| description: 'Onlook subscription upgrade', | |
| }); | |
| const invoice = await stripe.invoices.create({ | |
| customer: currentSubscription.customer as string, | |
| auto_advance: true, | |
| collection_method: 'charge_automatically', | |
| }); | |
| if (!invoice.id) { | |
| throw new Error('Invoice not created'); | |
| } | |
| await stripe.invoices.pay(invoice.id); | |
| } | |
| // Only after successful payment do we apply the subscription update without proration | |
| const updatedSubscription = await stripe.subscriptions.update(subscriptionId, { | |
| items: [ | |
| { | |
| id: subscriptionItemId, | |
| price: priceId, | |
| }, | |
| ], | |
| proration_behavior: 'none', | |
| }); | |
| return updatedSubscription; |
| await stripe.invoiceItems.create({ | ||
| customer: updatedSubscription.customer as string, | ||
| amount: priceDifferenceAmount, | ||
| description: 'Onlook subscription upgrade', | ||
| }); |
There was a problem hiding this comment.
Missing currency on invoice item will cause a Stripe error
When creating invoice items with an explicit amount, Stripe requires a currency. If you don’t adopt the larger refactor above, minimally include the currency from the item’s price.
Apply this minimal diff:
await stripe.invoiceItems.create({
customer: updatedSubscription.customer as string,
amount: priceDifferenceAmount,
+ currency: newItem.price?.currency || currentItem.price?.currency || 'usd',
description: 'Onlook subscription upgrade',
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/stripe/src/functions.ts around lines 169 to 173, the
invoiceItems.create call is missing the required currency when supplying an
explicit amount; add a currency property using the currency from the relevant
Price object (for example currency: item.price.currency or currency:
price.currency — whichever variable you used to compute priceDifferenceAmount),
or extract it from updatedSubscription items if needed, and pass that currency
into the invoiceItems.create payload.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Introduces
upgradeSubscriptionto handle subscription upgrades without proration and modifies Next.js config to ignore build errors.upgradeSubscriptioninfunctions.tsto handle subscription upgrades without proration.updateSubscriptionwithupgradeSubscriptioninsubscription.tsfor immediate billing of price differences.next.config.mjsto ignore TypeScript and ESLint errors during builds.upgradeSubscription.This description was created by
for af957d7. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores