Skip to content

Commit 7c9be16

Browse files
committed
fix(api): non-existent tax code
1 parent 8edef47 commit 7c9be16

13 files changed

Lines changed: 630 additions & 362 deletions

File tree

api/spec/packages/aip/src/customers/credits/operations.tsp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ namespace Credits.Grants {
4848
):
4949
| Shared.CreateResponse<CreditGrant>
5050
| Common.NotFound
51+
| Common.Conflict
5152
| Common.ErrorResponses;
5253

5354
/**

api/v3/api.gen.go

Lines changed: 320 additions & 320 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/v3/openapi.yaml

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,8 @@ paths:
10811081
$ref: '#/components/responses/Forbidden'
10821082
'404':
10831083
$ref: '#/components/responses/NotFound'
1084+
'409':
1085+
$ref: '#/components/responses/Conflict'
10841086
tags:
10851087
- OpenMeter Customers
10861088
requestBody:
@@ -11651,6 +11653,21 @@ components:
1165111653
example: kong:trace:1234567890
1165211654
detail:
1165311655
example: Gone
11656+
ConflictError:
11657+
allOf:
11658+
- $ref: '#/components/schemas/BaseError'
11659+
- type: object
11660+
properties:
11661+
status:
11662+
example: 409
11663+
title:
11664+
example: Conflict
11665+
type:
11666+
example: https://httpstatuses.com/409
11667+
instance:
11668+
example: kong:trace:1234567890
11669+
detail:
11670+
example: Conflict
1165411671
CursorMetaPage:
1165511672
type: object
1165611673
required:
@@ -11688,21 +11705,6 @@ components:
1168811705
properties:
1168911706
page:
1169011707
$ref: '#/components/schemas/CursorMetaPage'
11691-
ConflictError:
11692-
allOf:
11693-
- $ref: '#/components/schemas/BaseError'
11694-
- type: object
11695-
properties:
11696-
status:
11697-
example: 409
11698-
title:
11699-
example: Conflict
11700-
type:
11701-
example: https://httpstatuses.com/409
11702-
instance:
11703-
example: kong:trace:1234567890
11704-
detail:
11705-
example: Conflict
1170611708
responses:
1170711709
BadRequest:
1170811710
description: Bad Request

e2e/customer_credits_v3_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@ package e2e
33
import (
44
"net/http"
55
"net/url"
6+
"strings"
67
"testing"
78
"time"
89

910
"github.com/oklog/ulid/v2"
1011
"github.com/stretchr/testify/assert"
1112
"github.com/stretchr/testify/require"
13+
14+
apiv3 "github.com/openmeterio/openmeter/api/v3"
1215
)
1316

1417
func TestV3CustomerCreditBalanceTimestampParamParsing(t *testing.T) {
@@ -35,3 +38,49 @@ func TestV3CustomerCreditBalanceTimestampParamParsing(t *testing.T) {
3538
assert.Contains(t, problem.Detail, "timestamp")
3639
})
3740
}
41+
42+
// TestV3CreateCreditGrantMissingTaxCode verifies the documented contract for
43+
// create-credit-grant: referencing a tax code that does not exist is rejected
44+
// with HTTP 400 (a validation error), not a 412/500. The OpenAPI spec documents
45+
// 400 for this operation, so this asserts the in-contract behavior end-to-end
46+
// through the HTTP layer rather than only at the service boundary.
47+
func TestV3CreateCreditGrantMissingTaxCode(t *testing.T) {
48+
c := newV3Client(t)
49+
50+
currency := apiv3.CurrencyCode("USD")
51+
status, customer, problem := c.CreateCustomer(apiv3.CreateCustomerRequest{
52+
Key: uniqueKey("credit_grant_taxcode_customer"),
53+
Name: "Credit Grant Tax Code Test Customer",
54+
Currency: &currency,
55+
})
56+
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)
57+
require.NotNil(t, customer)
58+
59+
// A structurally valid but non-existent tax code ULID.
60+
missingTaxCode := ulid.Make().String()
61+
62+
status, _, problem = c.CreateCreditGrant(customer.Id, apiv3.CreateCreditGrantRequest{
63+
Name: "grant with missing tax code",
64+
Amount: apiv3.Numeric("10"),
65+
Currency: currency,
66+
FundingMethod: apiv3.BillingCreditFundingMethodNone,
67+
TaxConfig: &apiv3.CreateCreditGrantTaxConfig{
68+
TaxCode: &apiv3.CreateResourceReference{Id: missingTaxCode},
69+
},
70+
})
71+
72+
require.Equal(t, http.StatusBadRequest, status, "missing tax code must be a 400 validation error, problem: %+v", problem)
73+
require.NotNil(t, problem)
74+
75+
// The offending tax code is named either in the top-level Detail or in the
76+
// structured validationErrors[] message, depending on which error layer
77+
// renders it. Accept either so the test asserts the contract, not the
78+
// rendering shape.
79+
mentionsTaxCode := strings.Contains(problem.Detail, "tax code")
80+
for _, ve := range problem.ValidationErrors() {
81+
if strings.Contains(ve.Message, "tax code") || strings.Contains(ve.Field, "tax_code") {
82+
mentionsTaxCode = true
83+
}
84+
}
85+
assert.True(t, mentionsTaxCode, "response should name the missing tax code, problem: %+v", problem)
86+
}

e2e/v3helpers_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,14 @@ func (c *v3Client) GetCustomerEntitlementAccess(customerID string) (int, *apiv3.
300300
return decodeTyped[apiv3.ListCustomerEntitlementAccessResponseData](c, status, raw, problem, http.StatusOK)
301301
}
302302

303+
// CreateCreditGrant posts a credit grant for the given customer. customerID is
304+
// the customer's ULID. Returns the decoded grant on 201, or (status, nil,
305+
// problem) otherwise so error-path tests can assert the status and problem body.
306+
func (c *v3Client) CreateCreditGrant(customerID string, body apiv3.CreateCreditGrantRequest) (int, *apiv3.BillingCreditGrant, *v3Problem) {
307+
status, raw, problem := c.do(http.MethodPost, "/customers/"+customerID+"/credits/grants", body)
308+
return decodeTyped[apiv3.BillingCreditGrant](c, status, raw, problem, http.StatusCreated)
309+
}
310+
303311
// --- Subscriptions ---
304312

305313
func (c *v3Client) CreateSubscription(body apiv3.BillingSubscriptionCreate) (int, *apiv3.BillingSubscription, *v3Problem) {

openmeter/billing/charges/charge.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,33 @@ func (c ChargeIntent) GetUniqueReferenceID() (*string, error) {
385385
return nil, fmt.Errorf("invalid charge type: %s", c.t)
386386
}
387387

388-
// Meta returns the shared meta.Intent embedded in every charge type.
388+
// TaxCodeID returns the intent's configured tax code ID.
389+
// It is empty when no tax code is set.
390+
func (i ChargeIntent) TaxCodeID() (string, error) {
391+
switch i.t {
392+
case meta.ChargeTypeFlatFee:
393+
if i.flatFee == nil {
394+
return "", fmt.Errorf("flat fee is nil")
395+
}
396+
397+
return i.flatFee.TaxConfig.TaxCodeID, nil
398+
case meta.ChargeTypeUsageBased:
399+
if i.usageBased == nil {
400+
return "", fmt.Errorf("usage based is nil")
401+
}
402+
403+
return i.usageBased.TaxConfig.TaxCodeID, nil
404+
case meta.ChargeTypeCreditPurchase:
405+
if i.creditPurchase == nil {
406+
return "", fmt.Errorf("credit purchase is nil")
407+
}
408+
409+
return i.creditPurchase.TaxConfig.TaxCodeID, nil
410+
}
411+
412+
return "", fmt.Errorf("unsupported charge type: %s", i.t)
413+
}
414+
389415
// WithTaxCodeID returns a copy of the intent with TaxCodeID set to id.
390416
// Existing tax behavior and other intent fields are preserved.
391417
func (i ChargeIntent) WithTaxCodeID(id string) (ChargeIntent, error) {

openmeter/billing/charges/creditpurchase/adapter/charge.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/openmeterio/openmeter/openmeter/billing/charges/creditpurchase"
1010
"github.com/openmeterio/openmeter/openmeter/billing/charges/meta"
11+
metaadapter "github.com/openmeterio/openmeter/openmeter/billing/charges/meta/adapter"
1112
"github.com/openmeterio/openmeter/openmeter/billing/charges/models/chargemeta"
1213
"github.com/openmeterio/openmeter/openmeter/ent/db"
1314
dbchargecreditpurchase "github.com/openmeterio/openmeter/openmeter/ent/db/chargecreditpurchase"
@@ -89,7 +90,7 @@ func (a *adapter) CreateCharge(ctx context.Context, in creditpurchase.CreateChar
8990

9091
dbCreditPurchase, err := create.Save(ctx)
9192
if err != nil {
92-
return creditpurchase.Charge{}, err
93+
return creditpurchase.Charge{}, metaadapter.MapChargeConstraintError(err)
9394
}
9495

9596
err = tx.metaAdapter.RegisterCharges(ctx, meta.RegisterChargesInput{

openmeter/billing/charges/flatfee/adapter/charge.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/openmeterio/openmeter/openmeter/billing/charges/flatfee"
1111
"github.com/openmeterio/openmeter/openmeter/billing/charges/meta"
12+
metaadapter "github.com/openmeterio/openmeter/openmeter/billing/charges/meta/adapter"
1213
"github.com/openmeterio/openmeter/openmeter/billing/charges/models/chargemeta"
1314
"github.com/openmeterio/openmeter/openmeter/ent/db"
1415
dbchargeflatfee "github.com/openmeterio/openmeter/openmeter/ent/db/chargeflatfee"
@@ -208,7 +209,7 @@ func (a *adapter) CreateCharges(ctx context.Context, in flatfee.CreateChargesInp
208209

209210
entities, err := tx.db.ChargeFlatFee.CreateBulk(creates...).Save(ctx)
210211
if err != nil {
211-
return nil, err
212+
return nil, metaadapter.MapChargeConstraintError(err)
212213
}
213214

214215
// Let's reserve the charge IDs
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package adapter
2+
3+
import (
4+
"fmt"
5+
6+
"entgo.io/ent/dialect/sql/sqlgraph"
7+
8+
entdb "github.com/openmeterio/openmeter/openmeter/ent/db"
9+
"github.com/openmeterio/openmeter/pkg/models"
10+
)
11+
12+
// MapChargeConstraintError translates an ent DB constraint violation errors.
13+
func MapChargeConstraintError(err error) error {
14+
if err == nil || !entdb.IsConstraintError(err) {
15+
return err
16+
}
17+
18+
switch {
19+
case sqlgraph.IsUniqueConstraintError(err):
20+
return models.NewGenericConflictError(
21+
fmt.Errorf("charge conflicts with an existing charge: %w", err),
22+
)
23+
case sqlgraph.IsForeignKeyConstraintError(err):
24+
return models.NewGenericValidationError(
25+
fmt.Errorf("charge references a resource that does not exist: %w", err),
26+
)
27+
default:
28+
return models.NewGenericValidationError(
29+
fmt.Errorf("charge violates a database constraint: %w", err),
30+
)
31+
}
32+
}

openmeter/billing/charges/service/create.go

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/openmeterio/openmeter/pkg/clock"
2222
"github.com/openmeterio/openmeter/pkg/currencyx"
2323
"github.com/openmeterio/openmeter/pkg/framework/transaction"
24+
"github.com/openmeterio/openmeter/pkg/models"
2425
"github.com/openmeterio/openmeter/pkg/ref"
2526
"github.com/openmeterio/openmeter/pkg/slicesx"
2627
)
@@ -43,29 +44,9 @@ func (s *service) applyDefaultTaxCodes(ctx context.Context, namespace string, in
4344
})
4445

4546
return slicesx.MapWithErr(intents, func(intent charges.ChargeIntent) (charges.ChargeIntent, error) {
46-
var taxCodeID string
47-
48-
switch intent.Type() {
49-
case meta.ChargeTypeFlatFee:
50-
flatFee, err := intent.AsFlatFeeIntent()
51-
if err != nil {
52-
return charges.ChargeIntent{}, err
53-
}
54-
taxCodeID = flatFee.TaxConfig.TaxCodeID
55-
case meta.ChargeTypeCreditPurchase:
56-
creditPurchase, err := intent.AsCreditPurchaseIntent()
57-
if err != nil {
58-
return charges.ChargeIntent{}, err
59-
}
60-
taxCodeID = creditPurchase.TaxConfig.TaxCodeID
61-
case meta.ChargeTypeUsageBased:
62-
usageBased, err := intent.AsUsageBasedIntent()
63-
if err != nil {
64-
return charges.ChargeIntent{}, err
65-
}
66-
taxCodeID = usageBased.TaxConfig.TaxCodeID
67-
default:
68-
return charges.ChargeIntent{}, fmt.Errorf("unsupported charge type: %s", intent.Type())
47+
taxCodeID, err := intent.TaxCodeID()
48+
if err != nil {
49+
return charges.ChargeIntent{}, err
6950
}
7051

7152
if taxCodeID != "" {
@@ -87,6 +68,44 @@ func (s *service) applyDefaultTaxCodes(ctx context.Context, namespace string, in
8768
})
8869
}
8970

71+
// validateTaxCodesExist verifies every distinct non-empty tax code referenced by the intents
72+
// exists.
73+
func (s *service) validateTaxCodesExist(ctx context.Context, namespace string, intents charges.ChargeIntents) error {
74+
seen := make(map[string]struct{}, len(intents))
75+
76+
for _, intent := range intents {
77+
taxCodeID, err := intent.TaxCodeID()
78+
if err != nil {
79+
return err
80+
}
81+
82+
if taxCodeID == "" {
83+
continue
84+
}
85+
86+
if _, ok := seen[taxCodeID]; ok {
87+
continue
88+
}
89+
seen[taxCodeID] = struct{}{}
90+
91+
_, err = s.taxCodeService.GetTaxCode(ctx, taxcode.GetTaxCodeInput{
92+
NamespacedID: models.NamespacedID{Namespace: namespace, ID: taxCodeID},
93+
})
94+
if err != nil {
95+
if taxcode.IsTaxCodeNotFoundError(err) {
96+
return models.NewGenericValidationError(
97+
models.NewValidationError("tax_code_not_found", fmt.Sprintf("referenced tax code %q does not exist", taxCodeID)).
98+
WithPathString("tax_config", "tax_code"),
99+
)
100+
}
101+
102+
return err
103+
}
104+
}
105+
106+
return nil
107+
}
108+
90109
func (s *service) Create(ctx context.Context, input charges.CreateInput) (charges.Charges, error) {
91110
result, err := s.create(ctx, input)
92111
if err != nil {
@@ -127,6 +146,10 @@ func (s *service) create(ctx context.Context, input charges.CreateInput) (*charg
127146
return nil, err
128147
}
129148

149+
if err := s.validateTaxCodesExist(ctx, input.Namespace, input.Intents); err != nil {
150+
return nil, err
151+
}
152+
130153
result, err := transaction.Run(ctx, s.adapter, func(ctx context.Context) (*chargesWithInvoiceNowActions, error) {
131154
intentsByType, err := input.Intents.ByType()
132155
if err != nil {

0 commit comments

Comments
 (0)