getTransactionsWithPageInfo won't iterate through pages when pageFilter is defined

[GusGold]

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.

fireblocks-sdk-js/src/fireblocks-sdk.ts

Lines 679 to 686 in ede50aa

	public async getTransactionsWithPageInfo(pageFilter?: TransactionPageFilter, nextOrPreviousPath?: string): Promise<TransactionPageResponse> {
	if (pageFilter) {
	return await this.apiClient.issueGetRequestForTransactionPages(`/v1/transactions?${queryString.stringify(pageFilter)}`);
	} else if (nextOrPreviousPath) {
	const index = nextOrPreviousPath.indexOf("/v1/");
	const path = nextOrPreviousPath.substring(index, nextOrPreviousPath.length);
	return await this.apiClient.issueGetRequestForTransactionPages(path);
	}

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)