Skip to content

Commit 3a58a03

Browse files
borbelyr-kongborbelyr-kong
committed
refactor(taxcode): align resolver to Resolve, add TaxCodeReference accessor and rate-card tax code guards with tests
1 parent 2176f26 commit 3a58a03

5 files changed

Lines changed: 189 additions & 20 deletions

File tree

‎openmeter/productcatalog/plan/service/taxcode_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package service_test
22

33
import (
44
"context"
5+
"errors"
56
"testing"
67
"time"
78

@@ -885,6 +886,119 @@ func TestPlanTaxCodeBackfill(t *testing.T) {
885886
})
886887
}
887888

889+
func TestPlanPublishRejectsDeletedTaxCode(t *testing.T) {
890+
MonthPeriod := datetime.MustParseDuration(t, "P1M")
891+
892+
ctx, cancel := context.WithCancel(context.Background())
893+
defer cancel()
894+
895+
env := pctestutils.NewTestEnv(t)
896+
t.Cleanup(func() { env.Close(t) })
897+
env.DBSchemaMigrate(t)
898+
899+
namespace := pctestutils.NewTestNamespace(t)
900+
901+
// given:
902+
// - meters and a feature are provisioned
903+
// - a tax code is created and then deleted, leaving a dangling reference
904+
// - a draft plan has a rate card referencing that (now-deleted) tax code
905+
906+
err := env.Meter.ReplaceMeters(ctx, pctestutils.NewTestMeters(t, namespace))
907+
require.NoError(t, err)
908+
909+
result, err := env.Meter.ListMeters(ctx, meter.ListMetersParams{
910+
Page: pagination.Page{
911+
PageSize: 1000,
912+
PageNumber: 1,
913+
},
914+
Namespace: namespace,
915+
})
916+
require.NoError(t, err)
917+
require.NotEmpty(t, result.Items)
918+
919+
features := make([]feature.Feature, 0, len(result.Items))
920+
for _, m := range result.Items {
921+
feat, err := env.Feature.CreateFeature(ctx, pctestutils.NewTestFeatureFromMeter(t, &m))
922+
require.NoError(t, err)
923+
features = append(features, feat)
924+
}
925+
926+
// Provision organisation-default tax codes so DeleteTaxCode can proceed past the org-defaults check.
927+
invoicingTC, err := env.TaxCode.CreateTaxCode(ctx, taxcode.CreateTaxCodeInput{
928+
Namespace: namespace,
929+
Key: "org-default-invoicing",
930+
Name: "Org Default Invoicing",
931+
AppMappings: taxcode.TaxCodeAppMappings{
932+
{AppType: app.AppTypeStripe, TaxCode: "txcd_00000001"},
933+
},
934+
})
935+
require.NoError(t, err)
936+
937+
creditGrantTC, err := env.TaxCode.CreateTaxCode(ctx, taxcode.CreateTaxCodeInput{
938+
Namespace: namespace,
939+
Key: "org-default-credit-grant",
940+
Name: "Org Default Credit Grant",
941+
AppMappings: taxcode.TaxCodeAppMappings{
942+
{AppType: app.AppTypeStripe, TaxCode: "txcd_00000002"},
943+
},
944+
})
945+
require.NoError(t, err)
946+
947+
_, err = env.TaxCode.UpsertOrganizationDefaultTaxCodes(ctx, taxcode.UpsertOrganizationDefaultTaxCodesInput{
948+
Namespace: namespace,
949+
InvoicingTaxCodeID: invoicingTC.ID,
950+
CreditGrantTaxCodeID: creditGrantTC.ID,
951+
})
952+
require.NoError(t, err)
953+
954+
// Create a tax code with a Stripe app mapping so it resolves at create time.
955+
tcEntity, err := env.TaxCode.CreateTaxCode(ctx, taxcode.CreateTaxCodeInput{
956+
Namespace: namespace,
957+
Key: "stripe_txcd_40000001",
958+
Name: "txcd_40000001",
959+
AppMappings: taxcode.TaxCodeAppMappings{
960+
{AppType: app.AppTypeStripe, TaxCode: "txcd_40000001"},
961+
},
962+
})
963+
require.NoError(t, err)
964+
965+
taxCodeID := tcEntity.ID
966+
967+
// Create a DRAFT plan with a rate card referencing the tax code.
968+
input := newTestPlanInput(t, namespace, newTestFlatRateCard(features[0], &productcatalog.TaxConfig{
969+
TaxCodeID: lo.ToPtr(taxCodeID),
970+
}, &MonthPeriod))
971+
input.Key = "publish-deleted-taxcode"
972+
input.Name = "Publish Deleted TaxCode"
973+
974+
p, err := env.Plan.CreatePlan(ctx, input)
975+
require.NoError(t, err)
976+
977+
// Delete the tax code — the plan-reference delete hook is NOT registered in pctestutils,
978+
// so this succeeds and leaves a dangling reference (intended for this test).
979+
err = env.TaxCode.DeleteTaxCode(ctx, taxcode.DeleteTaxCodeInput{
980+
NamespacedID: models.NamespacedID{Namespace: namespace, ID: taxCodeID},
981+
})
982+
require.NoError(t, err)
983+
984+
// when: publishing the plan
985+
publishAt := time.Now().Truncate(time.Microsecond)
986+
_, err = env.Plan.PublishPlan(ctx, plan.PublishPlanInput{
987+
NamespacedID: p.NamespacedID,
988+
EffectivePeriod: productcatalog.EffectivePeriod{
989+
EffectiveFrom: &publishAt,
990+
EffectiveTo: nil,
991+
},
992+
})
993+
994+
// then: publish fails with a rate-card tax-code-not-found validation issue
995+
require.Error(t, err)
996+
997+
var vi models.ValidationIssue
998+
require.True(t, errors.As(err, &vi), "expected ValidationIssue wrapping ErrCodeRateCardTaxCodeNotFound, got %T: %v", err, err)
999+
require.Equal(t, productcatalog.ErrCodeRateCardTaxCodeNotFound, vi.Code())
1000+
}
1001+
8881002
func TestPlanWithAddonTaxCode(t *testing.T) {
8891003
MonthPeriod := datetime.MustParseDuration(t, "P1M")
8901004

‎openmeter/productcatalog/ratecard.go‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,18 @@ func (r *RateCardMeta) SetFeature(id, key *string) {
116116
r.FeatureKey = key
117117
}
118118

119+
// TaxCodeReference returns the ID of the tax code this rate card references, or nil
120+
// when no tax code is set. It is the single read point for the referenced tax code ID:
121+
// today the ID lives on TaxConfig, but it is migrating onto RateCardMeta directly, so
122+
// routing all reads through this accessor keeps callers stable across that move.
123+
func (r RateCardMeta) TaxCodeReference() *string {
124+
if r.TaxConfig == nil {
125+
return nil
126+
}
127+
128+
return r.TaxConfig.TaxCodeID
129+
}
130+
119131
func (r RateCardMeta) Clone() RateCardMeta {
120132
clone := RateCardMeta{
121133
Key: r.Key,
@@ -938,7 +950,7 @@ func ValidateRateCardsWithFeatures(ctx context.Context, resolver NamespacedFeatu
938950
}
939951
}
940952

941-
func ValidateRateCardsWithTaxCodes(ctx context.Context, resolver NamespacedTaxCodeResolver) func(cards RateCards) error {
953+
func ValidateRateCardsWithTaxCodes(ctx context.Context, taxCodeResolver NamespacedTaxCodeResolver) func(cards RateCards) error {
942954
return func(rateCards RateCards) error {
943955
var errs []error
944956

@@ -952,11 +964,21 @@ func ValidateRateCardsWithTaxCodes(ctx context.Context, resolver NamespacedTaxCo
952964
),
953965
)
954966

955-
if rc.TaxConfig == nil || rc.TaxConfig.TaxCodeID == nil {
967+
taxCodeID := rc.TaxCodeReference()
968+
if taxCodeID == nil {
969+
continue
970+
}
971+
972+
// An explicitly empty tax code reference is a malformed user reference, not an
973+
// internal failure: resolving it would surface a generic system error, so classify
974+
// it as a not-found validation issue here instead.
975+
if *taxCodeID == "" {
976+
errs = append(errs, models.ErrorWithFieldPrefix(rateCardFieldSelector, ErrRateCardTaxCodeNotFound))
977+
956978
continue
957979
}
958980

959-
tc, err := resolver.ResolveTaxCode(ctx, *rc.TaxConfig.TaxCodeID)
981+
tc, err := taxCodeResolver.Resolve(ctx, *taxCodeID)
960982
if err != nil {
961983
if models.IsGenericNotFoundError(err) || taxcode.IsTaxCodeNotFoundError(err) {
962984
errs = append(errs, models.ErrorWithFieldPrefix(rateCardFieldSelector, ErrRateCardTaxCodeNotFound))
@@ -967,6 +989,9 @@ func ValidateRateCardsWithTaxCodes(ctx context.Context, resolver NamespacedTaxCo
967989
continue
968990
}
969991

992+
// Defensive: the NamespacedTaxCodeResolver interface permits a (nil, nil) return.
993+
// The concrete resolver never returns it, but guard against a nil code to avoid a
994+
// nil-pointer panic and treat it as not-found.
970995
if tc == nil || tc.DeletedAt != nil {
971996
errs = append(errs, models.ErrorWithFieldPrefix(rateCardFieldSelector, ErrRateCardTaxCodeNotFound))
972997
}

‎openmeter/productcatalog/taxcoderesolver.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import (
88

99
// TaxCodeResolver resolves tax codes across namespaces and can create a namespace-scoped resolver.
1010
type TaxCodeResolver interface {
11-
ResolveTaxCode(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error)
11+
Resolve(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error)
1212
WithNamespace(namespace string) NamespacedTaxCodeResolver
1313
}
1414

1515
// NamespacedTaxCodeResolver resolves tax codes within a fixed namespace.
1616
type NamespacedTaxCodeResolver interface {
17-
ResolveTaxCode(ctx context.Context, id string) (*taxcode.TaxCode, error)
17+
Resolve(ctx context.Context, id string) (*taxcode.TaxCode, error)
1818
Namespace() string
1919
}

‎openmeter/productcatalog/taxcoderesolver/resolver.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ type namespacedResolver struct {
2626

2727
func (n *namespacedResolver) Namespace() string { return n.namespace }
2828

29-
func (n *namespacedResolver) ResolveTaxCode(ctx context.Context, id string) (*taxcode.TaxCode, error) {
30-
return n.resolver.ResolveTaxCode(ctx, n.namespace, id)
29+
func (n *namespacedResolver) Resolve(ctx context.Context, id string) (*taxcode.TaxCode, error) {
30+
return n.resolver.Resolve(ctx, n.namespace, id)
3131
}
3232

3333
var _ productcatalog.TaxCodeResolver = (*resolver)(nil)
@@ -40,7 +40,7 @@ func (r *resolver) WithNamespace(namespace string) productcatalog.NamespacedTaxC
4040
return &namespacedResolver{resolver: r, namespace: namespace}
4141
}
4242

43-
func (r *resolver) ResolveTaxCode(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error) {
43+
func (r *resolver) Resolve(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error) {
4444
if namespace == "" {
4545
return nil, errors.New("namespace is not set")
4646
}

‎openmeter/productcatalog/taxcoderesolver_test.go‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type stubTaxCodeResolver struct {
2222

2323
func (s stubTaxCodeResolver) Namespace() string { return s.namespace }
2424

25-
func (s stubTaxCodeResolver) ResolveTaxCode(_ context.Context, _ string) (*taxcode.TaxCode, error) {
25+
func (s stubTaxCodeResolver) Resolve(_ context.Context, _ string) (*taxcode.TaxCode, error) {
2626
return s.result, s.err
2727
}
2828

@@ -85,39 +85,39 @@ func TestValidateRateCardsWithTaxCodes(t *testing.T) {
8585
const taxCodeID = "01JBP3SGZ20Y7VRVC351TDFXYZ"
8686

8787
t.Run("valid tax code returns no error", func(t *testing.T) {
88-
resolver := stubTaxCodeResolver{
88+
taxCodeResolver := stubTaxCodeResolver{
8989
namespace: "test",
9090
result: validTaxCode(taxCodeID),
9191
}
9292

9393
cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)}
94-
err := ValidateRateCardsWithTaxCodes(ctx, resolver)(cards)
94+
err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards)
9595
require.NoError(t, err)
9696
})
9797

9898
t.Run("deleted tax code returns ErrCodeRateCardTaxCodeNotFound", func(t *testing.T) {
99-
resolver := stubTaxCodeResolver{
99+
taxCodeResolver := stubTaxCodeResolver{
100100
namespace: "test",
101101
result: deletedTaxCode(taxCodeID),
102102
}
103103

104104
cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)}
105-
err := ValidateRateCardsWithTaxCodes(ctx, resolver)(cards)
105+
err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards)
106106
require.Error(t, err)
107107

108108
var vi models.ValidationIssue
109109
require.True(t, errors.As(err, &vi), "expected ValidationIssue, got %T: %v", err, err)
110110
require.Equal(t, ErrCodeRateCardTaxCodeNotFound, vi.Code())
111111
})
112112

113-
t.Run("not-found error from resolver returns ErrCodeRateCardTaxCodeNotFound", func(t *testing.T) {
114-
resolver := stubTaxCodeResolver{
113+
t.Run("not-found error from taxCodeResolver returns ErrCodeRateCardTaxCodeNotFound", func(t *testing.T) {
114+
taxCodeResolver := stubTaxCodeResolver{
115115
namespace: "test",
116116
err: taxcode.NewTaxCodeNotFoundError(taxCodeID),
117117
}
118118

119119
cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)}
120-
err := ValidateRateCardsWithTaxCodes(ctx, resolver)(cards)
120+
err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards)
121121
require.Error(t, err)
122122

123123
var vi models.ValidationIssue
@@ -126,14 +126,44 @@ func TestValidateRateCardsWithTaxCodes(t *testing.T) {
126126
})
127127

128128
t.Run("rate card without TaxConfig is skipped", func(t *testing.T) {
129-
resolver := stubTaxCodeResolver{
129+
taxCodeResolver := stubTaxCodeResolver{
130130
namespace: "test",
131-
// err set to ensure resolver is never called
132-
err: errors.New("resolver should not be called"),
131+
// err set to ensure taxCodeResolver is never called
132+
err: errors.New("taxCodeResolver should not be called"),
133133
}
134134

135135
cards := RateCards{makeFlatFeeRateCardNoTaxConfig("rc-no-tax")}
136-
err := ValidateRateCardsWithTaxCodes(ctx, resolver)(cards)
136+
err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards)
137137
require.NoError(t, err)
138138
})
139+
140+
t.Run("empty tax code ID returns ErrCodeRateCardTaxCodeNotFound without calling resolver", func(t *testing.T) {
141+
// given: a rate card with an explicitly empty TaxCodeID, and a resolver that errors if called
142+
taxCodeResolver := stubTaxCodeResolver{
143+
namespace: "test",
144+
err: errors.New("resolver should not be called"),
145+
}
146+
147+
cards := RateCards{
148+
&FlatFeeRateCard{
149+
RateCardMeta: RateCardMeta{
150+
Key: "rc-empty-id",
151+
Name: "rc-empty-id",
152+
TaxConfig: &TaxConfig{
153+
TaxCodeID: lo.ToPtr(""),
154+
},
155+
},
156+
},
157+
}
158+
159+
// when: running the validator
160+
err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards)
161+
162+
// then: error is a ValidationIssue with ErrCodeRateCardTaxCodeNotFound
163+
require.Error(t, err)
164+
165+
var vi models.ValidationIssue
166+
require.True(t, errors.As(err, &vi), "expected ValidationIssue, got %T: %v", err, err)
167+
require.Equal(t, ErrCodeRateCardTaxCodeNotFound, vi.Code())
168+
})
139169
}

0 commit comments

Comments
 (0)