blnkfinance/blnk

/refund-transaction cannot set meta_data/description on the reversal — it blind-inherits the original's

オープン

#291 opened on 2026/05/18

 (2 件のコメント) (0 件のリアクション) (1 人の担当者)Go (129 件のフォーク)auto 404
good first issue

Repository metrics

Stars
 (475 個のスター)
PR merge metrics
 (PR metrics pending)

説明

Issue: /refund-transaction cannot set meta_data / description on the reversal — it blind-inherits the original's

Type: enhancement / API consistency Component: transaction.go (RefundTransaction, prepareRefundTransaction), api/transactions.go (RefundTransaction handler) Version checked: main @ b37af21 (2026-05-16)

Summary

When a transaction is refunded, the reversal transaction inherits the entire meta_data and description of the original, and there is no way for the caller to override either. A refund of a transaction tagged meta_data.type = "deposit" is itself stored with meta_data.type = "deposit" — so any system that classifies ledger rows by metadata cannot distinguish a refund from the thing it reversed.

Where it happens

prepareRefundTransaction (transaction.go:2347) builds the reversal with a whole-struct copy:

func prepareRefundTransaction(originalTxn *model.Transaction, skipQueue bool) *model.Transaction {
	newTransaction := *originalTxn          // copies meta_data, description, everything
	newTransaction.TransactionID = model.GenerateUUIDWithSuffix("txn")
	newTransaction.Reference     = model.GenerateUUIDWithSuffix("ref")
	newTransaction.ParentTransaction = originalTxn.TransactionID
	newTransaction.Source        = originalTxn.Destination  // swap
	newTransaction.Destination   = originalTxn.Source       // swap
	newTransaction.AllowOverdraft = true
	newTransaction.SkipQueue     = skipQueue
	...
}

Only id / reference / parent / source / destination / overdraft / skipqueue are overridden. MetaData and Description ride along unchanged, and nothing in the call chain (RefundTransactionprepareRefundTransaction, or the POST /refund-transaction/:id handler) exposes a way to set them.

Why it matters

POST /transactions already lets a caller set meta_data and description at creation. A refund is a transaction — it is created via the same QueueTransaction path — yet the refund endpoint is the one transaction-creating path that cannot. That asymmetry forces a caller who needs a correctly-classified refund to stop using /refund-transaction entirely and hand-roll the reversal as a plain POST /transactions with source/destination swapped — re-implementing exactly what prepareRefundTransaction already does, just to get control of two fields.

Post-hoc POST /{id}/metadata is not a workaround: the refund settles asynchronously into a separate child, and a metadata update only propagates to children created after the update — it races the settle worker and cannot reliably win.

Proposed fix

Let the optional refund request body — already introduced for skip_queue in 341d868 — also carry meta_data and description, and apply them in prepareRefundTransaction. Fully additive and backward-compatible: an absent body or absent fields reproduce today's behavior exactly.

1. api/transactions.go — extend refundTransactionRequest

type refundTransactionRequest struct {
	SkipQueue bool `json:"skip_queue"`

	// MetaData, when present, is MERGED onto the reversal's metadata,
	// overriding inherited keys of the same name. Absent keys keep the
	// value inherited from the original transaction. Use it to classify
	// the reversal (e.g. {"type": "refund"}) distinctly from the
	// original it reverses.
	MetaData map[string]interface{} `json:"meta_data,omitempty"`

	// Description, when non-empty, replaces the reversal's description
	// (which otherwise copies the original transaction's).
	Description string `json:"description,omitempty"`
}

The handler already binds this struct; no handler-flow change beyond passing the two new fields through.

2. transaction.go — apply the overrides in prepareRefundTransaction

After the existing field overrides:

	if description != "" {
		newTransaction.Description = description
	}
	if len(metaData) > 0 {
		// MERGE, not replace: the reversal must keep Blnk-internal keys
		// the queue sets (e.g. QUEUED_PARENT_TRANSACTION). The caller's
		// keys win on conflict.
		merged := make(map[string]interface{}, len(originalTxn.MetaData)+len(metaData))
		for k, v := range originalTxn.MetaData {
			merged[k] = v
		}
		for k, v := range metaData {
			merged[k] = v
		}
		newTransaction.MetaData = merged
	}

Merge (not replace) is important — a blind replace would drop QUEUED_PARENT_TRANSACTION and break lifecycle-chain lookups.

3. Threading the new parameters

prepareRefundTransaction, RefundTransaction, and RefundWorkerWithOptions each currently take a bare skipQueue bool. Adding meta_data + description raises a design choice:

Option A — extend positionally (consistent with the current codebase, which threads parameters positionally everywhere, e.g. ProcessTransactionInBatches's 7 params):

func prepareRefundTransaction(originalTxn *model.Transaction, skipQueue bool,
	metaData map[string]interface{}, description string) *model.Transaction
func (l *Blnk) RefundTransaction(ctx context.Context, transactionID string,
	skipQueue bool, metaData map[string]interface{}, description string) (*model.Transaction, error)
func (l *Blnk) RefundWorkerWithOptions(skipQueue bool,
	metaData map[string]interface{}, description string) transactionWorker

Option B — introduce a RefundOptions struct. RefundWorkerWithOptions is already named "WithOptions" while taking a single bool; a struct would make the name accurate and absorb any future knob without another signature change:

type RefundOptions struct {
	SkipQueue   bool
	MetaData    map[string]interface{}
	Description string
}
func (l *Blnk) RefundTransaction(ctx context.Context, transactionID string, opts RefundOptions) (*model.Transaction, error)
func (l *Blnk) RefundWorkerWithOptions(opts RefundOptions) transactionWorker
func prepareRefundTransaction(originalTxn *model.Transaction, opts RefundOptions) *model.Transaction

Recommendation: Option B. It is the cleaner fit for a "WithOptions" function, keeps the next addition signature-stable, and the migration is mechanical — RefundOptions{SkipQueue: x} at the 3 call sites (transaction.go:741, :774, :2398) and the handler. RefundWorker (the legacy non-options worker) becomes RefundTransaction(ctx, id, RefundOptions{SkipQueue: originalTxn.SkipQueue}). But Option A is acceptable if the maintainers prefer to stay positional.

Blast radius

Small and contained — non-test call sites:

  • RefundTransaction: transaction.go:741, :774, :2398
  • prepareRefundTransaction: transaction.go:2398 (one)
  • RefundWorkerWithOptions: api/transactions.go:222 (one)
  • RefundWorker (legacy): transaction.go:2474 (one)

Tests

Mirror TestRefundWorkerWithOptions (transaction_test.go:6964):

  • TestRefundWithMetaDataOverride — refund a txn whose meta_data.type is "deposit" with body {"meta_data":{"type":"refund"}}; assert the reversal has type=refund AND still carries the original's other keys (merge, not replace).
  • TestRefundWithDescriptionOverride — assert description is replaced when supplied, inherited when omitted.
  • Assert an empty body still reproduces the inherited-metadata behavior (backward-compat regression guard).

Also: a docs gap noticed nearby

reference/refund-transaction.mdx still documents the endpoint as path-param-only and never mentions the skip_queue body added in 341d868. Whatever the resolution here, that page needs updating to describe the request body.

コントリビューターガイド