diff --git a/src/logic/safe/store/actions/transactions/__tests__/loadGatewayTransactions.test.ts b/src/logic/safe/store/actions/transactions/__tests__/loadGatewayTransactions.test.ts new file mode 100644 index 0000000000..8ee4c81c5d --- /dev/null +++ b/src/logic/safe/store/actions/transactions/__tests__/loadGatewayTransactions.test.ts @@ -0,0 +1,79 @@ +import * as gatewaySDK from '@gnosis.pm/safe-react-gateway-sdk' +import { FilterType } from 'src/routes/safe/components/Transactions/TxList/Filter' +import { _getHistoryPageUrl, _getTxHistory } from '../fetchTransactions/loadGatewayTransactions' + +jest.mock('@gnosis.pm/safe-react-gateway-sdk', () => { + const original = jest.requireActual('@gnosis.pm/safe-react-gateway-sdk') + return { + ...original, + getIncomingTransfers: jest.fn, + getMultisigTransactions: jest.fn, + getModuleTransactions: jest.fn, + getTransactionHistory: jest.fn, + } +}) + +describe('loadGatewayTransactions', () => { + beforeEach(() => { + jest.resetAllMocks() + }) + + describe('getTxHistory', () => { + it('fetches incoming transfers according to type', async () => { + const spy = jest.spyOn(gatewaySDK, 'getIncomingTransfers') + + await _getTxHistory('4', '0x123', { type: FilterType.INCOMING, token_address: '0x456' }) + + expect(spy).toHaveBeenCalledWith('4', '0x123', { token_address: '0x456' }, undefined) + }) + + it('fetches multisig transfers according to type', async () => { + const spy = jest.spyOn(gatewaySDK, 'getMultisigTransactions') + + await _getTxHistory('4', '0x123', { type: FilterType.MULTISIG, to: '0x456' }) + + expect(spy).toHaveBeenCalledWith('4', '0x123', { to: '0x456', executed: 'true' }, undefined) + }) + + it('fetches module transfers according to type', async () => { + const spy = jest.spyOn(gatewaySDK, 'getModuleTransactions') + + await _getTxHistory('4', '0x123', { type: FilterType.MODULE, to: '0x456' }) + + expect(spy).toHaveBeenCalledWith('4', '0x123', { to: '0x456' }, undefined) + }) + + it('fetches historical transfers by default', async () => { + const spy = jest.spyOn(gatewaySDK, 'getTransactionHistory') + + await _getTxHistory('4', '0x123') + + expect(spy).toHaveBeenCalledWith('4', '0x123', undefined) + }) + }) + + describe('getHistoryPageUrl', () => { + it('returns the pageUrl when a falsy pageUrl is provided', () => { + // SDK types should allow for `null` in TransactionListPage['next' | 'previous'] as it's returned by gateway + expect(_getHistoryPageUrl(null as unknown as undefined, { type: FilterType.INCOMING })).toBe(null) + }) + + it('returns the pageUrl when a no filter is provided', () => { + expect(_getHistoryPageUrl('http://test123.com', undefined)).toBe('http://test123.com') + expect(_getHistoryPageUrl('http://test456.com', {})).toBe('http://test456.com') + }) + + it('returns the pageUrl if it is an invalid URL', () => { + expect(_getHistoryPageUrl('test123', { type: FilterType.INCOMING })).toBe('test123') + }) + + it('appends only defined filter values to the pageUrl', () => { + expect(_getHistoryPageUrl('http://test123.com', { type: FilterType.INCOMING, value: undefined })).toBe( + 'http://test123.com/?type=Incoming', + ) + expect( + _getHistoryPageUrl('http://test456.com', { type: FilterType.MULTISIG, execution_date__gte: undefined }), + ).toBe('http://test456.com/?type=Outgoing') + }) + }) +}) diff --git a/src/logic/safe/store/actions/transactions/fetchTransactions/loadGatewayTransactions.ts b/src/logic/safe/store/actions/transactions/fetchTransactions/loadGatewayTransactions.ts index cb58790b22..d58abee23e 100644 --- a/src/logic/safe/store/actions/transactions/fetchTransactions/loadGatewayTransactions.ts +++ b/src/logic/safe/store/actions/transactions/fetchTransactions/loadGatewayTransactions.ts @@ -16,25 +16,16 @@ import { getMultisigFilter, getModuleFilter, } from 'src/routes/safe/components/Transactions/TxList/Filter/utils' -import { ChainId } from 'src/config/chain.d' -import { operations } from '@gnosis.pm/safe-react-gateway-sdk/dist/types/api' /*************/ /* HISTORY */ /*************/ -const historyPointers: { - [chainId: string]: { - [safeAddress: string]: { - next?: string - previous?: string - } - } -} = {} -const getHistoryTxListPage = async ( - chainId: ChainId, +export const _getTxHistory = async ( + chainId: string, safeAddress: string, filter?: FilterForm | Partial, + next?: string, ): Promise => { let txListPage: TransactionListPage = { next: undefined, @@ -42,26 +33,17 @@ const getHistoryTxListPage = async ( results: [], } - const { next } = historyPointers[chainId]?.[safeAddress] || {} - - let query: - | operations['incoming_transfers' | 'incoming_transfers' | 'module_transactions']['parameters']['query'] - | undefined - switch (filter?.[FILTER_TYPE_FIELD_NAME]) { case FilterType.INCOMING: { - query = filter ? getIncomingFilter(filter) : undefined - txListPage = await getIncomingTransfers(chainId, safeAddress, query, next) + txListPage = await getIncomingTransfers(chainId, safeAddress, getIncomingFilter(filter), next) break } case FilterType.MULTISIG: { - query = filter ? getMultisigFilter(filter, true) : undefined - txListPage = await getMultisigTransactions(chainId, safeAddress, query, next) + txListPage = await getMultisigTransactions(chainId, safeAddress, getMultisigFilter(filter, true), next) break } case FilterType.MODULE: { - query = filter ? getModuleFilter(filter) : undefined - txListPage = await getModuleTransactions(chainId, safeAddress, query, next) + txListPage = await getModuleTransactions(chainId, safeAddress, getModuleFilter(filter), next) break } default: { @@ -69,31 +51,41 @@ const getHistoryTxListPage = async ( } } - const getPageUrl = (pageUrl?: string): string | undefined => { - if (!pageUrl || !query) { - return pageUrl - } + return txListPage +} - let url: URL +export const _getHistoryPageUrl = (pageUrl?: string, filter?: FilterForm | Partial): undefined | string => { + if (!pageUrl || !filter || Object.keys(filter).length === 0) { + return pageUrl + } - try { - url = new URL(pageUrl) - } catch { - return pageUrl - } + let url: URL - Object.entries(query).forEach(([key, value]) => { + try { + url = new URL(pageUrl) + } catch { + return pageUrl + } + + Object.entries(filter) + .filter(([, value]) => value !== undefined) + .forEach(([key, value]) => { url.searchParams.set(key, String(value)) }) - return url.toString() - } + return url.toString() +} - historyPointers[chainId][safeAddress].next = getPageUrl(txListPage?.next) - historyPointers[chainId][safeAddress].previous = getPageUrl(txListPage?.previous) +const historyPointers: { [chainId: string]: { [safeAddress: string]: { next?: string; previous?: string } } } = {} - return txListPage -} +const getHistoryPointer = ( + next?: string, + previous?: string, + filter?: FilterForm | Partial, +): { next?: string; previous?: string } => ({ + next: _getHistoryPageUrl(next, filter), + previous: _getHistoryPageUrl(previous, filter), +}) /** * Fetch next page if there is a next pointer for the safeAddress. @@ -102,17 +94,26 @@ const getHistoryTxListPage = async ( */ export const loadPagedHistoryTransactions = async ( safeAddress: string, + filter?: FilterForm | Partial, ): Promise<{ values: HistoryGatewayResponse['results']; next?: string } | undefined> => { const chainId = _getChainId() - - if (!historyPointers[chainId]?.[safeAddress]?.next) { + // if `historyPointers[safeAddress] is `undefined` it means `loadHistoryTransactions` wasn't called + // if `historyPointers[safeAddress].next is `null`, it means it reached the last page in gateway-client + if (!historyPointers[chainId][safeAddress]?.next) { throw new CodedException(Errors._608) } try { - const { results, next } = await getHistoryTxListPage(chainId, safeAddress) + const { results, next, previous } = await _getTxHistory( + chainId, + checksumAddress(safeAddress), + filter, + historyPointers[chainId][safeAddress].next, + ) + + historyPointers[chainId][safeAddress] = getHistoryPointer(next, previous, filter) - return { values: results, next } + return { values: results, next: historyPointers[chainId][safeAddress].next } } catch (e) { throw new CodedException(Errors._602, e.message) } @@ -123,20 +124,14 @@ export const loadHistoryTransactions = async ( filter?: FilterForm | Partial, ): Promise => { const chainId = _getChainId() + try { + const { results, next, previous } = await _getTxHistory(chainId, checksumAddress(safeAddress), filter) - if (!historyPointers[chainId]) { - historyPointers[chainId] = {} - } - - if (!historyPointers[chainId][safeAddress] || filter) { - historyPointers[chainId][safeAddress] = { - next: undefined, - previous: undefined, + if (!historyPointers[chainId]) { + historyPointers[chainId] = {} } - } - try { - const { results } = await getHistoryTxListPage(chainId, safeAddress, filter) + historyPointers[chainId][safeAddress] = getHistoryPointer(next, previous, filter) return results } catch (e) { diff --git a/src/routes/safe/components/Transactions/TxList/Filter/index.tsx b/src/routes/safe/components/Transactions/TxList/Filter/index.tsx index 6d54093888..30ffdd4995 100644 --- a/src/routes/safe/components/Transactions/TxList/Filter/index.tsx +++ b/src/routes/safe/components/Transactions/TxList/Filter/index.tsx @@ -1,4 +1,4 @@ -import { ReactElement, useCallback, useEffect, useState } from 'react' +import { ReactElement, useEffect, useState } from 'react' import { Controller, DefaultValues, useForm } from 'react-hook-form' import styled from 'styled-components' import ExpandMoreIcon from '@material-ui/icons/ExpandMore' @@ -144,28 +144,29 @@ const Filter = (): ReactElement => { }) } - const clearFilter = useCallback( - ({ clearSearch = true } = {}) => { - if (search && clearSearch) { - history.replace(pathname) - dispatch(loadTransactions({ chainId, safeAddress: checksumAddress(safeAddress) })) - reset(defaultValues) - } + const clearFilter = () => { + history.replace({ search: '' }) + dispatch(loadTransactions({ chainId, safeAddress: checksumAddress(safeAddress) })) + reset(defaultValues) + hideFilter() + } - hideFilter() - }, - [search, history, pathname, chainId, dispatch, reset, safeAddress], - ) + const filterType = watch(FILTER_TYPE_FIELD_NAME) useEffect(() => { + // We cannot rely on cleanup as setting search params (in onSubmit) unmounts the component + const unsubscribe = history.listen((newLocation) => { + const shouldResetHistory = filterType && newLocation.pathname !== pathname + if (shouldResetHistory) { + dispatch(loadTransactions({ chainId, safeAddress: checksumAddress(safeAddress) })) + } + }) + return () => { - // If search is programatically cleared on unmount, the router routes back to here - // Search is inherently cleared when unmounting either way - clearFilter({ clearSearch: false }) + unsubscribe() } - }, [clearFilter]) - - const filterType = watch(FILTER_TYPE_FIELD_NAME) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) const onSubmit = (filter: FilterForm) => { // Don't apply the same filter twice @@ -175,7 +176,7 @@ const Filter = (): ReactElement => { const query = Object.fromEntries(Object.entries(filter).filter(([, value]) => !!value)) - history.replace({ pathname, search: `?${new URLSearchParams(query).toString()}` }) + history.replace({ search: `?${new URLSearchParams(query).toString()}` }) dispatch(loadTransactions({ chainId, safeAddress: checksumAddress(safeAddress), filter }))