[PM-39925] feat: add the invoice preview projection to Bit.Invoicing - #8209
[PM-39925] feat: add the invoice preview projection to Bit.Invoicing#8209kdenney wants to merge 39 commits into
Conversation
kdenney
added
t:feature
🤖 Bitwarden Claude Code ReviewOverall Assessment: REQUEST CHANGES Reviewed the invoice-preview projection at commit Code Review Details
Dependency Changes
Not a net-new dependency — |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## billing/PM-39925/invoice-preview-scaffolding #8209 +/- ##
===============================================================================
Coverage ? 63.41%
===============================================================================
Files ? 2422
Lines ? 104438
Branches ? 9484
===============================================================================
Hits ? 66225
Misses ? 35941
Partials ? 2272 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
925c754 to
3474e23
Compare
3474e23 to
73a92e9
Compare
73a92e9 to
785c017
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
785c017 to
1605dea
Compare
Claude Code validationResult: Pass Validated one changed file —
CriticalNone. Major
Minor
Verified clean
Checks run
The three script checks ( Verdict rationale: |
1605dea to
1fa9ea1
Compare
1fa9ea1 to
3a77505
Compare
amorask-bitwarden
left a comment
amorask-bitwarden
left a comment
There was a problem hiding this comment.
Just a few ⛏️ items. Please ensure the correct purchasable_reference metadata is applied to the Stripe prices for Test / Prod as well.
…to lineItemsByReference
Address the codecov patch-coverage gaps and the review comment on the proration tax doc: - DiscountMapper: a cart-wide coupon Stripe echoes onto a line stays cart-level and is not attached as an item-level discount - InvoicePreviewBuilder: the subscription path's unplaceable-item (still counts toward the total) and duplicate-reference branches - PurchasableReferences: ProductOf returns null for an unknown reference - SubscriptionPreview/PendingSubscriptionChange serialization envelope Also fix the PurchasableProration.Tax doc to describe the sum of Stripe's per-line tax rather than a proportional share of the invoice tax total.
- Document the per-status contract for SubscriptionPreview's conditionally populated fields (CancelAt, Canceled, Suspension, GracePeriod). - Throw on a duplicate purchasable reference in both Build overloads, matching the missing-PM-seat behavior; both signal a misconfigured subscription. - Remove the unreachable null-product arm in the proration switch; a reference that passes IsKnown always maps to a product.
…e preview A mid-cycle Secrets Manager removal produces an upcoming invoice with an sm-seat proration credit but no recurring sm-seat line. BuildSecretsManagerItems keyed off the recurring line, so it returned null and dropped the summarized proration bucket while Total/AmountDue still carried the credit -- the cart's visible rows no longer summed to its stated total. Make SecretsManagerInvoiceItems.Seats optional and build the section whenever a seats line or a proration bucket is present; return null only when both are absent. The Password Manager side keeps throwing on a missing seats line, which is a Stripe misconfiguration since PM seats are always present. Verified live against Stripe create_preview (2026-06-24.dahlia); covered by InvoicePreviewBuilderSmRemovalTests.
…d in self-host Bit.Invoicing is cloud-only. Instead of leaving IInvoicePreviewService resolvable everywhere, register it through a factory that throws when IBitwardenEnvironment.SelfHosted is true, so a self-host code path that wrongly depends on it fails with a clear, intentional error rather than a generic DI failure or silent misbehavior. - Register the concrete InvoicePreviewService and resolve it from the guard factory for IInvoicePreviewService (both remain the same singleton). - The internal client and builder are only reachable through the service, so the guard covers them transitively; no separate guards needed. - Add a test asserting resolution throws in self-host and returns the service in cloud.
… cost The subscription Build overload computed item cost from Price.UnitAmount, which Stripe leaves null for fractional-cent per-unit prices. The `?? 0` fallback then zeroed that item, understating the total the adjacent comment promises is never understated. Read UnitAmountDecimal instead, matching GetBitwardenSubscriptionQuery and ProviderBillingController. Add a regression test for a fractional-cent price (unit_amount absent, unit_amount_decimal set) and give the existing subscription fixtures the unit_amount_decimal Stripe returns alongside unit_amount.
ResolveInvoiceDiscounts added total_discount_amounts entries by DiscountId without checking for null, so a discount with no id would throw ArgumentNullException and fail the entire preview with a 500 — unlike the line loop, which already skips empty DiscountIds, and unlike every other malformed-data case in the mapper, which logs and drops. Guard the null id (log and skip) and switch the add to an indexer so a duplicate id is also non-fatal, keeping the whole mapper consistent with its log-and-skip design.
…items
InvoicePreviewItem.Cost was the Stripe line amount (quantity x unit),
but the client cart item treats an item's Cost as the per-unit price,
matching the existing CartItem contract ("The unit-cost of the cart
item") which is built from Price.UnitAmountDecimal. The mismatch meant
the invoice-preview cart would show a line total where the client
expected a unit price.
Read the per-unit amount from Price.UnitAmountDecimal in both Build
paths so Cost is consistently a unit price; the subscription path still
sums quantity x unit into the envelope Total. Tests updated, including
the SM-removal reconciliation checks which now multiply Cost by Quantity.
a72b6a3 to
72c51a8
Compare
| // 30-day months, minimum one, matching the legacy proration display. | ||
| var days = (lineEnd.Value - invoice.PeriodEnd).TotalDays; |
There was a problem hiding this comment.
Months collapses to 1 on the invoice shape this PR captured live, because lineEnd == invoice.PeriodEnd there.
Details and fix
lineEnd - invoice.PeriodEnd only measures the prorated span when the preview is an immediate proration invoice — i.e. when proration_behavior = always_invoice makes invoice.period_end equal "now". That is exactly the assumption the legacy code documents:
// Use invoice periodEnd here instead of UtcNow ... the previewInvoice's periodEnd is the
// same as UtcNow anyway because of the proration behavior (always_invoice)
NewPlanProratedMonths = CalculateNewPlanProratedMonths(invoicePreview.PeriodEnd, passwordManagerItem.CurrentPeriodEnd)(src/Core/Billing/Premium/Commands/PreviewPremiumUpgradeProrationCommand.cs:130-132, which sets ProrationBehavior = AlwaysInvoice on its own options.)
IInvoicePreviewService takes caller-supplied InvoiceCreatePreviewOptions, so it does not control the proration behavior. With the default create_prorations, the preview is the next scheduled invoice and invoice.period_end is the current period end — the same instant the proration line ends. That is the shape in this PR's own live-captured fixture:
"period_end": 1789769920,
"lines": { "data": [
{ "amount": -1548, ..., "period": { "start": 1788387520, "end": 1789769920 } },(test/Libraries/Invoicing.Test/InvoicePreviewBuilderSmRemovalTests.cs:20-25)
1789769920 - 1789769920 = 0 days → Math.Max(1, 0) → Months = 1. For a monthly plan that happens to be right; for an annual mid-cycle change with 7 months left it would still render 1. ProrationMapperTests only exercises synthetic invoices where the line end is later than the invoice end, so nothing catches this, and InvoicePreviewBuilderSmRemovalTests does not assert Months.
The proration line already carries the span, so reading it is shape-independent — it equals the remaining term under both always_invoice and create_prorations:
var period = lines.Select(line => line.Period).FirstOrDefault(p => p is not null);
if (period is null)
{
return 0;
}
// 30-day months, minimum one, matching the legacy proration display.
var days = (period.End - period.Start).TotalDays;
return Math.Max(1, (int)Math.Round(days / 30, MidpointRounding.AwayFromZero));A test asserting Months on the in_preview_sm_removal fixture would lock the behavior in either way.
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-39925
📔 Objective
Fills in
Bit.Invoicingwith the invoice-preview projection: it fetches an upcoming Stripe invoice (or reads a subscription's current items when there is no upcoming invoice, such as a canceled or suspended subscription) and projects it into a vendor-neutralInvoicePreviewrecord family for the cart screens to render.IInvoicePreviewService; the builder, mappers, reference table, and Stripe client are internal, registered withTryAddSingleton.purchasable_referencevalue on the Stripe price metadata (pm-seat,pm-storage,sm-seat,sm-service-account), routed through a central reference-to-product table. There is deliberately no fallback toStripe.Price.Id: an unresolved or unknown reference is logged and skipped, and a missing required Password Manager seats line throws before the preview is built.DiscountMappersplits coupons into cart-level and item-level buckets, matching item-scoped coupons onto their lines byDiscountId(the line-level discount object is unexpanded on real Stripe responses, so only the id is reliable). Unresolved or unattached coupons are logged rather than dropped silently.ProrationMapperfolds each product's proration lines into a single credit, charge, and total row.100m, never integer100.purchasable_referencemetadata key and its reference values to Core'sStripeConstants, and covers the projection with tests built on deserialized, production-shaped Stripe JSON rather than hand-built object graphs.Deliberate divergences from the technical breakdown:
invoice.TotalTaxes). This projection instead sums Stripe's own per-line tax (InvoiceLineItem.Taxes) for the bucket. The proportional formula divided a pre-tax numerator (the line amount, which excludes tax) by a tax-inclusive denominator (invoice.Total), so it understated the tax whenever the invoice carried any; summing the tax Stripe has already computed also honors the breakdown's own rule that totals, tax, and discounts come straight from Stripe with no manual server-side tax calculation.InvoicePreviewDiscountrecord rather than extending Core'sBitwardenDiscount. A required appliedAmountwould breakBitwardenDiscount's two implicit Stripe operators and its existing assignment sites, the projection never uses those operators, and the two paths disagree on units (the legacy value is cents, the projection's is dollars).InvoicePreviewOptions. The publicIInvoicePreviewServicetakes Stripe types (InvoiceCreatePreviewOptions,Subscription) directly.Bit.Invoicingis itself the Stripe boundary and is permitted to reference Stripe types, so a domain-options wrapper would protect no boundary (the breakdown contradicts itself on this point). The boundary the READMEs enforce is behavioral: consumers must not call Stripe, but passing Stripe types across the surface is allowed.StripeExceptionis not wrapped in this library. Vendor-exception-to-domain translation belongs to the futureBit.Integrations.Billing; the endpoint groups' exception handling already logs server-side and returns a generic 500, so no raw Stripe detail leaks.Note on the 5-level expand.
lines.data.pricing.price_details.pricelooks like it exceeds Stripe's documented 4-level expand limit, but it doesn't:.datalist accessors and inline sub-hashes (pricing,price_details) don't count as levels. Verified live againstcreate_previewAPI — the expand returns the full price object, and Stripe only rejects at 7 segments (…price.product.default_price).This branch is stacked on the scaffolding PR; its base is
billing/PM-39925/invoice-preview-scaffolding, which should be reviewed and merged first.Stack created with GitHub Stacks CLI • Give Feedback 💬