Skip to content

Commit 8e5e07a

Browse files
borbelyr-kongborbelyr-kong
committed
fix(taxcode): fix review comments
1 parent 52371f9 commit 8e5e07a

3 files changed

Lines changed: 170 additions & 78 deletions

File tree

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

Lines changed: 108 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

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ func (e *addonHook) PreDelete(ctx context.Context, tc *taxcode.TaxCode) error {
5656
productcatalog.AddonStatusActive,
5757
productcatalog.AddonStatusDraft,
5858
productcatalog.AddonStatusArchived,
59+
productcatalog.AddonStatusInvalid,
5960
},
6061
TaxCodes: &filter.FilterString{
6162
In: &[]string{

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

Lines changed: 61 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,113 +1,96 @@
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/addon"
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-
// stubAddonService implements addon.Service for testing the addon hook.
17-
// Only ListAddons has real behavior; all other methods panic.
18-
type stubAddonService struct {
19-
listResult pagination.Result[addon.Addon]
20-
listErr error
21-
lastInput addon.ListAddonsInput
22-
}
23-
24-
func (s *stubAddonService) ListAddons(ctx context.Context, params addon.ListAddonsInput) (pagination.Result[addon.Addon], error) {
25-
s.lastInput = params
26-
return s.listResult, s.listErr
27-
}
28-
29-
func (s *stubAddonService) CreateAddon(_ context.Context, _ addon.CreateAddonInput) (*addon.Addon, error) {
30-
panic("not implemented")
31-
}
32-
33-
func (s *stubAddonService) DeleteAddon(_ context.Context, _ addon.DeleteAddonInput) error {
34-
panic("not implemented")
35-
}
36-
37-
func (s *stubAddonService) GetAddon(_ context.Context, _ addon.GetAddonInput) (*addon.Addon, error) {
38-
panic("not implemented")
39-
}
18+
func TestAddonHookPreDelete(t *testing.T) {
19+
// Setup real services backed by Postgres.
20+
env := pctestutils.NewTestEnv(t)
21+
t.Cleanup(func() { env.Close(t) })
22+
env.DBSchemaMigrate(t)
4023

41-
func (s *stubAddonService) UpdateAddon(_ context.Context, _ addon.UpdateAddonInput) (*addon.Addon, error) {
42-
panic("not implemented")
43-
}
24+
// Register the addon hook on the real taxcode service.
25+
addonHook, err := hooks.NewAddonHook(hooks.AddonHookConfig{AddonService: env.Addon})
26+
require.NoError(t, err)
27+
env.TaxCode.RegisterHooks(addonHook)
4428

45-
func (s *stubAddonService) PublishAddon(_ context.Context, _ addon.PublishAddonInput) (*addon.Addon, error) {
46-
panic("not implemented")
47-
}
29+
ns := pctestutils.NewTestNamespace(t)
4830

49-
func (s *stubAddonService) ArchiveAddon(_ context.Context, _ addon.ArchiveAddonInput) (*addon.Addon, error) {
50-
panic("not implemented")
51-
}
52-
53-
func (s *stubAddonService) NextAddon(_ context.Context, _ addon.NextAddonInput) (*addon.Addon, error) {
54-
panic("not implemented")
55-
}
56-
57-
var _ addon.Service = (*stubAddonService)(nil)
58-
59-
const addonTestTaxCodeID = "01234567890123456789012346"
60-
61-
func TestAddonHook_PreDelete(t *testing.T) {
62-
tc := &taxcode.TaxCode{
63-
NamespacedID: models.NamespacedID{
64-
Namespace: "test-ns",
65-
ID: addonTestTaxCodeID,
66-
},
67-
}
31+
// Provision organization-default tax codes so DeleteTaxCode can proceed past
32+
// the org-defaults check and reach the pre-delete hook.
33+
setupNamespaceDefaults(t, env, ns)
6834

6935
t.Run("blocks deletion when an add-on references the tax code", func(t *testing.T) {
70-
// given: an addon service that returns one matching add-on
71-
stub := &stubAddonService{
72-
listResult: pagination.Result[addon.Addon]{
73-
Items: []addon.Addon{
74-
{ManagedModel: models.ManagedModel{}, NamespacedID: models.NamespacedID{ID: "addon-abc"}},
75-
},
76-
TotalCount: 1,
36+
// given: a tax code that an add-on will reference
37+
referenced, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{
38+
Namespace: ns,
39+
Key: "addon-referenced",
40+
Name: "Referenced Tax Code",
41+
AppMappings: taxcode.TaxCodeAppMappings{
42+
{AppType: app.AppTypeStripe, TaxCode: "txcd_20000003"},
7743
},
78-
}
44+
})
45+
require.NoError(t, err)
7946

80-
hook, err := hooks.NewAddonHook(hooks.AddonHookConfig{AddonService: stub})
47+
// given: an add-on whose rate card references the tax code
48+
addonInput := pctestutils.NewTestAddon(t, ns,
49+
&productcatalog.FlatFeeRateCard{
50+
RateCardMeta: productcatalog.RateCardMeta{
51+
Key: "rc-1",
52+
Name: "RC 1",
53+
TaxConfig: &productcatalog.TaxConfig{
54+
TaxCodeID: lo.ToPtr(referenced.ID),
55+
},
56+
Price: productcatalog.NewPriceFrom(productcatalog.FlatPrice{
57+
Amount: decimal.NewFromInt(0),
58+
PaymentTerm: productcatalog.InArrearsPaymentTerm,
59+
}),
60+
},
61+
},
62+
)
63+
addonInput.Key = "addon-with-taxcode"
64+
_, err = env.Addon.CreateAddon(t.Context(), addonInput)
8165
require.NoError(t, err)
8266

83-
// when: PreDelete is called
84-
err = hook.PreDelete(t.Context(), tc)
67+
// when: attempting to delete the referenced tax code
68+
err = env.TaxCode.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{
69+
NamespacedID: models.NamespacedID{Namespace: ns, ID: referenced.ID},
70+
})
8571

8672
// then: an error is returned and it is a TaxCodeReferencedByAddon error
8773
require.Error(t, err)
8874
require.True(t, taxcode.IsTaxCodeReferencedByAddonError(err),
8975
"expected TaxCodeReferencedByAddon error, got: %v", err)
90-
91-
// and: the stub received a ListAddonsInput 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, addonTestTaxCodeID)
9576
})
9677

9778
t.Run("allows deletion when no add-on references the tax code", func(t *testing.T) {
98-
// given: an addon service that returns no matching add-ons
99-
stub := &stubAddonService{
100-
listResult: pagination.Result[addon.Addon]{
101-
Items: []addon.Addon{},
102-
TotalCount: 0,
79+
// given: a tax code that no add-on references
80+
unreferenced, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{
81+
Namespace: ns,
82+
Key: "addon-unreferenced",
83+
Name: "Unreferenced Tax Code",
84+
AppMappings: taxcode.TaxCodeAppMappings{
85+
{AppType: app.AppTypeStripe, TaxCode: "txcd_20000004"},
10386
},
104-
}
105-
106-
hook, err := hooks.NewAddonHook(hooks.AddonHookConfig{AddonService: stub})
87+
})
10788
require.NoError(t, err)
10889

109-
// when: PreDelete is called
110-
err = hook.PreDelete(t.Context(), tc)
90+
// when: deleting the unreferenced tax code
91+
err = env.TaxCode.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{
92+
NamespacedID: models.NamespacedID{Namespace: ns, ID: unreferenced.ID},
93+
})
11194

11295
// then: no error is returned
11396
require.NoError(t, err)

0 commit comments

Comments
 (0)