feat(payments): EN-618 EN-621 EN-622 EN-624 EN-625 orders & conversions commands#161
Open
thierrycoopman wants to merge 4 commits into
Open
feat(payments): EN-618 EN-621 EN-622 EN-624 EN-625 orders & conversions commands#161thierrycoopman wants to merge 4 commits into
thierrycoopman wants to merge 4 commits into
Conversation
Adds `fctl payments orders` and `fctl payments conversions` (list + get)
aligned with the upcoming payments v3.3 `GET /v3/orders` and
`GET /v3/conversions` endpoints. Filter flags, pagination, JSON output,
help screens, and renderers ship today; `Run()` returns a clear sentinel
error until formance-sdk-go/v3 exposes the new operations (see EN-1012),
at which point each stub becomes a one-line wire-up to
`Payments.V3.{ListOrders,GetOrder,ListConversions,GetConversion}`.
Per-package inline helpers follow the existing cmd/payments convention;
cross-package DRY is left as a future optimization.
Contributor
WalkthroughThis PR introduces two new CLI command modules—Orders and Conversions—under the Payments CLI. Each module provides list and show operations with filtering, cursor-based pagination, API integration, and table rendering. Both follow the same architectural pattern with root commands, list/show implementations, and tests. ChangesOrders and Conversions Modules
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Payments v3.3 endpoints shipped in formance-sdk-go v4.0.0 (2026-05-29) as a breaking major: pkg/models/components was removed and the models were split into per-domain packages (pkg/models/payments, ledger, wallets, ...). The PR's original "awaiting v3.x release" framing no longer holds. Updates each of the 4 stubs (orders list/get, conversions list/get) to: - block on EN-1012 (fctl-wide migration to formance-sdk-go/v4) instead of a non-existent v3 release - carry the precise v4 paste-in wiring with the right import paths (operations + paymentsmodels), request shape (RequestBody not V3QueryBuilder), response shape (V3OrdersCursorResponse.Cursor / V3GetOrderResponse.V3Order, etc.), and mapping caveats (typed-string enum casts, V3Metadata field name, V3OrderAdjustmentRaw empty struct) No functional change; Run() still returns a clear sentinel error and go build / go vet stay green. Co-authored-by: Cursor <cursoragent@cursor.com>
…conversions Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # cmd/payments/root.go
Replace the awaiting-SDK stubs with real calls now that fctl is on formance-sdk-go/v4 (payments v3.3 endpoints): ListOrders/GetOrder/ ListConversions/GetConversion, with toOrder/toOrderAdjustment/toConversion mappers (enum casting, V3Metadata, nullable big.Int amounts). - Query filter keys use the payments snake_case column names; the endpoint whitelists them, so camelCase was rejected with a VALIDATION error. - Drop the unsupported --created-at-from/to flags (no created_at filter key). - V3 query endpoints take a query body (first page) or an opaque cursor (subsequent pages), never both. - Add stdlib unit tests for the mappers and the query builder.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/payments/orders/list_test.go (1)
44-92: ⚡ Quick winExtend
TestToOrderto assert full adjustment-field mapping.Please add assertions for currently unverified adjustment fields (notably
ReferenceandRaw) so mapper regressions are caught by tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/payments/orders/list_test.go` around lines 44 - 92, TestToOrder's assertions don't cover all mapped adjustment fields so regressions for adjustment.Reference and adjustment.Raw can slip by; update the TestToOrder test (which calls toOrder with paymentsmodels.V3Order and its Adjustments) to assert that got.Adjustments[0].Reference equals the source Adjustments[0].Reference value and that got.Adjustments[0].Raw matches the source Adjustments[0].Raw (or its expected decoded/typed representation), and also add checks for any other unmapped adjustment fields present on paymentsmodels.V3OrderAdjustment to ensure full field mapping in toOrder.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/payments/conversions/list.go`:
- Around line 130-131: The CLI currently forwards the --status value directly
(see usage around buildQuery(cmd) and fctl.GetCursor(cmd)); add a local enum of
allowed status values (e.g., pending, completed, failed) and validate/normalize
the status flag before calling buildQuery or making the API request, returning a
clear CLI error if the value is invalid; ensure you read the flag from the same
source used by buildQuery (the cmd or query struct), perform optional case
normalization, and only attach the status to the outgoing query when it passes
validation.
In `@cmd/payments/orders/list.go`:
- Around line 236-246: The mapper function toOrderAdjustment is not populating
the OrderAdjustment.Raw field so raw provider payloads are lost; update
toOrderAdjustment to assign the raw payload from the source model
(paymentsmodels.V3OrderAdjustment) into OrderAdjustment.Raw (e.g., map a.Raw or
the appropriate source property) and ensure any type conversions are handled, or
if paymentsmodels.V3OrderAdjustment truly has no raw/opaque payload field then
remove Raw from the OrderAdjustment struct to keep the local model contract
consistent; reference toOrderAdjustment, OrderAdjustment, and
paymentsmodels.V3OrderAdjustment when making the change.
---
Nitpick comments:
In `@cmd/payments/orders/list_test.go`:
- Around line 44-92: TestToOrder's assertions don't cover all mapped adjustment
fields so regressions for adjustment.Reference and adjustment.Raw can slip by;
update the TestToOrder test (which calls toOrder with paymentsmodels.V3Order and
its Adjustments) to assert that got.Adjustments[0].Reference equals the source
Adjustments[0].Reference value and that got.Adjustments[0].Raw matches the
source Adjustments[0].Raw (or its expected decoded/typed representation), and
also add checks for any other unmapped adjustment fields present on
paymentsmodels.V3OrderAdjustment to ensure full field mapping in toOrder.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9746b727-ab0e-4094-8744-1700d755b667
📒 Files selected for processing (9)
cmd/payments/conversions/list.gocmd/payments/conversions/list_test.gocmd/payments/conversions/root.gocmd/payments/conversions/show.gocmd/payments/orders/list.gocmd/payments/orders/list_test.gocmd/payments/orders/root.gocmd/payments/orders/show.gocmd/payments/root.go
Comment on lines
+130
to
+131
| query := c.buildQuery(cmd) | ||
| cursor, err := fctl.GetCursor(cmd) |
Contributor
There was a problem hiding this comment.
Validate --status before sending the request.
--status is currently forwarded as-is; typos only fail at API time. Add a local enum check (and optional normalization) to fail fast with a clear CLI error.
Suggested patch
import (
"fmt"
"math/big"
+ "strings"
"time"
@@
query := c.buildQuery(cmd)
+ status := strings.ToUpper(strings.TrimSpace(fctl.GetString(cmd, c.statusFlag)))
+ if status != "" {
+ switch status {
+ case "PENDING", "COMPLETED", "FAILED", "UNKNOWN":
+ if err := cmd.Flags().Set(c.statusFlag, status); err != nil {
+ return nil, err
+ }
+ default:
+ return nil, fmt.Errorf("invalid --status %q (allowed: PENDING|COMPLETED|FAILED|UNKNOWN)", status)
+ }
+ }
+ query = c.buildQuery(cmd)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/payments/conversions/list.go` around lines 130 - 131, The CLI currently
forwards the --status value directly (see usage around buildQuery(cmd) and
fctl.GetCursor(cmd)); add a local enum of allowed status values (e.g., pending,
completed, failed) and validate/normalize the status flag before calling
buildQuery or making the API request, returning a clear CLI error if the value
is invalid; ensure you read the flag from the same source used by buildQuery
(the cmd or query struct), perform optional case normalization, and only attach
the status to the outgoing query when it passes validation.
Comment on lines
+236
to
+246
| func toOrderAdjustment(a paymentsmodels.V3OrderAdjustment) OrderAdjustment { | ||
| return OrderAdjustment{ | ||
| ID: a.ID, | ||
| Reference: a.Reference, | ||
| CreatedAt: a.CreatedAt, | ||
| Status: string(a.V3OrderStatusEnum), | ||
| BaseQuantityFilled: a.BaseQuantityFilled, | ||
| Fee: a.Fee, | ||
| FeeAsset: a.FeeAsset, | ||
| Metadata: a.V3Metadata, | ||
| } |
Contributor
There was a problem hiding this comment.
toOrderAdjustment drops the Raw payload declared by OrderAdjustment.
OrderAdjustment includes Raw (Line 61), but the mapper never assigns it, so that field is always lost in rendered/JSON output. This breaks the local model contract and can hide provider diagnostic details.
Suggested fix
func toOrderAdjustment(a paymentsmodels.V3OrderAdjustment) OrderAdjustment {
return OrderAdjustment{
ID: a.ID,
Reference: a.Reference,
CreatedAt: a.CreatedAt,
Status: string(a.V3OrderStatusEnum),
BaseQuantityFilled: a.BaseQuantityFilled,
Fee: a.Fee,
FeeAsset: a.FeeAsset,
Metadata: a.V3Metadata,
+ Raw: a.Raw,
}
}If paymentsmodels.V3OrderAdjustment does not expose a raw field, remove Raw from the local struct to keep contracts aligned.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/payments/orders/list.go` around lines 236 - 246, The mapper function
toOrderAdjustment is not populating the OrderAdjustment.Raw field so raw
provider payloads are lost; update toOrderAdjustment to assign the raw payload
from the source model (paymentsmodels.V3OrderAdjustment) into
OrderAdjustment.Raw (e.g., map a.Raw or the appropriate source property) and
ensure any type conversions are handled, or if paymentsmodels.V3OrderAdjustment
truly has no raw/opaque payload field then remove Raw from the OrderAdjustment
struct to keep the local model contract consistent; reference toOrderAdjustment,
OrderAdjustment, and paymentsmodels.V3OrderAdjustment when making the change.
thierrycoopman
changed the title
laouji
reviewed
Jun 5, 2026
Contributor
laouji
left a comment
laouji
left a comment
There was a problem hiding this comment.
would be great if you could provide screen shots so reviewers don't have to go to the trouble of booting up a stack and testing manually 🙂
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two read-only command trees under
fctl payments:fctl payments orders {list,get}— wrapsGET /v3/ordersandGET /v3/orders/{orderID}fctl payments conversions {list,get}— wrapsGET /v3/conversionsandGET /v3/conversions/{conversionID}Each
Run()callsstackClient.Payments.V3.{ListOrders,GetOrder,ListConversions,GetConversion}and maps the SDK models onto local nil-safe store types. Filter flags contribute$matchclauses under a top-level$and, using the payments snake_case storage column names (the endpoint whitelists keys). Pagination follows the V3 query convention: a query body on the first page, an opaque cursor thereafter — never both.SDK
Unblocked: fctl is on
formance-sdk-go/v4, which exposes the payments v3.3 orders/conversions endpoints.origin/mainwas merged into this branch to pick up the v4 migration.Jira
Resolves EN-618, EN-621, EN-622, EN-624, EN-625.
Related: EN-1012.
Test plan
go build ./...,go vet ./...,gofmt -l,golangci-lint run,go mod tidyall cleango test ./...(mapper + query-builder unit tests, stdlib only)orders {list,get}andconversions {list,get}