fireblocks / fireblocks-sdk-js

Typescript & Javascript SDK for developers using Fireblocks API
https://docs.fireblocks.com/api/swagger-ui/
MIT License
72 stars 69 forks source link

`getTransactionsWithPageInfo` won't iterate through pages when `pageFilter` is defined #268

Open GusGold opened 8 months ago

GusGold commented 8 months ago

getTransactionsWithPageInfo accepts both pageFilter as well as a nextOrPreviousPath argument. The current logic will only use pageFilter when it is defined, even if nextOrPreviousPath is also defined. This leads to unexpected behaviour where providing nextOrPreviousPath on subsequent calls to attempt to page through the transaction list will not page at all, and instead keep on returning the same result set from the first page.

https://github.com/fireblocks/fireblocks-sdk-js/blob/ede50aa4827d97666b21873b3d0c9ae7ddc027dc/src/fireblocks-sdk.ts#L679-L686

I'd propose either swapping the logic around such that nextOrPreviousPath takes priority over pageFilter so that consumers of the SDK can do something like

const fireblocksClient = new FireblocksSDK(...)
let transactions: TransactionResponse[] = []
let nextPage: string | undefined
do {
  const response = await fireblocksClient.getTransactionsWithPageInfo(fbFilters, nextPage)
  transactions = transactions.concat(response.transactions)
  nextPage = response.pageDetails.nextPage
} while (nextPage.length)
// `transactions` now contains all transactions

As it currently stands, the above code is also the way to reproduce this current unexpected behaviour, where transactions will just be full of duplicates from the first page of results.

Alternatively, throw an error when both pageFilter and nextOrPreviousPath are defined, to force consumer to implement the sdk is the following kind of way, which will make it clear and remove the possibility to assume behaviour

const response = await fireblocksClient.getTransactionsWithPageInfo(nextPage ? undefined : fbFilters, nextPage)