Add license discount support - #2313
Conversation
Apply product-scoped discounts to license attach and update flows, preserve discounted stored credits, and cover discount composition and lifecycle behavior.
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
| if (usableRows.length === 0) { | ||
| ctx.logger.warn( | ||
| `[storedInvoiceCreditForPrice] No usable stored charge row for cusProduct=${customerProduct.id} price=${price.id}; falling back to catalog synthesis`, | ||
| ); | ||
| return { lineItems: [], resolved: false }; |
There was a problem hiding this comment.
resolved: true with empty lineItems silently drops catalog fallback
When usable charge rows exist but every one of them hits the action.type === "skip" branch from augmentBillingContextForAnchorResetRefund, the loop finishes with lineItems = [] and returns { lineItems: [], resolved: true }. The caller in licenseInvoiceCreditFromStoredLineItems interprets resolved: true as "stored data is authoritative" and returns [] instead of [catalogCredit]. If the catalog path would have produced a non-zero credit (e.g., it doesn't pass through the same skip gate), the customer silently receives no refund for that license seat.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/billing/v2/utils/lineItems/storedInvoiceCreditForPrice.ts
Line: 58-62
Comment:
**`resolved: true` with empty `lineItems` silently drops catalog fallback**
When usable charge rows exist but every one of them hits the `action.type === "skip"` branch from `augmentBillingContextForAnchorResetRefund`, the loop finishes with `lineItems = []` and returns `{ lineItems: [], resolved: true }`. The caller in `licenseInvoiceCreditFromStoredLineItems` interprets `resolved: true` as "stored data is authoritative" and returns `[]` instead of `[catalogCredit]`. If the catalog path would have produced a non-zero credit (e.g., it doesn't pass through the same skip gate), the customer silently receives no refund for that license seat.
How can I resolve this? If you propose a fix, please make it concise.| return discountAmounts.map((da) => { | ||
| const discount = da.discount; | ||
| const discount = discountsById.get(da.discount.id) ?? da.discount; | ||
| const coupon = discount.source?.coupon; |
There was a problem hiding this comment.
percent_off silently dropped for discounts not in enriched map
If a discount ID appears in discount_amounts but is absent from both stripeDiscounts (invoice level) and stripeLineItem.discounts (e.g., a deleted/expired discount on a historical invoice), the fallback da.discount is used. That object comes from the "data.discount_amounts.discount" expansion, which does not include source.coupon, so discount.source?.coupon resolves to undefined and percentOff is silently stored as undefined. Adding a debug log makes the miss observable.
| return discountAmounts.map((da) => { | |
| const discount = da.discount; | |
| const discount = discountsById.get(da.discount.id) ?? da.discount; | |
| const coupon = discount.source?.coupon; | |
| return discountAmounts.map((da) => { | |
| const enriched = discountsById.get(da.discount.id); | |
| if (!enriched) { | |
| console.debug( | |
| `[stripeDiscountsToDbDiscounts] discount ${da.discount.id} not found in enriched map; percent_off will be missing`, | |
| ); | |
| } | |
| const discount = enriched ?? da.discount; | |
| const coupon = discount.source?.coupon; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/billing/v2/providers/stripe/utils/invoiceLines/convertToDbLineItem/stripeDiscountsToDbDiscounts.ts
Line: 31-33
Comment:
**`percent_off` silently dropped for discounts not in enriched map**
If a discount ID appears in `discount_amounts` but is absent from both `stripeDiscounts` (invoice level) and `stripeLineItem.discounts` (e.g., a deleted/expired discount on a historical invoice), the fallback `da.discount` is used. That object comes from the `"data.discount_amounts.discount"` expansion, which does not include `source.coupon`, so `discount.source?.coupon` resolves to `undefined` and `percentOff` is silently stored as `undefined`. Adding a debug log makes the miss observable.
```suggestion
return discountAmounts.map((da) => {
const enriched = discountsById.get(da.discount.id);
if (!enriched) {
console.debug(
`[stripeDiscountsToDbDiscounts] discount ${da.discount.id} not found in enriched map; percent_off will be missing`,
);
}
const discount = enriched ?? da.discount;
const coupon = discount.source?.coupon;
```
How can I resolve this? If you propose a fix, please make it concise.| const stripeDiscounts = stripeLineItems.some( | ||
| (lineItem) => (lineItem.discount_amounts?.length ?? 0) > 0, | ||
| ) | ||
| ? ( | ||
| await getStripeInvoice({ | ||
| stripeClient: stripeCli, | ||
| invoiceId: stripeInvoiceId, | ||
| expand: ["discounts.source.coupon"], | ||
| }) | ||
| ).discounts | ||
| : []; |
There was a problem hiding this comment.
Extra Stripe API call may be unnecessary after line-item expand change
getStripeInvoiceLineItems now expands "data.discounts.source.coupon", which should already include every discount object (including subscription-level ones) that applies to each line item. The invoice-level getStripeInvoice fetch is triggered whenever any line item has discount_amounts, even if all those discounts are already fully expanded in stripeLineItem.discounts. In practice this adds an extra API round-trip per invoiced payment that carries any discount. Consider checking whether stripeLineItem.discounts already covers all IDs referenced in discount_amounts before making the fetch, or document why the separate fetch remains necessary despite the newer expansion.
Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/internal/billing/v2/workflows/storeInvoiceLineItems/storeInvoiceLineItems.ts
Line: 49-59
Comment:
**Extra Stripe API call may be unnecessary after line-item expand change**
`getStripeInvoiceLineItems` now expands `"data.discounts.source.coupon"`, which should already include every discount object (including subscription-level ones) that applies to each line item. The invoice-level `getStripeInvoice` fetch is triggered whenever any line item has `discount_amounts`, even if all those discounts are already fully expanded in `stripeLineItem.discounts`. In practice this adds an extra API round-trip per invoiced payment that carries any discount. Consider checking whether `stripeLineItem.discounts` already covers all IDs referenced in `discount_amounts` before making the fetch, or document why the separate fetch remains necessary despite the newer expansion.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
2 issues found across 25 files
Confidence score: 2/5
- In
server/src/internal/billing/v2/utils/lineItems/storedInvoiceCreditForPrice.ts,currentPeriodRefundsis not filtered by the target product, so refunds for one license type can incorrectly consume credit for another that shares price/period, leading to under- or mis-credited refunds — scope refunds by both price/period and the supplied product before merging. - In
server/src/internal/billing/v2/utils/lineItems/getRefundLineItems.ts, if any parent price is missing a stored row the flow falls back to catalog regeneration and can credit discounted licenses at undiscounted amounts, creating incorrect refund totals and financial inconsistency — preserve stored-license credit behavior when partial stored data is missing (or fail safely) and add a regression test for discounted-license refunds.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/internal/billing/v2/utils/lineItems/storedInvoiceCreditForPrice.ts">
<violation number="1" location="server/src/internal/billing/v2/utils/lineItems/storedInvoiceCreditForPrice.ts:65">
P1: Refunding one license type can reduce or eliminate the credit for another license type that shares its price and billing period. Filter `currentPeriodRefunds` by the supplied target product too, matching the charge-row isolation.</violation>
</file>
<file name="server/src/internal/billing/v2/utils/lineItems/getRefundLineItems.ts">
<violation number="1" location="server/src/internal/billing/v2/utils/lineItems/getRefundLineItems.ts:34">
P2: A missing stored row for any parent price makes this refund discard the stored license credit and regenerate the license from catalog state, so a discounted license can again be credited at its undiscounted amount. Preserve `licenseCredits` in the fallback path while excluding the corresponding synthesized license entries.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const currentPeriodRefunds = refundRows.filter( | ||
| (row) => | ||
| row.customer_product_ids.includes(customerProduct.id) && | ||
| row.effective_period_end != null && | ||
| row.effective_period_start != null && | ||
| row.effective_period_start <= now && | ||
| row.effective_period_end > now, | ||
| ); |
There was a problem hiding this comment.
P1: Refunding one license type can reduce or eliminate the credit for another license type that shares its price and billing period. Filter currentPeriodRefunds by the supplied target product too, matching the charge-row isolation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/billing/v2/utils/lineItems/storedInvoiceCreditForPrice.ts, line 65:
<comment>Refunding one license type can reduce or eliminate the credit for another license type that shares its price and billing period. Filter `currentPeriodRefunds` by the supplied target product too, matching the charge-row isolation.</comment>
<file context>
@@ -0,0 +1,116 @@
+ return { lineItems: [], resolved: false };
+ }
+
+ const currentPeriodRefunds = refundRows.filter(
+ (row) =>
+ row.customer_product_ids.includes(customerProduct.id) &&
</file context>
| const currentPeriodRefunds = refundRows.filter( | |
| (row) => | |
| row.customer_product_ids.includes(customerProduct.id) && | |
| row.effective_period_end != null && | |
| row.effective_period_start != null && | |
| row.effective_period_start <= now && | |
| row.effective_period_end > now, | |
| ); | |
| const currentPeriodRefunds = refundRows.filter( | |
| (row) => | |
| row.customer_product_ids.includes(customerProduct.id) && | |
| (!product || | |
| row.internal_product_id === product.internal_id || | |
| row.product_id === product.id) && | |
| row.effective_period_end != null && | |
| row.effective_period_start != null && | |
| row.effective_period_start <= now && | |
| row.effective_period_end > now, | |
| ); |
| const licenseCredits = (customerProduct.customer_licenses ?? []).flatMap( | ||
| (customerLicense) => | ||
| customerLicenseToLineItems({ | ||
| licenseInvoiceCreditFromStoredLineItems({ |
There was a problem hiding this comment.
P2: A missing stored row for any parent price makes this refund discard the stored license credit and regenerate the license from catalog state, so a discounted license can again be credited at its undiscounted amount. Preserve licenseCredits in the fallback path while excluding the corresponding synthesized license entries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/internal/billing/v2/utils/lineItems/getRefundLineItems.ts, line 34:
<comment>A missing stored row for any parent price makes this refund discard the stored license credit and regenerate the license from catalog state, so a discounted license can again be credited at its undiscounted amount. Preserve `licenseCredits` in the fallback path while excluding the corresponding synthesized license entries.</comment>
<file context>
@@ -29,16 +29,13 @@ export const getRefundLineItems = ({
const licenseCredits = (customerProduct.customer_licenses ?? []).flatMap(
(customerLicense) =>
- customerLicenseToLineItems({
+ licenseInvoiceCreditFromStoredLineItems({
ctx,
billingContext,
</file context>
Prevents re-discounting finalized stored credits, tightens discount regression coverage, and preserves imported Stripe price IDs on custom license prices.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
There was a problem hiding this comment.
1 issue found across 19 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="shared/models/productV2Models/productItemModels/productItemModels.ts">
<violation number="1" location="shared/models/productV2Models/productItemModels/productItemModels.ts:202">
P1: Imported custom/base-price Stripe IDs are still dropped: this field never reaches the rebuilt `FixedPriceConfig`, despite the comment saying it does. Preserve `item.stripe_price_id` in `toPrice` (and round-trip mapper where applicable) so custom license prices reuse the imported Stripe price.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }), | ||
| /** Set only by producers that know the item corresponds to an existing | ||
| * Stripe price of the same shape. Flows into Price.config when rebuilt. */ | ||
| stripe_price_id: z.string().nullish().meta({ |
There was a problem hiding this comment.
P1: Imported custom/base-price Stripe IDs are still dropped: this field never reaches the rebuilt FixedPriceConfig, despite the comment saying it does. Preserve item.stripe_price_id in toPrice (and round-trip mapper where applicable) so custom license prices reuse the imported Stripe price.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At shared/models/productV2Models/productItemModels/productItemModels.ts, line 202:
<comment>Imported custom/base-price Stripe IDs are still dropped: this field never reaches the rebuilt `FixedPriceConfig`, despite the comment saying it does. Preserve `item.stripe_price_id` in `toPrice` (and round-trip mapper where applicable) so custom license prices reuse the imported Stripe price.</comment>
<file context>
@@ -197,12 +197,19 @@ export const ProductItemSchema = z.object({
}),
+ /** Set only by producers that know the item corresponds to an existing
+ * Stripe price of the same shape. Flows into Price.config when rebuilt. */
+ stripe_price_id: z.string().nullish().meta({
+ internal: true,
+ }),
</file context>
Summary
Validation
bunx tsgo --build --noEmitSummary by cubic
Adds product-scoped, stacked discounts to license attach/update and uses stored, already-discounted invoice rows for proration credits so previews, invoices, and Stripe stay in sync. Also prevents double-discounting finalized credits and preserves Stripe price IDs on custom license prices for reliable back-sync.
New Features
percent_offby expanding couponapplies_to, merging invoice- and line-level discounts, and ordering update/checkout discount params to match preview.Bug Fixes
amountAfterDiscountsFinalized); refunds use the net amount actually paid.stripe_price_idon custom license prices and during license back-sync for accurate product scoping.Written for commit 015f0e6. Summary will update on new commits.
Greptile Summary
This PR introduces product-scoped and stacked license discounts across attach and subscription-update flows, preserves discounted stored invoice credits during refunds and upgrades, and persists Stripe
percent_offdata on stored line items.storedInvoiceCreditForPriceandlicenseInvoiceCreditFromStoredLineItemshelpers, refactoring duplicated stored-credit logic; orders percent discounts before fixed discounts in Stripe params to match preview math; addsamountAfterDiscountsFinalizedflag so finalized credits are never re-discounted.stripe_price_idthrough theProductItem → BasePriceParams → Pricepipeline (fixing silent loss during license back-sync); stores couponpercent_offonDbInvoiceLineItem.discountsby enriching the discount lookup map with invoice-level coupon data; prevents discount re-application on stored refund credits via the newamountAfterDiscountsFinalizedguard indiscountAppliesToLineItem.stripe_price_idtoProductItemSchemaandamountAfterDiscountsFinalizedtoLineItemSchema; widenstoBasePriceParamsto acceptBasePriceParamsdirectly so internal IDs flow through plan-diff paths.Confidence Score: 4/5
The refund credit path is safe for the happy path but has a gap when all stored rows are anchor-skipped or yield zero credit: the price is marked as resolved so the catalog fallback is suppressed, leaving the customer with no refund for that period.
The stored-credit abstraction returns resolved=true even when every usable charge row is skipped by the anchor-reset gate or produces a zero credit amount. Both callers treat resolved=true as authoritative and suppress the catalog fallback, so an anchor-reset scenario during a discounted license period can silently drop the refund entirely. The rest of the change — discount ordering, finalized-flag guard, stripe_price_id propagation, and percent_off storage — is well-structured and comprehensively tested.
Files Needing Attention: server/src/internal/billing/v2/utils/lineItems/storedInvoiceCreditForPrice.ts — the resolved=true / empty lineItems case; server/src/internal/billing/v2/utils/lineItems/licenseInvoiceCreditFromStoredLineItems.ts — inherits the same gap from the shared helper.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[getRefundLineItems] --> B[invoiceCreditFromStoredLineItems] A --> C[licenseInvoiceCreditFromStoredLineItems] C --> D[customerLicenseToLineItems - catalog credits] D --> E{for each catalogCredit} E --> F[storedInvoiceCreditForPrice] F --> G{resolved?} G -->|yes and non-empty| H[return stored credits with finalized flag] G -->|yes but empty - all skipped| I[catalog fallback silently dropped] G -->|no| J[return catalogCredit as fallback] B --> K[storedInvoiceCreditForPrice per cusPrice] K --> L{usable rows found?} L -->|no| M[resolved=false, anyMissed=true] L -->|yes| N{all rows skipped or zero credit?} N -->|yes| O[resolved=true with empty lineItems] N -->|no| P[chargeRowToRefundLineItem with finalized flag] P --> Q[discountAppliesToLineItem] Q --> R{amountAfterDiscountsFinalized set?} R -->|true| S[skip discount - no re-application] R -->|false| T[apply normal applies-to check]Reviews (2): Last reviewed commit: "Merge branch 'dev' into feat/licenses-di..." | Re-trigger Greptile
Context used (3)