From 07aaab881139ab57173365e357d65bf491fece9b Mon Sep 17 00:00:00 2001 From: Satish Ravi Date: Wed, 18 Oct 2023 11:53:17 -0700 Subject: [PATCH] fix(tokens): avoid filters in hooks (#4319) ### Description More context [here](https://valora-app.slack.com/archives/C025V1D6F3J/p1697203116036359?thread_ts=1696932462.287309&cid=C025V1D6F3J) ### Test plan Unit tests ### Related issues N/A ### Backwards compatibility Yes --- src/tokens/Assets.test.tsx | 56 +++++++++++++++++++++++++- src/tokens/Assets.tsx | 47 ++++++++++++++++++---- src/tokens/TokenDetails.tsx | 4 +- src/tokens/hooks.test.tsx | 78 ++----------------------------------- src/tokens/hooks.ts | 77 +++++------------------------------- src/tokens/selectors.ts | 65 ++++++++++++++++++++++++------- 6 files changed, 160 insertions(+), 167 deletions(-) diff --git a/src/tokens/Assets.test.tsx b/src/tokens/Assets.test.tsx index 9db2c33f104..bcdcf58352e 100644 --- a/src/tokens/Assets.test.tsx +++ b/src/tokens/Assets.test.tsx @@ -4,7 +4,7 @@ import { Provider } from 'react-redux' import ValoraAnalytics from 'src/analytics/ValoraAnalytics' import { navigate } from 'src/navigator/NavigationService' import { Screens } from 'src/navigator/Screens' -import { getFeatureGate } from 'src/statsig' +import { getDynamicConfigParams, getFeatureGate } from 'src/statsig' import { StatsigFeatureGates } from 'src/statsig/types' import AssetsScreen from 'src/tokens/Assets' import { NetworkId } from 'src/transactions/types' @@ -20,6 +20,7 @@ import { mockNftNullMetadata, mockPositions, mockShortcuts, + mockTokenBalances, } from 'test/values' jest.mock('src/statsig', () => { @@ -31,6 +32,8 @@ jest.mock('src/statsig', () => { } }) +const ethTokenId = 'ethereum-sepolia:native' + const storeWithTokenBalances = { tokens: { tokenBalances: { @@ -332,4 +335,55 @@ describe('AssetsScreen', () => { fireEvent.press(getByText('assets.claimRewards')) expect(navigate).toHaveBeenCalledWith(Screens.DappShortcutsRewards) }) + + it('displays tokens with balance and ones marked with showZeroBalance in the expected order', () => { + jest.mocked(getDynamicConfigParams).mockReturnValueOnce({ + showBalances: [NetworkId['celo-alfajores'], NetworkId['ethereum-sepolia']], + }) + const store = createMockStore({ + tokens: { + tokenBalances: { + ...mockTokenBalances, + [ethTokenId]: { + tokenId: ethTokenId, + balance: '0', + priceUsd: '5', + networkId: NetworkId['ethereum-sepolia'], + showZeroBalance: true, + isNative: true, + symbol: 'ETH', + }, + ['token1']: { + tokenId: 'token1', + networkId: NetworkId['celo-alfajores'], + balance: '10', + symbol: 'TK1', + }, + ['token2']: { + tokenId: 'token2', + networkId: NetworkId['celo-alfajores'], + balance: '0', + symbol: 'TK2', + }, + ['token3']: { + tokenId: 'token3', + networkId: NetworkId['ethereum-sepolia'], + balance: '20', + symbol: 'TK3', + }, + }, + }, + }) + + const { getAllByTestId } = render( + + + + ) + + expect(getAllByTestId('TokenBalanceItem')).toHaveLength(6) + ;['POOF', 'TK3', 'TK1', 'CELO', 'ETH', 'cUSD'].map((symbol, index) => { + expect(getAllByTestId('TokenBalanceItem')[index]).toHaveTextContent(symbol) + }) + }) }) diff --git a/src/tokens/Assets.tsx b/src/tokens/Assets.tsx index d952dd1b16d..ddede7f1675 100644 --- a/src/tokens/Assets.tsx +++ b/src/tokens/Assets.tsx @@ -18,12 +18,12 @@ import Animated, { useSharedValue, } from 'react-native-reanimated' import { useSafeAreaInsets } from 'react-native-safe-area-context' -import { useSelector } from 'react-redux' import { AssetsEvents } from 'src/analytics/Events' import ValoraAnalytics from 'src/analytics/ValoraAnalytics' import Button, { BtnSizes, BtnTypes } from 'src/components/Button' import { AssetsTokenBalance } from 'src/components/TokenBalance' import Touchable from 'src/components/Touchable' +import { TOKEN_MIN_AMOUNT } from 'src/config' import ImageErrorIcon from 'src/icons/ImageErrorIcon' import { useDollarsToLocalAmount } from 'src/localCurrency/hooks' import { getLocalCurrencySymbol } from 'src/localCurrency/selectors' @@ -46,6 +46,7 @@ import { totalPositionsBalanceUsdSelector, } from 'src/positions/selectors' import { Position } from 'src/positions/types' +import useSelector from 'src/redux/useSelector' import { getFeatureGate } from 'src/statsig' import { StatsigFeatureGates } from 'src/statsig/types' import Colors from 'src/styles/colors' @@ -55,13 +56,14 @@ import { Shadow, Spacing, getShadowStyle } from 'src/styles/styles' import variables from 'src/styles/variables' import { PositionItem } from 'src/tokens/AssetItem' import { TokenBalanceItem } from 'src/tokens/TokenBalanceItem' -import { - useTokenPricesAreStale, - useTokensForAssetsScreen, - useTotalTokenBalance, -} from 'src/tokens/hooks' +import { useTokenPricesAreStale, useTotalTokenBalance } from 'src/tokens/hooks' +import { tokensListSelector } from 'src/tokens/selectors' import { TokenBalance } from 'src/tokens/slice' -import { getSupportedNetworkIdsForTokenBalances, getTokenAnalyticsProps } from 'src/tokens/utils' +import { + getSupportedNetworkIdsForTokenBalances, + getTokenAnalyticsProps, + usdBalance, +} from 'src/tokens/utils' const DEVICE_WIDTH_BREAKPOINT = 340 const NUM_OF_NFTS_PER_ROW = 2 @@ -116,7 +118,36 @@ function AssetsScreen({ navigation, route }: Props) { const activeTab = route.params?.activeTab ?? AssetTabType.Tokens const supportedNetworkIds = getSupportedNetworkIdsForTokenBalances() - const tokens = useTokensForAssetsScreen() + const allTokens = useSelector((state) => tokensListSelector(state, supportedNetworkIds)) + const tokens = useMemo( + () => + allTokens + .filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT) || tokenInfo.showZeroBalance) + .sort((token1, token2) => { + // Sorts by usd balance, then token balance, then zero balance natives by + // network id, then zero balance non natives by network id + const usdBalanceCompare = usdBalance(token2).comparedTo(usdBalance(token1)) + if (usdBalanceCompare) { + return usdBalanceCompare + } + + const balanceCompare = token2.balance.comparedTo(token1.balance) + if (balanceCompare) { + return balanceCompare + } + + if (token1.isNative && !token2.isNative) { + return -1 + } + if (!token1.isNative && token2.isNative) { + return 1 + } + + return token1.networkId.localeCompare(token2.networkId) + }), + [allTokens] + ) + const localCurrencySymbol = useSelector(getLocalCurrencySymbol) const totalTokenBalanceLocal = useTotalTokenBalance() ?? new BigNumber(0) const tokensAreStale = useTokenPricesAreStale(supportedNetworkIds) diff --git a/src/tokens/TokenDetails.tsx b/src/tokens/TokenDetails.tsx index 6b687d2c163..1e40f569ae3 100644 --- a/src/tokens/TokenDetails.tsx +++ b/src/tokens/TokenDetails.tsx @@ -35,9 +35,9 @@ import { TokenBalanceItem } from 'src/tokens/TokenBalanceItem' import { useCashInTokens, useCashOutTokens, - useSendableTokens, useSwappableTokens, useTokenInfo, + useTokensForSend, } from 'src/tokens/hooks' import { TokenBalance } from 'src/tokens/slice' import { TokenDetailsActionName } from 'src/tokens/types' @@ -138,7 +138,7 @@ function PriceInfo({ token }: { token: TokenBalance }) { function Actions({ token }: { token: TokenBalance }) { const { t } = useTranslation() - const sendableTokens = useSendableTokens() + const sendableTokens = useTokensForSend() const swappableTokens = useSwappableTokens() const cashInTokens = useCashInTokens() const cashOutTokens = useCashOutTokens() diff --git a/src/tokens/hooks.test.tsx b/src/tokens/hooks.test.tsx index b97a0fb89af..76349a45aff 100644 --- a/src/tokens/hooks.test.tsx +++ b/src/tokens/hooks.test.tsx @@ -9,11 +9,10 @@ import { useCashInTokens, useCashOutTokens, useLocalToTokenAmountByAddress, - useSendableTokens, useSwappableTokens, useTokenPricesAreStale, useTokenToLocalAmountByAddress, - useTokensForAssetsScreen, + useTokensForSend, } from 'src/tokens/hooks' import { TokenBalance } from 'src/tokens/slice' import { NetworkId } from 'src/transactions/types' @@ -219,80 +218,11 @@ describe('token to fiat exchanges', () => { }) }) -describe('useTokensForAssetsScreen', () => { - it('returns tokens with balance and tokens with showZeroBalance set to true', () => { - const store = createMockStore({ tokens: { tokenBalances: mockTokenBalances } }) - - const { getByTestId } = render( - - - - ) - - expect(getByTestId('tokenIDs').props.children).toEqual([ - mockPoofTokenId, - mockCeloTokenId, - mockCusdTokenId, - ]) - }) - - it('sorts by usd balance, then balance if price is not available, then zero balance tokens with natives first', () => { - jest.mocked(getDynamicConfigParams).mockReturnValueOnce({ - showBalances: [NetworkId['celo-alfajores'], NetworkId['ethereum-sepolia']], - }) - const store = createMockStore({ - tokens: { - tokenBalances: { - ...mockTokenBalances, - [ethTokenId]: { - tokenId: ethTokenId, - balance: '0', - priceUsd: '5', - networkId: NetworkId['ethereum-sepolia'], - showZeroBalance: true, - isNative: true, - }, - ['token1']: { - tokenId: 'token1', - networkId: NetworkId['celo-alfajores'], - balance: '10', - }, - ['token2']: { - tokenId: 'token2', - networkId: NetworkId['celo-alfajores'], - balance: '0', - }, - ['token3']: { - tokenId: 'token3', - networkId: NetworkId['ethereum-sepolia'], - balance: '20', - }, - }, - }, - }) - - const { getByTestId } = render( - - - - ) - - expect(getByTestId('tokenIDs').props.children).toEqual([ - mockPoofTokenId, - 'token3', - 'token1', - mockCeloTokenId, - ethTokenId, - mockCusdTokenId, - ]) - }) -}) - -describe('useSendableTokens', () => { +describe('useTokensForSend', () => { it('returns tokens with balance', () => { const { getByTestId } = render( - + ) @@ -309,7 +239,7 @@ describe('useSendableTokens', () => { }) const { getByTestId } = render( - + ) diff --git a/src/tokens/hooks.ts b/src/tokens/hooks.ts index 15fdc92d83b..36a6aad675f 100644 --- a/src/tokens/hooks.ts +++ b/src/tokens/hooks.ts @@ -1,18 +1,20 @@ import BigNumber from 'bignumber.js' -import DeviceInfo from 'react-native-device-info' -import { TIME_UNTIL_TOKEN_INFO_BECOMES_STALE, TOKEN_MIN_AMOUNT } from 'src/config' +import { TIME_UNTIL_TOKEN_INFO_BECOMES_STALE } from 'src/config' import { usdToLocalCurrencyRateSelector } from 'src/localCurrency/selectors' import useSelector from 'src/redux/useSelector' import { getDynamicConfigParams } from 'src/statsig' import { DynamicConfigs } from 'src/statsig/constants' import { StatsigDynamicConfigs } from 'src/statsig/types' import { - tokenCompareByUsdBalanceThenByName, + cashInTokensByNetworkIdSelector, + cashOutTokensByNetworkIdSelector, + swappableTokensByNetworkIdSelector, tokensByAddressSelector, tokensByCurrencySelector, tokensByIdSelector, tokensListSelector, tokensListWithAddressSelector, + tokensWithTokenBalanceSelector, tokensWithUsdValueSelector, totalTokenBalanceSelector, } from 'src/tokens/selectors' @@ -22,12 +24,9 @@ import { convertTokenToLocalAmount, getSupportedNetworkIdsForSend, getSupportedNetworkIdsForTokenBalances, - isCicoToken, - usdBalance, } from 'src/tokens/utils' import { NetworkId } from 'src/transactions/types' import { Currency } from 'src/utils/currencies' -import { isVersionBelowMinimum } from 'src/utils/versionCheck' import networkConfig from 'src/web3/networkConfig' /** @@ -49,44 +48,12 @@ export function useTotalTokenBalance() { export function useTokensWithTokenBalance() { const supportedNetworkIds = getSupportedNetworkIdsForTokenBalances() - const tokens = useSelector((state) => tokensListSelector(state, supportedNetworkIds)) - return tokens.filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT)) + return useSelector((state) => tokensWithTokenBalanceSelector(state, supportedNetworkIds)) } export function useTokensForSend() { const supportedNetworkIds = getSupportedNetworkIdsForSend() - const tokens = useSelector((state) => tokensListSelector(state, supportedNetworkIds)) - return tokens.filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT)) -} - -export function useTokensForAssetsScreen() { - const supportedNetworkIds = getSupportedNetworkIdsForTokenBalances() - const tokens = useSelector((state) => tokensListSelector(state, supportedNetworkIds)) - - return tokens - .filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT) || tokenInfo.showZeroBalance) - .sort((token1, token2) => { - // Sorts by usd balance, then token balance, then zero balance natives by - // network id, then zero balance non natives by network id - const usdBalanceCompare = usdBalance(token2).comparedTo(usdBalance(token1)) - if (usdBalanceCompare) { - return usdBalanceCompare - } - - const balanceCompare = token2.balance.comparedTo(token1.balance) - if (balanceCompare) { - return balanceCompare - } - - if (token1.isNative && !token2.isNative) { - return -1 - } - if (!token1.isNative && token2.isNative) { - return 1 - } - - return token1.networkId.localeCompare(token2.networkId) - }) + return useSelector((state) => tokensWithTokenBalanceSelector(state, supportedNetworkIds)) } export function useTokensInfoUnavailable(networkIds: NetworkId[]) { @@ -117,49 +84,25 @@ export function useTokenPricesAreStale(networkIds: NetworkId[]) { } } -export function useSendableTokens() { - const networkIdsForSend = getDynamicConfigParams( - DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES] - ).showSend - const tokens = useSelector((state) => tokensListSelector(state, networkIdsForSend)) - return tokens.filter((tokenInfo) => tokenInfo.balance.gt(TOKEN_MIN_AMOUNT)) -} - export function useSwappableTokens() { - const appVersion = DeviceInfo.getVersion() const networkIdsForSwap = getDynamicConfigParams( DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES] ).showSwap - const tokens = useSelector((state) => tokensListSelector(state, networkIdsForSwap)) - return tokens - .filter( - (tokenInfo) => - tokenInfo.isSwappable || - (tokenInfo.minimumAppVersionToSwap && - !isVersionBelowMinimum(appVersion, tokenInfo.minimumAppVersionToSwap)) - ) - .sort(tokenCompareByUsdBalanceThenByName) + return useSelector((state) => swappableTokensByNetworkIdSelector(state, networkIdsForSwap)) } export function useCashInTokens() { const networkIdsForCico = getDynamicConfigParams( DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES] ).showCico - const tokens = useSelector((state) => tokensListSelector(state, networkIdsForCico)) - return tokens.filter((tokenInfo) => tokenInfo.isCashInEligible && isCicoToken(tokenInfo.symbol)) + return useSelector((state) => cashInTokensByNetworkIdSelector(state, networkIdsForCico)) } export function useCashOutTokens() { const networkIdsForCico = getDynamicConfigParams( DynamicConfigs[StatsigDynamicConfigs.MULTI_CHAIN_FEATURES] ).showCico - const tokens = useSelector((state) => tokensListSelector(state, networkIdsForCico)) - return tokens.filter( - (tokenInfo) => - tokenInfo.balance.gt(TOKEN_MIN_AMOUNT) && - tokenInfo.isCashOutEligible && - isCicoToken(tokenInfo.symbol) - ) + return useSelector((state) => cashOutTokensByNetworkIdSelector(state, networkIdsForCico)) } export function useTokenInfo(tokenId?: string): TokenBalance | undefined { diff --git a/src/tokens/selectors.ts b/src/tokens/selectors.ts index d1881757b22..7b69002bea5 100644 --- a/src/tokens/selectors.ts +++ b/src/tokens/selectors.ts @@ -1,4 +1,5 @@ import BigNumber from 'bignumber.js' +import _ from 'lodash' import deviceInfoModule from 'react-native-device-info' import { createSelector } from 'reselect' import { @@ -18,8 +19,11 @@ import { NetworkId } from 'src/transactions/types' import { Currency } from 'src/utils/currencies' import { isVersionBelowMinimum } from 'src/utils/versionCheck' import networkConfig from 'src/web3/networkConfig' -import { sortByUsdBalance, sortFirstStableThenCeloThenOthersByUsdBalance } from './utils' -import _ from 'lodash' +import { + isCicoToken, + sortByUsdBalance, + sortFirstStableThenCeloThenOthersByUsdBalance, +} from './utils' type TokenBalanceWithPriceUsd = TokenBalance & { priceUsd: BigNumber @@ -211,20 +215,12 @@ export function tokenCompareByUsdBalanceThenByName(token1: TokenBalance, token2: } /** - * @deprecated + * @deprecated use swappableTokensByNetworkIdSelector or useSwappableTokens hook */ -export const swappableTokensSelector = createSelector(tokensByUsdBalanceSelector, (tokens) => { - const appVersion = deviceInfoModule.getVersion() - - return tokens - .filter( - (tokenInfo) => - tokenInfo.isSwappable || - (tokenInfo.minimumAppVersionToSwap && - !isVersionBelowMinimum(appVersion, tokenInfo.minimumAppVersionToSwap)) - ) - .sort(tokenCompareByUsdBalanceThenByName) -}) +export const swappableTokensSelector = createSelector( + (state: RootState) => swappableTokensByNetworkIdSelector(state, [networkConfig.defaultNetworkId]), + (tokens) => tokens.filter((tokenInfo) => !!tokenInfo.address) as TokenBalanceWithAddress[] +) /** * @deprecated @@ -343,5 +339,44 @@ export const tokensInfoUnavailableSelector = createSelector( } ) +export const tokensWithTokenBalanceSelector = createSelector( + (state: RootState, networkIds: NetworkId[]) => tokensListSelector(state, networkIds), + (tokens) => { + return tokens.filter((token) => token.balance.gt(TOKEN_MIN_AMOUNT)) + } +) + +export const swappableTokensByNetworkIdSelector = createSelector( + (state: RootState, networkIds: NetworkId[]) => tokensListSelector(state, networkIds), + (tokens) => { + const appVersion = deviceInfoModule.getVersion() + return tokens + .filter( + (tokenInfo) => + tokenInfo.isSwappable || + (tokenInfo.minimumAppVersionToSwap && + !isVersionBelowMinimum(appVersion, tokenInfo.minimumAppVersionToSwap)) + ) + .sort(tokenCompareByUsdBalanceThenByName) + } +) + +export const cashInTokensByNetworkIdSelector = createSelector( + (state: RootState, networkIds: NetworkId[]) => tokensListSelector(state, networkIds), + (tokens) => + tokens.filter((tokenInfo) => tokenInfo.isCashInEligible && isCicoToken(tokenInfo.symbol)) +) + +export const cashOutTokensByNetworkIdSelector = createSelector( + (state: RootState, networkIds: NetworkId[]) => tokensListSelector(state, networkIds), + (tokens) => + tokens.filter( + (tokenInfo) => + tokenInfo.balance.gt(TOKEN_MIN_AMOUNT) && + tokenInfo.isCashOutEligible && + isCicoToken(tokenInfo.symbol) + ) +) + export const visualizeNFTsEnabledInHomeAssetsPageSelector = (state: RootState) => state.app.visualizeNFTsEnabledInHomeAssetsPage