From bc19856d5d9ad1831e1722c84fe6161bed7a0a5a Mon Sep 17 00:00:00 2001 From: Niranjana Binoy <43930900+NiranjanaBinoy@users.noreply.github.com> Date: Fri, 24 Feb 2023 14:21:55 -0500 Subject: [PATCH] Transaction-list-item-details pop up to display the correct token information on token approve item (#17422) --- test/data/transaction-data.json | 64 +++++++++++++++++++ .../tests/custom-token-add-approve.spec.js | 16 ++--- test/e2e/tests/nfts.spec.js | 6 +- .../transaction-list-item.component.test.js | 3 + ui/hooks/useTransactionDisplayData.js | 57 +++++++++++++++-- ui/hooks/useTransactionDisplayData.test.js | 24 +++++-- 6 files changed, 148 insertions(+), 22 deletions(-) diff --git a/test/data/transaction-data.json b/test/data/transaction-data.json index 24d1deaf7ac7..c9f367aa3878 100644 --- a/test/data/transaction-data.json +++ b/test/data/transaction-data.json @@ -1208,5 +1208,69 @@ }, "hasRetried": false, "hasCancelled": false + }, + { + "initialTransaction": { + "blockNumber": "6195527", + "id": 8401691827212777, + "metamaskNetworkId": "4", + "status": "confirmed", + "time": 1585088013000, + "txParams": { + "from": "0xe18035bf8712672935fdb4e5e431b1a0183d2dfc", + "to": "0xabca64466f257793eaa52fcfff5066894b76a149", + "nonce": "0x1", + "value": "0x0", + "data": "0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef4000000000000000000000000000000000000000000000000000000000000c350", + "gas": "0xea60", + "maxFeePerGas": "0x4a817c800", + "maxPriorityFeePerGas": "0x4a817c800" + }, + "hash": "0xbcb195f393f4468945b4045cd41bcdbc2f19ad75ae92a32cf153a3004e42009a", + "type": "approve", + "origin": "https://metamask.github.io" + }, + "transactions": [ + { + "blockNumber": "6195527", + "id": 8401691827212777, + "metamaskNetworkId": "4", + "status": "confirmed", + "time": 1585088013000, + "txParams": { + "from": "0xe18035bf8712672935fdb4e5e431b1a0183d2dfc", + "to": "0xabca64466f257793eaa52fcfff5066894b76a149", + "nonce": "0x1", + "value": "0x0", + "data": "0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef4000000000000000000000000000000000000000000000000000000000000c350", + "gas": "0xea60", + "maxFeePerGas": "0x4a817c800", + "maxPriorityFeePerGas": "0x4a817c800" + }, + "hash": "0xbcb195f393f4468945b4045cd41bcdbc2f19ad75ae92a32cf153a3004e42009a", + "type": "approve" + } + ], + "primaryTransaction": { + "blockNumber": "6195527", + "id": 8401691827212777, + "metamaskNetworkId": "4", + "status": "confirmed", + "time": 1585088013000, + "txParams": { + "from": "0xe18035bf8712672935fdb4e5e431b1a0183d2dfc", + "to": "0xabca64466f257793eaa52fcfff5066894b76a149", + "nonce": "0x1", + "value": "0x0", + "data": "0x095ea7b30000000000000000000000009bc5baf874d2da8d216ae9f137804184ee5afef4000000000000000000000000000000000000000000000000000000000000c350", + "gas": "0xea60", + "maxFeePerGas": "0x4a817c800", + "maxPriorityFeePerGas": "0x4a817c800" + }, + "hash": "0xbcb195f393f4468945b4045cd41bcdbc2f19ad75ae92a32cf153a3004e42009a", + "type": "approve" + }, + "hasRetried": false, + "hasCancelled": false } ] diff --git a/test/e2e/tests/custom-token-add-approve.spec.js b/test/e2e/tests/custom-token-add-approve.spec.js index 4eb9fdb75d9f..62700d389016 100644 --- a/test/e2e/tests/custom-token-add-approve.spec.js +++ b/test/e2e/tests/custom-token-add-approve.spec.js @@ -196,11 +196,11 @@ describe('Create token, approve token and approve token without gas', function ( const approveTokenTask = await driver.waitForSelector({ // Selects only the very first transaction list item immediately following the 'Pending' header css: '.transaction-list__completed-transactions .transaction-list-item:first-child .list-item__heading', - text: 'Approve token spending cap', + text: 'Approve TST spending cap', }); assert.equal( await approveTokenTask.getText(), - 'Approve token spending cap', + 'Approve TST spending cap', ); }, ); @@ -339,11 +339,11 @@ describe('Create token, approve token and approve token without gas', function ( const approveTokenTask = await driver.waitForSelector({ // Select only the heading of the first entry in the transaction list. css: '.transaction-list__completed-transactions .transaction-list-item:first-child .list-item__heading', - text: 'Approve token spending cap', + text: 'Approve TST spending cap', }); assert.equal( await approveTokenTask.getText(), - 'Approve token spending cap', + 'Approve TST spending cap', ); }, ); @@ -426,11 +426,11 @@ describe('Create token, approve token and approve token without gas', function ( const approveTokenTask = await driver.waitForSelector({ // Select only the heading of the first entry in the transaction list. css: '.transaction-list__completed-transactions .transaction-list-item:first-child .list-item__heading', - text: 'Approve token spending cap', + text: 'Approve TST spending cap', }); assert.equal( await approveTokenTask.getText(), - 'Approve token spending cap', + 'Approve TST spending cap', ); }, ); @@ -490,11 +490,11 @@ describe('Create token, approve token and approve token without gas', function ( // check transaction in Activity tab const approveTokenTask = await driver.waitForSelector({ css: '.transaction-list__completed-transactions .transaction-list-item:first-child .list-item__heading', - text: 'Approve token spending cap', + text: 'Approve TST spending cap', }); assert.equal( await approveTokenTask.getText(), - 'Approve token spending cap', + 'Approve TST spending cap', ); }, ); diff --git a/test/e2e/tests/nfts.spec.js b/test/e2e/tests/nfts.spec.js index 16623644d67b..5de71dfe10de 100644 --- a/test/e2e/tests/nfts.spec.js +++ b/test/e2e/tests/nfts.spec.js @@ -131,7 +131,7 @@ describe('NFTs', function () { // Verify transaction const completedTx = await driver.findElement('.list-item__title'); const completedTxText = await completedTx.getText(); - assert.equal(completedTxText, 'Approve token spending cap'); + assert.equal(completedTxText, 'Approve TDC spending cap'); }, ); }); @@ -200,7 +200,7 @@ describe('NFTs', function () { // Verify transaction const completedTx = await driver.findElement('.list-item__title'); const completedTxText = await completedTx.getText(); - assert.equal(completedTxText, 'Approve Token with no spend limit'); + assert.equal(completedTxText, 'Approve TDC with no spend limit'); }, ); }); @@ -272,7 +272,7 @@ describe('NFTs', function () { // Verify transaction const completedTx = await driver.findElement('.list-item__title'); const completedTxText = await completedTx.getText(); - assert.equal(completedTxText, 'Approve Token with no spend limit'); + assert.equal(completedTxText, 'Approve TDC with no spend limit'); }, ); }); diff --git a/ui/components/app/transaction-list-item/transaction-list-item.component.test.js b/ui/components/app/transaction-list-item/transaction-list-item.component.test.js index 58cfcc5526be..66f0bc16ceed 100644 --- a/ui/components/app/transaction-list-item/transaction-list-item.component.test.js +++ b/ui/components/app/transaction-list-item/transaction-list-item.component.test.js @@ -16,6 +16,7 @@ import { import { useGasFeeEstimates } from '../../../hooks/useGasFeeEstimates'; import { GasEstimateTypes } from '../../../../shared/constants/gas'; +import { getTokens } from '../../../ducks/metamask/metamask'; import TransactionListItem from '.'; const FEE_MARKET_ESTIMATE_RETURN_VALUE = { @@ -88,6 +89,8 @@ const generateUseSelectorRouter = (opts) => (selector) => { ); } else if (selector === getShouldShowFiat) { return opts.shouldShowFiat ?? false; + } else if (selector === getTokens) { + return opts.tokens ?? []; } return undefined; }; diff --git a/ui/hooks/useTransactionDisplayData.js b/ui/hooks/useTransactionDisplayData.js index f9ea93f1b5a7..c9a3a939b72f 100644 --- a/ui/hooks/useTransactionDisplayData.js +++ b/ui/hooks/useTransactionDisplayData.js @@ -1,5 +1,10 @@ import { useDispatch, useSelector } from 'react-redux'; -import { getKnownMethodData } from '../selectors/selectors'; +import { useEffect, useState } from 'react'; +import { + getDetectedTokensInCurrentNetwork, + getKnownMethodData, + getTokenList, +} from '../selectors/selectors'; import { getStatusKey, getTransactionTypeTitle, @@ -7,6 +12,7 @@ import { import { camelCaseToCapitalize } from '../helpers/utils/common.util'; import { PRIMARY, SECONDARY } from '../helpers/constants/common'; import { + getAssetDetails, getTokenAddressParam, getTokenIdParam, } from '../helpers/utils/token-util'; @@ -93,6 +99,8 @@ export function useTransactionDisplayData(transactionGroup) { const currentAsset = useCurrentAsset(); const knownTokens = useSelector(getTokens); const knownNfts = useSelector(getNfts); + const detectedTokens = useSelector(getDetectedTokensInCurrentNetwork) || []; + const tokenList = useSelector(getTokenList); const t = useI18nContext(); const { initialTransaction, primaryTransaction } = transactionGroup; @@ -121,18 +129,53 @@ export function useTransactionDisplayData(transactionGroup) { // This value is used to determine whether we should look inside txParams.data // to pull out and render token related information const isTokenCategory = TOKEN_CATEGORY_HASH[type]; - // these values are always instantiated because they are either // used by or returned from hooks. Hooks must be called at the top level, // so as an additional safeguard against inappropriately associating token // transfers, we pass an additional argument to these hooks that will be // false for non-token transactions. This additional argument forces the // hook to return null - const token = - isTokenCategory && - knownTokens.find(({ address }) => - isEqualCaseInsensitive(address, recipientAddress), - ); + let token = null; + const [currentAssetDetails, setCurrentAssetDetails] = useState(null); + + if (isTokenCategory) { + token = + knownTokens.find(({ address }) => + isEqualCaseInsensitive(address, recipientAddress), + ) || + detectedTokens.find(({ address }) => + isEqualCaseInsensitive(address, recipientAddress), + ) || + tokenList[recipientAddress.toLowerCase()]; + } + useEffect(() => { + async function getAndSetAssetDetails() { + if (isTokenCategory && !token) { + const assetDetails = await getAssetDetails( + recipientAddress, + senderAddress, + initialTransaction?.txParams?.data, + knownNfts, + ); + setCurrentAssetDetails(assetDetails); + } + } + getAndSetAssetDetails(); + }, [ + isTokenCategory, + token, + recipientAddress, + senderAddress, + initialTransaction?.txParams?.data, + knownNfts, + ]); + if (currentAssetDetails) { + token = { + address: currentAssetDetails.toAddress, + symbol: currentAssetDetails.symbol, + decimals: currentAssetDetails.decimals, + }; + } const tokenData = useTokenData( initialTransaction?.txParams?.data, diff --git a/ui/hooks/useTransactionDisplayData.test.js b/ui/hooks/useTransactionDisplayData.test.js index e61ee354df8d..d925a392b398 100644 --- a/ui/hooks/useTransactionDisplayData.test.js +++ b/ui/hooks/useTransactionDisplayData.test.js @@ -10,7 +10,11 @@ import { getCurrentCurrency, getCurrentChainId, } from '../selectors'; -import { getTokens, getNativeCurrency } from '../ducks/metamask/metamask'; +import { + getTokens, + getNativeCurrency, + getNfts, +} from '../ducks/metamask/metamask'; import { getMessage } from '../helpers/utils/i18n-helper'; import messages from '../../app/_locales/en/messages.json'; import { ASSET_ROUTE, DEFAULT_ROUTE } from '../helpers/constants/routes'; @@ -143,6 +147,18 @@ const expectedResults = [ isPending: false, displayedStatusKey: TransactionStatus.confirmed, }, + { + title: 'Approve ABC spending cap', + category: TransactionGroupCategory.approval, + subtitle: `metamask.github.io`, + subtitleContainsOrigin: true, + primaryCurrency: '0.00000000000005 ABC', + senderAddress: '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc', + recipientAddress: '0xabca64466f257793eaa52fcfff5066894b76a149', + secondaryCurrency: undefined, + displayedStatusKey: TransactionStatus.confirmed, + isPending: false, + }, ]; let useSelector, useI18nContext, useTokenFiatAmount; @@ -166,9 +182,7 @@ describe('useTransactionDisplayData', () => { useTokenFiatAmountHooks, 'useTokenFiatAmount', ); - useTokenFiatAmount.returns((tokenAddress) => { - return tokenAddress ? '1 TST' : undefined; - }); + useTokenFiatAmount.returns(undefined); useI18nContext = sinon.stub(i18nhooks, 'useI18nContext'); useI18nContext.returns((key, variables) => getMessage('en', messages, key, variables), @@ -194,6 +208,8 @@ describe('useTransactionDisplayData', () => { return 'ETH'; } else if (selector === getCurrentChainId) { return CHAIN_IDS.MAINNET; + } else if (selector === getNfts) { + return []; } return null; });