chore: address tax code review feedback

Author: turip

api/v3/handlers/customers/charges/convert.go

@@ -375,11 +375,15 @@ func convertTaxCodeConfigToAPI(cfg productcatalog.TaxCodeConfig) *api.BillingTax
 		return nil
 	}
 
-	return &api.BillingTaxConfig{
-		Behavior:  (*api.BillingTaxBehavior)(cfg.Behavior),
-		TaxCode:   &api.TaxCodeReference{Id: cfg.TaxCodeID},
-		TaxCodeId: lo.ToPtr(cfg.TaxCodeID),
+	out := &api.BillingTaxConfig{
+		Behavior: (*api.BillingTaxBehavior)(cfg.Behavior),
 	}
+	if cfg.TaxCodeID != "" {
+		out.TaxCode = &api.TaxCodeReference{Id: cfg.TaxCodeID}
+		out.TaxCodeId = lo.ToPtr(cfg.TaxCodeID)
+	}
+
+	return out
 }
 
 // convertAPIChargeStatus maps an API status string to its domain equivalent.

api/v3/handlers/customers/charges/convert_test.go

@@ -43,6 +43,15 @@ func TestConvertTaxCodeConfigToAPI(t *testing.T) {
 				TaxCodeId: lo.ToPtr(api.ULID("01JTEST00000000000000000002")),
 			},
 		},
+		{
+			name: "behavior only",
+			input: productcatalog.TaxCodeConfig{
+				Behavior: lo.ToPtr(productcatalog.InclusiveTaxBehavior),
+			},
+			want: &api.BillingTaxConfig{
+				Behavior: lo.ToPtr(api.BillingTaxBehaviorInclusive),
+			},
+		},
 	}
 
 	for _, tt := range tests {

openmeter/billing/charges/charge.go

@@ -412,7 +412,7 @@ func (i ChargeIntent) Meta() (meta.Intent, error) {
 }
 
 // WithTaxCodeID returns a copy of the intent with TaxCodeID set to id.
-// If TaxConfig is nil a new one is created; all other fields are preserved.
+// Existing tax behavior and other intent fields are preserved.
 func (i ChargeIntent) WithTaxCodeID(id string) (ChargeIntent, error) {
 	switch i.t {
 	case meta.ChargeTypeFlatFee:

openmeter/billing/charges/models/chargemeta/mixin.go

@@ -87,6 +87,7 @@ func (metaMixin) Fields() []ent.Field {
 			Optional().
 			Nillable(),
 		field.String("tax_code_id").
+			NotEmpty().
 			SchemaType(map[string]string{
 				dialect.Postgres: "char(26)",
 			}),

openmeter/billing/charges/service/taxcode_test.go

@@ -98,10 +98,9 @@ func (s *TaxCodePersistenceTestSuite) TestFlatFeeChargePersistsTaxConfig() {
 		flatFee, err := readBack.AsFlatFeeCharge()
 		s.NoError(err)
 
-		s.Require().NotNil(flatFee.Intent.TaxConfig, "TaxConfig must be populated on read")
 		s.Require().NotNil(flatFee.Intent.TaxConfig.Behavior, "TaxBehavior must be persisted")
 		s.Equal(productcatalog.InclusiveTaxBehavior, *flatFee.Intent.TaxConfig.Behavior)
-		s.Require().NotNil(flatFee.Intent.TaxConfig.TaxCodeID, "TaxCodeID must be persisted as FK")
+		s.Require().NotEmpty(flatFee.Intent.TaxConfig.TaxCodeID, "TaxCodeID must be persisted as FK")
 		s.Equal(tc.ID, flatFee.Intent.TaxConfig.TaxCodeID)
 	})
 
@@ -135,7 +134,6 @@ func (s *TaxCodePersistenceTestSuite) TestFlatFeeChargePersistsTaxConfig() {
 		s.NoError(err)
 
 		// nil TaxConfig intents get the namespace default invoicing TaxCodeID stamped.
-		s.Require().NotNil(flatFee.Intent.TaxConfig, "TaxConfig must be stamped with the default invoicing tax code")
 		s.Require().NotEmpty(flatFee.Intent.TaxConfig.TaxCodeID, "default invoicing TaxCodeID must be stamped")
 		s.Equal(defaults.InvoicingTaxCodeID, flatFee.Intent.TaxConfig.TaxCodeID)
 		s.Nil(flatFee.Intent.TaxConfig.Behavior, "Behavior must remain nil when only default TaxCodeID is stamped")
@@ -196,10 +194,9 @@ func (s *TaxCodePersistenceTestSuite) TestUsageBasedChargePersistsTaxConfig() {
 		usageBased, err := readBack.AsUsageBasedCharge()
 		s.NoError(err)
 
-		s.Require().NotNil(usageBased.Intent.TaxConfig, "TaxConfig must be populated on read")
 		s.Require().NotNil(usageBased.Intent.TaxConfig.Behavior, "TaxBehavior must be persisted")
 		s.Equal(productcatalog.ExclusiveTaxBehavior, *usageBased.Intent.TaxConfig.Behavior)
-		s.Require().NotNil(usageBased.Intent.TaxConfig.TaxCodeID, "TaxCodeID must be persisted as FK")
+		s.Require().NotEmpty(usageBased.Intent.TaxConfig.TaxCodeID, "TaxCodeID must be persisted as FK")
 		s.Equal(tc.ID, usageBased.Intent.TaxConfig.TaxCodeID)
 	})
 
@@ -233,7 +230,6 @@ func (s *TaxCodePersistenceTestSuite) TestUsageBasedChargePersistsTaxConfig() {
 		s.NoError(err)
 
 		// nil TaxConfig intents get the namespace default invoicing TaxCodeID stamped.
-		s.Require().NotNil(usageBased.Intent.TaxConfig, "TaxConfig must be stamped with the default invoicing tax code")
 		s.Require().NotEmpty(usageBased.Intent.TaxConfig.TaxCodeID, "default invoicing TaxCodeID must be stamped")
 		s.Equal(defaults.InvoicingTaxCodeID, usageBased.Intent.TaxConfig.TaxCodeID)
 		s.Nil(usageBased.Intent.TaxConfig.Behavior, "Behavior must remain nil when only default TaxCodeID is stamped")
@@ -293,10 +289,9 @@ func (s *TaxCodePersistenceTestSuite) TestCreditPurchaseChargePersistsTaxConfig(
 		cp, err := readBack.AsCreditPurchaseCharge()
 		s.NoError(err)
 
-		s.Require().NotNil(cp.Intent.TaxConfig, "TaxConfig must be populated on read")
 		s.Require().NotNil(cp.Intent.TaxConfig.Behavior, "TaxBehavior must be persisted")
 		s.Equal(productcatalog.InclusiveTaxBehavior, *cp.Intent.TaxConfig.Behavior)
-		s.Require().NotNil(cp.Intent.TaxConfig.TaxCodeID, "TaxCodeID must be persisted as FK")
+		s.Require().NotEmpty(cp.Intent.TaxConfig.TaxCodeID, "TaxCodeID must be persisted as FK")
 		s.Equal(tc.ID, cp.Intent.TaxConfig.TaxCodeID)
 	})
 
@@ -333,7 +328,6 @@ func (s *TaxCodePersistenceTestSuite) TestCreditPurchaseChargePersistsTaxConfig(
 		s.NoError(err)
 
 		// nil TaxConfig intents get the namespace default credit-grant TaxCodeID stamped.
-		s.Require().NotNil(cp.Intent.TaxConfig, "TaxConfig must be stamped with the default credit grant tax code")
 		s.Require().NotEmpty(cp.Intent.TaxConfig.TaxCodeID, "default credit grant TaxCodeID must be stamped")
 		s.Equal(defaults.CreditGrantTaxCodeID, cp.Intent.TaxConfig.TaxCodeID)
 		s.Nil(cp.Intent.TaxConfig.Behavior, "Behavior must remain nil when only default TaxCodeID is stamped")
@@ -471,7 +465,7 @@ func (s *TaxCodePersistenceTestSuite) TestCreditPurchaseInvoiceSettlementPropaga
 }
 
 // TestCreditPurchaseInvoiceSettlementNilTaxConfigGetsDefaultCreditGrantTaxCodeStamped verifies that
-// when Intent.TaxConfig is nil the gathering line's TaxConfig is populated with the namespace
+// when Intent.TaxConfig is zero the gathering line's TaxConfig is populated with the namespace
 // default credit-grant tax code ID stamped by applyDefaultTaxCodes.
 func (s *TaxCodePersistenceTestSuite) TestCreditPurchaseInvoiceSettlementNilTaxConfigGetsDefaultCreditGrantTaxCodeStamped() {
 	ctx := s.T().Context()
@@ -599,7 +593,6 @@ func (s *TaxCodePersistenceTestSuite) TestFlatFeeCreditOnlyHandlerReceivesTaxCon
 	s.NoError(err)
 	s.Require().Len(advancedCharges, 1)
 
-	s.Require().NotNil(capturedInput.Charge.Intent.TaxConfig, "handler must receive TaxConfig after DB roundtrip")
 	s.Require().NotNil(capturedInput.Charge.Intent.TaxConfig.Behavior)
 	s.Equal(productcatalog.InclusiveTaxBehavior, *capturedInput.Charge.Intent.TaxConfig.Behavior)
 	s.Require().NotEmpty(capturedInput.Charge.Intent.TaxConfig.TaxCodeID)
@@ -682,7 +675,6 @@ func (s *TaxCodePersistenceTestSuite) TestUsageBasedCreditOnlyHandlerReceivesTax
 	_, err = s.Charges.AdvanceCharges(ctx, charges.AdvanceChargesInput{Customer: cust.GetID()})
 	s.NoError(err)
 
-	s.Require().NotNil(capturedInput.Charge.Intent.TaxConfig, "handler must receive TaxConfig after DB roundtrip")
 	s.Require().NotNil(capturedInput.Charge.Intent.TaxConfig.Behavior)
 	s.Equal(productcatalog.ExclusiveTaxBehavior, *capturedInput.Charge.Intent.TaxConfig.Behavior)
 	s.Require().NotEmpty(capturedInput.Charge.Intent.TaxConfig.TaxCodeID)
@@ -893,12 +885,10 @@ func (s *TaxCodePersistenceTestSuite) TestTaxConfigInListCharges() {
 
 			if ff.Intent.Intent.UniqueReferenceID != nil && *ff.Intent.Intent.UniqueReferenceID == "flat-fee-list-no-taxcode" {
 				// nil TaxConfig intents get the default invoicing TaxCodeID stamped.
-				s.Require().NotNil(ff.Intent.TaxConfig, "flat fee charge without explicit tax config must have default invoicing TaxCodeID stamped")
 				s.Require().NotEmpty(ff.Intent.TaxConfig.TaxCodeID)
 				s.Equal(defaults.InvoicingTaxCodeID, ff.Intent.TaxConfig.TaxCodeID)
 				s.Nil(ff.Intent.TaxConfig.Behavior)
 			} else {
-				s.Require().NotNil(ff.Intent.TaxConfig, "flat fee charge must carry TaxConfig in list response")
 				s.Require().NotNil(ff.Intent.TaxConfig.Behavior)
 				s.Equal(productcatalog.InclusiveTaxBehavior, *ff.Intent.TaxConfig.Behavior)
 				s.Require().NotEmpty(ff.Intent.TaxConfig.TaxCodeID)
@@ -908,7 +898,6 @@ func (s *TaxCodePersistenceTestSuite) TestTaxConfigInListCharges() {
 		case meta.ChargeTypeUsageBased:
 			ub, err := charge.AsUsageBasedCharge()
 			s.Require().NoError(err)
-			s.Require().NotNil(ub.Intent.TaxConfig, "usage-based charge must carry TaxConfig in list response")
 			s.Require().NotNil(ub.Intent.TaxConfig.Behavior)
 			s.Equal(productcatalog.InclusiveTaxBehavior, *ub.Intent.TaxConfig.Behavior)
 			s.Require().NotEmpty(ub.Intent.TaxConfig.TaxCodeID)
@@ -993,7 +982,7 @@ func (s *TaxCodePersistenceTestSuite) TestFlatFeeInvoiceSettlementPropagatesTaxC
 }
 
 // TestFlatFeeInvoiceSettlementNilTaxConfigGetsDefaultInvoicingTaxCodeStampedOnGatheringLine verifies
-// that when Intent.TaxConfig is nil, the flat-fee CreditThenInvoice gathering line's TaxConfig is
+// that when Intent.TaxConfig is zero, the flat-fee CreditThenInvoice gathering line's TaxConfig is
 // populated with the namespace default invoicing tax code ID stamped by applyDefaultTaxCodes.
 func (s *TaxCodePersistenceTestSuite) TestFlatFeeInvoiceSettlementNilTaxConfigGetsDefaultInvoicingTaxCodeStampedOnGatheringLine() {
 	ctx := s.T().Context()