feat(ledger): add capability to sort transactions by timestamp #1606
Conversation
WalkthroughThe changes made introduce the ability to order transactions in the Ledger API by adding support for "order" and "reverse" query parameters. Adjustments span across query construction, API controllers, and related tests, ensuring transactions can be efficiently ordered and handled during pagination. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Ledger API
participant Db as Database
Client->>API: GET /transactions?order=effective&reverse=true
API->>API: Parse query parameters
API->>Db: Construct and execute GetTransactionsQuery
Db-->>API: Return ordered transactions
API-->>Client: Return response with ordered transactions
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
components/ledger/openapi.yamlis excluded by!**/*.yamlcomponents/ledger/openapi/v2.yamlis excluded by!**/*.yamlreleases/sdks/go/gen.yamlis excluded by!**/*.yaml
Files selected for processing (12)
- components/ledger/internal/api/v2/controllers_transactions.go (1 hunks)
- components/ledger/internal/api/v2/controllers_transactions_test.go (4 hunks)
- components/ledger/internal/storage/ledgerstore/transactions.go (3 hunks)
- libs/go-libs/bun/bunpaginate/pagination.go (1 hunks)
- libs/go-libs/bun/bunpaginate/pagination_column.go (3 hunks)
- libs/go-libs/bun/bunpaginate/pagination_offset.go (1 hunks)
- releases/sdks/go/.speakeasy/gen.lock (4 hunks)
- releases/sdks/go/docs/pkg/models/operations/order.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2listtransactionsrequest.md (1 hunks)
- releases/sdks/go/formance.go (1 hunks)
- releases/sdks/go/pkg/models/operations/v2listtransactions.go (4 hunks)
- tests/integration/suite/ledger-list-transactions.go (11 hunks)
Files skipped from review due to trivial changes (3)
- libs/go-libs/bun/bunpaginate/pagination_offset.go
- releases/sdks/go/docs/pkg/models/operations/order.md
- releases/sdks/go/formance.go
Additional comments not posted (28)
libs/go-libs/bun/bunpaginate/pagination_column.go (2)
9-9: Import Addition ApprovedThe import for
timeis necessary for handlingtime.Timetypes in the pagination logic.
64-84: Handling Different Pagination ID Types ApprovedThe code correctly handles various types for pagination IDs, including
time.Time,libtime.Time,*BigInt,*big.Int, andint64. The use ofbig.NewIntensures consistent conversion tobig.Int.libs/go-libs/bun/bunpaginate/pagination.go (1)
67-71: Addition ofWithColumnMethod ApprovedThe
WithColumnmethod allows setting the column for pagination. The method is simple and follows the pattern of otherWith*methods in the file.releases/sdks/go/pkg/models/operations/v2listtransactions.go (5)
14-18: Addition ofOrderType and Constant ApprovedThe
Ordertype and its constantOrderEffectiveare necessary for specifying the order of transactions.
20-22: Addition ofToPointerMethod ApprovedThe
ToPointermethod is useful for converting anOrdervalue to a pointer, which is often needed in Go for optional fields.
23-35: Addition ofUnmarshalJSONMethod ApprovedThe
UnmarshalJSONmethod correctly handles the JSON unmarshalling for theOrdertype, ensuring that only valid values are accepted.
48-53: Addition ofOrderandReverseFields ApprovedThe addition of the
OrderandReversefields to theV2ListTransactionsRequeststruct allows specifying the order and direction of the transactions.
95-100: Addition ofGetOrderandGetReverseMethods ApprovedThe
GetOrderandGetReversemethods provide convenient access to theOrderandReversefields.Also applies to: 116-121
components/ledger/internal/api/v2/controllers_transactions.go (1)
60-69: Handling oforderandreverseQuery Parameters ApprovedThe code correctly handles the
orderandreversequery parameters, setting the appropriate fields in theGetTransactionsQueryobject.releases/sdks/go/docs/pkg/models/operations/v2listtransactionsrequest.md (2)
12-12: Field Addition:OrderThe
Orderfield is correctly documented and integrated into theV2ListTransactionsRequeststruct.
15-15: Field Addition:ReverseThe
Reversefield is correctly documented and integrated into theV2ListTransactionsRequeststruct.components/ledger/internal/storage/ledgerstore/transactions.go (2)
166-166: Modification:buildTransactionQueryMethodThe changes to the
buildTransactionQuerymethod are correctly implemented to include the new column expression.
406-411: Method Addition:WithColumnThe
WithColumnmethod is correctly implemented and integrated into theGetTransactionsQuerystruct.components/ledger/internal/api/v2/controllers_transactions_test.go (1)
Line range hint
570-717: Modification:TestGetTransactionsFunctionThe changes to the
TestGetTransactionsfunction are correctly implemented to include the newexpectQueryobject.tests/integration/suite/ledger-list-transactions.go (11)
8-8: Import Approved:sortpackage.The addition of the
sortpackage is necessary for sorting transactions by timestamp.
38-38: Change Approved: Use ofJustBeforeEach.The use of
JustBeforeEachensures that the ledger is created just before each test runs, which is appropriate for setup steps that should be executed in the latest possible moment.
54-63: Change Approved: Creating transactions with adjusted timestamps.The logic for adjusting the timestamps ensures that transactions have different effective dates, which is necessary for testing the ordering functionality.
78-78: Change Approved: Setting transaction timestamp.The
Timestampfield of the transaction is set using a pointer to the adjustedtxTimestamp, which is crucial for testing the sorting functionality.
136-160: Change Approved: Updating test setup for listing transactions.The changes ensure that the request for listing transactions includes the necessary parameters for pagination and filtering.
171-182: Change Approved: Adding context for testing effective ordering.The new context verifies that transactions are correctly ordered by their effective timestamp, which is the primary objective of the PR.
189-189: Change Approved: Ensuring cursor stability.Creating a new transaction ensures the stability of the cursor when paginating through transactions, which is important for accurate pagination tests.
231-231: Change Approved: Following previous cursor.The
JustBeforeEachblock ensures that the previous cursor is followed correctly, which is necessary for testing pagination in both directions.
260-260: Change Approved: Listing transactions with a single match filter.The
JustBeforeEachblock sets up the necessary conditions for testing the listing of transactions with a specific filter, ensuring the correct functionality of the filtering feature.
302-302: Change Approved: Listing transactions with multiple match filters.The
JustBeforeEachblock sets up the necessary conditions for testing the listing of transactions with multiple filters, ensuring the correct functionality of complex filtering scenarios.
360-360: Change Approved: Listing transactions with an invalid filter.The
JustBeforeEachblock sets up the necessary conditions for testing the error handling when an invalid filter is used, ensuring robust validation.releases/sdks/go/.speakeasy/gen.lock (3)
8-8: Release version update.The
releaseVersionis updated to0.0.1. Ensure this version number is consistent across the codebase and documentation.Verification successful
Release version update verified.
The
releaseVersionis updated to0.0.1and this version number is consistent across the relevant parts of the codebase, specifically inreleases/sdks/go/formance.go.
releases/sdks/go/formance.go:SDKVersion: "0.0.1"releases/sdks/go/formance.go:UserAgent: "speakeasy-sdk/go 0.0.1 ..."Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the release version across the codebase and documentation. # Test: Search for the release version. Expect: Only occurrences of the new version. rg --type go --type md '0.0.1'Length of output: 4966
16-16: New feature added:downloadStreams.The
downloadStreamsfeature is added. Ensure this feature is appropriately documented and integrated.
9-9: Configuration checksum update.The
configChecksumis updated toa4b5c74b8763c6fdc98e6adfde02c782. Ensure this checksum is correctly generated and matches the current configuration.Verification successful
Configuration checksum update verified.
The
configChecksumvaluea4b5c74b8763c6fdc98e6adfde02c782matches the checksum of the configuration filereleases/sdks/go/gen.yaml.
releases/sdks/go/gen.yamlScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the configuration checksum. # Test: Generate the configuration checksum and compare with the updated value. # Note: Replace `<config-file>` with the actual configuration file path. md5sum <config-file> | grep 'a4b5c74b8763c6fdc98e6adfde02c782'Length of output: 199
Script:
#!/bin/bash # Find potential configuration files in the repository fd -e json -e yaml -e yml -e toml -e ini --type f # Calculate the checksums of these files and compare with the given checksum for file in $(fd -e json -e yaml -e yml -e toml -e ini --type f); do md5sum "$file" done | grep 'a4b5c74b8763c6fdc98e6adfde02c782'Length of output: 73500
gfyrag
force-pushed
the
feat/ledger-timstamp-based-pagination
branch
from
833f7ec
to
483e99b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
components/ledger/internal/api/v2/controllers_transactions.go (2)
62-64: Ensure consistency in handling query parameters.The logic for setting the
Columnfield based on the "order" parameter is correct. However, consider using constants or enums for the "order" values to avoid hardcoding strings.- if r.URL.Query().Get("order") == "effective" { + const orderEffective = "effective" + if r.URL.Query().Get("order") == orderEffective {
65-67: Ensure consistency in handling query parameters.The logic for setting the
Orderfield based on the "reverse" parameter is correct. However, consider using constants or enums for the "reverse" values to avoid hardcoding strings.- if r.URL.Query().Get("reverse") == "true" { + const reverseTrue = "true" + if r.URL.Query().Get("reverse") == reverseTrue {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
components/ledger/openapi.yamlis excluded by!**/*.yamlcomponents/ledger/openapi/v2.yamlis excluded by!**/*.yamlreleases/sdks/go/gen.yamlis excluded by!**/*.yaml
Files selected for processing (12)
- components/ledger/internal/api/v2/controllers_transactions.go (1 hunks)
- components/ledger/internal/api/v2/controllers_transactions_test.go (4 hunks)
- components/ledger/internal/storage/ledgerstore/transactions.go (3 hunks)
- libs/go-libs/bun/bunpaginate/pagination.go (1 hunks)
- libs/go-libs/bun/bunpaginate/pagination_column.go (3 hunks)
- libs/go-libs/bun/bunpaginate/pagination_offset.go (1 hunks)
- releases/sdks/go/.speakeasy/gen.lock (2 hunks)
- releases/sdks/go/docs/pkg/models/operations/order.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2listtransactionsrequest.md (1 hunks)
- releases/sdks/go/formance.go (1 hunks)
- releases/sdks/go/pkg/models/operations/v2listtransactions.go (4 hunks)
- tests/integration/suite/ledger-list-transactions.go (11 hunks)
Files skipped from review due to trivial changes (1)
- releases/sdks/go/docs/pkg/models/operations/order.md
Files skipped from review as they are similar to previous changes (10)
- components/ledger/internal/api/v2/controllers_transactions_test.go
- components/ledger/internal/storage/ledgerstore/transactions.go
- libs/go-libs/bun/bunpaginate/pagination.go
- libs/go-libs/bun/bunpaginate/pagination_column.go
- libs/go-libs/bun/bunpaginate/pagination_offset.go
- releases/sdks/go/.speakeasy/gen.lock
- releases/sdks/go/docs/pkg/models/operations/v2listtransactionsrequest.md
- releases/sdks/go/formance.go
- releases/sdks/go/pkg/models/operations/v2listtransactions.go
- tests/integration/suite/ledger-list-transactions.go
gfyrag
force-pushed
the
feat/ledger-timstamp-based-pagination
branch
from
483e99b
to
73075a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
components/ledger/openapi.yamlis excluded by!**/*.yamlcomponents/ledger/openapi/v2.yamlis excluded by!**/*.yaml
Files selected for processing (11)
- components/ledger/internal/api/v2/controllers_transactions.go (1 hunks)
- components/ledger/internal/api/v2/controllers_transactions_test.go (4 hunks)
- components/ledger/internal/storage/ledgerstore/transactions.go (3 hunks)
- libs/go-libs/bun/bunpaginate/pagination.go (1 hunks)
- libs/go-libs/bun/bunpaginate/pagination_column.go (3 hunks)
- libs/go-libs/bun/bunpaginate/pagination_offset.go (1 hunks)
- releases/sdks/go/.speakeasy/gen.lock (2 hunks)
- releases/sdks/go/docs/pkg/models/operations/order.md (1 hunks)
- releases/sdks/go/docs/pkg/models/operations/v2listtransactionsrequest.md (1 hunks)
- releases/sdks/go/pkg/models/operations/v2listtransactions.go (4 hunks)
- tests/integration/suite/ledger-list-transactions.go (11 hunks)
Files skipped from review as they are similar to previous changes (10)
- components/ledger/internal/api/v2/controllers_transactions_test.go
- components/ledger/internal/storage/ledgerstore/transactions.go
- libs/go-libs/bun/bunpaginate/pagination.go
- libs/go-libs/bun/bunpaginate/pagination_column.go
- libs/go-libs/bun/bunpaginate/pagination_offset.go
- releases/sdks/go/.speakeasy/gen.lock
- releases/sdks/go/docs/pkg/models/operations/order.md
- releases/sdks/go/docs/pkg/models/operations/v2listtransactionsrequest.md
- releases/sdks/go/pkg/models/operations/v2listtransactions.go
- tests/integration/suite/ledger-list-transactions.go
Additional comments not posted (4)
components/ledger/internal/api/v2/controllers_transactions.go (4)
60-61: Initialize the query object correctly.The initialization of
qwithledgerstore.NewGetTransactionsQuery(*options)seems correct.
62-64: Ensure correct column assignment for ordering.The condition checks if the "order" query parameter is set to "effective" and assigns the "timestamp" column for ordering. This looks correct.
65-67: Verify the order direction assignment.The condition checks if the "reverse" query parameter is set to "true" and assigns ascending order. Ensure this logic aligns with the intended behavior.
Is the ascending order intended when "reverse" is set to true? Typically, "reverse" implies descending order.
68-69: Pointer conversion for the query object.The conversion of the query object to a pointer using
pointer.For(q)is correct.
Fixes ENG-1155
Add two query params on GET //transactions :