From f1de905be7f8b52b8868f24caf984dcf949b6844 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Fri, 2 Jun 2023 18:33:10 +0530 Subject: [PATCH] Fix details when transferring NFT not added to wallet (#19045) --- test/e2e/nft/erc721-interaction.spec.js | 2 +- .../confirm-page-container-container.test.js | 20 ++++++ .../confirm-page-container.component.js | 6 +- .../app/confirm-subtitle/confirm-subtitle.js | 9 ++- .../confirm-subtitle/confirm-subtitle.test.js | 43 +++++++++++- ui/helpers/utils/transactions.util.js | 10 +++ ui/pages/confirm-approve/confirm-approve.js | 1 + .../confirm-token-transaction-base.js | 1 + .../confirm-transaction-base.component.js | 7 +- .../send-asset-row.component.js | 29 ++++++--- .../send-asset-row/send-asset-row.test.js | 65 +++++++++++++++++++ 11 files changed, 173 insertions(+), 20 deletions(-) create mode 100644 ui/pages/send/send-content/send-asset-row/send-asset-row.test.js diff --git a/test/e2e/nft/erc721-interaction.spec.js b/test/e2e/nft/erc721-interaction.spec.js index d0603225a..f8c703354 100644 --- a/test/e2e/nft/erc721-interaction.spec.js +++ b/test/e2e/nft/erc721-interaction.spec.js @@ -50,7 +50,7 @@ describe('ERC721 NFTs testdapp interaction', function () { // Confirm transfer await driver.waitForSelector({ - css: '.confirm-page-container-summary__title', + css: '.mm-text--heading-md', text: 'TestDappCollectibles', }); await driver.clickElement({ text: 'Confirm', tag: 'button' }); diff --git a/ui/components/app/confirm-page-container/confirm-page-container-container.test.js b/ui/components/app/confirm-page-container/confirm-page-container-container.test.js index 6dc3afcea..6432fad41 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container-container.test.js +++ b/ui/components/app/confirm-page-container/confirm-page-container-container.test.js @@ -282,6 +282,26 @@ describe('Confirm Page Container Container Test', () => { }); }); + describe('Rendering NetworkAccountBalanceHeader', () => { + const store = configureMockStore()(mockState); + + it('should render NetworkAccountBalanceHeader if displayAccountBalanceHeader is true', () => { + const { getByText } = renderWithProvider( + , + store, + ); + expect(getByText('Balance')).toBeInTheDocument(); + }); + + it('should not render NetworkAccountBalanceHeader if displayAccountBalanceHeader is false', () => { + const { queryByText } = renderWithProvider( + , + store, + ); + expect(queryByText('Balance')).toBeNull(); + }); + }); + describe('Contact/AddressBook name should appear in recipient header', () => { it('should not show add to address dialog if recipient is in contact list and should display contact name', () => { const addressBookName = 'test save name'; diff --git a/ui/components/app/confirm-page-container/confirm-page-container.component.js b/ui/components/app/confirm-page-container/confirm-page-container.component.js index e36af4144..cd489fea8 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container.component.js +++ b/ui/components/app/confirm-page-container/confirm-page-container.component.js @@ -98,6 +98,7 @@ const ConfirmPageContainer = (props) => { txData, assetStandard, isApprovalOrRejection, + displayAccountBalanceHeader, ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) noteComponent, ///: END:ONLY_INCLUDE_IN @@ -169,9 +170,7 @@ const ConfirmPageContainer = (props) => {
- {assetStandard === TokenStandard.ERC20 || - assetStandard === TokenStandard.ERC721 || - assetStandard === TokenStandard.ERC1155 ? ( + {displayAccountBalanceHeader ? ( { const shouldShowFiat = useSelector(getShouldShowFiat); const { isNftTransfer } = useTransactionInfo(txData); - if (!shouldShowFiat && !isNftTransfer) { + if (!shouldShowFiat && !isNftTransfer && !isNFTAssetStandard(assetStandard)) { return null; } @@ -48,6 +50,7 @@ const ConfirmSubTitle = ({ }; ConfirmSubTitle.propTypes = { + assetStandard: PropTypes.string, hexTransactionAmount: PropTypes.string, subtitleComponent: PropTypes.element, txData: PropTypes.object.isRequired, diff --git a/ui/components/app/confirm-subtitle/confirm-subtitle.test.js b/ui/components/app/confirm-subtitle/confirm-subtitle.test.js index b8218029a..946c19b5f 100644 --- a/ui/components/app/confirm-subtitle/confirm-subtitle.test.js +++ b/ui/components/app/confirm-subtitle/confirm-subtitle.test.js @@ -1,7 +1,8 @@ import React from 'react'; -import { hexToDecimal } from '../../../../shared/modules/conversion.utils'; +import { ERC1155, ERC721 } from '@metamask/controller-utils'; import mockState from '../../../../test/data/mock-state.json'; +import { hexToDecimal } from '../../../../shared/modules/conversion.utils'; import { renderWithProvider } from '../../../../test/lib/render-helpers'; import configureStore from '../../../store/store'; import ConfirmSubTitle from './confirm-subtitle'; @@ -42,7 +43,7 @@ describe('ConfirmSubTitle', () => { expect(container.firstChild).toStrictEqual(null); }); - it('should not null if showFiatInTestnets preference if false but it is NFT Transfer', async () => { + it('should not return null if it is NFT Transfer', async () => { mockState.metamask.preferences.showFiatInTestnets = false; mockState.metamask.allNftContracts = { [mockState.metamask.selectedAddress]: { @@ -67,6 +68,44 @@ describe('ConfirmSubTitle', () => { expect(await findByText('0.00001')).toBeInTheDocument(); }); + it('should not return null if assetStandard is ERC1155', async () => { + mockState.metamask.preferences.showFiatInTestnets = false; + store = configureStore(mockState); + + const { findByText } = renderWithProvider( + , + store, + ); + expect(await findByText('0.00001')).toBeInTheDocument(); + }); + + it('should not return null if assetStandard is ERC712', async () => { + mockState.metamask.preferences.showFiatInTestnets = false; + store = configureStore(mockState); + + const { findByText } = renderWithProvider( + , + store, + ); + expect(await findByText('0.00001')).toBeInTheDocument(); + }); + it('should render subtitleComponent if passed', () => { const { getByText } = renderWithProvider( + assetStandard === ERC1155 || assetStandard === ERC721; diff --git a/ui/pages/confirm-approve/confirm-approve.js b/ui/pages/confirm-approve/confirm-approve.js index a7c36df55..d0fadc086 100644 --- a/ui/pages/confirm-approve/confirm-approve.js +++ b/ui/pages/confirm-approve/confirm-approve.js @@ -319,6 +319,7 @@ export default function ConfirmApprove({ hideSenderToRecipient customTxParamsData={customData} assetStandard={assetStandard} + displayAccountBalanceHeader /> ); diff --git a/ui/pages/confirm-token-transaction-base/confirm-token-transaction-base.js b/ui/pages/confirm-token-transaction-base/confirm-token-transaction-base.js index ee0b5c869..bdd52e21b 100644 --- a/ui/pages/confirm-token-transaction-base/confirm-token-transaction-base.js +++ b/ui/pages/confirm-token-transaction-base/confirm-token-transaction-base.js @@ -188,6 +188,7 @@ export default function ConfirmTokenTransactionBase({ return ( ); } @@ -837,6 +840,7 @@ export default class ConfirmTransactionBase extends Component { image, isApprovalOrRejection, assetStandard, + displayAccountBalanceHeader, title, ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) isNoteToTraderSupported, @@ -952,6 +956,7 @@ export default class ConfirmTransactionBase extends Component { isApprovalOrRejection={isApprovalOrRejection} assetStandard={assetStandard} txData={txData} + displayAccountBalanceHeader={displayAccountBalanceHeader} /> ); diff --git a/ui/pages/send/send-content/send-asset-row/send-asset-row.component.js b/ui/pages/send/send-content/send-asset-row/send-asset-row.component.js index b39ea987b..29b6672cf 100644 --- a/ui/pages/send/send-content/send-asset-row/send-asset-row.component.js +++ b/ui/pages/send/send-content/send-asset-row/send-asset-row.component.js @@ -154,8 +154,8 @@ export default class SendAssetRow extends Component { isEqualCaseInsensitive(address, details.address) && tokenId === details.tokenId, ); - if (nft) { - return this.renderNft(nft); + if (nft || details) { + return this.renderNft(nft ?? details); } } return this.renderNativeCurrency(); @@ -184,8 +184,13 @@ export default class SendAssetRow extends Component { renderNativeCurrency(insideDropdown = false) { const { t } = this.context; - const { accounts, selectedAddress, nativeCurrency, nativeCurrencyImage } = - this.props; + const { + accounts, + selectedAddress, + nativeCurrency, + nativeCurrencyImage, + sendAsset, + } = this.props; const { sendableTokens, sendableNfts } = this.state; @@ -204,11 +209,15 @@ export default class SendAssetRow extends Component { onClick={() => this.selectToken(AssetType.native)} >
- + {sendAsset?.type === AssetType.NFT && sendAsset?.details?.image ? ( + + ) : ( + + )}
@@ -277,7 +286,7 @@ export default class SendAssetRow extends Component {
- {nftCollection.name || name} + {nftCollection?.name || name}
diff --git a/ui/pages/send/send-content/send-asset-row/send-asset-row.test.js b/ui/pages/send/send-content/send-asset-row/send-asset-row.test.js new file mode 100644 index 000000000..61aea6f0b --- /dev/null +++ b/ui/pages/send/send-content/send-asset-row/send-asset-row.test.js @@ -0,0 +1,65 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; + +import mockSendState from '../../../../../test/data/mock-send-state.json'; +import { renderWithProvider } from '../../../../../test/lib/render-helpers'; +import SendAssetRow from './send-asset-row.component'; + +const mockStore = configureMockStore()(mockSendState); + +const props = { + tokens: [], + selectedAddress: '0x0', + nfts: [], + collections: [], + sendAsset: { + balance: '0x1', + details: { + address: '0xb9b9c1592e737a2c71d80d3b8495049990161e0c', + tokenId: '2', + standard: 'ERC721', + tokenURI: + 'data:application/json;base64,eyJuYW1lIjogIlRlc3QgRGFwcCBDb2xsZWN0aWJsZXMgIzIiLCAiZGVzY3JpcHRpb24iOiAiVGVzdCBEYXBwIENvbGxlY3RpYmxlcyBmb3IgdGVzdGluZy4iLCAiaW1hZ2UiOiAiZGF0YTppbWFnZS9zdmcreG1sO2Jhc2U2NCxQSE4yWnlCb1pXbG5hSFE5SWpNMU1DSWdkMmxrZEdnOUlqTTFNQ0lnZG1sbGQwSnZlRDBpTUNBd0lERXdNQ0F4TURBaUlIaHRiRzV6UFNKb2RIUndPaTh2ZDNkM0xuY3pMbTl5Wnk4eU1EQXdMM04yWnlJK1BHUmxabk0rUEhCaGRHZ2dhV1E5SWsxNVVHRjBhQ0lnWm1sc2JEMGlibTl1WlNJZ2MzUnliMnRsUFNKeVpXUWlJR1E5SWsweE1DdzVNQ0JST1RBc09UQWdPVEFzTkRVZ1VUa3dMREV3SURVd0xERXdJRkV4TUN3eE1DQXhNQ3cwTUNCUk1UQXNOekFnTkRVc056QWdVVGN3TERjd0lEYzFMRFV3SWlBdlBqd3ZaR1ZtY3o0OGRHVjRkRDQ4ZEdWNGRGQmhkR2dnYUhKbFpqMGlJMDE1VUdGMGFDSStVWFZwWTJzZ1luSnZkMjRnWm05NElHcDFiWEJ6SUc5MlpYSWdkR2hsSUd4aGVua2daRzluTGp3dmRHVjRkRkJoZEdnK1BDOTBaWGgwUGp3dmMzWm5QZz09IiwgImF0dHJpYnV0ZXMiOiBbeyJ0cmFpdF90eXBlIjogIlRva2VuIElkIiwgInZhbHVlIjogIjIifV19', + symbol: 'TDC', + name: 'TestDappCollectibles', + image: + 'data:image/svg+xml;base64,PHN2ZyBoZWlnaHQ9IjM1MCIgd2lkdGg9IjM1MCIgdmlld0JveD0iMCAwIDEwMCAxMDAiIHhtbG5zPSJodHRwOi8vd3d3LnczLm9yZy8yMDAwL3N2ZyI+PGRlZnM+PHBhdGggaWQ9Ik15UGF0aCIgZmlsbD0ibm9uZSIgc3Ryb2tlPSJyZWQiIGQ9Ik0xMCw5MCBROTAsOTAgOTAsNDUgUTkwLDEwIDUwLDEwIFExMCwxMCAxMCw0MCBRMTAsNzAgNDUsNzAgUTcwLDcwIDc1LDUwIiAvPjwvZGVmcz48dGV4dD48dGV4dFBhdGggaHJlZj0iI015UGF0aCI+UXVpY2sgYnJvd24gZm94IGp1bXBzIG92ZXIgdGhlIGxhenkgZG9nLjwvdGV4dFBhdGg+PC90ZXh0Pjwvc3ZnPg==', + }, + error: null, + type: 'NFT', + }, + accounts: { + '0x0': { + address: '0x0', + balance: '0x4cc191087ecfcee6', + }, + '0x1': { + address: '0x1', + balance: '0x041813d3a1fc67ed', + }, + }, +}; + +describe('SendAssetRow', () => { + describe('Sending NFT', () => { + it('should render NFT image even if NFT is not imported into metamask', async () => { + const { getByRole } = renderWithProvider( + , + mockStore, + ); + + const image = getByRole('img'); + expect(image).toBeDefined(); + expect(image).toHaveAttribute('src', props.sendAsset.details.image); + }); + + it('should render token id even if NFT is not imported into metamask', async () => { + const { getByText } = renderWithProvider( + , + mockStore, + ); + + expect(getByText('Token ID:')).toBeInTheDocument(); + }); + }); +});