From e5229d8be3cd351e6f69f7900d802f6bd5648a6e Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Mon, 1 Jun 2026 16:42:11 +0200 Subject: [PATCH 1/7] fix(billing): merge tax-code Stripe+ID atomically and refresh tax-config snapshot at sync --- api/v3/handlers/billingprofiles/convert.go | 1 - openmeter/billing/adapter/gatheringlines.go | 4 +- .../billing/adapter/stdinvoicelinemapper.go | 16 +- openmeter/billing/gatheringinvoice.go | 4 +- openmeter/billing/httpdriver/invoiceline.go | 12 +- .../billing/service/invoicecalc/taxconfig.go | 3 +- .../service/invoicecalc/taxconfig_test.go | 108 +++++++++++- openmeter/billing/service/stdinvoicestate.go | 33 ++++ openmeter/billing/stdinvoiceline.go | 8 +- openmeter/billing/taxconfig.go | 159 ++++++++++++++++++ .../service/sync_credittheninvoice_test.go | 2 +- openmeter/ent/db/billinginvoiceline.go | 2 +- openmeter/ent/db/billinginvoiceline_create.go | 10 +- openmeter/ent/db/billinginvoiceline_update.go | 8 +- openmeter/ent/db/entmixinaccessor.go | 2 +- openmeter/ent/db/mutation.go | 12 +- openmeter/ent/db/setorclear.go | 4 +- openmeter/ent/schema/billing.go | 2 +- openmeter/productcatalog/tax.go | 28 ++- openmeter/productcatalog/tax_test.go | 79 +++------ openmeter/taxcode/taxcode.go | 32 ++++ test/billing/taxcode_dual_write_test.go | 4 +- 22 files changed, 407 insertions(+), 126 deletions(-) create mode 100644 openmeter/billing/taxconfig.go diff --git a/api/v3/handlers/billingprofiles/convert.go b/api/v3/handlers/billingprofiles/convert.go index 23ca5830d8..1e355ccc85 100644 --- a/api/v3/handlers/billingprofiles/convert.go +++ b/api/v3/handlers/billingprofiles/convert.go @@ -69,7 +69,6 @@ var ( // goverter:ignore TaxCode ToAPIBillingTaxConfig func(config *productcatalog.TaxConfig) (*api.BillingTaxConfig, error) // goverter:map Stripe Stripe - // goverter:ignore TaxCode FromAPIBillingTaxConfig func(config *api.BillingTaxConfig) (*productcatalog.TaxConfig, error) ) diff --git a/openmeter/billing/adapter/gatheringlines.go b/openmeter/billing/adapter/gatheringlines.go index eb19f40be3..70bd8b483c 100644 --- a/openmeter/billing/adapter/gatheringlines.go +++ b/openmeter/billing/adapter/gatheringlines.go @@ -221,7 +221,7 @@ func (a *adapter) updateGatheringLines(ctx context.Context, lines billing.Gather } if line.TaxConfig != nil { - create = create.SetTaxConfig(*line.TaxConfig). + create = create.SetTaxConfig(*billing.FromProductCatalog(line.TaxConfig)). SetNillableTaxCodeID(line.TaxConfig.TaxCodeID). SetNillableTaxBehavior(line.TaxConfig.Behavior) } @@ -310,7 +310,7 @@ func (a *adapter) mapGatheringInvoiceLineFromDB(schemaLevel int, dbLine *db.Bill Currency: dbLine.Currency, TaxConfig: productcatalog.BackfillTaxConfig( - lo.EmptyableToPtr(dbLine.TaxConfig), + lo.EmptyableToPtr(dbLine.TaxConfig).ToProductCatalog(), dbLine.TaxBehavior, taxCodeFromInvoiceLineEdge(dbLine), ), diff --git a/openmeter/billing/adapter/stdinvoicelinemapper.go b/openmeter/billing/adapter/stdinvoicelinemapper.go index b70427a7bb..6ec5ccb75d 100644 --- a/openmeter/billing/adapter/stdinvoicelinemapper.go +++ b/openmeter/billing/adapter/stdinvoicelinemapper.go @@ -318,28 +318,28 @@ func taxCodeFromInvoiceLineEdge(dbLine *db.BillingInvoiceLine) *taxcode.TaxCode // TODO[later]: change the billing-facing types to expose TaxCodeSnapshot and TaxCodeReference // fields explicitly so it is obvious what is the immutable invoice snapshot and what is the live // reference to the tax entity. -func backfillTaxConfigReferences(snapshottedTaxConfig *productcatalog.TaxConfig, persistedTaxBehavior *productcatalog.TaxBehavior, resolvedTaxCode *taxcode.TaxCode) *productcatalog.TaxConfig { - backfilledTaxConfig := productcatalog.BackfillTaxConfig(snapshottedTaxConfig, persistedTaxBehavior, resolvedTaxCode) +func backfillTaxConfigReferences(snapshottedTaxConfig *billing.TaxConfig, persistedTaxBehavior *productcatalog.TaxBehavior, resolvedTaxCode *taxcode.TaxCode) *billing.TaxConfig { + backfilledTaxConfig := productcatalog.BackfillTaxConfig(snapshottedTaxConfig.ToProductCatalog(), persistedTaxBehavior, resolvedTaxCode) if backfilledTaxConfig == nil || resolvedTaxCode == nil { - return backfilledTaxConfig + return billing.FromProductCatalog(backfilledTaxConfig) } if backfilledTaxConfig.TaxCodeID != nil && *backfilledTaxConfig.TaxCodeID != resolvedTaxCode.ID { - return backfilledTaxConfig + return billing.FromProductCatalog(backfilledTaxConfig) } if backfilledTaxConfig.Stripe != nil { mapping, ok := resolvedTaxCode.GetAppMapping(app.AppTypeStripe) if !ok || mapping.TaxCode != backfilledTaxConfig.Stripe.Code { - return backfilledTaxConfig + return billing.FromProductCatalog(backfilledTaxConfig) } } - cloned := backfilledTaxConfig.Clone() - cloned.TaxCode = resolvedTaxCode + result := billing.FromProductCatalog(backfilledTaxConfig) + result.TaxCode = resolvedTaxCode - return &cloned + return result } func (a *adapter) mapStandardInvoiceDetailedLineAmountDiscountFromDB(dbDiscount *db.BillingStandardInvoiceDetailedLineAmountDiscount) (billing.AmountLineDiscountManaged, error) { diff --git a/openmeter/billing/gatheringinvoice.go b/openmeter/billing/gatheringinvoice.go index cf620d0bab..470e4fa42c 100644 --- a/openmeter/billing/gatheringinvoice.go +++ b/openmeter/billing/gatheringinvoice.go @@ -717,9 +717,9 @@ func (g GatheringLine) AsNewStandardLine(invoiceID string) (*StandardLine, error return nil, fmt.Errorf("cloning annotations: %w", err) } - var taxConfig *productcatalog.TaxConfig + var taxConfig *TaxConfig if g.TaxConfig != nil { - taxConfig = lo.ToPtr(g.TaxConfig.Clone()) + taxConfig = FromProductCatalog(lo.ToPtr(g.TaxConfig.Clone())) } var subscription *SubscriptionReference diff --git a/openmeter/billing/httpdriver/invoiceline.go b/openmeter/billing/httpdriver/invoiceline.go index 72d5c705c0..8e24b2676b 100644 --- a/openmeter/billing/httpdriver/invoiceline.go +++ b/openmeter/billing/httpdriver/invoiceline.go @@ -160,7 +160,7 @@ func mapCreateLineToEntity(line api.InvoicePendingLineCreate, ns string) (*billi }, InvoiceAt: line.InvoiceAt, - TaxConfig: rateCardParsed.TaxConfig, + TaxConfig: billing.FromProductCatalog(rateCardParsed.TaxConfig), RateCardDiscounts: rateCardParsed.Discounts, }, UsageBased: &billing.UsageBasedLine{ @@ -362,7 +362,7 @@ func mapInvoiceLineToAPI(line *billing.StandardLine) (api.InvoiceLine, error) { To: line.Period.To, }, - TaxConfig: mapTaxConfigToAPI(line.TaxConfig), + TaxConfig: mapTaxConfigToAPI(line.TaxConfig.ToProductCatalog()), FeatureKey: lo.EmptyableToPtr(line.UsageBased.FeatureKey), MeteredQuantity: decimalPtrToStringPtrIfNotEqual(line.UsageBased.MeteredQuantity, line.UsageBased.Quantity), @@ -373,7 +373,7 @@ func mapInvoiceLineToAPI(line *billing.StandardLine) (api.InvoiceLine, error) { Price: lo.ToPtr(price), RateCard: &api.InvoiceUsageBasedRateCard{ - TaxConfig: mapTaxConfigToAPI(line.TaxConfig), + TaxConfig: mapTaxConfigToAPI(line.TaxConfig.ToProductCatalog()), Price: lo.ToPtr(price), FeatureKey: lo.EmptyableToPtr(line.UsageBased.FeatureKey), }, @@ -634,7 +634,7 @@ func mapSimulationLineToEntity(line api.InvoiceSimulationLine) (*billing.Standar }, InvoiceAt: line.InvoiceAt.Truncate(streaming.MinimumWindowSizeDuration), - TaxConfig: rateCardParsed.TaxConfig, + TaxConfig: billing.FromProductCatalog(rateCardParsed.TaxConfig), RateCardDiscounts: rateCardParsed.Discounts, }, UsageBased: &billing.UsageBasedLine{ @@ -679,7 +679,7 @@ func standardLineFromInvoiceLineReplaceUpdate(line api.InvoiceLineReplaceUpdate, }, InvoiceAt: line.InvoiceAt.Truncate(streaming.MinimumWindowSizeDuration), - TaxConfig: rateCardParsed.TaxConfig, + TaxConfig: billing.FromProductCatalog(rateCardParsed.TaxConfig), RateCardDiscounts: rateCardParsed.Discounts, }, UsageBased: &billing.UsageBasedLine{ @@ -760,7 +760,7 @@ func mergeStandardLineFromInvoiceLineReplaceUpdate(existing *billing.StandardLin existing.Period.To = line.Period.To.Truncate(streaming.MinimumWindowSizeDuration) existing.InvoiceAt = line.InvoiceAt.Truncate(streaming.MinimumWindowSizeDuration) - existing.TaxConfig = rateCardParsed.TaxConfig + existing.TaxConfig = billing.FromProductCatalog(rateCardParsed.TaxConfig) existing.UsageBased.Price = rateCardParsed.Price existing.UsageBased.FeatureKey = rateCardParsed.FeatureKey diff --git a/openmeter/billing/service/invoicecalc/taxconfig.go b/openmeter/billing/service/invoicecalc/taxconfig.go index 3d8e668e15..fdf71376c3 100644 --- a/openmeter/billing/service/invoicecalc/taxconfig.go +++ b/openmeter/billing/service/invoicecalc/taxconfig.go @@ -4,7 +4,6 @@ import ( "github.com/samber/lo" "github.com/openmeterio/openmeter/openmeter/billing" - "github.com/openmeterio/openmeter/openmeter/productcatalog" ) // SnapshotTaxConfigIntoLines merges the invoice's DefaultTaxConfig into each line and @@ -16,7 +15,7 @@ func SnapshotTaxConfigIntoLines(invoice *billing.StandardInvoice, deps StandardI } for _, line := range invoice.Lines.OrEmpty() { - line.TaxConfig = productcatalog.MergeTaxConfigs(invoice.Workflow.Config.Invoicing.DefaultTaxConfig, line.TaxConfig) + line.TaxConfig = billing.MergeTaxConfigs(billing.FromProductCatalog(invoice.Workflow.Config.Invoicing.DefaultTaxConfig), line.TaxConfig) if line.TaxConfig == nil || line.TaxConfig.Stripe == nil { continue diff --git a/openmeter/billing/service/invoicecalc/taxconfig_test.go b/openmeter/billing/service/invoicecalc/taxconfig_test.go index 99d646c6c4..abdf550639 100644 --- a/openmeter/billing/service/invoicecalc/taxconfig_test.go +++ b/openmeter/billing/service/invoicecalc/taxconfig_test.go @@ -14,6 +14,63 @@ import ( "github.com/openmeterio/openmeter/pkg/models" ) +// TestBug2_TaxConfigEqualDetectsTaxCode is the regression guard for the fix: billing.TaxConfig.Equal +// now includes the resolved TaxCode entity. Two configs that are identical except for TaxCode +// (one nil, one stamped) must compare as NOT equal, so the adapter diff guard re-upserts the line +// and the snapshot is persisted to the tax_config JSONB column. +func TestBug2_TaxConfigEqualDetectsTaxCode(t *testing.T) { + tc1 := taxcode.TaxCode{ + NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_10000000"}, + }, + } + + // Persisted state: TaxCodeID set, but the TaxCode entity snapshot is absent. + persistedState := &billing.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + TaxCode: nil, + } + + // In-memory state after SnapshotTaxConfigIntoLines stamps the entity. Identical except TaxCode. + expectedState := &billing.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + TaxCode: &tc1, + } + + assert.False(t, persistedState.Equal(expectedState), + "Equal must detect the stamped TaxCode so the line is re-upserted and the snapshot persisted") + + // Deep-equal sub-case: two configs whose TaxCode has the same ID/namespace/key/name but + // different AppMappings must compare as NOT equal (shallow ID comparison would miss this). + tc2 := taxcode.TaxCode{ + NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_20000000"}, + }, + } + + leftConfig := &billing.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + TaxCode: &tc1, + } + rightConfig := &billing.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + TaxCode: &tc2, + } + + assert.False(t, leftConfig.Equal(rightConfig), + "Equal must detect different AppMappings even when TaxCode.ID is the same") +} + func TestSnapshotTaxConfigIntoLines(t *testing.T) { tc1 := taxcode.TaxCode{ NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, @@ -47,7 +104,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { newLine := func(tc *productcatalog.TaxConfig) *billing.StandardLine { return &billing.StandardLine{ StandardLineBase: billing.StandardLineBase{ - TaxConfig: tc, + TaxConfig: billing.FromProductCatalog(tc), }, } } @@ -56,7 +113,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { name string invoice billing.StandardInvoice deps StandardInvoiceCalculatorDependencies - wantTC *productcatalog.TaxConfig + wantTC *billing.TaxConfig wantNoErr bool }{ { @@ -69,7 +126,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { }), ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &productcatalog.TaxConfig{Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}}, + wantTC: &billing.TaxConfig{Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}}, wantNoErr: true, }, { @@ -82,7 +139,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { }), ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &productcatalog.TaxConfig{ + wantTC: &billing.TaxConfig{ Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, TaxCodeID: lo.ToPtr("tc-1"), TaxCode: &tc1, @@ -100,13 +157,46 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { }), ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &productcatalog.TaxConfig{ + wantTC: &billing.TaxConfig{ Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, TaxCodeID: lo.ToPtr("already-set"), TaxCode: &tc1, }, wantNoErr: true, }, + { + // Reproduces the dedup production scenario: + // line.TaxCodeID points to the loser UUID (written by a code path that resolved + // the tax code outside of GetOrCreateByAppMapping), while deps.TaxCodes maps the + // Stripe code to the winner entity. + // + // Expected: TaxCode snapshot IS stamped with winner data (so reads of tax_config.tax_code + // return the correct entity); TaxCodeID is NOT overwritten (stays as loser) because the + // non-nil guard prevents it. The dedup migration and a separate backfill are needed to + // repair the TaxCodeID → winner realignment. + // + // If this test fails (TaxCode is nil), the bug is inside SnapshotTaxConfigIntoLines itself. + // If this test passes, the production missing-snapshot bug is in a layer outside this + // function (persistence path or a code path that bypasses calculateInvoice entirely). + name: "dedup scenario: loser TaxCodeID is preserved but winner entity snapshot is stamped", + invoice: newInvoice( + billing.StandardInvoiceStatusDraftCollecting, + nil, + newLine(&productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("loser-uuid"), + }), + ), + deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, + wantTC: &billing.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("loser-uuid"), // not overwritten to winner + TaxCode: &tc1, // winner entity IS stamped + }, + wantNoErr: true, + }, { name: "gracefully skips when stripe code is absent from deps", invoice: newInvoice( @@ -117,7 +207,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { }), ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{}}, - wantTC: &productcatalog.TaxConfig{ + wantTC: &billing.TaxConfig{ Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, }, wantNoErr: true, @@ -133,7 +223,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { newLine(nil), ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &productcatalog.TaxConfig{ + wantTC: &billing.TaxConfig{ Behavior: lo.ToPtr(productcatalog.InclusiveTaxBehavior), Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, TaxCodeID: lo.ToPtr("tc-1"), @@ -156,7 +246,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { "txcd_10000000": tc1, "txcd_20000000": tc2, }}, - wantTC: &productcatalog.TaxConfig{ + wantTC: &billing.TaxConfig{ Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_20000000"}, TaxCodeID: lo.ToPtr("tc-2"), TaxCode: &tc2, @@ -173,7 +263,7 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { }), ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &productcatalog.TaxConfig{ + wantTC: &billing.TaxConfig{ Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), }, wantNoErr: true, diff --git a/openmeter/billing/service/stdinvoicestate.go b/openmeter/billing/service/stdinvoicestate.go index 3967b04c5d..e0d92ca0cb 100644 --- a/openmeter/billing/service/stdinvoicestate.go +++ b/openmeter/billing/service/stdinvoicestate.go @@ -168,6 +168,8 @@ func allocateStateMachine() *InvoiceStateMachine { Permit(billing.TriggerDelete, billing.StandardInvoiceStatusDeleteInProgress). Permit(billing.TriggerFailed, billing.StandardInvoiceStatusDraftSyncFailed). OnActive(statelessx.AllOf( + out.snapshotLineTaxConfigs, + out.requireDBSave, out.syncDraftInvoice, )) @@ -229,6 +231,8 @@ func allocateStateMachine() *InvoiceStateMachine { Permit(billing.TriggerFailed, billing.StandardInvoiceStatusIssuingSyncFailed). Permit(billing.TriggerDelete, billing.StandardInvoiceStatusDeleteInProgress). OnActive(statelessx.AllOf( + out.snapshotLineTaxConfigs, + out.requireDBSave, out.finalizeInvoice, )) @@ -771,6 +775,35 @@ func (m *InvoiceStateMachine) calculateInvoice(ctx context.Context) error { }) } +// snapshotLineTaxConfigs re-stamps the resolved TaxCode entity onto each line's TaxConfig +// immediately before the invoice is synced to / finalized by the external invoicing app, so the +// persisted and externally-sent snapshot reflects the tax code's point-in-time state at the +// moment of sync rather than the possibly-stale snapshot taken during the draft calculation states. +// +// It runs in the OnActive of draft.syncing and issuing.syncing. StatusDetails.Immutable flips to +// true at draft.syncing (the state drops TriggerUpdated), but the line is only truly immutable once +// finalized at issuing.syncing; this refresh is a controlled, system-driven update, not a user edit. +func (m *InvoiceStateMachine) snapshotLineTaxConfigs(ctx context.Context) error { + taxCodes, err := m.Service.resolveTaxCodes(ctx, resolveTaxCodesInput{ + Namespace: m.Invoice.Namespace, + Invoice: &m.Invoice, + // Read-only: the tax codes were already resolved/created during the draft states; we must + // not mint new tax_codes rows during sync/finalization. + ReadOnly: true, + }) + if err != nil { + return fmt.Errorf("resolving tax codes: %w", err) + } + + if err := invoicecalc.SnapshotTaxConfigIntoLines(&m.Invoice, invoicecalc.StandardInvoiceCalculatorDependencies{ + TaxCodes: taxCodes, + }); err != nil { + return fmt.Errorf("snapshotting tax configs: %w", err) + } + + return nil +} + // syncDraftInvoice syncs the draft invoice with the external system. func (m *InvoiceStateMachine) syncDraftInvoice(ctx context.Context) error { if err := m.validateNamespaceLockdown(); err != nil { diff --git a/openmeter/billing/stdinvoiceline.go b/openmeter/billing/stdinvoiceline.go index 7b0ee290f3..5204b8abcc 100644 --- a/openmeter/billing/stdinvoiceline.go +++ b/openmeter/billing/stdinvoiceline.go @@ -45,9 +45,9 @@ type StandardLineBase struct { ChildUniqueReferenceID *string `json:"childUniqueReferenceID,omitempty"` - TaxConfig *productcatalog.TaxConfig `json:"taxOverrides,omitempty"` - RateCardDiscounts Discounts `json:"rateCardDiscounts,omitempty"` - CreditsApplied CreditsApplied `json:"creditsApplied,omitempty"` + TaxConfig *TaxConfig `json:"taxOverrides,omitempty"` + RateCardDiscounts Discounts `json:"rateCardDiscounts,omitempty"` + CreditsApplied CreditsApplied `json:"creditsApplied,omitempty"` ExternalIDs externalid.LineExternalIDs `json:"externalIDs,omitempty"` Subscription *SubscriptionReference `json:"subscription,omitempty"` @@ -406,7 +406,7 @@ func (i StandardLine) ToGatheringLineBase() (GatheringLineBase, error) { InvoiceAt: i.InvoiceAt, Price: lo.FromPtr(i.UsageBased.Price), FeatureKey: i.UsageBased.FeatureKey, - TaxConfig: i.TaxConfig, + TaxConfig: i.TaxConfig.ToProductCatalog(), RateCardDiscounts: i.RateCardDiscounts, ChildUniqueReferenceID: i.ChildUniqueReferenceID, Subscription: i.Subscription, diff --git a/openmeter/billing/taxconfig.go b/openmeter/billing/taxconfig.go new file mode 100644 index 0000000000..c7b812e9d6 --- /dev/null +++ b/openmeter/billing/taxconfig.go @@ -0,0 +1,159 @@ +package billing + +import ( + "errors" + "fmt" + + "github.com/samber/lo" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" + "github.com/openmeterio/openmeter/openmeter/taxcode" + "github.com/openmeterio/openmeter/pkg/models" +) + +// TaxConfig is the billing-layer tax configuration. It extends productcatalog.TaxConfig with +// TaxCode, the resolved entity snapshot stamped at invoice snapshot time. +type TaxConfig struct { + Behavior *productcatalog.TaxBehavior `json:"behavior,omitempty"` + Stripe *productcatalog.StripeTaxConfig `json:"stripe,omitempty"` + TaxCodeID *string `json:"tax_code_id,omitempty"` + // TaxCode is the resolved TaxCode entity, stamped at invoice snapshot time. + TaxCode *taxcode.TaxCode `json:"tax_code,omitempty"` +} + +// FromProductCatalog promotes a productcatalog.TaxConfig into a billing.TaxConfig. +// TaxCode is left nil; stamp it after resolving the entity. +func FromProductCatalog(c *productcatalog.TaxConfig) *TaxConfig { + if c == nil { + return nil + } + return &TaxConfig{ + Behavior: c.Behavior, + Stripe: c.Stripe, + TaxCodeID: c.TaxCodeID, + } +} + +// ToProductCatalog strips the billing-only TaxCode field and returns the intent-level config. +func (c *TaxConfig) ToProductCatalog() *productcatalog.TaxConfig { + if c == nil { + return nil + } + return &productcatalog.TaxConfig{ + Behavior: c.Behavior, + Stripe: c.Stripe, + TaxCodeID: c.TaxCodeID, + } +} + +func (c *TaxConfig) Equal(v *TaxConfig) bool { + if c == nil && v == nil { + return true + } + + if c == nil || v == nil { + return false + } + + if (c.Behavior != nil && v.Behavior == nil) || (c.Behavior == nil && v.Behavior != nil) { + return false + } + + if c.Behavior != nil && *c.Behavior != *v.Behavior { + return false + } + + if (c.TaxCodeID != nil && v.TaxCodeID == nil) || (c.TaxCodeID == nil && v.TaxCodeID != nil) { + return false + } + + if c.TaxCodeID != nil && *c.TaxCodeID != *v.TaxCodeID { + return false + } + + if !c.TaxCode.Equal(v.TaxCode) { + return false + } + + return c.Stripe.Equal(v.Stripe) +} + +func (c *TaxConfig) Validate() error { + var errs []error + + if c.Behavior != nil { + if err := c.Behavior.Validate(); err != nil { + errs = append(errs, err) + } + } + + if c.Stripe != nil { + if err := c.Stripe.Validate(); err != nil { + errs = append(errs, fmt.Errorf("invalid stripe config: %w", err)) + } + } + + return models.NewNillableGenericValidationError(errors.Join(errs...)) +} + +func (c TaxConfig) Clone() TaxConfig { + out := TaxConfig{} + + if c.Behavior != nil { + out.Behavior = lo.ToPtr(*c.Behavior) + } + + if c.Stripe != nil { + out.Stripe = lo.ToPtr(c.Stripe.Clone()) + } + + if c.TaxCodeID != nil { + out.TaxCodeID = lo.ToPtr(*c.TaxCodeID) + } + + if c.TaxCode != nil { + tc := *c.TaxCode + tc.AppMappings = append(taxcode.TaxCodeAppMappings(nil), c.TaxCode.AppMappings...) + if c.TaxCode.Description != nil { + tc.Description = lo.ToPtr(*c.TaxCode.Description) + } + out.TaxCode = &tc + } + + return out +} + +// MergeTaxConfigs merges two billing TaxConfigs with overrides taking precedence. +// +// Stripe and TaxCodeID are two encodings of the same intent-level tax-code identity, so they +// merge as a unit: a config that overrides only the Stripe code must not inherit the base's +// (different) TaxCodeID, which would leave the result pointing at two different tax entities. +// +// TaxCode (resolved entity) is intentionally excluded: merge operates on intent-level configs, +// not snapshotted invoice lines. +func MergeTaxConfigs(base, overrides *TaxConfig) *TaxConfig { + if base != nil && overrides != nil { + stripe, taxCodeID := base.Stripe, base.TaxCodeID + if overrides.Stripe != nil || overrides.TaxCodeID != nil { + stripe, taxCodeID = overrides.Stripe, overrides.TaxCodeID + } + + return &TaxConfig{ + Behavior: lo.CoalesceOrEmpty(overrides.Behavior, base.Behavior), + Stripe: stripe, + TaxCodeID: taxCodeID, + } + } + + if overrides != nil { + c := overrides.Clone() + return &c + } + + if base != nil { + c := base.Clone() + return &c + } + + return nil +} diff --git a/openmeter/billing/worker/subscriptionsync/service/sync_credittheninvoice_test.go b/openmeter/billing/worker/subscriptionsync/service/sync_credittheninvoice_test.go index 26d52c8d14..1c460e76cd 100644 --- a/openmeter/billing/worker/subscriptionsync/service/sync_credittheninvoice_test.go +++ b/openmeter/billing/worker/subscriptionsync/service/sync_credittheninvoice_test.go @@ -7358,7 +7358,7 @@ func (s *CreditThenInvoiceTestSuite) assertStandardLineTaxConfigs(lines []*billi s.Require().NotEmpty(lines) for _, line := range lines { s.Require().NotNil(line.TaxConfig) - s.True(expected.Equal(line.TaxConfig), "line %s tax config: expected %+v, got %+v", line.ID, expected, line.TaxConfig) + s.True(expected.Equal(line.TaxConfig.ToProductCatalog()), "line %s tax config: expected %+v, got %+v", line.ID, expected, line.TaxConfig) } } diff --git a/openmeter/ent/db/billinginvoiceline.go b/openmeter/ent/db/billinginvoiceline.go index eb2cab0f50..2b54752609 100644 --- a/openmeter/ent/db/billinginvoiceline.go +++ b/openmeter/ent/db/billinginvoiceline.go @@ -56,7 +56,7 @@ type BillingInvoiceLine struct { // Currency holds the value of the "currency" field. Currency currencyx.Code `json:"currency,omitempty"` // TaxConfig holds the value of the "tax_config" field. - TaxConfig productcatalog.TaxConfig `json:"tax_config,omitempty"` + TaxConfig billing.TaxConfig `json:"tax_config,omitempty"` // TaxCodeID holds the value of the "tax_code_id" field. TaxCodeID *string `json:"tax_code_id,omitempty"` // TaxBehavior holds the value of the "tax_behavior" field. diff --git a/openmeter/ent/db/billinginvoiceline_create.go b/openmeter/ent/db/billinginvoiceline_create.go index 04299d2b73..a06644a096 100644 --- a/openmeter/ent/db/billinginvoiceline_create.go +++ b/openmeter/ent/db/billinginvoiceline_create.go @@ -133,13 +133,13 @@ func (_c *BillingInvoiceLineCreate) SetCurrency(v currencyx.Code) *BillingInvoic } // SetTaxConfig sets the "tax_config" field. -func (_c *BillingInvoiceLineCreate) SetTaxConfig(v productcatalog.TaxConfig) *BillingInvoiceLineCreate { +func (_c *BillingInvoiceLineCreate) SetTaxConfig(v billing.TaxConfig) *BillingInvoiceLineCreate { _c.mutation.SetTaxConfig(v) return _c } // SetNillableTaxConfig sets the "tax_config" field if the given value is not nil. -func (_c *BillingInvoiceLineCreate) SetNillableTaxConfig(v *productcatalog.TaxConfig) *BillingInvoiceLineCreate { +func (_c *BillingInvoiceLineCreate) SetNillableTaxConfig(v *billing.TaxConfig) *BillingInvoiceLineCreate { if v != nil { _c.SetTaxConfig(*v) } @@ -1537,7 +1537,7 @@ func (u *BillingInvoiceLineUpsert) ClearDescription() *BillingInvoiceLineUpsert } // SetTaxConfig sets the "tax_config" field. -func (u *BillingInvoiceLineUpsert) SetTaxConfig(v productcatalog.TaxConfig) *BillingInvoiceLineUpsert { +func (u *BillingInvoiceLineUpsert) SetTaxConfig(v billing.TaxConfig) *BillingInvoiceLineUpsert { u.Set(billinginvoiceline.FieldTaxConfig, v) return u } @@ -2213,7 +2213,7 @@ func (u *BillingInvoiceLineUpsertOne) ClearDescription() *BillingInvoiceLineUpse } // SetTaxConfig sets the "tax_config" field. -func (u *BillingInvoiceLineUpsertOne) SetTaxConfig(v productcatalog.TaxConfig) *BillingInvoiceLineUpsertOne { +func (u *BillingInvoiceLineUpsertOne) SetTaxConfig(v billing.TaxConfig) *BillingInvoiceLineUpsertOne { return u.Update(func(s *BillingInvoiceLineUpsert) { s.SetTaxConfig(v) }) @@ -3143,7 +3143,7 @@ func (u *BillingInvoiceLineUpsertBulk) ClearDescription() *BillingInvoiceLineUps } // SetTaxConfig sets the "tax_config" field. -func (u *BillingInvoiceLineUpsertBulk) SetTaxConfig(v productcatalog.TaxConfig) *BillingInvoiceLineUpsertBulk { +func (u *BillingInvoiceLineUpsertBulk) SetTaxConfig(v billing.TaxConfig) *BillingInvoiceLineUpsertBulk { return u.Update(func(s *BillingInvoiceLineUpsert) { s.SetTaxConfig(v) }) diff --git a/openmeter/ent/db/billinginvoiceline_update.go b/openmeter/ent/db/billinginvoiceline_update.go index 962550b3df..003512c6e8 100644 --- a/openmeter/ent/db/billinginvoiceline_update.go +++ b/openmeter/ent/db/billinginvoiceline_update.go @@ -135,13 +135,13 @@ func (_u *BillingInvoiceLineUpdate) ClearDescription() *BillingInvoiceLineUpdate } // SetTaxConfig sets the "tax_config" field. -func (_u *BillingInvoiceLineUpdate) SetTaxConfig(v productcatalog.TaxConfig) *BillingInvoiceLineUpdate { +func (_u *BillingInvoiceLineUpdate) SetTaxConfig(v billing.TaxConfig) *BillingInvoiceLineUpdate { _u.mutation.SetTaxConfig(v) return _u } // SetNillableTaxConfig sets the "tax_config" field if the given value is not nil. -func (_u *BillingInvoiceLineUpdate) SetNillableTaxConfig(v *productcatalog.TaxConfig) *BillingInvoiceLineUpdate { +func (_u *BillingInvoiceLineUpdate) SetNillableTaxConfig(v *billing.TaxConfig) *BillingInvoiceLineUpdate { if v != nil { _u.SetTaxConfig(*v) } @@ -2085,13 +2085,13 @@ func (_u *BillingInvoiceLineUpdateOne) ClearDescription() *BillingInvoiceLineUpd } // SetTaxConfig sets the "tax_config" field. -func (_u *BillingInvoiceLineUpdateOne) SetTaxConfig(v productcatalog.TaxConfig) *BillingInvoiceLineUpdateOne { +func (_u *BillingInvoiceLineUpdateOne) SetTaxConfig(v billing.TaxConfig) *BillingInvoiceLineUpdateOne { _u.mutation.SetTaxConfig(v) return _u } // SetNillableTaxConfig sets the "tax_config" field if the given value is not nil. -func (_u *BillingInvoiceLineUpdateOne) SetNillableTaxConfig(v *productcatalog.TaxConfig) *BillingInvoiceLineUpdateOne { +func (_u *BillingInvoiceLineUpdateOne) SetNillableTaxConfig(v *billing.TaxConfig) *BillingInvoiceLineUpdateOne { if v != nil { _u.SetTaxConfig(*v) } diff --git a/openmeter/ent/db/entmixinaccessor.go b/openmeter/ent/db/entmixinaccessor.go index 95d0c25347..aca1d6d3f4 100644 --- a/openmeter/ent/db/entmixinaccessor.go +++ b/openmeter/ent/db/entmixinaccessor.go @@ -445,7 +445,7 @@ func (e *BillingInvoiceLine) GetCurrency() currencyx.Code { return e.Currency } -func (e *BillingInvoiceLine) GetTaxConfig() productcatalog.TaxConfig { +func (e *BillingInvoiceLine) GetTaxConfig() billing.TaxConfig { return e.TaxConfig } diff --git a/openmeter/ent/db/mutation.go b/openmeter/ent/db/mutation.go index 03182c6a81..957900f60d 100644 --- a/openmeter/ent/db/mutation.go +++ b/openmeter/ent/db/mutation.go @@ -17321,7 +17321,7 @@ type BillingInvoiceLineMutation struct { name *string description *string currency *currencyx.Code - tax_config *productcatalog.TaxConfig + tax_config *billing.TaxConfig tax_behavior *productcatalog.TaxBehavior amount *alpacadecimal.Decimal taxes_total *alpacadecimal.Decimal @@ -17877,12 +17877,12 @@ func (m *BillingInvoiceLineMutation) ResetCurrency() { } // SetTaxConfig sets the "tax_config" field. -func (m *BillingInvoiceLineMutation) SetTaxConfig(pc productcatalog.TaxConfig) { - m.tax_config = &pc +func (m *BillingInvoiceLineMutation) SetTaxConfig(bc billing.TaxConfig) { + m.tax_config = &bc } // TaxConfig returns the value of the "tax_config" field in the mutation. -func (m *BillingInvoiceLineMutation) TaxConfig() (r productcatalog.TaxConfig, exists bool) { +func (m *BillingInvoiceLineMutation) TaxConfig() (r billing.TaxConfig, exists bool) { v := m.tax_config if v == nil { return @@ -17893,7 +17893,7 @@ func (m *BillingInvoiceLineMutation) TaxConfig() (r productcatalog.TaxConfig, ex // OldTaxConfig returns the old "tax_config" field's value of the BillingInvoiceLine entity. // If the BillingInvoiceLine object wasn't provided to the builder, the object is fetched from the database. // An error is returned if the mutation operation is not UpdateOne, or the database query fails. -func (m *BillingInvoiceLineMutation) OldTaxConfig(ctx context.Context) (v productcatalog.TaxConfig, err error) { +func (m *BillingInvoiceLineMutation) OldTaxConfig(ctx context.Context) (v billing.TaxConfig, err error) { if !m.op.Is(OpUpdateOne) { return v, errors.New("OldTaxConfig is only allowed on UpdateOne operations") } @@ -20493,7 +20493,7 @@ func (m *BillingInvoiceLineMutation) SetField(name string, value ent.Value) erro m.SetCurrency(v) return nil case billinginvoiceline.FieldTaxConfig: - v, ok := value.(productcatalog.TaxConfig) + v, ok := value.(billing.TaxConfig) if !ok { return fmt.Errorf("unexpected type %T for field %s", value, name) } diff --git a/openmeter/ent/db/setorclear.go b/openmeter/ent/db/setorclear.go index 2d1e0e11cc..2e7034f49d 100644 --- a/openmeter/ent/db/setorclear.go +++ b/openmeter/ent/db/setorclear.go @@ -1153,14 +1153,14 @@ func (u *BillingInvoiceLineUpdateOne) SetOrClearDescription(value *string) *Bill return u.SetDescription(*value) } -func (u *BillingInvoiceLineUpdate) SetOrClearTaxConfig(value *productcatalog.TaxConfig) *BillingInvoiceLineUpdate { +func (u *BillingInvoiceLineUpdate) SetOrClearTaxConfig(value *billing.TaxConfig) *BillingInvoiceLineUpdate { if value == nil { return u.ClearTaxConfig() } return u.SetTaxConfig(*value) } -func (u *BillingInvoiceLineUpdateOne) SetOrClearTaxConfig(value *productcatalog.TaxConfig) *BillingInvoiceLineUpdateOne { +func (u *BillingInvoiceLineUpdateOne) SetOrClearTaxConfig(value *billing.TaxConfig) *BillingInvoiceLineUpdateOne { if value == nil { return u.ClearTaxConfig() } diff --git a/openmeter/ent/schema/billing.go b/openmeter/ent/schema/billing.go index 262077704f..5fec400465 100644 --- a/openmeter/ent/schema/billing.go +++ b/openmeter/ent/schema/billing.go @@ -292,7 +292,7 @@ func (InvoiceLineBaseMixin) Fields() []ent.Field { dialect.Postgres: "varchar(3)", }), - field.JSON("tax_config", productcatalog.TaxConfig{}). + field.JSON("tax_config", billing.TaxConfig{}). SchemaType(map[string]string{ dialect.Postgres: "jsonb", }). diff --git a/openmeter/productcatalog/tax.go b/openmeter/productcatalog/tax.go index b9f4bb443f..7d7fa0b249 100644 --- a/openmeter/productcatalog/tax.go +++ b/openmeter/productcatalog/tax.go @@ -39,9 +39,6 @@ type TaxConfig struct { Behavior *TaxBehavior `json:"behavior,omitempty"` Stripe *StripeTaxConfig `json:"stripe,omitempty"` TaxCodeID *string `json:"tax_code_id,omitempty"` - // TaxCode is the resolved TaxCode entity, stamped at invoice snapshot time. - // Present only on invoice lines (persisted in JSONB); nil on profile/rate-card configs. - TaxCode *taxcode.TaxCode `json:"tax_code,omitempty"` } func (c *TaxConfig) Equal(v *TaxConfig) bool { @@ -105,26 +102,25 @@ func (c TaxConfig) Clone() TaxConfig { out.TaxCodeID = lo.ToPtr(*c.TaxCodeID) } - if c.TaxCode != nil { - tc := *c.TaxCode - tc.AppMappings = append(taxcode.TaxCodeAppMappings(nil), c.TaxCode.AppMappings...) - if c.TaxCode.Description != nil { - tc.Description = lo.ToPtr(*c.TaxCode.Description) - } - out.TaxCode = &tc - } - return out } +// MergeTaxConfigs merges two TaxConfigs with overrides taking precedence. +// +// Stripe and TaxCodeID are two encodings of the same intent-level tax-code identity, so they +// merge as a unit: a config that overrides only the Stripe code must not inherit the base's +// (different) TaxCodeID, which would leave the result pointing at two different tax entities. func MergeTaxConfigs(base, overrides *TaxConfig) *TaxConfig { if base != nil && overrides != nil { - // TaxCode (resolved entity) is intentionally excluded: merge operates on - // intent-level configs, not snapshotted invoice lines. + stripe, taxCodeID := base.Stripe, base.TaxCodeID + if overrides.Stripe != nil || overrides.TaxCodeID != nil { + stripe, taxCodeID = overrides.Stripe, overrides.TaxCodeID + } + return &TaxConfig{ Behavior: lo.CoalesceOrEmpty(overrides.Behavior, base.Behavior), - Stripe: lo.CoalesceOrEmpty(overrides.Stripe, base.Stripe), - TaxCodeID: lo.CoalesceOrEmpty(overrides.TaxCodeID, base.TaxCodeID), + Stripe: stripe, + TaxCodeID: taxCodeID, } } diff --git a/openmeter/productcatalog/tax_test.go b/openmeter/productcatalog/tax_test.go index 9ce6cc4bfc..9c1a75a25a 100644 --- a/openmeter/productcatalog/tax_test.go +++ b/openmeter/productcatalog/tax_test.go @@ -406,7 +406,11 @@ func TestMergeTaxConfigs(t *testing.T) { }, }, { - Name: "Right overrides left partially - TaxCodeID", + // The override supplies a new TaxCodeID but no Stripe code. Because Stripe and + // TaxCodeID are an atomic pair, the merge must take the override's pair (new + // TaxCodeID, nil Stripe) rather than leaking the base's Stripe through. The + // base's Stripe references a different tax entity and must not appear in the result. + Name: "Right overrides left partially - TaxCodeID only clears base Stripe (atomic pair)", Left: &TaxConfig{ Behavior: lo.ToPtr(InclusiveTaxBehavior), TaxCodeID: lo.ToPtr("01AN4Z07BY79KA1307SR9X4MV3"), @@ -421,9 +425,27 @@ func TestMergeTaxConfigs(t *testing.T) { Expected: &TaxConfig{ Behavior: lo.ToPtr(InclusiveTaxBehavior), TaxCodeID: lo.ToPtr("01AN4Z07BY79KA1307SR9X4MV4"), - Stripe: &StripeTaxConfig{ - Code: "txcd_99999999", - }, + Stripe: nil, // base Stripe must NOT leak through + }, + }, + { + // Regression guard: base has both Stripe and TaxCodeID (pointing at the same entity); + // override has ONLY a new Stripe code (no TaxCodeID). The merged result must carry the + // override's Stripe and must NOT inherit the base's TaxCodeID (which references a + // different entity). Leaking the base TaxCodeID alongside the override Stripe would + // produce an inconsistent pair that breaks snapshot readback. + Name: "Right overrides left partially - Stripe only does not leak base TaxCodeID", + Left: &TaxConfig{ + Stripe: &StripeTaxConfig{Code: "txcd_10000000"}, + TaxCodeID: lo.ToPtr("01AN4Z07BY79KA1307SR9X4MV3"), + }, + Right: &TaxConfig{ + Stripe: &StripeTaxConfig{Code: "txcd_20000000"}, + // TaxCodeID intentionally nil — override owns a different code entity. + }, + Expected: &TaxConfig{ + Stripe: &StripeTaxConfig{Code: "txcd_20000000"}, + TaxCodeID: nil, // base TaxCodeID must NOT leak through }, }, { @@ -449,21 +471,6 @@ func TestMergeTaxConfigs(t *testing.T) { }, }, }, - { - Name: "TaxCode resolved entity is dropped — merge is intent-level only", - Left: &TaxConfig{ - TaxCodeID: lo.ToPtr("01AN4Z07BY79KA1307SR9X4MV3"), - TaxCode: &taxcode.TaxCode{NamespacedID: models.NamespacedID{ID: "01AN4Z07BY79KA1307SR9X4MV3"}}, - }, - Right: &TaxConfig{ - Behavior: lo.ToPtr(ExclusiveTaxBehavior), - }, - Expected: &TaxConfig{ - TaxCodeID: lo.ToPtr("01AN4Z07BY79KA1307SR9X4MV3"), - Behavior: lo.ToPtr(ExclusiveTaxBehavior), - // TaxCode intentionally omitted from merge result - }, - }, } for _, test := range tests { @@ -515,40 +522,6 @@ func TestTaxConfigClone(t *testing.T) { assert.False(t, original.Equal(&cloned)) } -func TestTaxConfigCloneWithTaxCode(t *testing.T) { - desc := "Software - SaaS" - original := TaxConfig{ - Stripe: &StripeTaxConfig{Code: "txcd_10000000"}, - TaxCode: &taxcode.TaxCode{ - NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, - Description: &desc, - AppMappings: taxcode.TaxCodeAppMappings{ - {AppType: app.AppTypeStripe, TaxCode: "txcd_10000000"}, - }, - }, - } - - cloned := original.Clone() - - // TaxCode pointer must not be shared - assert.NotSame(t, original.TaxCode, cloned.TaxCode) - - // Mutating clone's AppMappings must not affect original - cloned.TaxCode.AppMappings = append(cloned.TaxCode.AppMappings, taxcode.TaxCodeAppMapping{ - AppType: app.AppTypeStripe, TaxCode: "txcd_99999999", - }) - assert.Len(t, original.TaxCode.AppMappings, 1, "original AppMappings slice must not grow") - - // Mutating clone's Description must not affect original - *cloned.TaxCode.Description = "mutated" - assert.Equal(t, "Software - SaaS", *original.TaxCode.Description) - - // Clone of config with nil TaxCode must have nil TaxCode - nilTCConfig := TaxConfig{Stripe: &StripeTaxConfig{Code: "txcd_10000000"}} - clonedNil := nilTCConfig.Clone() - assert.Nil(t, clonedNil.TaxCode) -} - func TestBackfillTaxConfig(t *testing.T) { newTaxCode := func(id, stripeCode string) *taxcode.TaxCode { tc := &taxcode.TaxCode{ diff --git a/openmeter/taxcode/taxcode.go b/openmeter/taxcode/taxcode.go index 2448f8192b..9d66d13296 100644 --- a/openmeter/taxcode/taxcode.go +++ b/openmeter/taxcode/taxcode.go @@ -86,6 +86,38 @@ type TaxCode struct { Annotations models.Annotations `json:"annotations,omitempty"` } +// Equal returns true when both TaxCode values carry identical semantic data. +// ManagedModel timestamps (CreatedAt, UpdatedAt, DeletedAt) are excluded; all other fields are compared. +func (t *TaxCode) Equal(v *TaxCode) bool { + if t == nil && v == nil { + return true + } + if t == nil || v == nil { + return false + } + if t.ID != v.ID || t.Namespace != v.Namespace { + return false + } + if t.Key != v.Key || t.Name != v.Name { + return false + } + if (t.Description == nil) != (v.Description == nil) { + return false + } + if t.Description != nil && *t.Description != *v.Description { + return false + } + if len(t.AppMappings) != len(v.AppMappings) { + return false + } + for i := range t.AppMappings { + if t.AppMappings[i] != v.AppMappings[i] { + return false + } + } + return true +} + // IsManagedBySystem returns true when this tax code was auto-created by the system. func (t TaxCode) IsManagedBySystem() bool { v, ok := t.Annotations[AnnotationKeyManagedBy] diff --git a/test/billing/taxcode_dual_write_test.go b/test/billing/taxcode_dual_write_test.go index 740a05c2e0..0084fc062a 100644 --- a/test/billing/taxcode_dual_write_test.go +++ b/test/billing/taxcode_dual_write_test.go @@ -744,9 +744,9 @@ func (s *TaxCodeDualWriteTestSuite) TestSimulateInvoiceReadOnly() { PaymentTerm: productcatalog.InAdvancePaymentTerm, ManagedBy: billing.ManuallyManagedLine, }) - line.TaxConfig = &productcatalog.TaxConfig{ + line.TaxConfig = billing.FromProductCatalog(&productcatalog.TaxConfig{ Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_99999999"}, - } + }) result, err := s.BillingService.SimulateInvoice(ctx, billing.SimulateInvoiceInput{ Namespace: ns, From 318718c3b8d5feef2dee783a7739ebfbd6ae64dd Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Mon, 1 Jun 2026 16:52:42 +0200 Subject: [PATCH 2/7] fix(billing): remove comments --- .../service/invoicecalc/taxconfig_test.go | 37 +------------------ 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/openmeter/billing/service/invoicecalc/taxconfig_test.go b/openmeter/billing/service/invoicecalc/taxconfig_test.go index abdf550639..f7555ff7cf 100644 --- a/openmeter/billing/service/invoicecalc/taxconfig_test.go +++ b/openmeter/billing/service/invoicecalc/taxconfig_test.go @@ -14,11 +14,11 @@ import ( "github.com/openmeterio/openmeter/pkg/models" ) -// TestBug2_TaxConfigEqualDetectsTaxCode is the regression guard for the fix: billing.TaxConfig.Equal +// TaxConfigEqualDetectsTaxCode is the regression guard for the fix: billing.TaxConfig.Equal // now includes the resolved TaxCode entity. Two configs that are identical except for TaxCode // (one nil, one stamped) must compare as NOT equal, so the adapter diff guard re-upserts the line // and the snapshot is persisted to the tax_config JSONB column. -func TestBug2_TaxConfigEqualDetectsTaxCode(t *testing.T) { +func TaxConfigEqualDetectsTaxCode(t *testing.T) { tc1 := taxcode.TaxCode{ NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, AppMappings: taxcode.TaxCodeAppMappings{ @@ -164,39 +164,6 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { }, wantNoErr: true, }, - { - // Reproduces the dedup production scenario: - // line.TaxCodeID points to the loser UUID (written by a code path that resolved - // the tax code outside of GetOrCreateByAppMapping), while deps.TaxCodes maps the - // Stripe code to the winner entity. - // - // Expected: TaxCode snapshot IS stamped with winner data (so reads of tax_config.tax_code - // return the correct entity); TaxCodeID is NOT overwritten (stays as loser) because the - // non-nil guard prevents it. The dedup migration and a separate backfill are needed to - // repair the TaxCodeID → winner realignment. - // - // If this test fails (TaxCode is nil), the bug is inside SnapshotTaxConfigIntoLines itself. - // If this test passes, the production missing-snapshot bug is in a layer outside this - // function (persistence path or a code path that bypasses calculateInvoice entirely). - name: "dedup scenario: loser TaxCodeID is preserved but winner entity snapshot is stamped", - invoice: newInvoice( - billing.StandardInvoiceStatusDraftCollecting, - nil, - newLine(&productcatalog.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), - TaxCodeID: lo.ToPtr("loser-uuid"), - }), - ), - deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), - TaxCodeID: lo.ToPtr("loser-uuid"), // not overwritten to winner - TaxCode: &tc1, // winner entity IS stamped - }, - wantNoErr: true, - }, { name: "gracefully skips when stripe code is absent from deps", invoice: newInvoice( From 30c83960d6d9b204fcffbba7d4e6a187bc52656f Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Mon, 1 Jun 2026 17:24:20 +0200 Subject: [PATCH 3/7] fix(billing): fix review comment --- openmeter/billing/adapter/stdinvoicelinemapper.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openmeter/billing/adapter/stdinvoicelinemapper.go b/openmeter/billing/adapter/stdinvoicelinemapper.go index 6ec5ccb75d..a8fb9c381b 100644 --- a/openmeter/billing/adapter/stdinvoicelinemapper.go +++ b/openmeter/billing/adapter/stdinvoicelinemapper.go @@ -319,6 +319,10 @@ func taxCodeFromInvoiceLineEdge(dbLine *db.BillingInvoiceLine) *taxcode.TaxCode // fields explicitly so it is obvious what is the immutable invoice snapshot and what is the live // reference to the tax entity. func backfillTaxConfigReferences(snapshottedTaxConfig *billing.TaxConfig, persistedTaxBehavior *productcatalog.TaxBehavior, resolvedTaxCode *taxcode.TaxCode) *billing.TaxConfig { + if snapshottedTaxConfig == nil { + return billing.FromProductCatalog(productcatalog.BackfillTaxConfig(nil, persistedTaxBehavior, resolvedTaxCode)) + } + backfilledTaxConfig := productcatalog.BackfillTaxConfig(snapshottedTaxConfig.ToProductCatalog(), persistedTaxBehavior, resolvedTaxCode) if backfilledTaxConfig == nil || resolvedTaxCode == nil { From 53787c49cc196e703470f3d228e7e62588176cf9 Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Tue, 2 Jun 2026 11:16:16 +0200 Subject: [PATCH 4/7] fix(billing): fix review comment --- openmeter/billing/adapter/gatheringlines.go | 2 +- .../service/invoicecalc/taxconfig_test.go | 86 ++++++++++++------- openmeter/billing/taxconfig.go | 47 ++++------ openmeter/productcatalog/tax.go | 4 + 4 files changed, 76 insertions(+), 63 deletions(-) diff --git a/openmeter/billing/adapter/gatheringlines.go b/openmeter/billing/adapter/gatheringlines.go index 70bd8b483c..c597889e4b 100644 --- a/openmeter/billing/adapter/gatheringlines.go +++ b/openmeter/billing/adapter/gatheringlines.go @@ -221,7 +221,7 @@ func (a *adapter) updateGatheringLines(ctx context.Context, lines billing.Gather } if line.TaxConfig != nil { - create = create.SetTaxConfig(*billing.FromProductCatalog(line.TaxConfig)). + create = create.SetNillableTaxConfig(billing.FromProductCatalog(line.TaxConfig)). SetNillableTaxCodeID(line.TaxConfig.TaxCodeID). SetNillableTaxBehavior(line.TaxConfig.Behavior) } diff --git a/openmeter/billing/service/invoicecalc/taxconfig_test.go b/openmeter/billing/service/invoicecalc/taxconfig_test.go index f7555ff7cf..323cf1caa6 100644 --- a/openmeter/billing/service/invoicecalc/taxconfig_test.go +++ b/openmeter/billing/service/invoicecalc/taxconfig_test.go @@ -28,18 +28,22 @@ func TaxConfigEqualDetectsTaxCode(t *testing.T) { // Persisted state: TaxCodeID set, but the TaxCode entity snapshot is absent. persistedState := &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), - TaxCodeID: lo.ToPtr("tc-1"), - TaxCode: nil, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + }, + TaxCode: nil, } // In-memory state after SnapshotTaxConfigIntoLines stamps the entity. Identical except TaxCode. expectedState := &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), - TaxCodeID: lo.ToPtr("tc-1"), - TaxCode: &tc1, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + }, + TaxCode: &tc1, } assert.False(t, persistedState.Equal(expectedState), @@ -55,16 +59,20 @@ func TaxConfigEqualDetectsTaxCode(t *testing.T) { } leftConfig := &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), - TaxCodeID: lo.ToPtr("tc-1"), - TaxCode: &tc1, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + }, + TaxCode: &tc1, } rightConfig := &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), - TaxCodeID: lo.ToPtr("tc-1"), - TaxCode: &tc2, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxCodeID: lo.ToPtr("tc-1"), + }, + TaxCode: &tc2, } assert.False(t, leftConfig.Equal(rightConfig), @@ -125,8 +133,8 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, }), ), - deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &billing.TaxConfig{Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}}, + deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, + wantTC: &billing.TaxConfig{TaxConfig: productcatalog.TaxConfig{Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}}}, wantNoErr: true, }, { @@ -140,9 +148,11 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, wantTC: &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - TaxCodeID: lo.ToPtr("tc-1"), - TaxCode: &tc1, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + TaxCodeID: lo.ToPtr("tc-1"), + }, + TaxCode: &tc1, }, wantNoErr: true, }, @@ -158,9 +168,11 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, wantTC: &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - TaxCodeID: lo.ToPtr("already-set"), - TaxCode: &tc1, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + TaxCodeID: lo.ToPtr("already-set"), + }, + TaxCode: &tc1, }, wantNoErr: true, }, @@ -175,7 +187,9 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{}}, wantTC: &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + }, }, wantNoErr: true, }, @@ -191,10 +205,12 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, wantTC: &billing.TaxConfig{ - Behavior: lo.ToPtr(productcatalog.InclusiveTaxBehavior), - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, - TaxCodeID: lo.ToPtr("tc-1"), - TaxCode: &tc1, + TaxConfig: productcatalog.TaxConfig{ + Behavior: lo.ToPtr(productcatalog.InclusiveTaxBehavior), + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, + TaxCodeID: lo.ToPtr("tc-1"), + }, + TaxCode: &tc1, }, wantNoErr: true, }, @@ -214,9 +230,11 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { "txcd_20000000": tc2, }}, wantTC: &billing.TaxConfig{ - Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_20000000"}, - TaxCodeID: lo.ToPtr("tc-2"), - TaxCode: &tc2, + TaxConfig: productcatalog.TaxConfig{ + Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_20000000"}, + TaxCodeID: lo.ToPtr("tc-2"), + }, + TaxCode: &tc2, }, wantNoErr: true, }, @@ -231,7 +249,9 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { ), deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, wantTC: &billing.TaxConfig{ - Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + TaxConfig: productcatalog.TaxConfig{ + Behavior: lo.ToPtr(productcatalog.ExclusiveTaxBehavior), + }, }, wantNoErr: true, }, diff --git a/openmeter/billing/taxconfig.go b/openmeter/billing/taxconfig.go index c7b812e9d6..3ef107270d 100644 --- a/openmeter/billing/taxconfig.go +++ b/openmeter/billing/taxconfig.go @@ -14,9 +14,7 @@ import ( // TaxConfig is the billing-layer tax configuration. It extends productcatalog.TaxConfig with // TaxCode, the resolved entity snapshot stamped at invoice snapshot time. type TaxConfig struct { - Behavior *productcatalog.TaxBehavior `json:"behavior,omitempty"` - Stripe *productcatalog.StripeTaxConfig `json:"stripe,omitempty"` - TaxCodeID *string `json:"tax_code_id,omitempty"` + productcatalog.TaxConfig // TaxCode is the resolved TaxCode entity, stamped at invoice snapshot time. TaxCode *taxcode.TaxCode `json:"tax_code,omitempty"` } @@ -27,10 +25,9 @@ func FromProductCatalog(c *productcatalog.TaxConfig) *TaxConfig { if c == nil { return nil } + return &TaxConfig{ - Behavior: c.Behavior, - Stripe: c.Stripe, - TaxCodeID: c.TaxCodeID, + TaxConfig: c.Clone(), } } @@ -39,11 +36,8 @@ func (c *TaxConfig) ToProductCatalog() *productcatalog.TaxConfig { if c == nil { return nil } - return &productcatalog.TaxConfig{ - Behavior: c.Behavior, - Stripe: c.Stripe, - TaxCodeID: c.TaxCodeID, - } + + return lo.ToPtr(c.TaxConfig.Clone()) } func (c *TaxConfig) Equal(v *TaxConfig) bool { @@ -55,30 +49,23 @@ func (c *TaxConfig) Equal(v *TaxConfig) bool { return false } - if (c.Behavior != nil && v.Behavior == nil) || (c.Behavior == nil && v.Behavior != nil) { - return false - } - - if c.Behavior != nil && *c.Behavior != *v.Behavior { - return false - } - - if (c.TaxCodeID != nil && v.TaxCodeID == nil) || (c.TaxCodeID == nil && v.TaxCodeID != nil) { - return false - } - - if c.TaxCodeID != nil && *c.TaxCodeID != *v.TaxCodeID { + // none of them are nil + if !c.TaxConfig.Equal(&v.TaxConfig) { return false } - if !c.TaxCode.Equal(v.TaxCode) { + if c.TaxCode == nil || v.TaxCode == nil { return false } - return c.Stripe.Equal(v.Stripe) + return c.TaxCode.Equal(v.TaxCode) } func (c *TaxConfig) Validate() error { + if c == nil { + return nil + } + var errs []error if c.Behavior != nil { @@ -139,9 +126,11 @@ func MergeTaxConfigs(base, overrides *TaxConfig) *TaxConfig { } return &TaxConfig{ - Behavior: lo.CoalesceOrEmpty(overrides.Behavior, base.Behavior), - Stripe: stripe, - TaxCodeID: taxCodeID, + TaxConfig: productcatalog.TaxConfig{ + Behavior: lo.CoalesceOrEmpty(overrides.Behavior, base.Behavior), + Stripe: stripe, + TaxCodeID: taxCodeID, + }, } } diff --git a/openmeter/productcatalog/tax.go b/openmeter/productcatalog/tax.go index 7d7fa0b249..03c480cc72 100644 --- a/openmeter/productcatalog/tax.go +++ b/openmeter/productcatalog/tax.go @@ -70,6 +70,10 @@ func (c *TaxConfig) Equal(v *TaxConfig) bool { } func (c *TaxConfig) Validate() error { + if c == nil { + return nil + } + var errs []error if c.Behavior != nil { From fb22927e6311227c7696657d3115a6bcd3cc5b0c Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Tue, 2 Jun 2026 11:26:29 +0200 Subject: [PATCH 5/7] fix(billing): fix review comment --- openmeter/billing/service/invoicecalc/taxconfig_test.go | 4 ++-- openmeter/taxcode/taxcode.go | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/openmeter/billing/service/invoicecalc/taxconfig_test.go b/openmeter/billing/service/invoicecalc/taxconfig_test.go index 323cf1caa6..323a86821e 100644 --- a/openmeter/billing/service/invoicecalc/taxconfig_test.go +++ b/openmeter/billing/service/invoicecalc/taxconfig_test.go @@ -133,8 +133,8 @@ func TestSnapshotTaxConfigIntoLines(t *testing.T) { Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}, }), ), - deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, - wantTC: &billing.TaxConfig{TaxConfig: productcatalog.TaxConfig{Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}}}, + deps: StandardInvoiceCalculatorDependencies{TaxCodes: TaxCodes{"txcd_10000000": tc1}}, + wantTC: &billing.TaxConfig{TaxConfig: productcatalog.TaxConfig{Stripe: &productcatalog.StripeTaxConfig{Code: "txcd_10000000"}}}, wantNoErr: true, }, { diff --git a/openmeter/taxcode/taxcode.go b/openmeter/taxcode/taxcode.go index 9d66d13296..258924b17f 100644 --- a/openmeter/taxcode/taxcode.go +++ b/openmeter/taxcode/taxcode.go @@ -87,7 +87,8 @@ type TaxCode struct { } // Equal returns true when both TaxCode values carry identical semantic data. -// ManagedModel timestamps (CreatedAt, UpdatedAt, DeletedAt) are excluded; all other fields are compared. +// Compares ID, Namespace, Key, Name, Description, and AppMappings. +// ManagedModel timestamps, Metadata, and Annotations are excluded. func (t *TaxCode) Equal(v *TaxCode) bool { if t == nil && v == nil { return true From 78041fd49bd3b8f2753bbee5030a01259c92b10d Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Wed, 3 Jun 2026 13:17:40 +0200 Subject: [PATCH 6/7] fix(billing): change from deep taxcode equal to id equal --- openmeter/app/stripe/appinvoice.go | 2 +- openmeter/billing/httpdriver/invoiceline.go | 2 +- .../service/invoicecalc/taxconfig_test.go | 10 +++--- openmeter/billing/service/stdinvoicestate.go | 33 ------------------- openmeter/billing/stdinvoice.go | 5 ++- openmeter/billing/taxconfig.go | 24 ++++---------- openmeter/taxcode/errors.go | 10 ++++++ openmeter/taxcode/service/taxcode.go | 11 ++++++- 8 files changed, 36 insertions(+), 61 deletions(-) diff --git a/openmeter/app/stripe/appinvoice.go b/openmeter/app/stripe/appinvoice.go index 74befb9715..0726eeef32 100644 --- a/openmeter/app/stripe/appinvoice.go +++ b/openmeter/app/stripe/appinvoice.go @@ -571,7 +571,7 @@ func getCreditStripeAddInvoiceItemParams(calculator StripeCalculator, line billi return params } -func applyTaxSettingsToInvoiceItem(add *stripe.InvoiceItemParams, taxConfig *productcatalog.TaxConfig) *stripe.InvoiceItemParams { +func applyTaxSettingsToInvoiceItem(add *stripe.InvoiceItemParams, taxConfig *billing.TaxConfig) *stripe.InvoiceItemParams { if taxConfig != nil && !lo.IsEmpty(taxConfig) { if taxConfig.Behavior != nil { add.TaxBehavior = getStripeTaxBehavior(taxConfig.Behavior) diff --git a/openmeter/billing/httpdriver/invoiceline.go b/openmeter/billing/httpdriver/invoiceline.go index 8e24b2676b..6972e6536e 100644 --- a/openmeter/billing/httpdriver/invoiceline.go +++ b/openmeter/billing/httpdriver/invoiceline.go @@ -324,7 +324,7 @@ func mapInvoiceLineToAPI(line *billing.StandardLine) (api.InvoiceLine, error) { return api.InvoiceLine{}, fmt.Errorf("failed to map price: %w", err) } - children, err := mapDetailedLinesToAPI(line.DetailedLines, line.InvoiceAt, line.TaxConfig) + children, err := mapDetailedLinesToAPI(line.DetailedLines, line.InvoiceAt, line.TaxConfig.ToProductCatalog()) if err != nil { return api.InvoiceLine{}, fmt.Errorf("failed to map children: %w", err) } diff --git a/openmeter/billing/service/invoicecalc/taxconfig_test.go b/openmeter/billing/service/invoicecalc/taxconfig_test.go index 323a86821e..4c7486a1d4 100644 --- a/openmeter/billing/service/invoicecalc/taxconfig_test.go +++ b/openmeter/billing/service/invoicecalc/taxconfig_test.go @@ -18,7 +18,7 @@ import ( // now includes the resolved TaxCode entity. Two configs that are identical except for TaxCode // (one nil, one stamped) must compare as NOT equal, so the adapter diff guard re-upserts the line // and the snapshot is persisted to the tax_config JSONB column. -func TaxConfigEqualDetectsTaxCode(t *testing.T) { +func TestTaxConfigEqualDetectsTaxCode(t *testing.T) { tc1 := taxcode.TaxCode{ NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, AppMappings: taxcode.TaxCodeAppMappings{ @@ -49,8 +49,8 @@ func TaxConfigEqualDetectsTaxCode(t *testing.T) { assert.False(t, persistedState.Equal(expectedState), "Equal must detect the stamped TaxCode so the line is re-upserted and the snapshot persisted") - // Deep-equal sub-case: two configs whose TaxCode has the same ID/namespace/key/name but - // different AppMappings must compare as NOT equal (shallow ID comparison would miss this). + // Same-ID sub-case: two configs whose TaxCode shares the same ID but differs in other fields + // (e.g. AppMappings) must compare as equal — equality is ID-only. tc2 := taxcode.TaxCode{ NamespacedID: models.NamespacedID{Namespace: "ns", ID: "tc-1"}, AppMappings: taxcode.TaxCodeAppMappings{ @@ -75,8 +75,8 @@ func TaxConfigEqualDetectsTaxCode(t *testing.T) { TaxCode: &tc2, } - assert.False(t, leftConfig.Equal(rightConfig), - "Equal must detect different AppMappings even when TaxCode.ID is the same") + assert.True(t, leftConfig.Equal(rightConfig), + "Equal uses ID-only comparison: same TaxCode.ID means equal regardless of other TaxCode fields") } func TestSnapshotTaxConfigIntoLines(t *testing.T) { diff --git a/openmeter/billing/service/stdinvoicestate.go b/openmeter/billing/service/stdinvoicestate.go index e0d92ca0cb..3967b04c5d 100644 --- a/openmeter/billing/service/stdinvoicestate.go +++ b/openmeter/billing/service/stdinvoicestate.go @@ -168,8 +168,6 @@ func allocateStateMachine() *InvoiceStateMachine { Permit(billing.TriggerDelete, billing.StandardInvoiceStatusDeleteInProgress). Permit(billing.TriggerFailed, billing.StandardInvoiceStatusDraftSyncFailed). OnActive(statelessx.AllOf( - out.snapshotLineTaxConfigs, - out.requireDBSave, out.syncDraftInvoice, )) @@ -231,8 +229,6 @@ func allocateStateMachine() *InvoiceStateMachine { Permit(billing.TriggerFailed, billing.StandardInvoiceStatusIssuingSyncFailed). Permit(billing.TriggerDelete, billing.StandardInvoiceStatusDeleteInProgress). OnActive(statelessx.AllOf( - out.snapshotLineTaxConfigs, - out.requireDBSave, out.finalizeInvoice, )) @@ -775,35 +771,6 @@ func (m *InvoiceStateMachine) calculateInvoice(ctx context.Context) error { }) } -// snapshotLineTaxConfigs re-stamps the resolved TaxCode entity onto each line's TaxConfig -// immediately before the invoice is synced to / finalized by the external invoicing app, so the -// persisted and externally-sent snapshot reflects the tax code's point-in-time state at the -// moment of sync rather than the possibly-stale snapshot taken during the draft calculation states. -// -// It runs in the OnActive of draft.syncing and issuing.syncing. StatusDetails.Immutable flips to -// true at draft.syncing (the state drops TriggerUpdated), but the line is only truly immutable once -// finalized at issuing.syncing; this refresh is a controlled, system-driven update, not a user edit. -func (m *InvoiceStateMachine) snapshotLineTaxConfigs(ctx context.Context) error { - taxCodes, err := m.Service.resolveTaxCodes(ctx, resolveTaxCodesInput{ - Namespace: m.Invoice.Namespace, - Invoice: &m.Invoice, - // Read-only: the tax codes were already resolved/created during the draft states; we must - // not mint new tax_codes rows during sync/finalization. - ReadOnly: true, - }) - if err != nil { - return fmt.Errorf("resolving tax codes: %w", err) - } - - if err := invoicecalc.SnapshotTaxConfigIntoLines(&m.Invoice, invoicecalc.StandardInvoiceCalculatorDependencies{ - TaxCodes: taxCodes, - }); err != nil { - return fmt.Errorf("snapshotting tax configs: %w", err) - } - - return nil -} - // syncDraftInvoice syncs the draft invoice with the external system. func (m *InvoiceStateMachine) syncDraftInvoice(ctx context.Context) error { if err := m.validateNamespaceLockdown(); err != nil { diff --git a/openmeter/billing/stdinvoice.go b/openmeter/billing/stdinvoice.go index 9535aff6cf..5ff5c25103 100644 --- a/openmeter/billing/stdinvoice.go +++ b/openmeter/billing/stdinvoice.go @@ -13,7 +13,6 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing/models/externalid" "github.com/openmeterio/openmeter/openmeter/billing/models/totals" "github.com/openmeterio/openmeter/openmeter/customer" - "github.com/openmeterio/openmeter/openmeter/productcatalog" "github.com/openmeterio/openmeter/openmeter/streaming" "github.com/openmeterio/openmeter/pkg/currencyx" "github.com/openmeterio/openmeter/pkg/expand" @@ -450,7 +449,7 @@ func (i *StandardInvoice) getLeafLines() DetailedLines { type DetailedLineWithResolvedTaxConfig struct { DetailedLine - TaxConfig *productcatalog.TaxConfig + TaxConfig *TaxConfig } // GetLeafLinesWithResolvedTaxConfig returns invoice leaf lines together with the effective tax @@ -459,7 +458,7 @@ func (i *StandardInvoice) GetLeafLinesWithResolvedTaxConfig() []DetailedLineWith out := make([]DetailedLineWithResolvedTaxConfig, 0) for _, parentLine := range i.Lines.OrEmpty() { - taxConfig := productcatalog.MergeTaxConfigs(i.Workflow.Config.Invoicing.DefaultTaxConfig, parentLine.TaxConfig) + taxConfig := MergeTaxConfigs(FromProductCatalog(i.Workflow.Config.Invoicing.DefaultTaxConfig), parentLine.TaxConfig) for _, line := range parentLine.DetailedLines { out = append(out, DetailedLineWithResolvedTaxConfig{ DetailedLine: line, diff --git a/openmeter/billing/taxconfig.go b/openmeter/billing/taxconfig.go index 3ef107270d..62c6b8c56f 100644 --- a/openmeter/billing/taxconfig.go +++ b/openmeter/billing/taxconfig.go @@ -3,6 +3,7 @@ package billing import ( "errors" "fmt" + "slices" "github.com/samber/lo" @@ -49,16 +50,15 @@ func (c *TaxConfig) Equal(v *TaxConfig) bool { return false } - // none of them are nil - if !c.TaxConfig.Equal(&v.TaxConfig) { + if (c.TaxCode != nil && v.TaxCode == nil) || (c.TaxCode == nil && v.TaxCode != nil) { return false } - if c.TaxCode == nil || v.TaxCode == nil { + if c.TaxCode != nil && c.TaxCode.ID != v.TaxCode.ID { return false } - return c.TaxCode.Equal(v.TaxCode) + return c.TaxConfig.Equal(&v.TaxConfig) } func (c *TaxConfig) Validate() error { @@ -84,23 +84,13 @@ func (c *TaxConfig) Validate() error { } func (c TaxConfig) Clone() TaxConfig { - out := TaxConfig{} - - if c.Behavior != nil { - out.Behavior = lo.ToPtr(*c.Behavior) - } - - if c.Stripe != nil { - out.Stripe = lo.ToPtr(c.Stripe.Clone()) - } - - if c.TaxCodeID != nil { - out.TaxCodeID = lo.ToPtr(*c.TaxCodeID) + out := TaxConfig{ + TaxConfig: c.TaxConfig.Clone(), } if c.TaxCode != nil { tc := *c.TaxCode - tc.AppMappings = append(taxcode.TaxCodeAppMappings(nil), c.TaxCode.AppMappings...) + tc.AppMappings = slices.Clone(c.TaxCode.AppMappings) if c.TaxCode.Description != nil { tc.Description = lo.ToPtr(*c.TaxCode.Description) } diff --git a/openmeter/taxcode/errors.go b/openmeter/taxcode/errors.go index 793ddcc222..b79203ba7f 100644 --- a/openmeter/taxcode/errors.go +++ b/openmeter/taxcode/errors.go @@ -146,3 +146,13 @@ func IsTaxCodeIsOrganizationDefaultError(err error) bool { var vi models.ValidationIssue return errors.As(err, &vi) && vi.Code() == ErrCodeTaxCodeIsOrganizationDefault } + +// ErrTaxCodeOrphanedKey is returned by GetOrCreateByAppMapping when a key derived from +// the Stripe code already exists but no longer carries that app mapping (the mapping was +// changed after the key was auto-created). The typed error prevents a raw constraint error from +// poisoning the pg transaction (25P02). +var ErrTaxCodeOrphanedKey = errors.New("tax code key exists but app mapping is orphaned") + +func IsTaxCodeOrphanedKeyError(err error) bool { + return errors.Is(err, ErrTaxCodeOrphanedKey) +} diff --git a/openmeter/taxcode/service/taxcode.go b/openmeter/taxcode/service/taxcode.go index 6c19dcfbf0..56507ec071 100644 --- a/openmeter/taxcode/service/taxcode.go +++ b/openmeter/taxcode/service/taxcode.go @@ -101,7 +101,16 @@ func (s *Service) GetOrCreateByAppMapping(ctx context.Context, input taxcode.Get if err != nil { // Another request may have created it concurrently. if models.IsGenericConflictError(err) { - return s.adapter.GetTaxCodeByAppMapping(ctx, taxcode.GetTaxCodeByAppMappingInput(input)) + tc, retryErr := s.adapter.GetTaxCodeByAppMapping(ctx, taxcode.GetTaxCodeByAppMappingInput(input)) + if retryErr != nil { + if taxcode.IsTaxCodeNotFoundError(retryErr) { + // The key derived from this Stripe code exists but its app mapping was changed + // after auto-creation (orphaned key). Avoid poisoning the pg tx. + return taxcode.TaxCode{}, fmt.Errorf("resolving orphaned tax code key for %q: %w", input.TaxCode, taxcode.ErrTaxCodeOrphanedKey) + } + return taxcode.TaxCode{}, retryErr + } + return tc, nil } return taxcode.TaxCode{}, err From 0d656471e1c37730eff2650ef5a01407797c8ea3 Mon Sep 17 00:00:00 2001 From: Robert Borbely Date: Wed, 3 Jun 2026 14:29:20 +0200 Subject: [PATCH 7/7] fix(billing): fix review comment --- openmeter/taxcode/taxcode.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/openmeter/taxcode/taxcode.go b/openmeter/taxcode/taxcode.go index 258924b17f..77e98a23fc 100644 --- a/openmeter/taxcode/taxcode.go +++ b/openmeter/taxcode/taxcode.go @@ -93,39 +93,54 @@ func (t *TaxCode) Equal(v *TaxCode) bool { if t == nil && v == nil { return true } + if t == nil || v == nil { return false } + if t.ID != v.ID || t.Namespace != v.Namespace { return false } + if t.Key != v.Key || t.Name != v.Name { return false } + if (t.Description == nil) != (v.Description == nil) { return false } + if t.Description != nil && *t.Description != *v.Description { return false } + if len(t.AppMappings) != len(v.AppMappings) { return false } - for i := range t.AppMappings { - if t.AppMappings[i] != v.AppMappings[i] { + + left := lo.SliceToMap(t.AppMappings, func(m TaxCodeAppMapping) (app.AppType, string) { + return m.AppType, m.TaxCode + }) + + for _, m := range v.AppMappings { + code, ok := left[m.AppType] + if !ok || code != m.TaxCode { return false } } + return true } // IsManagedBySystem returns true when this tax code was auto-created by the system. func (t TaxCode) IsManagedBySystem() bool { v, ok := t.Annotations[AnnotationKeyManagedBy] + if !ok { return false } s, ok := v.(string) + return ok && s == AnnotationValueManagedBySystem }