diff --git a/api/v3/handlers/taxcodes/delete.go b/api/v3/handlers/taxcodes/delete.go index 55b280351f..68d0c51645 100644 --- a/api/v3/handlers/taxcodes/delete.go +++ b/api/v3/handlers/taxcodes/delete.go @@ -18,8 +18,6 @@ type ( DeleteTaxCodeHandler = httptransport.HandlerWithArgs[DeleteTaxCodeRequest, DeleteTaxCodeResponse, DeleteTaxCodeParams] ) -const deleteTaxCodeEnabled = false - // DeleteTaxCode returns a handler for deleting a tax code. func (h *handler) DeleteTaxCode() DeleteTaxCodeHandler { return httptransport.NewHandlerWithArgs( @@ -37,10 +35,6 @@ func (h *handler) DeleteTaxCode() DeleteTaxCodeHandler { }, nil }, func(ctx context.Context, request DeleteTaxCodeRequest) (DeleteTaxCodeResponse, error) { - if !deleteTaxCodeEnabled { - return nil, apierrors.NewForbiddenErrorDetail(ctx, "deleting tax codes is not allowed") - } - err := h.service.DeleteTaxCode(ctx, taxcode.DeleteTaxCodeInput{ NamespacedID: models.NamespacedID{ Namespace: request.Namespace, diff --git a/app/common/productcatalog.go b/app/common/productcatalog.go index abf6ac1ef7..c69e5fd676 100644 --- a/app/common/productcatalog.go +++ b/app/common/productcatalog.go @@ -25,6 +25,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon" planaddonadapter "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/adapter" planaddonservice "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/service" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" "github.com/openmeterio/openmeter/openmeter/streaming" "github.com/openmeterio/openmeter/openmeter/taxcode" "github.com/openmeterio/openmeter/openmeter/watermill/eventbus" @@ -49,6 +50,7 @@ var Cost = wire.NewSet( var Plan = wire.NewSet( NewPlanService, + NewTaxCodeResolver, ) var Addon = wire.NewSet( @@ -71,6 +73,8 @@ func NewFeatureConnector( var NewFeatureResolver = featureresolver.New +var NewTaxCodeResolver = taxcoderesolver.New + func NewCostService( featureConnector feature.FeatureConnector, meterService meter.Service, @@ -88,6 +92,7 @@ func NewPlanService( logger *slog.Logger, db *entdb.Client, featureResolver productcatalog.FeatureResolver, + taxCodeResolver productcatalog.TaxCodeResolver, taxCodeService taxcode.Service, publisher eventbus.Publisher, ) (plan.Service, error) { @@ -102,6 +107,7 @@ func NewPlanService( return planservice.New(planservice.Config{ Adapter: adapter, FeatureResolver: featureResolver, + TaxCodeResolver: taxCodeResolver, TaxCode: taxCodeService, Logger: logger.With("subsystem", "productcatalog.plan"), Publisher: publisher, diff --git a/app/common/taxcode.go b/app/common/taxcode.go index 3ea8005462..3c7d122def 100644 --- a/app/common/taxcode.go +++ b/app/common/taxcode.go @@ -1,6 +1,7 @@ package common import ( + "fmt" "log/slog" "strings" @@ -10,9 +11,11 @@ import ( "github.com/openmeterio/openmeter/app/config" "github.com/openmeterio/openmeter/openmeter/app" entdb "github.com/openmeterio/openmeter/openmeter/ent/db" + "github.com/openmeterio/openmeter/openmeter/productcatalog/plan" "github.com/openmeterio/openmeter/openmeter/taxcode" taxcodeadapter "github.com/openmeterio/openmeter/openmeter/taxcode/adapter" taxcodeservice "github.com/openmeterio/openmeter/openmeter/taxcode/service" + taxcodehooks "github.com/openmeterio/openmeter/openmeter/taxcode/service/hooks" ) var TaxCode = wire.NewSet( @@ -42,6 +45,28 @@ func NewTaxCodeService( }) } +// TaxCodePlanHook prevents deleting tax codes that are still referenced by plans. +type TaxCodePlanHook taxcodehooks.PlanHook + +// NewTaxCodePlanServiceHook builds the plan-reference hook and registers it +// on the tax code service. It depends on both the plan and tax code services so wire constructs +// it only after both exist, avoiding a construction cycle (plan already depends on tax code). +func NewTaxCodePlanServiceHook( + planService plan.Service, + taxCodeService taxcode.Service, +) (TaxCodePlanHook, error) { + h, err := taxcodehooks.NewPlanHook(taxcodehooks.PlanHookConfig{ + PlanService: planService, + }) + if err != nil { + return nil, fmt.Errorf("failed to create tax code plan hook: %w", err) + } + + taxCodeService.RegisterHooks(h) + + return h, nil +} + func NewTaxCodeNamespaceHandler( logger *slog.Logger, service taxcode.Service, diff --git a/cmd/billing-worker/wire_gen.go b/cmd/billing-worker/wire_gen.go index d13e704fd8..2c22052c10 100644 --- a/cmd/billing-worker/wire_gen.go +++ b/cmd/billing-worker/wire_gen.go @@ -14,6 +14,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/meter" "github.com/openmeterio/openmeter/openmeter/namespace" "github.com/openmeterio/openmeter/openmeter/productcatalog/featureresolver" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" "github.com/openmeterio/openmeter/openmeter/streaming" "github.com/openmeterio/openmeter/openmeter/watermill/driver/kafka" "github.com/openmeterio/openmeter/openmeter/watermill/router" @@ -280,7 +281,18 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } - planService, err := common.NewPlanService(logger, client, featureResolver, taxcodeService, eventbusPublisher) + taxCodeResolver, err := taxcoderesolver.New(taxcodeService) + if err != nil { + cleanup7() + cleanup6() + cleanup5() + cleanup4() + cleanup3() + cleanup2() + cleanup() + return Application{}, nil, err + } + planService, err := common.NewPlanService(logger, client, featureResolver, taxCodeResolver, taxcodeService, eventbusPublisher) if err != nil { cleanup7() cleanup6() diff --git a/cmd/jobs/internal/wire_gen.go b/cmd/jobs/internal/wire_gen.go index 4c9c7da5f7..ba5aa3d226 100644 --- a/cmd/jobs/internal/wire_gen.go +++ b/cmd/jobs/internal/wire_gen.go @@ -26,6 +26,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/productcatalog/feature" "github.com/openmeterio/openmeter/openmeter/productcatalog/featureresolver" "github.com/openmeterio/openmeter/openmeter/productcatalog/plan" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" "github.com/openmeterio/openmeter/openmeter/registry" "github.com/openmeterio/openmeter/openmeter/secret" "github.com/openmeterio/openmeter/openmeter/streaming" @@ -289,7 +290,18 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } - planService, err := common.NewPlanService(logger, client, featureResolver, taxcodeService, eventbusPublisher) + taxCodeResolver, err := taxcoderesolver.New(taxcodeService) + if err != nil { + cleanup7() + cleanup6() + cleanup5() + cleanup4() + cleanup3() + cleanup2() + cleanup() + return Application{}, nil, err + } + planService, err := common.NewPlanService(logger, client, featureResolver, taxCodeResolver, taxcodeService, eventbusPublisher) if err != nil { cleanup7() cleanup6() diff --git a/cmd/server/wire.go b/cmd/server/wire.go index 4c4387fd04..9ce1228f9d 100644 --- a/cmd/server/wire.go +++ b/cmd/server/wire.go @@ -99,6 +99,7 @@ type Application struct { StreamingConnector streaming.Connector TaxCodeNamespaceHandler *taxcode.NamespaceHandler TaxCodeService taxcode.Service + TaxCodePlanHook common.TaxCodePlanHook TelemetryServer common.TelemetryServer TerminationChecker *common.TerminationChecker RuntimeMetricsCollector common.RuntimeMetricsCollector @@ -148,6 +149,7 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl common.Server, common.TaxCode, common.TaxCodeNamespaceHandler, + common.NewTaxCodePlanServiceHook, common.Subscription, common.Lockr, common.Secret, diff --git a/cmd/server/wire_gen.go b/cmd/server/wire_gen.go index a631c9d7b2..0ac112972f 100644 --- a/cmd/server/wire_gen.go +++ b/cmd/server/wire_gen.go @@ -32,6 +32,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/productcatalog/featureresolver" "github.com/openmeterio/openmeter/openmeter/productcatalog/plan" "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" "github.com/openmeterio/openmeter/openmeter/progressmanager" "github.com/openmeterio/openmeter/openmeter/registry" "github.com/openmeterio/openmeter/openmeter/secret" @@ -295,7 +296,18 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } - planService, err := common.NewPlanService(logger, client, featureResolver, taxcodeService, eventbusPublisher) + taxCodeResolver, err := taxcoderesolver.New(taxcodeService) + if err != nil { + cleanup7() + cleanup6() + cleanup5() + cleanup4() + cleanup3() + cleanup2() + cleanup() + return Application{}, nil, err + } + planService, err := common.NewPlanService(logger, client, featureResolver, taxCodeResolver, taxcodeService, eventbusPublisher) if err != nil { cleanup7() cleanup6() @@ -765,6 +777,18 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } + taxCodePlanHook, err := common.NewTaxCodePlanServiceHook(planService, taxcodeService) + if err != nil { + cleanup8() + cleanup7() + cleanup6() + cleanup5() + cleanup4() + cleanup3() + cleanup2() + cleanup() + return Application{}, nil, err + } health := common.NewHealthChecker(logger) telemetryHandler := common.NewTelemetryHandler(metricsTelemetryConfig, health, logger) v10, cleanup9 := common.NewTelemetryServer(telemetryConfig, telemetryHandler) @@ -860,6 +884,7 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl StreamingConnector: connector, TaxCodeNamespaceHandler: taxcodeNamespaceHandler, TaxCodeService: taxcodeService, + TaxCodePlanHook: taxCodePlanHook, TelemetryServer: v10, TerminationChecker: terminationChecker, RuntimeMetricsCollector: runtimeMetricsCollector, @@ -933,6 +958,7 @@ type Application struct { StreamingConnector streaming.Connector TaxCodeNamespaceHandler *taxcode.NamespaceHandler TaxCodeService taxcode.Service + TaxCodePlanHook common.TaxCodePlanHook TelemetryServer common.TelemetryServer TerminationChecker *common.TerminationChecker RuntimeMetricsCollector common.RuntimeMetricsCollector diff --git a/openmeter/billing/service/invoiceupdate_test.go b/openmeter/billing/service/invoiceupdate_test.go index e44f539e84..9a71f9b11e 100644 --- a/openmeter/billing/service/invoiceupdate_test.go +++ b/openmeter/billing/service/invoiceupdate_test.go @@ -1000,6 +1000,8 @@ func (s *invoiceUpdateTaxCodeService) UpsertOrganizationDefaultTaxCodes(context. return taxcode.OrganizationDefaultTaxCodes{}, errors.New("UpsertOrganizationDefaultTaxCodes is not supported in this test") } +func (s *invoiceUpdateTaxCodeService) RegisterHooks(...models.ServiceHook[taxcode.TaxCode]) {} + type preallocatingInvoiceLineAdapter struct { billing.Adapter } diff --git a/openmeter/productcatalog/errors.go b/openmeter/productcatalog/errors.go index ef74d0f50c..425b46c7a0 100644 --- a/openmeter/productcatalog/errors.go +++ b/openmeter/productcatalog/errors.go @@ -121,6 +121,16 @@ var ErrRateCardFeatureArchived = models.NewValidationIssue( commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest), ) +const ErrCodeRateCardTaxCodeNotFound models.ErrorCode = "rate_card_tax_code_not_found" + +var ErrRateCardTaxCodeNotFound = models.NewValidationIssue( + ErrCodeRateCardTaxCodeNotFound, + "tax code not found", + models.WithFieldString("taxConfig"), + models.WithCriticalSeverity(), + commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest), +) + const ErrCodeRateCardFeatureMismatch models.ErrorCode = "rate_card_feature_mismatch" var ErrRateCardFeatureMismatch = models.NewValidationIssue( diff --git a/openmeter/productcatalog/plan.go b/openmeter/productcatalog/plan.go index 2b19b299dc..69a232951b 100644 --- a/openmeter/productcatalog/plan.go +++ b/openmeter/productcatalog/plan.go @@ -365,3 +365,24 @@ func ValidatePlanWithFeatures(ctx context.Context, resolver NamespacedFeatureRes return errors.Join(errs...) } } + +func ValidatePlanWithTaxCodes(ctx context.Context, resolver NamespacedTaxCodeResolver) models.ValidatorFunc[Plan] { + return func(p Plan) error { + var errs []error + + for _, phase := range p.Phases { + phaseFieldSelector := models.NewFieldSelectorGroup( + models.NewFieldSelector("phases"). + WithExpression( + models.NewFieldAttrValue("key", phase.Key), + ), + ) + + if err := ValidateRateCardsWithTaxCodes(ctx, resolver)(phase.RateCards); err != nil { + errs = append(errs, models.ErrorWithFieldPrefix(phaseFieldSelector, err)) + } + } + + return errors.Join(errs...) + } +} diff --git a/openmeter/productcatalog/plan/adapter/adapter_test.go b/openmeter/productcatalog/plan/adapter/adapter_test.go index 4364c9feda..23ec231324 100644 --- a/openmeter/productcatalog/plan/adapter/adapter_test.go +++ b/openmeter/productcatalog/plan/adapter/adapter_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/openmeterio/openmeter/openmeter/app" entdb "github.com/openmeterio/openmeter/openmeter/ent/db" "github.com/openmeterio/openmeter/openmeter/meter" "github.com/openmeterio/openmeter/openmeter/productcatalog" @@ -17,8 +18,11 @@ import ( "github.com/openmeterio/openmeter/openmeter/productcatalog/plan" "github.com/openmeterio/openmeter/openmeter/productcatalog/plan/adapter" pctestutils "github.com/openmeterio/openmeter/openmeter/productcatalog/testutils" + "github.com/openmeterio/openmeter/openmeter/taxcode" + tctestutils "github.com/openmeterio/openmeter/openmeter/taxcode/testutils" "github.com/openmeterio/openmeter/openmeter/testutils" "github.com/openmeterio/openmeter/pkg/clock" + "github.com/openmeterio/openmeter/pkg/filter" "github.com/openmeterio/openmeter/pkg/models" "github.com/openmeterio/openmeter/pkg/pagination" ) @@ -390,6 +394,10 @@ func TestPostgresAdapter(t *testing.T) { t.Run("ListStatusFilter", func(t *testing.T) { testListPlanStatusFilter(t.Context(), t, env.PlanRepository) }) + + t.Run("ListTaxCodeFilter", func(t *testing.T) { + testListPlanTaxCodeFilter(t, env) + }) }) } @@ -568,3 +576,112 @@ func testListPlanStatusFilter(ctx context.Context, t *testing.T, repo plan.Repos }) } } + +func testListPlanTaxCodeFilter(t *testing.T, env *pctestutils.TestEnv) { + ns := pctestutils.NewTestNamespace(t) + + // Create two tax codes with distinct Stripe app mappings so the plan service + // can resolve them and populate the rate-card tax_code_id FK. + tcEnv := tctestutils.NewTestEnvFromClient(t, env.Client, env.Logger) + + taxA := tcEnv.CreateTaxCode(t, ns, taxcode.CreateTaxCodeInput{ + Key: "tax-a", + Name: "Tax A", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_10000001"}, + }, + }) + + taxB := tcEnv.CreateTaxCode(t, ns, taxcode.CreateTaxCodeInput{ + Key: "tax-b", + Name: "Tax B", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_10000002"}, + }, + }) + + // Helper: build a minimal single-phase plan with a FlatFeeRateCard referencing the given taxCodeID. + makePlanInput := func(key string, taxCodeID string) plan.CreatePlanInput { + return pctestutils.NewTestPlan(t, ns, + pctestutils.WithPlanKey(key), + pctestutils.WithPlanPhases(productcatalog.Phase{ + PhaseMeta: productcatalog.PhaseMeta{ + Key: "default", + Name: "Default", + }, + RateCards: []productcatalog.RateCard{ + &productcatalog.FlatFeeRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: "rc-1", + Name: "RC 1", + TaxConfig: &productcatalog.TaxConfig{ + TaxCodeID: lo.ToPtr(taxCodeID), + }, + Price: productcatalog.NewPriceFrom(productcatalog.FlatPrice{ + Amount: decimal.NewFromInt(0), + PaymentTerm: productcatalog.InArrearsPaymentTerm, + }), + }, + BillingCadence: &pctestutils.MonthPeriod, + }, + }, + }), + ) + } + + // given: two plans each referencing a different tax code + planA, err := env.Plan.CreatePlan(t.Context(), makePlanInput("plan-a", taxA.ID)) + require.NoError(t, err, "creating planA must not fail") + + planB, err := env.Plan.CreatePlan(t.Context(), makePlanInput("plan-b", taxB.ID)) + require.NoError(t, err, "creating planB must not fail") + + t.Run("filter by taxA returns only planA", func(t *testing.T) { + // when: listing plans filtered by taxA + result, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{ + Namespaces: []string{ns}, + TaxCodes: &filter.FilterString{In: lo.ToPtr([]string{taxA.ID})}, + Page: pagination.Page{PageSize: 100, PageNumber: 1}, + }) + + // then: only planA is returned + require.NoError(t, err) + ids := planIDs(result) + require.ElementsMatch(t, []string{planA.ID}, ids) + }) + + t.Run("filter by taxB returns only planB", func(t *testing.T) { + // when: listing plans filtered by taxB + result, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{ + Namespaces: []string{ns}, + TaxCodes: &filter.FilterString{In: lo.ToPtr([]string{taxB.ID})}, + Page: pagination.Page{PageSize: 100, PageNumber: 1}, + }) + + // then: only planB is returned + require.NoError(t, err) + ids := planIDs(result) + require.ElementsMatch(t, []string{planB.ID}, ids) + }) + + t.Run("filter by nonexistent tax code returns empty", func(t *testing.T) { + // when: listing plans filtered by a tax code id that doesn't exist + result, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{ + Namespaces: []string{ns}, + TaxCodes: &filter.FilterString{In: lo.ToPtr([]string{"01000000000000000000000000"})}, + Page: pagination.Page{PageSize: 100, PageNumber: 1}, + }) + + // then: no plans are returned + require.NoError(t, err) + require.Empty(t, result.Items) + }) +} + +func planIDs(result pagination.Result[plan.Plan]) []string { + ids := make([]string, 0, len(result.Items)) + for _, p := range result.Items { + ids = append(ids, p.ID) + } + return ids +} diff --git a/openmeter/productcatalog/plan/adapter/plan.go b/openmeter/productcatalog/plan/adapter/plan.go index 0d26f32d66..ac42240e4c 100644 --- a/openmeter/productcatalog/plan/adapter/plan.go +++ b/openmeter/productcatalog/plan/adapter/plan.go @@ -60,6 +60,17 @@ func (a *adapter) ListPlans(ctx context.Context, params plan.ListPlansInput) (pa query = filter.ApplyToQuery(query, params.Name, plandb.FieldName) query = filter.ApplyToQuery(query, params.Currency, plandb.FieldCurrency) + if params.TaxCodes != nil { + if p := filter.SelectPredicate[predicate.PlanRateCard](filter.Filter(*params.TaxCodes), ratecarddb.FieldTaxCodeID); p != nil { + query = query.Where(plandb.HasPhasesWith( + phasedb.HasRatecardsWith( + *p, + ratecarddb.DeletedAtIsNil(), + ), + )) + } + } + if !params.IncludeDeleted { query = query.Where(plandb.DeletedAtIsNil()) } diff --git a/openmeter/productcatalog/plan/service.go b/openmeter/productcatalog/plan/service.go index 5ea12e67b6..fcf9b01b4e 100644 --- a/openmeter/productcatalog/plan/service.go +++ b/openmeter/productcatalog/plan/service.go @@ -106,6 +106,9 @@ type ListPlansInput struct { // Currency filters plans by their currency field (AND semantics, supports eq/neq/contains/oeq). Currency *filter.FilterString + + // TaxCodes filters plans by their use of tax codes (AND semantics, supports eq/neq/contains/oeq). + TaxCodes *filter.FilterString } func (i ListPlansInput) Validate() error { @@ -125,6 +128,11 @@ func (i ListPlansInput) Validate() error { errs = append(errs, err) } } + if i.TaxCodes != nil { + if err := i.TaxCodes.Validate(); err != nil { + errs = append(errs, err) + } + } if i.OrderBy != "" { if err := i.OrderBy.Validate(); err != nil { errs = append(errs, err) diff --git a/openmeter/productcatalog/plan/service/plan.go b/openmeter/productcatalog/plan/service/plan.go index 5a1b484b72..990010dfa9 100644 --- a/openmeter/productcatalog/plan/service/plan.go +++ b/openmeter/productcatalog/plan/service/plan.go @@ -437,6 +437,16 @@ func (s service) PublishPlan(ctx context.Context, params plan.PublishPlanInput) ) } + // Validate plan with tax codes + err = pp.ValidateWith( + productcatalog.ValidatePlanWithTaxCodes(ctx, s.taxCodeResolver.WithNamespace(params.Namespace)), + ) + if err != nil { + errs = append(errs, fmt.Errorf("invalid plan [id=%s key=%s version=%d]: %w", + p.ID, p.Key, p.Version, err), + ) + } + // Check for incompatible add-ons assigned to this plan if p.Addons == nil { diff --git a/openmeter/productcatalog/plan/service/service.go b/openmeter/productcatalog/plan/service/service.go index b0c7512d95..fe0dd72b29 100644 --- a/openmeter/productcatalog/plan/service/service.go +++ b/openmeter/productcatalog/plan/service/service.go @@ -17,6 +17,7 @@ type Config struct { Publisher eventbus.Publisher FeatureResolver productcatalog.FeatureResolver + TaxCodeResolver productcatalog.TaxCodeResolver } func New(config Config) (plan.Service, error) { @@ -28,6 +29,10 @@ func New(config Config) (plan.Service, error) { return nil, errors.New("feature resolver is required") } + if config.TaxCodeResolver == nil { + return nil, errors.New("tax code resolver is required") + } + if config.Logger == nil { return nil, errors.New("logger is required") } @@ -47,6 +52,7 @@ func New(config Config) (plan.Service, error) { publisher: config.Publisher, featureResolver: config.FeatureResolver, + taxCodeResolver: config.TaxCodeResolver, }, nil } @@ -59,4 +65,5 @@ type service struct { publisher eventbus.Publisher featureResolver productcatalog.FeatureResolver + taxCodeResolver productcatalog.TaxCodeResolver } diff --git a/openmeter/productcatalog/plan/service/taxcode_test.go b/openmeter/productcatalog/plan/service/taxcode_test.go index 9b53d3a8e4..e82c8d0797 100644 --- a/openmeter/productcatalog/plan/service/taxcode_test.go +++ b/openmeter/productcatalog/plan/service/taxcode_test.go @@ -2,6 +2,7 @@ package service_test import ( "context" + "errors" "testing" "time" @@ -885,6 +886,119 @@ func TestPlanTaxCodeBackfill(t *testing.T) { }) } +func TestPlanPublishRejectsDeletedTaxCode(t *testing.T) { + MonthPeriod := datetime.MustParseDuration(t, "P1M") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + env := pctestutils.NewTestEnv(t) + t.Cleanup(func() { env.Close(t) }) + env.DBSchemaMigrate(t) + + namespace := pctestutils.NewTestNamespace(t) + + // given: + // - meters and a feature are provisioned + // - a tax code is created and then deleted, leaving a dangling reference + // - a draft plan has a rate card referencing that (now-deleted) tax code + + err := env.Meter.ReplaceMeters(ctx, pctestutils.NewTestMeters(t, namespace)) + require.NoError(t, err) + + result, err := env.Meter.ListMeters(ctx, meter.ListMetersParams{ + Page: pagination.Page{ + PageSize: 1000, + PageNumber: 1, + }, + Namespace: namespace, + }) + require.NoError(t, err) + require.NotEmpty(t, result.Items) + + features := make([]feature.Feature, 0, len(result.Items)) + for _, m := range result.Items { + feat, err := env.Feature.CreateFeature(ctx, pctestutils.NewTestFeatureFromMeter(t, &m)) + require.NoError(t, err) + features = append(features, feat) + } + + // Provision organization-default tax codes so DeleteTaxCode can proceed past the org-defaults check. + invoicingTC, err := env.TaxCode.CreateTaxCode(ctx, taxcode.CreateTaxCodeInput{ + Namespace: namespace, + Key: "org-default-invoicing", + Name: "Org Default Invoicing", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_00000001"}, + }, + }) + require.NoError(t, err) + + creditGrantTC, err := env.TaxCode.CreateTaxCode(ctx, taxcode.CreateTaxCodeInput{ + Namespace: namespace, + Key: "org-default-credit-grant", + Name: "Org Default Credit Grant", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_00000002"}, + }, + }) + require.NoError(t, err) + + _, err = env.TaxCode.UpsertOrganizationDefaultTaxCodes(ctx, taxcode.UpsertOrganizationDefaultTaxCodesInput{ + Namespace: namespace, + InvoicingTaxCodeID: invoicingTC.ID, + CreditGrantTaxCodeID: creditGrantTC.ID, + }) + require.NoError(t, err) + + // Create a tax code with a Stripe app mapping so it resolves at create time. + tcEntity, err := env.TaxCode.CreateTaxCode(ctx, taxcode.CreateTaxCodeInput{ + Namespace: namespace, + Key: "stripe_txcd_40000001", + Name: "txcd_40000001", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_40000001"}, + }, + }) + require.NoError(t, err) + + taxCodeID := tcEntity.ID + + // Create a DRAFT plan with a rate card referencing the tax code. + input := newTestPlanInput(t, namespace, newTestFlatRateCard(features[0], &productcatalog.TaxConfig{ + TaxCodeID: lo.ToPtr(taxCodeID), + }, &MonthPeriod)) + input.Key = "publish-deleted-taxcode" + input.Name = "Publish Deleted TaxCode" + + p, err := env.Plan.CreatePlan(ctx, input) + require.NoError(t, err) + + // Delete the tax code — the plan-reference delete hook is NOT registered in pctestutils, + // so this succeeds and leaves a dangling reference (intended for this test). + err = env.TaxCode.DeleteTaxCode(ctx, taxcode.DeleteTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: namespace, ID: taxCodeID}, + }) + require.NoError(t, err) + + // when: publishing the plan + publishAt := time.Now().Truncate(time.Microsecond) + _, err = env.Plan.PublishPlan(ctx, plan.PublishPlanInput{ + NamespacedID: p.NamespacedID, + EffectivePeriod: productcatalog.EffectivePeriod{ + EffectiveFrom: &publishAt, + EffectiveTo: nil, + }, + }) + + // then: publish fails with a rate-card tax-code-not-found validation issue + require.Error(t, err) + + var vi models.ValidationIssue + require.True(t, errors.As(err, &vi), "expected ValidationIssue wrapping ErrCodeRateCardTaxCodeNotFound, got %T: %v", err, err) + require.Equal(t, productcatalog.ErrCodeRateCardTaxCodeNotFound, vi.Code()) +} + func TestPlanWithAddonTaxCode(t *testing.T) { MonthPeriod := datetime.MustParseDuration(t, "P1M") diff --git a/openmeter/productcatalog/ratecard.go b/openmeter/productcatalog/ratecard.go index d3b690f833..82c26f23ce 100644 --- a/openmeter/productcatalog/ratecard.go +++ b/openmeter/productcatalog/ratecard.go @@ -121,6 +121,18 @@ func (r *RateCardMeta) SetFeature(id, key *string) { r.FeatureKey = key } +// TaxCodeReference returns the ID of the tax code this rate card references, or nil +// when no tax code is set. It is the single read point for the referenced tax code ID: +// today the ID lives on TaxConfig, but it is migrating onto RateCardMeta directly, so +// routing all reads through this accessor keeps callers stable across that move. +func (r RateCardMeta) TaxCodeReference() *string { + if r.TaxConfig == nil { + return nil + } + + return r.TaxConfig.TaxCodeID +} + func (r RateCardMeta) Clone() RateCardMeta { clone := RateCardMeta{ Key: r.Key, @@ -973,3 +985,54 @@ func ValidateRateCardsWithFeatures(ctx context.Context, resolver NamespacedFeatu return errors.Join(errs...) } } + +func ValidateRateCardsWithTaxCodes(ctx context.Context, taxCodeResolver NamespacedTaxCodeResolver) func(cards RateCards) error { + return func(rateCards RateCards) error { + var errs []error + + for _, rateCard := range rateCards { + rc := rateCard.AsMeta() + + rateCardFieldSelector := models.NewFieldSelectorGroup( + models.NewFieldSelector("rateCards"). + WithExpression( + models.NewFieldAttrValue("key", rateCard.Key()), + ), + ) + + taxCodeID := rc.TaxCodeReference() + if taxCodeID == nil { + continue + } + + // An explicitly empty tax code reference is a malformed user reference, not an + // internal failure: resolving it would surface a generic system error, so classify + // it as a not-found validation issue here instead. + if *taxCodeID == "" { + errs = append(errs, models.ErrorWithFieldPrefix(rateCardFieldSelector, ErrRateCardTaxCodeNotFound)) + + continue + } + + tc, err := taxCodeResolver.Resolve(ctx, *taxCodeID) + if err != nil { + if models.IsGenericNotFoundError(err) || taxcode.IsTaxCodeNotFoundError(err) { + errs = append(errs, models.ErrorWithFieldPrefix(rateCardFieldSelector, ErrRateCardTaxCodeNotFound)) + } else { + errs = append(errs, fmt.Errorf("failed to resolve tax code for ratecard: %w", err)) + } + + continue + } + + // Defensive: the NamespacedTaxCodeResolver interface permits a (nil, nil) return. + // The concrete resolver never returns it, but guard against a nil code to avoid a + // nil-pointer panic and treat it as not-found. + if tc == nil || tc.DeletedAt != nil { + errs = append(errs, models.ErrorWithFieldPrefix(rateCardFieldSelector, ErrRateCardTaxCodeNotFound)) + } + } + + return errors.Join(errs...) + } +} diff --git a/openmeter/productcatalog/tax_test.go b/openmeter/productcatalog/tax_test.go index 8288e97043..0eed202a68 100644 --- a/openmeter/productcatalog/tax_test.go +++ b/openmeter/productcatalog/tax_test.go @@ -719,6 +719,8 @@ func (s *stubTaxCodeService) UpsertOrganizationDefaultTaxCodes(_ context.Context panic("not implemented") } +func (s *stubTaxCodeService) RegisterHooks(_ ...models.ServiceHook[taxcode.TaxCode]) {} + func TestResolveTaxConfig(t *testing.T) { const ns = "test-ns" diff --git a/openmeter/productcatalog/taxcoderesolver.go b/openmeter/productcatalog/taxcoderesolver.go new file mode 100644 index 0000000000..c8c68860f9 --- /dev/null +++ b/openmeter/productcatalog/taxcoderesolver.go @@ -0,0 +1,19 @@ +package productcatalog + +import ( + "context" + + "github.com/openmeterio/openmeter/openmeter/taxcode" +) + +// TaxCodeResolver resolves tax codes across namespaces and can create a namespace-scoped resolver. +type TaxCodeResolver interface { + Resolve(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error) + WithNamespace(namespace string) NamespacedTaxCodeResolver +} + +// NamespacedTaxCodeResolver resolves tax codes within a fixed namespace. +type NamespacedTaxCodeResolver interface { + Resolve(ctx context.Context, id string) (*taxcode.TaxCode, error) + Namespace() string +} diff --git a/openmeter/productcatalog/taxcoderesolver/resolver.go b/openmeter/productcatalog/taxcoderesolver/resolver.go new file mode 100644 index 0000000000..7b180af69f --- /dev/null +++ b/openmeter/productcatalog/taxcoderesolver/resolver.go @@ -0,0 +1,56 @@ +package taxcoderesolver + +import ( + "context" + "errors" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" + "github.com/openmeterio/openmeter/openmeter/taxcode" + "github.com/openmeterio/openmeter/pkg/models" +) + +func New(service taxcode.Service) (productcatalog.TaxCodeResolver, error) { + if service == nil { + return nil, errors.New("tax code service is not set") + } + + return &resolver{service: service}, nil +} + +var _ productcatalog.NamespacedTaxCodeResolver = (*namespacedResolver)(nil) + +type namespacedResolver struct { + resolver *resolver + namespace string +} + +func (n *namespacedResolver) Namespace() string { return n.namespace } + +func (n *namespacedResolver) Resolve(ctx context.Context, id string) (*taxcode.TaxCode, error) { + return n.resolver.Resolve(ctx, n.namespace, id) +} + +var _ productcatalog.TaxCodeResolver = (*resolver)(nil) + +type resolver struct { + service taxcode.Service +} + +func (r *resolver) WithNamespace(namespace string) productcatalog.NamespacedTaxCodeResolver { + return &namespacedResolver{resolver: r, namespace: namespace} +} + +func (r *resolver) Resolve(ctx context.Context, namespace string, id string) (*taxcode.TaxCode, error) { + if namespace == "" { + return nil, errors.New("namespace is not set") + } + + tc, err := r.service.GetTaxCode(ctx, taxcode.GetTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: namespace, ID: id}, + }) + if err != nil { + return nil, err + } + + return &tc, nil +} diff --git a/openmeter/productcatalog/taxcoderesolver_test.go b/openmeter/productcatalog/taxcoderesolver_test.go new file mode 100644 index 0000000000..569d284344 --- /dev/null +++ b/openmeter/productcatalog/taxcoderesolver_test.go @@ -0,0 +1,169 @@ +package productcatalog + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/samber/lo" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/taxcode" + "github.com/openmeterio/openmeter/pkg/models" +) + +// stubTaxCodeResolver is a minimal NamespacedTaxCodeResolver for unit tests. +type stubTaxCodeResolver struct { + namespace string + result *taxcode.TaxCode + err error +} + +func (s stubTaxCodeResolver) Namespace() string { return s.namespace } + +func (s stubTaxCodeResolver) Resolve(_ context.Context, _ string) (*taxcode.TaxCode, error) { + return s.result, s.err +} + +// makeFlatFeeRateCardWithTaxCodeID builds a minimal FlatFeeRateCard with a TaxConfig pointing at id. +func makeFlatFeeRateCardWithTaxCodeID(key, taxCodeID string) RateCard { + return &FlatFeeRateCard{ + RateCardMeta: RateCardMeta{ + Key: key, + Name: key, + TaxConfig: &TaxConfig{ + TaxCodeID: lo.ToPtr(taxCodeID), + }, + }, + } +} + +// makeFlatFeeRateCardNoTaxConfig builds a minimal FlatFeeRateCard without any TaxConfig. +func makeFlatFeeRateCardNoTaxConfig(key string) RateCard { + return &FlatFeeRateCard{ + RateCardMeta: RateCardMeta{ + Key: key, + Name: key, + }, + } +} + +// validTaxCode returns a non-deleted TaxCode stub. +func validTaxCode(id string) *taxcode.TaxCode { + now := time.Now() + return &taxcode.TaxCode{ + NamespacedID: models.NamespacedID{Namespace: "test", ID: id}, + ManagedModel: models.ManagedModel{ + CreatedAt: now, + UpdatedAt: now, + DeletedAt: nil, + }, + Key: "tc-key", + Name: "Tax Code", + } +} + +// deletedTaxCode returns a TaxCode stub whose DeletedAt is set. +func deletedTaxCode(id string) *taxcode.TaxCode { + now := time.Now() + deleted := now.Add(-time.Hour) + return &taxcode.TaxCode{ + NamespacedID: models.NamespacedID{Namespace: "test", ID: id}, + ManagedModel: models.ManagedModel{ + CreatedAt: now.Add(-2 * time.Hour), + UpdatedAt: now.Add(-2 * time.Hour), + DeletedAt: &deleted, + }, + Key: "tc-key", + Name: "Tax Code", + } +} + +func TestValidateRateCardsWithTaxCodes(t *testing.T) { + ctx := context.Background() + const taxCodeID = "01JBP3SGZ20Y7VRVC351TDFXYZ" + + t.Run("valid tax code returns no error", func(t *testing.T) { + taxCodeResolver := stubTaxCodeResolver{ + namespace: "test", + result: validTaxCode(taxCodeID), + } + + cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)} + err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards) + require.NoError(t, err) + }) + + t.Run("deleted tax code returns ErrCodeRateCardTaxCodeNotFound", func(t *testing.T) { + taxCodeResolver := stubTaxCodeResolver{ + namespace: "test", + result: deletedTaxCode(taxCodeID), + } + + cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)} + err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards) + require.Error(t, err) + + var vi models.ValidationIssue + require.True(t, errors.As(err, &vi), "expected ValidationIssue, got %T: %v", err, err) + require.Equal(t, ErrCodeRateCardTaxCodeNotFound, vi.Code()) + }) + + t.Run("not-found error from taxCodeResolver returns ErrCodeRateCardTaxCodeNotFound", func(t *testing.T) { + taxCodeResolver := stubTaxCodeResolver{ + namespace: "test", + err: taxcode.NewTaxCodeNotFoundError(taxCodeID), + } + + cards := RateCards{makeFlatFeeRateCardWithTaxCodeID("rc-1", taxCodeID)} + err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards) + require.Error(t, err) + + var vi models.ValidationIssue + require.True(t, errors.As(err, &vi), "expected ValidationIssue, got %T: %v", err, err) + require.Equal(t, ErrCodeRateCardTaxCodeNotFound, vi.Code()) + }) + + t.Run("rate card without TaxConfig is skipped", func(t *testing.T) { + taxCodeResolver := stubTaxCodeResolver{ + namespace: "test", + // err set to ensure taxCodeResolver is never called + err: errors.New("taxCodeResolver should not be called"), + } + + cards := RateCards{makeFlatFeeRateCardNoTaxConfig("rc-no-tax")} + err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards) + require.NoError(t, err) + }) + + t.Run("empty tax code ID returns ErrCodeRateCardTaxCodeNotFound without calling resolver", func(t *testing.T) { + // given: a rate card with an explicitly empty TaxCodeID, and a resolver that errors if called + taxCodeResolver := stubTaxCodeResolver{ + namespace: "test", + err: errors.New("resolver should not be called"), + } + + cards := RateCards{ + &FlatFeeRateCard{ + RateCardMeta: RateCardMeta{ + Key: "rc-empty-id", + Name: "rc-empty-id", + TaxConfig: &TaxConfig{ + TaxCodeID: lo.ToPtr(""), + }, + }, + }, + } + + // when: running the validator + err := ValidateRateCardsWithTaxCodes(ctx, taxCodeResolver)(cards) + + // then: error is a ValidationIssue with ErrCodeRateCardTaxCodeNotFound + require.Error(t, err) + + var vi models.ValidationIssue + require.True(t, errors.As(err, &vi), "expected ValidationIssue, got %T: %v", err, err) + require.Equal(t, ErrCodeRateCardTaxCodeNotFound, vi.Code()) + }) +} diff --git a/openmeter/productcatalog/testutils/env.go b/openmeter/productcatalog/testutils/env.go index 7e5576f398..a8516d662a 100644 --- a/openmeter/productcatalog/testutils/env.go +++ b/openmeter/productcatalog/testutils/env.go @@ -22,6 +22,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon" planaddonadapter "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/adapter" planaddonservice "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/service" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" "github.com/openmeterio/openmeter/openmeter/taxcode" taxcodeadapter "github.com/openmeterio/openmeter/openmeter/taxcode/adapter" taxcodeservice "github.com/openmeterio/openmeter/openmeter/taxcode/service" @@ -117,6 +118,9 @@ func NewTestEnv(t *testing.T) *TestEnv { }) require.NoErrorf(t, err, "initializing tax code service must not fail") + taxCodeResolver, err := taxcoderesolver.New(taxCodeService) + require.NoErrorf(t, err, "failed to create tax code resolver: %v", err) + // Init plan service planAdapter, err := planadapter.New(planadapter.Config{ Client: client, @@ -128,6 +132,7 @@ func NewTestEnv(t *testing.T) *TestEnv { planService, err := planservice.New(planservice.Config{ Adapter: planAdapter, FeatureResolver: featureResolver, + TaxCodeResolver: taxCodeResolver, TaxCode: taxCodeService, Logger: logger, Publisher: publisher, diff --git a/openmeter/server/server_test.go b/openmeter/server/server_test.go index 7715bb1bac..6487f1ae46 100644 --- a/openmeter/server/server_test.go +++ b/openmeter/server/server_test.go @@ -2051,6 +2051,8 @@ func (n NoopTaxCodeService) UpsertOrganizationDefaultTaxCodes(ctx context.Contex return taxcode.OrganizationDefaultTaxCodes{}, nil } +func (n NoopTaxCodeService) RegisterHooks(_ ...models.ServiceHook[taxcode.TaxCode]) {} + // SubjectService methods var _ subject.Service = &NoopSubjectService{} diff --git a/openmeter/subscription/testutils/service.go b/openmeter/subscription/testutils/service.go index d0adcdf6b9..9a9757040f 100644 --- a/openmeter/subscription/testutils/service.go +++ b/openmeter/subscription/testutils/service.go @@ -24,6 +24,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon" planaddonrepo "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/adapter" planaddonservice "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/service" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" "github.com/openmeterio/openmeter/openmeter/registry" registrybuilder "github.com/openmeterio/openmeter/openmeter/registry/builder" streamingtestutils "github.com/openmeterio/openmeter/openmeter/streaming/testutils" @@ -188,8 +189,12 @@ func NewService(t *testing.T, dbDeps *DBDeps) SubscriptionDependencies { featureResolver, err := featureresolver.New(entitlementRegistry.Feature) require.NoErrorf(t, err, "failed to create feature resolver: %v", err) + taxCodeResolver, err := taxcoderesolver.New(taxCodeService) + require.NoErrorf(t, err, "failed to create tax code resolver: %v", err) + planService, err := planservice.New(planservice.Config{ FeatureResolver: featureResolver, + TaxCodeResolver: taxCodeResolver, Logger: logger, Adapter: planRepo, Publisher: publisher, diff --git a/openmeter/taxcode/adapter/taxcode.go b/openmeter/taxcode/adapter/taxcode.go index 744ccb4377..b12a97b0ef 100644 --- a/openmeter/taxcode/adapter/taxcode.go +++ b/openmeter/taxcode/adapter/taxcode.go @@ -110,11 +110,10 @@ func (a *adapter) GetTaxCode(ctx context.Context, input taxcode.GetTaxCodeInput) return entutils.TransactingRepo(ctx, a, func(ctx context.Context, a *adapter) (taxcode.TaxCode, error) { query := a.db.TaxCode.Query(). - Where(taxcodedb.Namespace(input.Namespace)). - Where(taxcodedb.ID(input.ID)) - if !input.IncludeDeleted { - query = query.Where(taxcodedb.DeletedAtIsNil()) - } + Where( + taxcodedb.Namespace(input.Namespace), + taxcodedb.ID(input.ID), + ) entity, err := query.Only(ctx) if err != nil { diff --git a/openmeter/taxcode/errors.go b/openmeter/taxcode/errors.go index 381add7b6e..b75189d227 100644 --- a/openmeter/taxcode/errors.go +++ b/openmeter/taxcode/errors.go @@ -162,3 +162,23 @@ var ErrTaxCodeOrphanedKey = errors.New("tax code key exists but app mapping is o func IsTaxCodeOrphanedKeyError(err error) bool { return errors.Is(err, ErrTaxCodeOrphanedKey) } + +const ErrCodeTaxCodeReferencedByPlan models.ErrorCode = "tax_code_referenced_by_plan" + +var ErrTaxCodeReferencedByPlan = models.NewValidationIssue( + ErrCodeTaxCodeReferencedByPlan, + "tax code cannot be deleted as it is referenced by one or more plans", + models.WithCriticalSeverity(), + commonhttp.WithHTTPStatusCodeAttribute(http.StatusConflict), +) + +func NewTaxCodeReferencedByPlanError(taxCodeID string, planIDs []string) error { + return ErrTaxCodeReferencedByPlan. + WithAttr("id", taxCodeID). + WithAttr("plan_ids", planIDs) +} + +func IsTaxCodeReferencedByPlanError(err error) bool { + var vi models.ValidationIssue + return errors.As(err, &vi) && vi.Code() == ErrCodeTaxCodeReferencedByPlan +} diff --git a/openmeter/taxcode/service.go b/openmeter/taxcode/service.go index 55e2d840f5..e059e939a6 100644 --- a/openmeter/taxcode/service.go +++ b/openmeter/taxcode/service.go @@ -12,6 +12,8 @@ import ( type Service interface { TaxCodeService OrganizationDefaultTaxCodesService + + models.ServiceHooks[TaxCode] } type TaxCodeService interface { @@ -133,10 +135,6 @@ func (i ListTaxCodesInput) Validate() error { type GetTaxCodeInput struct { models.NamespacedID - - // IncludeDeleted controls whether soft-deleted records are returned. - // Only for internal service-to-service use; never set from API handlers. - IncludeDeleted bool } func (i GetTaxCodeInput) Validate() error { diff --git a/openmeter/taxcode/service/hooks/planhook.go b/openmeter/taxcode/service/hooks/planhook.go new file mode 100644 index 0000000000..28e62b9ca0 --- /dev/null +++ b/openmeter/taxcode/service/hooks/planhook.go @@ -0,0 +1,84 @@ +package hooks + +import ( + "context" + "fmt" + + "github.com/samber/lo" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" + "github.com/openmeterio/openmeter/openmeter/productcatalog/plan" + "github.com/openmeterio/openmeter/openmeter/taxcode" + "github.com/openmeterio/openmeter/pkg/filter" + "github.com/openmeterio/openmeter/pkg/models" + "github.com/openmeterio/openmeter/pkg/pagination" +) + +type ( + PlanHook = models.ServiceHook[taxcode.TaxCode] + NoopPlanHook = models.NoopServiceHook[taxcode.TaxCode] +) + +type PlanHookConfig struct { + PlanService plan.Service +} + +func (e PlanHookConfig) Validate() error { + if e.PlanService == nil { + return fmt.Errorf("plan service is required") + } + + return nil +} + +var _ models.ServiceHook[taxcode.TaxCode] = (*planHook)(nil) + +type planHook struct { + NoopPlanHook + + planService plan.Service +} + +func NewPlanHook(config PlanHookConfig) (PlanHook, error) { + if err := config.Validate(); err != nil { + return nil, fmt.Errorf("invalid plan hook config: %w", err) + } + + return &planHook{ + planService: config.PlanService, + }, nil +} + +func (e *planHook) PreDelete(ctx context.Context, tc *taxcode.TaxCode) error { + affectedPlans, err := e.planService.ListPlans(ctx, plan.ListPlansInput{ + Namespaces: []string{tc.Namespace}, + Status: []productcatalog.PlanStatus{ + productcatalog.PlanStatusActive, + productcatalog.PlanStatusArchived, + productcatalog.PlanStatusDraft, + productcatalog.PlanStatusScheduled, + productcatalog.PlanStatusInvalid, + }, + TaxCodes: &filter.FilterString{ + In: &[]string{ + tc.ID, + }, + }, + Page: pagination.Page{ + PageSize: 5, + PageNumber: 1, + }, + }) + if err != nil { + return fmt.Errorf("failed to list plans: %w", err) + } + + if len(affectedPlans.Items) > 0 { + planIDs := lo.Map(affectedPlans.Items, func(item plan.Plan, _ int) string { + return item.ID + }) + return taxcode.NewTaxCodeReferencedByPlanError(tc.ID, planIDs) + } + + return nil +} diff --git a/openmeter/taxcode/service/hooks/planhook_test.go b/openmeter/taxcode/service/hooks/planhook_test.go new file mode 100644 index 0000000000..e4f60548eb --- /dev/null +++ b/openmeter/taxcode/service/hooks/planhook_test.go @@ -0,0 +1,137 @@ +package hooks_test + +import ( + "testing" + + decimal "github.com/alpacahq/alpacadecimal" + "github.com/samber/lo" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/app" + "github.com/openmeterio/openmeter/openmeter/productcatalog" + pctestutils "github.com/openmeterio/openmeter/openmeter/productcatalog/testutils" + "github.com/openmeterio/openmeter/openmeter/taxcode" + "github.com/openmeterio/openmeter/openmeter/taxcode/service/hooks" + "github.com/openmeterio/openmeter/pkg/models" +) + +// setupNamespaceDefaults provisions the organization-default tax codes that +// DeleteTaxCode requires to exist before it calls the pre-delete hook. +func setupNamespaceDefaults(t *testing.T, env *pctestutils.TestEnv, ns string) { + t.Helper() + + invoicing, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{ + Namespace: ns, + Key: "default-invoicing", + Name: "Provider Default", + }) + require.NoError(t, err) + + creditGrant, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{ + Namespace: ns, + Key: "default-credit-grant", + Name: "Non-Taxable", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_00000000"}, + }, + }) + require.NoError(t, err) + + _, err = env.TaxCode.UpsertOrganizationDefaultTaxCodes(t.Context(), taxcode.UpsertOrganizationDefaultTaxCodesInput{ + Namespace: ns, + InvoicingTaxCodeID: invoicing.ID, + CreditGrantTaxCodeID: creditGrant.ID, + }) + require.NoError(t, err) +} + +func TestPlanHookPreDelete(t *testing.T) { + // Setup real services backed by Postgres. + env := pctestutils.NewTestEnv(t) + t.Cleanup(func() { env.Close(t) }) + env.DBSchemaMigrate(t) + + // Register the plan hook on the real taxcode service. + planHook, err := hooks.NewPlanHook(hooks.PlanHookConfig{PlanService: env.Plan}) + require.NoError(t, err) + env.TaxCode.RegisterHooks(planHook) + + ns := pctestutils.NewTestNamespace(t) + + // Provision organization-default tax codes so DeleteTaxCode can proceed past + // the org-defaults check and reach the pre-delete hook. + setupNamespaceDefaults(t, env, ns) + + t.Run("blocks deletion when a plan references the tax code", func(t *testing.T) { + // given: a tax code that a plan will reference + referenced, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{ + Namespace: ns, + Key: "referenced", + Name: "Referenced Tax Code", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_20000001"}, + }, + }) + require.NoError(t, err) + + // given: a plan whose rate card references the tax code + planInput := pctestutils.NewTestPlan(t, ns, + pctestutils.WithPlanKey("plan-with-taxcode"), + pctestutils.WithPlanPhases(productcatalog.Phase{ + PhaseMeta: productcatalog.PhaseMeta{ + Key: "default", + Name: "Default", + }, + RateCards: []productcatalog.RateCard{ + &productcatalog.FlatFeeRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: "rc-1", + Name: "RC 1", + TaxConfig: &productcatalog.TaxConfig{ + TaxCodeID: lo.ToPtr(referenced.ID), + }, + Price: productcatalog.NewPriceFrom(productcatalog.FlatPrice{ + Amount: decimal.NewFromInt(0), + PaymentTerm: productcatalog.InArrearsPaymentTerm, + }), + }, + BillingCadence: &pctestutils.MonthPeriod, + }, + }, + }), + ) + _, err = env.Plan.CreatePlan(t.Context(), planInput) + require.NoError(t, err) + + // when: attempting to delete the referenced tax code + err = env.TaxCode.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: ns, ID: referenced.ID}, + }) + + // then: an error is returned and it is a TaxCodeReferencedByPlan error + require.Error(t, err) + require.True(t, taxcode.IsTaxCodeReferencedByPlanError(err), + "expected TaxCodeReferencedByPlan error, got: %v", err) + }) + + t.Run("allows deletion when no plan references the tax code", func(t *testing.T) { + // given: a tax code that no plan references + unreferenced, err := env.TaxCode.CreateTaxCode(t.Context(), taxcode.CreateTaxCodeInput{ + Namespace: ns, + Key: "unreferenced", + Name: "Unreferenced Tax Code", + AppMappings: taxcode.TaxCodeAppMappings{ + {AppType: app.AppTypeStripe, TaxCode: "txcd_20000002"}, + }, + }) + require.NoError(t, err) + + // when: deleting the unreferenced tax code + err = env.TaxCode.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: ns, ID: unreferenced.ID}, + }) + + // then: no error is returned + require.NoError(t, err) + }) +} diff --git a/openmeter/taxcode/service/organizationdefaulttaxcodes.go b/openmeter/taxcode/service/organizationdefaulttaxcodes.go index 5b03ba1863..82cd352939 100644 --- a/openmeter/taxcode/service/organizationdefaulttaxcodes.go +++ b/openmeter/taxcode/service/organizationdefaulttaxcodes.go @@ -24,16 +24,31 @@ func (s *Service) UpsertOrganizationDefaultTaxCodes(ctx context.Context, input t } return transaction.Run(ctx, s.adapter, func(ctx context.Context) (taxcode.OrganizationDefaultTaxCodes, error) { + // requireActiveTaxCode ensures the tax code belongs to the namespace and is not soft-deleted. + // GetTaxCode returns soft-deleted rows by ID (so billing can still resolve frozen Stripe + // mappings), so a deleted code must be rejected explicitly to prevent it from being + // designated as an organization default. + requireActiveTaxCode := func(ctx context.Context, namespace, id string) error { + tc, err := s.GetTaxCode(ctx, taxcode.GetTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: namespace, ID: id}, + }) + if err != nil { + return err + } + + if tc.IsDeleted() { + return taxcode.NewTaxCodeNotFoundError(id) + } + + return nil + } + // Ensure both tax code IDs belong to the namespace. - if _, err := s.GetTaxCode(ctx, taxcode.GetTaxCodeInput{ - NamespacedID: models.NamespacedID{Namespace: input.Namespace, ID: input.InvoicingTaxCodeID}, - }); err != nil { + if err := requireActiveTaxCode(ctx, input.Namespace, input.InvoicingTaxCodeID); err != nil { return taxcode.OrganizationDefaultTaxCodes{}, err } - if _, err := s.GetTaxCode(ctx, taxcode.GetTaxCodeInput{ - NamespacedID: models.NamespacedID{Namespace: input.Namespace, ID: input.CreditGrantTaxCodeID}, - }); err != nil { + if err := requireActiveTaxCode(ctx, input.Namespace, input.CreditGrantTaxCodeID); err != nil { return taxcode.OrganizationDefaultTaxCodes{}, err } diff --git a/openmeter/taxcode/service/organizationdefaulttaxcodes_test.go b/openmeter/taxcode/service/organizationdefaulttaxcodes_test.go index ba074c960c..bd076fd184 100644 --- a/openmeter/taxcode/service/organizationdefaulttaxcodes_test.go +++ b/openmeter/taxcode/service/organizationdefaulttaxcodes_test.go @@ -117,6 +117,44 @@ func TestOrganizationDefaultTaxCodesService(t *testing.T) { assert.True(t, taxcode.IsTaxCodeNotFoundError(err), "tax code from another namespace must not resolve") }) + t.Run("SoftDeleted/InvoicingTaxCodeIsDeleted", func(t *testing.T) { + dns := testutils.NameGenerator.Generate().Key + env.SetupNamespaceDefaults(t, dns) + deleted := env.CreateTaxCode(t, dns) + active := env.CreateTaxCode(t, dns) + + require.NoError(t, env.Service.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: dns, ID: deleted.ID}, + })) + + _, err := env.Service.UpsertOrganizationDefaultTaxCodes(t.Context(), taxcode.UpsertOrganizationDefaultTaxCodesInput{ + Namespace: dns, + InvoicingTaxCodeID: deleted.ID, + CreditGrantTaxCodeID: active.ID, + }) + require.Error(t, err) + assert.True(t, taxcode.IsTaxCodeNotFoundError(err), "soft-deleted tax code must not be settable as an organization default") + }) + + t.Run("SoftDeleted/CreditGrantTaxCodeIsDeleted", func(t *testing.T) { + dns := testutils.NameGenerator.Generate().Key + env.SetupNamespaceDefaults(t, dns) + active := env.CreateTaxCode(t, dns) + deleted := env.CreateTaxCode(t, dns) + + require.NoError(t, env.Service.DeleteTaxCode(t.Context(), taxcode.DeleteTaxCodeInput{ + NamespacedID: models.NamespacedID{Namespace: dns, ID: deleted.ID}, + })) + + _, err := env.Service.UpsertOrganizationDefaultTaxCodes(t.Context(), taxcode.UpsertOrganizationDefaultTaxCodesInput{ + Namespace: dns, + InvoicingTaxCodeID: active.ID, + CreditGrantTaxCodeID: deleted.ID, + }) + require.Error(t, err) + assert.True(t, taxcode.IsTaxCodeNotFoundError(err), "soft-deleted tax code must not be settable as an organization default") + }) + t.Run("Create", func(t *testing.T) { ns2 := testutils.NameGenerator.Generate().Key invoicing := env.CreateTaxCode(t, ns2) diff --git a/openmeter/taxcode/service/service.go b/openmeter/taxcode/service/service.go index 30756c80f5..fb15a400bc 100644 --- a/openmeter/taxcode/service/service.go +++ b/openmeter/taxcode/service/service.go @@ -5,6 +5,7 @@ import ( "log/slog" "github.com/openmeterio/openmeter/openmeter/taxcode" + "github.com/openmeterio/openmeter/pkg/models" ) var _ taxcode.Service = (*Service)(nil) @@ -12,6 +13,8 @@ var _ taxcode.Service = (*Service)(nil) type Service struct { adapter taxcode.Repository logger *slog.Logger + + hooks models.ServiceHookRegistry[taxcode.TaxCode] } type Config struct { @@ -33,6 +36,10 @@ func (c Config) Validate() error { return errors.Join(errs...) } +func (s *Service) RegisterHooks(hooks ...models.ServiceHook[taxcode.TaxCode]) { + s.hooks.RegisterHooks(hooks...) +} + func New(config Config) (*Service, error) { if err := config.Validate(); err != nil { return nil, err @@ -41,5 +48,7 @@ func New(config Config) (*Service, error) { return &Service{ adapter: config.Adapter, logger: config.Logger, + hooks: models.ServiceHookRegistry[taxcode.TaxCode]{}, + // TODO: add event publisher }, nil } diff --git a/openmeter/taxcode/service/taxcode.go b/openmeter/taxcode/service/taxcode.go index 3b29be465a..3fc7dfc984 100644 --- a/openmeter/taxcode/service/taxcode.go +++ b/openmeter/taxcode/service/taxcode.go @@ -16,7 +16,18 @@ func (s *Service) CreateTaxCode(ctx context.Context, input taxcode.CreateTaxCode } return transaction.Run(ctx, s.adapter, func(ctx context.Context) (taxcode.TaxCode, error) { - return s.adapter.CreateTaxCode(ctx, input) + tc, err := s.adapter.CreateTaxCode(ctx, input) + if err != nil { + return taxcode.TaxCode{}, err + } + + if err = s.hooks.PostCreate(ctx, &tc); err != nil { + return taxcode.TaxCode{}, err + } + + // TODO: add event publishing + + return tc, nil }) } @@ -26,16 +37,35 @@ func (s *Service) UpdateTaxCode(ctx context.Context, input taxcode.UpdateTaxCode } return transaction.Run(ctx, s.adapter, func(ctx context.Context) (taxcode.TaxCode, error) { - existing, err := s.adapter.GetTaxCode(ctx, taxcode.GetTaxCodeInput{NamespacedID: input.NamespacedID}) + tc, err := s.adapter.GetTaxCode(ctx, taxcode.GetTaxCodeInput{NamespacedID: input.NamespacedID}) if err != nil { return taxcode.TaxCode{}, err } - if existing.IsManagedBySystem() && !input.AllowAnnotations { + if tc.IsDeleted() { + return taxcode.TaxCode{}, models.NewGenericNotFoundError(taxcode.ErrTaxCodeNotFound) + } + + if tc.IsManagedBySystem() && !input.AllowAnnotations { return taxcode.TaxCode{}, models.NewGenericConflictError(taxcode.ErrTaxCodeManagedBySystem) } - return s.adapter.UpdateTaxCode(ctx, input) + if err = s.hooks.PreUpdate(ctx, &tc); err != nil { + return taxcode.TaxCode{}, err + } + + tc, err = s.adapter.UpdateTaxCode(ctx, input) + if err != nil { + return taxcode.TaxCode{}, err + } + + if err = s.hooks.PostUpdate(ctx, &tc); err != nil { + return taxcode.TaxCode{}, err + } + + // TODO: add event publishing + + return tc, nil }) } @@ -126,6 +156,10 @@ func (s *Service) GetOrCreateByAppMapping(ctx context.Context, input taxcode.Get return taxcode.TaxCode{}, err } + if err = s.hooks.PostCreate(ctx, &tc); err != nil { + return taxcode.TaxCode{}, err + } + return tc, nil }) } @@ -141,6 +175,10 @@ func (s *Service) DeleteTaxCode(ctx context.Context, input taxcode.DeleteTaxCode return err } + if existing.IsDeleted() { + return nil + } + if existing.IsManagedBySystem() && !input.AllowAnnotations { return models.NewGenericConflictError(taxcode.ErrTaxCodeManagedBySystem) } @@ -154,6 +192,24 @@ func (s *Service) DeleteTaxCode(ctx context.Context, input taxcode.DeleteTaxCode return models.NewGenericConflictError(taxcode.ErrTaxCodeIsOrganizationDefault) } - return s.adapter.DeleteTaxCode(ctx, input) + if err = s.hooks.PreDelete(ctx, &existing); err != nil { + return err + } + + err = s.adapter.DeleteTaxCode(ctx, input) + if err != nil { + return err + } + + deleted, err := s.adapter.GetTaxCode(ctx, taxcode.GetTaxCodeInput{NamespacedID: input.NamespacedID}) + if err != nil { + return err + } + + if err = s.hooks.PostDelete(ctx, &deleted); err != nil { + return err + } + + return nil }) } diff --git a/test/billing/subscription_suite.go b/test/billing/subscription_suite.go index 158cd7468c..a137f1b2b7 100644 --- a/test/billing/subscription_suite.go +++ b/test/billing/subscription_suite.go @@ -34,6 +34,7 @@ import ( planaddonrepo "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/adapter" planaddonservice "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/service" subscriptiontestutils "github.com/openmeterio/openmeter/openmeter/productcatalog/subscription/testutils" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" streamingtestutils "github.com/openmeterio/openmeter/openmeter/streaming/testutils" "github.com/openmeterio/openmeter/openmeter/subscription" subscriptionaddon "github.com/openmeterio/openmeter/openmeter/subscription/addon" @@ -129,8 +130,12 @@ func (s *SubscriptionMixin) SetupSuite(t *testing.T, deps SubscriptionMixInDepen featureResolver, err := featureresolver.New(deps.FeatureService) require.NoErrorf(t, err, "failed to create feature resolver: %v", err) + taxCodeResolver, err := taxcoderesolver.New(taxCodeService) + require.NoErrorf(t, err, "failed to create tax code resolver: %v", err) + planService, err := planservice.New(planservice.Config{ FeatureResolver: featureResolver, + TaxCodeResolver: taxCodeResolver, Adapter: planAdapter, TaxCode: taxCodeService, Logger: slog.Default(), diff --git a/test/customer/testenv.go b/test/customer/testenv.go index e6a34350d0..b24bbf4914 100644 --- a/test/customer/testenv.go +++ b/test/customer/testenv.go @@ -36,6 +36,7 @@ import ( planservice "github.com/openmeterio/openmeter/openmeter/productcatalog/plan/service" planaddonrepo "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/adapter" planaddonservice "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon/service" + "github.com/openmeterio/openmeter/openmeter/productcatalog/taxcoderesolver" registrybuilder "github.com/openmeterio/openmeter/openmeter/registry/builder" streamingtestutils "github.com/openmeterio/openmeter/openmeter/streaming/testutils" "github.com/openmeterio/openmeter/openmeter/subject" @@ -275,9 +276,13 @@ func NewTestEnv(t *testing.T, ctx context.Context) (TestEnv, error) { featureResolver, err := featureresolver.New(entitlementRegistry.Feature) require.NoErrorf(t, err, "failed to create feature resolver: %v", err) + taxCodeResolver, err := taxcoderesolver.New(taxCodeService) + require.NoErrorf(t, err, "failed to create tax code resolver: %v", err) + planService, err := planservice.New(planservice.Config{ Adapter: planAdapter, FeatureResolver: featureResolver, + TaxCodeResolver: taxCodeResolver, TaxCode: taxCodeService, Logger: logger.WithGroup("plan"), Publisher: publisher,