Skip to content

Commit cfb5372

Browse files
borbelyr-kongborbelyr-kong
committed
fix(taxcode): scope plan hook by namespace, guard soft-deleted codes on update, and use a service-backed plan hook test
1 parent d49b6cd commit cfb5372

4 files changed

Lines changed: 136 additions & 105 deletions

File tree

‎openmeter/taxcode/service/hooks/planhook.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ func NewPlanHook(config PlanHookConfig) (PlanHook, error) {
5151

5252
func (e *planHook) PreDelete(ctx context.Context, tc *taxcode.TaxCode) error {
5353
affectedPlans, err := e.planService.ListPlans(ctx, plan.ListPlansInput{
54+
Namespaces: []string{tc.Namespace},
5455
Status: []productcatalog.PlanStatus{
5556
productcatalog.PlanStatusActive,
5657
productcatalog.PlanStatusArchived,

‎openmeter/taxcode/service/hooks/planhook_test.go‎

Lines changed: 96 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,113 +1,135 @@
11
package hooks_test
22

33
import (
4-
"context"
54
"testing"
65

6+
decimal "github.com/alpacahq/alpacadecimal"
7+
"github.com/samber/lo"
78
"github.com/stretchr/testify/require"
89

9-
"github.com/openmeterio/openmeter/openmeter/productcatalog/plan"
10+
"github.com/openmeterio/openmeter/openmeter/app"
11+
"github.com/openmeterio/openmeter/openmeter/productcatalog"
12+
pctestutils "github.com/openmeterio/openmeter/openmeter/productcatalog/testutils"
1013
"github.com/openmeterio/openmeter/openmeter/taxcode"
1114
"github.com/openmeterio/openmeter/openmeter/taxcode/service/hooks"
1215
"github.com/openmeterio/openmeter/pkg/models"
13-
"github.com/openmeterio/openmeter/pkg/pagination"
1416
)
1517

16-
// stubPlanService implements plan.Service for testing the plan hook.
17-
// Only ListPlans has real behavior; all other methods panic.
18-
type stubPlanService struct {
19-
listResult pagination.Result[plan.Plan]
20-
listErr error
21-
lastInput plan.ListPlansInput
22-
}
23-
24-
func (s *stubPlanService) ListPlans(ctx context.Context, params plan.ListPlansInput) (pagination.Result[plan.Plan], error) {
25-
s.lastInput = params
26-
return s.listResult, s.listErr
27-
}
28-
29-
func (s *stubPlanService) CreatePlan(_ context.Context, _ plan.CreatePlanInput) (*plan.Plan, error) {
30-
panic("not implemented")
31-
}
32-
33-
func (s *stubPlanService) DeletePlan(_ context.Context, _ plan.DeletePlanInput) error {
34-
panic("not implemented")
35-
}
36-
37-
func (s *stubPlanService) GetPlan(_ context.Context, _ plan.GetPlanInput) (*plan.Plan, error) {
38-
panic("not implemented")
39-
}
18+
// setupNamespaceDefaults provisions the organisation-default tax codes that
19+
// DeleteTaxCode requires to exist before it calls the pre-delete hook.
20+
func setupNamespaceDefaults(t *testing.T, env *pctestutils.TestEnv, ns string) {
21+
t.Helper()
4022

41-
func (s *stubPlanService) UpdatePlan(_ context.Context, _ plan.UpdatePlanInput) (*plan.Plan, error) {
42-
panic("not implemented")
43-
}
44-
45-
func (s *stubPlanService) PublishPlan(_ context.Context, _ plan.PublishPlanInput) (*plan.Plan, error) {
46-
panic("not implemented")
47-
}
23+
invoicing, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{
24+
Namespace: ns,
25+
Key: "default-invoicing",
26+
Name: "Provider Default",
27+
})
28+
require.NoError(t, err)
29+
30+
creditGrant, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{
31+
Namespace: ns,
32+
Key: "default-credit-grant",
33+
Name: "Non-Taxable",
34+
AppMappings: taxcode.TaxCodeAppMappings{
35+
{AppType: app.AppTypeStripe, TaxCode: "txcd_00000000"},
36+
},
37+
})
38+
require.NoError(t, err)
4839

49-
func (s *stubPlanService) ArchivePlan(_ context.Context, _ plan.ArchivePlanInput) (*plan.Plan, error) {
50-
panic("not implemented")
40+
_, err = env.TaxCode.UpsertOrganizationDefaultTaxCodes(t.Context(), taxcode.UpsertOrganizationDefaultTaxCodesInput{
41+
Namespace: ns,
42+
InvoicingTaxCodeID: invoicing.ID,
43+
CreditGrantTaxCodeID: creditGrant.ID,
44+
})
45+
require.NoError(t, err)
5146
}
5247

53-
func (s *stubPlanService) NextPlan(_ context.Context, _ plan.NextPlanInput) (*plan.Plan, error) {
54-
panic("not implemented")
55-
}
48+
func TestPlanHookPreDelete(t *testing.T) {
49+
// Setup real services backed by Postgres.
50+
env := pctestutils.NewTestEnv(t)
51+
t.Cleanup(func() { env.Close(t) })
52+
env.DBSchemaMigrate(t)
5653

57-
var _ plan.Service = (*stubPlanService)(nil)
54+
// Register the plan hook on the real taxcode service.
55+
planHook, err := hooks.NewPlanHook(hooks.PlanHookConfig{PlanService: env.Plan})
56+
require.NoError(t, err)
57+
env.TaxCode.RegisterHooks(planHook)
5858

59-
const testTaxCodeID = "01234567890123456789012345"
59+
ns := pctestutils.NewTestNamespace(t)
6060

61-
func TestPlanHook_PreDelete(t *testing.T) {
62-
tc := &taxcode.TaxCode{
63-
NamespacedID: models.NamespacedID{
64-
Namespace: "test-ns",
65-
ID: testTaxCodeID,
66-
},
67-
}
61+
// Provision organisation-default tax codes so DeleteTaxCode can proceed past
62+
// the org-defaults check and reach the pre-delete hook.
63+
setupNamespaceDefaults(t, env, ns)
6864

6965
t.Run("blocks deletion when a plan references the tax code", func(t *testing.T) {
70-
// given: a plan service that returns one matching plan
71-
stub := &stubPlanService{
72-
listResult: pagination.Result[plan.Plan]{
73-
Items: []plan.Plan{
74-
{ManagedModel: models.ManagedModel{}, NamespacedID: models.NamespacedID{ID: "plan-abc"}},
75-
},
76-
TotalCount: 1,
66+
// given: a tax code that a plan will reference
67+
referenced, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{
68+
Namespace: ns,
69+
Key: "referenced",
70+
Name: "Referenced Tax Code",
71+
AppMappings: taxcode.TaxCodeAppMappings{
72+
{AppType: app.AppTypeStripe, TaxCode: "txcd_20000001"},
7773
},
78-
}
74+
})
75+
require.NoError(t, err)
7976

80-
hook, err := hooks.NewPlanHook(hooks.PlanHookConfig{PlanService: stub})
77+
// given: a plan whose rate card references the tax code
78+
planInput := pctestutils.NewTestPlan(t, ns,
79+
pctestutils.WithPlanKey("plan-with-taxcode"),
80+
pctestutils.WithPlanPhases(productcatalog.Phase{
81+
PhaseMeta: productcatalog.PhaseMeta{
82+
Key: "default",
83+
Name: "Default",
84+
},
85+
RateCards: []productcatalog.RateCard{
86+
&productcatalog.FlatFeeRateCard{
87+
RateCardMeta: productcatalog.RateCardMeta{
88+
Key: "rc-1",
89+
Name: "RC 1",
90+
TaxConfig: &productcatalog.TaxConfig{
91+
TaxCodeID: lo.ToPtr(referenced.ID),
92+
},
93+
Price: productcatalog.NewPriceFrom(productcatalog.FlatPrice{
94+
Amount: decimal.NewFromInt(0),
95+
PaymentTerm: productcatalog.InArrearsPaymentTerm,
96+
}),
97+
},
98+
BillingCadence: &pctestutils.MonthPeriod,
99+
},
100+
},
101+
}),
102+
)
103+
_, err = env.Plan.CreatePlan(t.Context(), planInput)
81104
require.NoError(t, err)
82105

83-
// when: PreDelete is called
84-
err = hook.PreDelete(t.Context(), tc)
106+
// when: attempting to delete the referenced tax code
107+
err = env.TaxCode.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{
108+
NamespacedID: models.NamespacedID{Namespace: ns, ID: referenced.ID},
109+
})
85110

86111
// then: an error is returned and it is a TaxCodeReferencedByPlan error
87112
require.Error(t, err)
88113
require.True(t, taxcode.IsTaxCodeReferencedByPlanError(err),
89114
"expected TaxCodeReferencedByPlan error, got: %v", err)
90-
91-
// and: the stub received a ListPlansInput whose TaxCodes.In contains the tax code id
92-
require.NotNil(t, stub.lastInput.TaxCodes)
93-
require.NotNil(t, stub.lastInput.TaxCodes.In)
94-
require.Contains(t, *stub.lastInput.TaxCodes.In, testTaxCodeID)
95115
})
96116

97117
t.Run("allows deletion when no plan references the tax code", func(t *testing.T) {
98-
// given: a plan service that returns no matching plans
99-
stub := &stubPlanService{
100-
listResult: pagination.Result[plan.Plan]{
101-
Items: []plan.Plan{},
102-
TotalCount: 0,
118+
// given: a tax code that no plan references
119+
unreferenced, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{
120+
Namespace: ns,
121+
Key: "unreferenced",
122+
Name: "Unreferenced Tax Code",
123+
AppMappings: taxcode.TaxCodeAppMappings{
124+
{AppType: app.AppTypeStripe, TaxCode: "txcd_20000002"},
103125
},
104-
}
105-
106-
hook, err := hooks.NewPlanHook(hooks.PlanHookConfig{PlanService: stub})
126+
})
107127
require.NoError(t, err)
108128

109-
// when: PreDelete is called
110-
err = hook.PreDelete(t.Context(), tc)
129+
// when: deleting the unreferenced tax code
130+
err = env.TaxCode.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{
131+
NamespacedID: models.NamespacedID{Namespace: ns, ID: unreferenced.ID},
132+
})
111133

112134
// then: no error is returned
113135
require.NoError(t, err)

‎openmeter/taxcode/service/organizationdefaulttaxcodes.go‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,34 +24,34 @@ func (s *Service) UpsertOrganizationDefaultTaxCodes(ctx context.Context, input t
2424
}
2525

2626
return transaction.Run(ctx, s.adapter, func(ctx context.Context) (taxcode.OrganizationDefaultTaxCodes, error) {
27-
// Ensure both tax code IDs belong to the namespace and are not soft-deleted.
28-
if err := s.requireActiveTaxCode(ctx, input.Namespace, input.InvoicingTaxCodeID); err != nil {
27+
// requireActiveTaxCode ensures the tax code belongs to the namespace and is not soft-deleted.
28+
// GetTaxCode returns soft-deleted rows by ID (so billing can still resolve frozen Stripe
29+
// mappings), so a deleted code must be rejected explicitly to prevent it from being
30+
// designated as an organization default.
31+
requireActiveTaxCode := func(ctx context.Context, namespace, id string) error {
32+
tc, err := s.GetTaxCode(ctx, taxcode.GetTaxCodeInput{
33+
NamespacedID: models.NamespacedID{Namespace: namespace, ID: id},
34+
})
35+
if err != nil {
36+
return err
37+
}
38+
39+
if tc.DeletedAt != nil {
40+
return taxcode.NewTaxCodeNotFoundError(id)
41+
}
42+
43+
return nil
44+
}
45+
46+
// Ensure both tax code IDs belong to the namespace.
47+
if err := requireActiveTaxCode(ctx, input.Namespace, input.InvoicingTaxCodeID); err != nil {
2948
return taxcode.OrganizationDefaultTaxCodes{}, err
3049
}
3150

32-
if err := s.requireActiveTaxCode(ctx, input.Namespace, input.CreditGrantTaxCodeID); err != nil {
51+
if err := requireActiveTaxCode(ctx, input.Namespace, input.CreditGrantTaxCodeID); err != nil {
3352
return taxcode.OrganizationDefaultTaxCodes{}, err
3453
}
3554

3655
return s.adapter.UpsertOrganizationDefaultTaxCodes(ctx, input)
3756
})
3857
}
39-
40-
// requireActiveTaxCode ensures the tax code belongs to the namespace and is not soft-deleted.
41-
// GetTaxCode returns soft-deleted rows by ID (so billing can still resolve frozen Stripe
42-
// mappings), so a deleted code must be rejected explicitly to prevent it from being
43-
// designated as an organization default.
44-
func (s *Service) requireActiveTaxCode(ctx context.Context, namespace, id string) error {
45-
tc, err := s.GetTaxCode(ctx, taxcode.GetTaxCodeInput{
46-
NamespacedID: models.NamespacedID{Namespace: namespace, ID: id},
47-
})
48-
if err != nil {
49-
return err
50-
}
51-
52-
if tc.DeletedAt != nil {
53-
return taxcode.NewTaxCodeNotFoundError(id)
54-
}
55-
56-
return nil
57-
}

‎openmeter/taxcode/service/taxcode.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,18 @@ func (s *Service) CreateTaxCode(ctx context.Context, input taxcode.CreateTaxCode
1616
}
1717

1818
return transaction.Run(ctx, s.adapter, func(ctx context.Context) (taxcode.TaxCode, error) {
19-
tx, err := s.adapter.CreateTaxCode(ctx, input)
19+
tc, err := s.adapter.CreateTaxCode(ctx, input)
2020
if err != nil {
2121
return taxcode.TaxCode{}, err
2222
}
2323

24-
if err = s.hooks.PostCreate(ctx, &tx); err != nil {
24+
if err = s.hooks.PostCreate(ctx, &tc); err != nil {
2525
return taxcode.TaxCode{}, err
2626
}
2727

2828
// TODO: add event publishing
2929

30-
return tx, nil
30+
return tc, nil
3131
})
3232
}
3333

@@ -37,31 +37,35 @@ func (s *Service) UpdateTaxCode(ctx context.Context, input taxcode.UpdateTaxCode
3737
}
3838

3939
return transaction.Run(ctx, s.adapter, func(ctx context.Context) (taxcode.TaxCode, error) {
40-
tx, err := s.adapter.GetTaxCode(ctx, taxcode.GetTaxCodeInput{NamespacedID: input.NamespacedID})
40+
tc, err := s.adapter.GetTaxCode(ctx, taxcode.GetTaxCodeInput{NamespacedID: input.NamespacedID})
4141
if err != nil {
4242
return taxcode.TaxCode{}, err
4343
}
4444

45-
if tx.IsManagedBySystem() && !input.AllowAnnotations {
45+
if tc.DeletedAt != nil {
46+
return taxcode.TaxCode{}, models.NewGenericNotFoundError(taxcode.ErrTaxCodeNotFound)
47+
}
48+
49+
if tc.IsManagedBySystem() && !input.AllowAnnotations {
4650
return taxcode.TaxCode{}, models.NewGenericConflictError(taxcode.ErrTaxCodeManagedBySystem)
4751
}
4852

49-
if err = s.hooks.PreUpdate(ctx, &tx); err != nil {
53+
if err = s.hooks.PreUpdate(ctx, &tc); err != nil {
5054
return taxcode.TaxCode{}, err
5155
}
5256

53-
tx, err = s.adapter.UpdateTaxCode(ctx, input)
57+
tc, err = s.adapter.UpdateTaxCode(ctx, input)
5458
if err != nil {
5559
return taxcode.TaxCode{}, err
5660
}
5761

58-
if err = s.hooks.PostUpdate(ctx, &tx); err != nil {
62+
if err = s.hooks.PostUpdate(ctx, &tc); err != nil {
5963
return taxcode.TaxCode{}, err
6064
}
6165

6266
// TODO: add event publishing
6367

64-
return tx, nil
68+
return tc, nil
6569
})
6670
}
6771

@@ -152,6 +156,10 @@ func (s *Service) GetOrCreateByAppMapping(ctx context.Context, input taxcode.Get
152156
return taxcode.TaxCode{}, err
153157
}
154158

159+
if err = s.hooks.PostCreate(ctx, &tc); err != nil {
160+
return taxcode.TaxCode{}, err
161+
}
162+
155163
return tc, nil
156164
})
157165
}

0 commit comments

Comments
 (0)