From c28efe4d73815378b059f57cfa6ee7bfce435aa3 Mon Sep 17 00:00:00 2001 From: Salim TOUBAL Date: Thu, 30 Jan 2025 22:26:05 +0100 Subject: [PATCH] fix: fix sort feature (#13277) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Fix a regression on sort token feature ## **Related issues** Fixes: #13260 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** https://github.com/user-attachments/assets/b9ccdfd4-6a9f-47b9-93f6-ec2687487158 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../Tokens/__snapshots__/index.test.tsx.snap | 164 +++++++++--------- app/components/UI/Tokens/index.tsx | 66 +++---- e2e/pages/wallet/TokenSortBottomSheet.js | 25 +++ e2e/pages/wallet/WalletView.js | 14 ++ .../assets/multichain/asset-sort.spec.js | 89 ++++++++++ e2e/utils/Gestures.js | 1 - 6 files changed, 235 insertions(+), 124 deletions(-) create mode 100644 e2e/pages/wallet/TokenSortBottomSheet.js create mode 100644 e2e/specs/assets/multichain/asset-sort.spec.js diff --git a/app/components/UI/Tokens/__snapshots__/index.test.tsx.snap b/app/components/UI/Tokens/__snapshots__/index.test.tsx.snap index 3fe1837784e..5afc5733b5d 100644 --- a/app/components/UI/Tokens/__snapshots__/index.test.tsx.snap +++ b/app/components/UI/Tokens/__snapshots__/index.test.tsx.snap @@ -476,7 +476,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is tokens={ [ { - "address": "0x0", + "address": "0x01", "balanceFiat": "", "chainId": "0x1", "decimals": 18, @@ -484,13 +484,13 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is "isETH": false, "isNative": false, "isStaked": false, - "name": "Ethereum", - "symbol": "ETH", - "token": "Ethereum", + "name": "Bat", + "symbol": "BAT", + "token": "Bat", "tokenFiatAmount": 0, }, { - "address": "0x01", + "address": "0x0", "balanceFiat": "", "chainId": "0x1", "decimals": 18, @@ -498,10 +498,10 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is "isETH": false, "isNative": false, "isStaked": false, - "name": "Bat", - "symbol": "BAT", - "token": "Bat", - "tokenFiatAmount": 0, + "name": "Ethereum", + "symbol": "ETH", + "token": "Ethereum", + "tokenFiatAmount": undefined, }, ] } @@ -510,7 +510,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is data={ [ { - "address": "0x0", + "address": "0x01", "balanceFiat": "", "chainId": "0x1", "decimals": 18, @@ -518,13 +518,13 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is "isETH": false, "isNative": false, "isStaked": false, - "name": "Ethereum", - "symbol": "ETH", - "token": "Ethereum", + "name": "Bat", + "symbol": "BAT", + "token": "Bat", "tokenFiatAmount": 0, }, { - "address": "0x01", + "address": "0x0", "balanceFiat": "", "chainId": "0x1", "decimals": 18, @@ -532,10 +532,10 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is "isETH": false, "isNative": false, "isStaked": false, - "name": "Bat", - "symbol": "BAT", - "token": "Bat", - "tokenFiatAmount": 0, + "name": "Ethereum", + "symbol": "ETH", + "token": "Ethereum", + "tokenFiatAmount": undefined, }, ] } @@ -587,7 +587,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is "paddingVertical": 10, } } - testID="asset-ETH" + testID="asset-BAT" > - Ethereum + Bat @@ -809,35 +809,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is } testID="fiat-balance-test-id" > - + < 0.00001 BAT - + $0 @@ -905,7 +849,7 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is "paddingVertical": 10, } } - testID="asset-BAT" + testID="asset-ETH" > - Bat + Ethereum @@ -1127,7 +1071,35 @@ exports[`Tokens Portfolio View should match the snapshot when portfolio view is } testID="fiat-balance-test-id" > - < 0.00001 BAT + - $0 + diff --git a/app/components/UI/Tokens/index.tsx b/app/components/UI/Tokens/index.tsx index 48684d1f0d3..74d98faa16e 100644 --- a/app/components/UI/Tokens/index.tsx +++ b/app/components/UI/Tokens/index.tsx @@ -207,51 +207,29 @@ const Tokens: React.FC = memo(({ tokens }) => { return [...nativeTokens, ...nonNativeTokens]; }; - const calculateTokenFiatBalances = (assets: TokenI[]) => { - const tokenFiatBalances: number[] = []; - - for (const token of assets) { + const calculateFiatBalances = (assets: TokenI[]) => + assets.map((token) => { const chainId = token.chainId as Hex; - const multiChainExchangeRates = debouncedMultiChainMarketData?.[chainId]; + const multiChainExchangeRates = multiChainMarketData?.[chainId]; const multiChainTokenBalances = - debouncedMultiChainTokenBalance?.[ - selectedInternalAccountAddress as Hex - ]?.[chainId]; + multiChainTokenBalance?.[selectedInternalAccountAddress as Hex]?.[ + chainId + ]; const nativeCurrency = networkConfigurationsByChainId[chainId].nativeCurrency; const multiChainConversionRate = - debouncedMultiChainCurrencyRates?.[nativeCurrency]?.conversionRate || 0; - - // Calculate fiat balance for the token - const fiatBalance = - token.isETH || token.isNative - ? parseFloat(token.balance) * multiChainConversionRate - : deriveBalanceFromAssetMarketDetails( - token, - multiChainTokenBalances || {}, - multiChainExchangeRates || {}, - multiChainConversionRate || 0, - currentCurrency || '', - ).balanceFiatCalculation; - - // Add the calculated balance to the array - tokenFiatBalances.push(fiatBalance || 0); - } - - const tokensWithBalances: typeof assets = []; - - for (let i = 0; i < assets.length; i++) { - const token = assets[i]; - const tokenWithBalance = { - ...token, - tokenFiatAmount: tokenFiatBalances[i], - }; - - tokensWithBalances.push(tokenWithBalance); - } - - return tokensWithBalances; - }; + multiChainCurrencyRates?.[nativeCurrency]?.conversionRate || 0; + + return token.isETH || token.isNative + ? parseFloat(token.balance) * multiChainConversionRate + : deriveBalanceFromAssetMarketDetails( + token, + multiChainExchangeRates || {}, + multiChainTokenBalances || {}, + multiChainConversionRate || 0, + currentCurrency || '', + ).balanceFiatCalculation; + }); const filterTokensByNetwork = (tokensToDisplay: TokenI[]): TokenI[] => { if (isAllNetworks && isPopularNetwork) { @@ -286,7 +264,13 @@ const Tokens: React.FC = memo(({ tokens }) => { const assets = categorizeTokens(filteredTokens); - const tokensWithBalances = calculateTokenFiatBalances(assets); + // Calculate fiat balances for tokens + const tokenFiatBalances = calculateFiatBalances(assets); + + const tokensWithBalances = assets.map((token, i) => ({ + ...token, + tokenFiatAmount: tokenFiatBalances[i], + })); const tokensSorted = sortAssets(tokensWithBalances, tokenSortConfig); endTrace({ diff --git a/e2e/pages/wallet/TokenSortBottomSheet.js b/e2e/pages/wallet/TokenSortBottomSheet.js new file mode 100644 index 00000000000..2d48522d96e --- /dev/null +++ b/e2e/pages/wallet/TokenSortBottomSheet.js @@ -0,0 +1,25 @@ +import { WalletViewSelectorsIDs } from '../../selectors/wallet/WalletView.selectors'; +import Gestures from '../../utils/Gestures'; +import Matchers from '../../utils/Matchers'; + +class SortModal { + get sortAlphabetically() { + return Matchers.getElementByID(WalletViewSelectorsIDs.SORT_ALPHABETICAL); + } + + get sortFiatAmount() { + return Matchers.getElementByID( + WalletViewSelectorsIDs.SORT_DECLINING_BALANCE, + ); + } + + async tapSortAlphabetically() { + await Gestures.waitAndTap(this.sortAlphabetically); + } + + async tapSortFiatAmount() { + await Gestures.waitAndTap(this.sortFiatAmount); + } +} + +export default new SortModal(); diff --git a/e2e/pages/wallet/WalletView.js b/e2e/pages/wallet/WalletView.js index e812774b37e..761c0d0428b 100644 --- a/e2e/pages/wallet/WalletView.js +++ b/e2e/pages/wallet/WalletView.js @@ -90,6 +90,10 @@ class WalletView { return Matchers.getElementByID(WalletViewSelectorsIDs.TOKEN_NETWORK_FILTER); } + get sortBy() { + return Matchers.getElementByID(WalletViewSelectorsIDs.SORT_BY); + } + get tokenNetworkFilterAll() { return Matchers.getElementByID( WalletViewSelectorsIDs.TOKEN_NETWORK_FILTER_ALL, @@ -172,6 +176,12 @@ class WalletView { return Matchers.getElementByText(tokenName); } + async getTokensInWallet() { + return Matchers.getElementByID( + WalletViewSelectorsIDs.TOKENS_CONTAINER_LIST, + ); + } + async nftIDInWallet(nftId) { return Matchers.getElementByID(nftId); } @@ -192,6 +202,10 @@ class WalletView { await Gestures.waitAndTap(this.tokenNetworkFilter); } + async tapSortBy() { + await Gestures.waitAndTap(this.sortBy); + } + async tapTokenNetworkFilterAll() { await Gestures.waitAndTap(this.tokenNetworkFilterAll); } diff --git a/e2e/specs/assets/multichain/asset-sort.spec.js b/e2e/specs/assets/multichain/asset-sort.spec.js new file mode 100644 index 00000000000..10ed60d0ad8 --- /dev/null +++ b/e2e/specs/assets/multichain/asset-sort.spec.js @@ -0,0 +1,89 @@ +import { SmokeAssets } from '../../../tags'; +import WalletView from '../../../pages/wallet/WalletView'; +import SortModal from '../../../pages/wallet/TokenSortBottomSheet'; +import FixtureBuilder, { + DEFAULT_FIXTURE_ACCOUNT, +} from '../../../fixtures/fixture-builder'; +import { + loadFixture, + startFixtureServer, +} from '../../../fixtures/fixture-helper'; +import FixtureServer from '../../../fixtures/fixture-server'; +import { getFixturesServerPort } from '../../../fixtures/utils'; +import { loginToApp } from '../../../viewHelper'; +import Assertions from '../../../utils/Assertions'; +import ConfirmAddAssetView from '../../../pages/wallet/ImportTokenFlow/ConfirmAddAsset'; +import TestHelpers from '../../../helpers'; + +import Tenderly from '../../../tenderly'; +import { CustomNetworks } from '../../../resources/networks.e2e'; +import ImportTokensView from '../../../pages/wallet/ImportTokenFlow/ImportTokensView'; + +const fixtureServer = new FixtureServer(); + +describe(SmokeAssets('Import Tokens'), () => { + beforeAll(async () => { + await Tenderly.addFunds( + CustomNetworks.Tenderly.Mainnet.providerConfig.rpcUrl, + DEFAULT_FIXTURE_ACCOUNT, + ); + + await TestHelpers.reverseServerPort(); + const fixture = new FixtureBuilder() + .withNetworkController(CustomNetworks.Tenderly.Mainnet) + .build(); + await startFixtureServer(fixtureServer); + await loadFixture(fixtureServer, { fixture }); + await TestHelpers.launchApp({ + permissions: { notifications: 'YES' }, + launchArgs: { fixtureServerPort: `${getFixturesServerPort()}` }, + }); + await loginToApp(); + }); + + it('should add a aave token', async () => { + await WalletView.tapImportTokensButton(); + await ImportTokensView.searchToken('AAVE'); + await ImportTokensView.tapOnToken(); // taps the first token in the returned list + await ImportTokensView.tapOnNextButton(); + + await TestHelpers.delay(500); + await Assertions.checkIfVisible(ConfirmAddAssetView.container); + + await ConfirmAddAssetView.tapOnConfirmButton(); + + await Assertions.checkIfVisible(WalletView.container); + }); + + it('should sort tokens alphabetically', async () => { + await WalletView.tapSortBy(); + await SortModal.tapSortAlphabetically(); + + const tokens = await WalletView.getTokensInWallet(); + const tokensAttributes = await tokens.getAttributes(); + const label = tokensAttributes.label; + + // Ensure `label` contains "Aave" followed (somewhere) by "Ethereum". + const textOrderRegex = new RegExp('Aave([\\s\\S]*?)Ethereum', 'i'); + const isMatch = label.match(textOrderRegex); + if (!isMatch) { + throw new Error('Expected label to match the regex, but it did not.'); + } + }); + + it('should sort tokens by fiat amount', async () => { + await WalletView.tapSortBy(); + await SortModal.tapSortFiatAmount(); + + const tokens = await WalletView.getTokensInWallet(); + const tokensAttributes = await tokens.getAttributes(); + const label = tokensAttributes.label; + + // Ensure `label` contains "Ethereum" followed (somewhere) by "Aave". + const textOrderRegex = new RegExp('Ethereum([\\s\\S]*?)Aave', 'i'); + const isMatch = label.match(textOrderRegex); + if (!isMatch) { + throw new Error('Expected label to match the regex, but it did not.'); + } + }); +}); diff --git a/e2e/utils/Gestures.js b/e2e/utils/Gestures.js index d7753b980a6..275f3e70ab9 100644 --- a/e2e/utils/Gestures.js +++ b/e2e/utils/Gestures.js @@ -47,7 +47,6 @@ class Gestures { await element(by.text(new RegExp(`^/${textPattern} .*$/`))).tap(); } - /** * Wait for an element to be visible and then tap it. *