Skip to content

Commit 90eb5a2

Browse files
GAlexIHUGAlexIHU
committed
fix(credits): address grant voiding review feedback
1 parent 5ff14c7 commit 90eb5a2

6 files changed

Lines changed: 203 additions & 114 deletions

File tree

‎api/v3/handlers/customers/credits/void_grant_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package customerscredits
22

33
import (
4-
"context"
54
"net/http"
65
"net/http/httptest"
76
"testing"
@@ -65,7 +64,7 @@ func TestConvertAPIVoidCreditGrantRequestRejectsUnsupportedPaymentAdjustment(t *
6564

6665
rec := httptest.NewRecorder()
6766
req := httptest.NewRequest(http.MethodPost, "/api/v3/customers/cust-1/credits/grants/grant-1/void", nil)
68-
handled := apierrors.GenericErrorEncoder()(context.Background(), err, rec, req)
67+
handled := apierrors.GenericErrorEncoder()(t.Context(), err, rec, req)
6968

7069
require.True(t, handled)
7170
require.Equal(t, http.StatusBadRequest, rec.Code)

‎openmeter/billing/creditgrant/service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ type Service interface {
2424
Get(ctx context.Context, input GetInput) (creditpurchase.Charge, error)
2525
List(ctx context.Context, input ListInput) (pagination.Result[creditpurchase.Charge], error)
2626
UpdateExternalSettlement(ctx context.Context, input UpdateExternalSettlementInput) (creditpurchase.Charge, error)
27-
// Void forfeits the grant's remaining unused value by booking ledger
28-
// breakage at the current server time.
27+
// Void forfeits the grant's remaining unused value by correcting the
28+
// original receivable issuance at the current server time.
2929
Void(ctx context.Context, input VoidInput) (creditpurchase.Charge, error)
3030
}
3131

‎openmeter/ledger/creditvoid/service.go‎

Lines changed: 12 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -121,90 +121,6 @@ type VoidCreditPurchaseResult struct {
121121
TransactionGroupID string
122122
}
123123

124-
type Record struct {
125-
ID models.NamespacedID
126-
Amount alpacadecimal.Decimal
127-
CreatedAt time.Time
128-
UpdatedAt time.Time
129-
DeletedAt *time.Time
130-
131-
CustomerID customer.CustomerID
132-
Currency currencyx.Code
133-
VoidedAt time.Time
134-
135-
SourceChargeID string
136-
137-
VoidTransactionGroupID string
138-
VoidTransactionID string
139-
140-
FBOSubAccountID string
141-
ReceivableSubAccountID string
142-
143-
Annotations models.Annotations
144-
}
145-
146-
type PendingRecord struct {
147-
Record
148-
}
149-
150-
type CreateRecordsInput struct {
151-
Records []Record
152-
}
153-
154-
func (i CreateRecordsInput) Validate() error {
155-
for idx, record := range i.Records {
156-
if err := record.Validate(); err != nil {
157-
return fmt.Errorf("records[%d]: %w", idx, err)
158-
}
159-
}
160-
161-
return nil
162-
}
163-
164-
func (r Record) Validate() error {
165-
var errs []error
166-
167-
if err := r.ID.Validate(); err != nil {
168-
errs = append(errs, fmt.Errorf("id: %w", err))
169-
}
170-
if !r.Amount.IsPositive() {
171-
errs = append(errs, errors.New("amount must be positive"))
172-
}
173-
if err := r.CustomerID.Validate(); err != nil {
174-
errs = append(errs, fmt.Errorf("customer id: %w", err))
175-
}
176-
if err := r.Currency.Validate(); err != nil {
177-
errs = append(errs, fmt.Errorf("currency: %w", err))
178-
}
179-
if r.VoidedAt.IsZero() {
180-
errs = append(errs, errors.New("voided at is required"))
181-
}
182-
if r.SourceChargeID == "" {
183-
errs = append(errs, errors.New("source charge id is required"))
184-
}
185-
if r.VoidTransactionGroupID == "" {
186-
errs = append(errs, errors.New("void transaction group id is required"))
187-
}
188-
if r.VoidTransactionID == "" {
189-
errs = append(errs, errors.New("void transaction id is required"))
190-
}
191-
if r.FBOSubAccountID == "" {
192-
errs = append(errs, errors.New("FBO sub-account id is required"))
193-
}
194-
if r.ReceivableSubAccountID == "" {
195-
errs = append(errs, errors.New("receivable sub-account id is required"))
196-
}
197-
198-
return errors.Join(errs...)
199-
}
200-
201-
type ListRecordsInput struct {
202-
CustomerID customer.CustomerID
203-
Currency *currencyx.Code
204-
AsOf time.Time
205-
Route ledger.RouteFilter
206-
}
207-
208124
type ListVoidedCreditImpactsInput struct {
209125
CustomerID customer.CustomerID
210126
Currency *currencyx.Code
@@ -275,11 +191,6 @@ func (i VoidImpact) Cursor() ledger.TransactionCursor {
275191
}
276192
}
277193

278-
type Adapter interface {
279-
CreateRecords(ctx context.Context, input CreateRecordsInput) error
280-
ListRecords(ctx context.Context, input ListRecordsInput) ([]Record, error)
281-
}
282-
283194
// voidPlan is the read-only outcome of the planning step: the concrete
284195
// remaining balance slices to forfeit, each with the open expiry breakage plan
285196
// it must release.
@@ -414,7 +325,7 @@ func (s *service) executeVoid(ctx context.Context, input VoidCreditPurchaseInput
414325
var (
415326
inputs []ledger.TransactionInput
416327
pendingBreakage []breakage.PendingRecord
417-
pendingVoidRecords []PendingRecord
328+
pendingVoidRecords []pendingVoidRecord
418329
)
419330
amount := alpacadecimal.Zero
420331

@@ -476,13 +387,13 @@ func (s *service) executeVoid(ctx context.Context, input VoidCreditPurchaseInput
476387
}, nil
477388
}
478389

479-
func (s *service) resolveVoidSlice(ctx context.Context, input VoidCreditPurchaseInput, voidedAt time.Time, slice voidSlice) (ledger.TransactionInput, PendingRecord, error) {
390+
func (s *service) resolveVoidSlice(ctx context.Context, input VoidCreditPurchaseInput, voidedAt time.Time, slice voidSlice) (ledger.TransactionInput, pendingVoidRecord, error) {
480391
recordID := newRecordID(input.CustomerID.Namespace)
481392
route := slice.fboAddress.Route().Route()
482393

483394
issueTx, err := s.originalIssueTransaction(ctx, input, voidedAt, slice)
484395
if err != nil {
485-
return nil, PendingRecord{}, err
396+
return nil, pendingVoidRecord{}, err
486397
}
487398

488399
inputs, err := transactions.CorrectTransaction(ctx, s.deps, transactions.CorrectionInput{
@@ -491,21 +402,21 @@ func (s *service) resolveVoidSlice(ctx context.Context, input VoidCreditPurchase
491402
OriginalTransaction: issueTx,
492403
})
493404
if err != nil {
494-
return nil, PendingRecord{}, fmt.Errorf("resolve issue correction: %w", err)
405+
return nil, pendingVoidRecord{}, fmt.Errorf("resolve issue correction: %w", err)
495406
}
496407
if len(inputs) != 1 {
497-
return nil, PendingRecord{}, fmt.Errorf("expected one issue correction transaction input, got %d", len(inputs))
408+
return nil, pendingVoidRecord{}, fmt.Errorf("expected one issue correction transaction input, got %d", len(inputs))
498409
}
499410

500411
correctedFBO, correctedReceivable, err := correctionEntrySubAccounts(inputs[0])
501412
if err != nil {
502-
return nil, PendingRecord{}, err
413+
return nil, pendingVoidRecord{}, err
503414
}
504415
if correctedFBO != slice.fboAddress.SubAccountID() {
505-
return nil, PendingRecord{}, fmt.Errorf("issue correction FBO sub-account %s does not match voided FBO sub-account %s", correctedFBO, slice.fboAddress.SubAccountID())
416+
return nil, pendingVoidRecord{}, fmt.Errorf("issue correction FBO sub-account %s does not match voided FBO sub-account %s", correctedFBO, slice.fboAddress.SubAccountID())
506417
}
507418

508-
record := PendingRecord{Record: Record{
419+
record := pendingVoidRecord{
509420
ID: recordID,
510421
Amount: slice.amount,
511422
CustomerID: input.CustomerID,
@@ -514,7 +425,7 @@ func (s *service) resolveVoidSlice(ctx context.Context, input VoidCreditPurchase
514425
SourceChargeID: input.ChargeID,
515426
FBOSubAccountID: correctedFBO,
516427
ReceivableSubAccountID: correctedReceivable,
517-
}}
428+
}
518429

519430
return transactions.WithAnnotations(inputs[0], creditVoidRecordAnnotations(recordID.ID)), record, nil
520431
}
@@ -627,15 +538,15 @@ func correctionEntrySubAccounts(input ledger.TransactionInput) (string, string,
627538
return fboSubAccountID, receivableSubAccountID, nil
628539
}
629540

630-
func (s *service) persistCommittedVoidRecords(ctx context.Context, pending []PendingRecord, group ledger.TransactionGroup) error {
541+
func (s *service) persistCommittedVoidRecords(ctx context.Context, pending []pendingVoidRecord, group ledger.TransactionGroup) error {
631542
if len(pending) == 0 {
632543
return nil
633544
}
634545
if group == nil {
635546
return errors.New("transaction group is required")
636547
}
637548

638-
pendingByID := make(map[string]PendingRecord, len(pending))
549+
pendingByID := make(map[string]pendingVoidRecord, len(pending))
639550
for _, item := range pending {
640551
pendingByID[item.ID.ID] = item
641552
}
@@ -653,10 +564,7 @@ func (s *service) persistCommittedVoidRecords(ctx context.Context, pending []Pen
653564
return fmt.Errorf("committed void transaction %s has unknown record id %s", tx.ID().ID, recordID)
654565
}
655566

656-
record := pendingRecord.Record
657-
record.VoidTransactionGroupID = groupID
658-
record.VoidTransactionID = tx.ID().ID
659-
record.Annotations = tx.Annotations()
567+
record := pendingRecord.committed(groupID, tx.ID().ID, tx.Annotations())
660568

661569
records = append(records, record)
662570
delete(pendingByID, recordID)

‎openmeter/ledger/creditvoid/types.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
package creditvoid
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"time"
8+
9+
"github.com/alpacahq/alpacadecimal"
10+
11+
"github.com/openmeterio/openmeter/openmeter/customer"
12+
"github.com/openmeterio/openmeter/openmeter/ledger"
13+
"github.com/openmeterio/openmeter/pkg/currencyx"
14+
"github.com/openmeterio/openmeter/pkg/models"
15+
)
16+
17+
type Record struct {
18+
ID models.NamespacedID
19+
Amount alpacadecimal.Decimal
20+
CreatedAt time.Time
21+
UpdatedAt time.Time
22+
DeletedAt *time.Time
23+
24+
CustomerID customer.CustomerID
25+
Currency currencyx.Code
26+
VoidedAt time.Time
27+
28+
SourceChargeID string
29+
30+
VoidTransactionGroupID string
31+
VoidTransactionID string
32+
33+
FBOSubAccountID string
34+
ReceivableSubAccountID string
35+
36+
Annotations models.Annotations
37+
}
38+
39+
// pendingVoidRecord is a pre-commit record plan. The committed ledger
40+
// transaction IDs are not known until after CommitGroup returns.
41+
type pendingVoidRecord struct {
42+
ID models.NamespacedID
43+
Amount alpacadecimal.Decimal
44+
CustomerID customer.CustomerID
45+
Currency currencyx.Code
46+
VoidedAt time.Time
47+
48+
SourceChargeID string
49+
50+
FBOSubAccountID string
51+
ReceivableSubAccountID string
52+
}
53+
54+
func (r pendingVoidRecord) committed(groupID, transactionID string, annotations models.Annotations) Record {
55+
return Record{
56+
ID: r.ID,
57+
Amount: r.Amount,
58+
CustomerID: r.CustomerID,
59+
Currency: r.Currency,
60+
VoidedAt: r.VoidedAt,
61+
SourceChargeID: r.SourceChargeID,
62+
VoidTransactionGroupID: groupID,
63+
VoidTransactionID: transactionID,
64+
FBOSubAccountID: r.FBOSubAccountID,
65+
ReceivableSubAccountID: r.ReceivableSubAccountID,
66+
Annotations: annotations,
67+
}
68+
}
69+
70+
type CreateRecordsInput struct {
71+
Records []Record
72+
}
73+
74+
func (i CreateRecordsInput) Validate() error {
75+
for idx, record := range i.Records {
76+
if err := record.Validate(); err != nil {
77+
return fmt.Errorf("records[%d]: %w", idx, err)
78+
}
79+
}
80+
81+
return nil
82+
}
83+
84+
func (r Record) Validate() error {
85+
var errs []error
86+
87+
if err := r.ID.Validate(); err != nil {
88+
errs = append(errs, fmt.Errorf("id: %w", err))
89+
}
90+
if !r.Amount.IsPositive() {
91+
errs = append(errs, errors.New("amount must be positive"))
92+
}
93+
if err := r.CustomerID.Validate(); err != nil {
94+
errs = append(errs, fmt.Errorf("customer id: %w", err))
95+
}
96+
if err := r.Currency.Validate(); err != nil {
97+
errs = append(errs, fmt.Errorf("currency: %w", err))
98+
}
99+
if r.VoidedAt.IsZero() {
100+
errs = append(errs, errors.New("voided at is required"))
101+
}
102+
if r.SourceChargeID == "" {
103+
errs = append(errs, errors.New("source charge id is required"))
104+
}
105+
if r.VoidTransactionGroupID == "" {
106+
errs = append(errs, errors.New("void transaction group id is required"))
107+
}
108+
if r.VoidTransactionID == "" {
109+
errs = append(errs, errors.New("void transaction id is required"))
110+
}
111+
if r.FBOSubAccountID == "" {
112+
errs = append(errs, errors.New("FBO sub-account id is required"))
113+
}
114+
if r.ReceivableSubAccountID == "" {
115+
errs = append(errs, errors.New("receivable sub-account id is required"))
116+
}
117+
118+
return errors.Join(errs...)
119+
}
120+
121+
type ListRecordsInput struct {
122+
CustomerID customer.CustomerID
123+
Currency *currencyx.Code
124+
AsOf time.Time
125+
Route ledger.RouteFilter
126+
}
127+
128+
type Adapter interface {
129+
CreateRecords(ctx context.Context, input CreateRecordsInput) error
130+
ListRecords(ctx context.Context, input ListRecordsInput) ([]Record, error)
131+
}

‎openmeter/ledger/customerbalance/ledger_loader.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (l *ledgerCreditTransactionLoader) Load(ctx context.Context, input creditTr
3030
Namespace: input.CustomerID.Namespace,
3131
Cursor: after,
3232
Before: before,
33-
Limit: input.Limit + 1,
33+
Limit: input.Limit,
3434
AccountIDs: []string{input.AccountID},
3535
Currency: input.Currency,
3636
AsOf: &input.AsOf,
@@ -52,13 +52,12 @@ func (l *ledgerCreditTransactionLoader) Load(ctx context.Context, input creditTr
5252
txs = append(txs, tx)
5353
}
5454

55-
if result.NextCursor == nil {
56-
hasMore = false
55+
if len(txs) > input.Limit {
56+
hasMore = true
5757
break
5858
}
5959

60-
hasMore = true
61-
if len(txs) > input.Limit {
60+
if result.NextCursor == nil {
6261
break
6362
}
6463

@@ -68,6 +67,9 @@ func (l *ledgerCreditTransactionLoader) Load(ctx context.Context, input creditTr
6867
after = result.NextCursor
6968
}
7069
}
70+
if len(txs) > input.Limit {
71+
txs = txs[:input.Limit]
72+
}
7173

7274
items, err := creditTransactionsFromLedgerTransactions(txs)
7375
if err != nil {

0 commit comments

Comments
 (0)