fix: Nft chainId and current global chainId arent always the same (#3…

…0517) ## **Description** Fixes a bug, where the network badge on the top of the nft in the nft details page did not correspond to the chain it belonged to. This resulted in navigating to send screen on a network that the NFT did not belong to. This PR introduces two things: 1. Show correct NFT chain badge on NFT details screen, regardless of which network user is on 2. When navigating to the send flow for an NFT, where the network the NFT belongs to is not the globally selected chainId, change the network on behalf of the user so that the globally selected chainId is the same as the NFT being sent. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30517?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to NFT Grid 2. Navigate to an NFT details screen on Ethereum Mainnet 3. Change global network to Linea 4. Should not see the network badge change on the NFT 5. Click send 6. See that network is changed on behalf of user from Linea back to Ethereum, where the NFT belongs ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/a2620999-6ce5-4d10-91ec-2531063ee2ca ### **After** https://github.com/user-attachments/assets/06d294eb-73d6-46ba-aa75-b463d6da0a66 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/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.
MetaMask · Feb 25, 2025 · 78c5d2c · 78c5d2c
1 parent 32af047
commit 78c5d2c
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 11 deletions.
diff --git a/ui/components/app/assets/nfts/nft-details/nft-details.stories.js b/ui/components/app/assets/nfts/nft-details/nft-details.stories.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import NftDetails from './nft-details';
+import { NftDetailsComponent } from './nft-details';
 
 const nft = {
   name: 'Catnip Spicywright',
@@ -18,6 +18,8 @@ const nft = {
   },
 };
 
+const nftChainId = '0x1';
+
 export default {
   title: 'Components/App/NftsDetail',
 
@@ -28,17 +30,18 @@ export default {
   },
   args: {
     nft,
+    nftChainId,
   },
 };
 
 export const DefaultStory = (args) => {
-  return <NftDetails {...args} />;
+  return <NftDetailsComponent {...args} />;
 };
 
 DefaultStory.storyName = 'Default';
 
 export const NoImage = (args) => {
-  return <NftDetails {...args} />;
+  return <NftDetailsComponent {...args} />;
 };
 
 NoImage.args = {

diff --git a/ui/components/app/assets/nfts/nft-details/nft-details.test.js b/ui/components/app/assets/nfts/nft-details/nft-details.test.js
@@ -1,5 +1,6 @@
 import { fireEvent, waitFor } from '@testing-library/react';
 import React from 'react';
+import { useParams } from 'react-router-dom';
 import configureMockStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
 import copyToClipboard from 'copy-to-clipboard';
@@ -39,6 +40,7 @@ jest.mock('react-router-dom', () => ({
   useHistory: () => ({
     push: mockHistoryPush,
   }),
+  useParams: jest.fn(),
 }));
 
 jest.mock('../../../../../ducks/send/index.js', () => ({
@@ -72,6 +74,7 @@ describe('NFT Details', () => {
   });
 
   it('should match minimal props and state snapshot', async () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.GOERLI });
     getAssetImageURL.mockResolvedValue(
       'https://bafybeiclzx7zfjvuiuwobn5ip3ogc236bjqfjzoblumf4pau4ep6dqramu.ipfs.dweb.link',
     );
@@ -88,6 +91,7 @@ describe('NFT Details', () => {
   });
 
   it(`should route to '/' route when the back button is clicked`, () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const { queryByTestId } = renderWithProvider(
       <NftDetails {...props} />,
       mockStore,
@@ -101,6 +105,7 @@ describe('NFT Details', () => {
   });
 
   it(`should call removeAndIgnoreNFT with proper nft details and route to '/' when removing nft`, async () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const { queryByTestId } = renderWithProvider(
       <NftDetails {...props} />,
       mockStore,
@@ -122,6 +127,7 @@ describe('NFT Details', () => {
   });
 
   it(`should call setRemoveNftMessage with error when removeAndIgnoreNft fails and route to '/'`, async () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const { queryByTestId } = renderWithProvider(
       <NftDetails {...props} />,
       mockStore,
@@ -145,6 +151,7 @@ describe('NFT Details', () => {
   });
 
   it('should copy nft address', async () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const { queryByTestId } = renderWithProvider(
       <NftDetails {...props} />,
       mockStore,
@@ -157,6 +164,7 @@ describe('NFT Details', () => {
   });
 
   it('should navigate to draft transaction send route with ERC721 data', async () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const nftProps = {
       nft: nfts[5],
     };
@@ -180,6 +188,7 @@ describe('NFT Details', () => {
   });
 
   it('should not render send button if isCurrentlyOwned is false', () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const sixthNftProps = {
       nft: nfts[6],
     };
@@ -195,6 +204,7 @@ describe('NFT Details', () => {
   });
 
   it('should render send button if it is an ERC1155', () => {
+    useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
     const nftProps = {
       nft: nfts[1],
     };
@@ -211,6 +221,7 @@ describe('NFT Details', () => {
 
   describe(`Alternative Networks' OpenSea Links`, () => {
     it('should open opeasea link with goeli testnet chainId', async () => {
+      useParams.mockReturnValue({ chainId: CHAIN_IDS.GOERLI });
       global.platform = { openTab: jest.fn() };
 
       const { queryByTestId } = renderWithProvider(
@@ -234,6 +245,7 @@ describe('NFT Details', () => {
     });
 
     it('should open tab to mainnet opensea url with nft info', async () => {
+      useParams.mockReturnValue({ chainId: CHAIN_IDS.MAINNET });
       global.platform = { openTab: jest.fn() };
 
       const mainnetState = {
@@ -266,11 +278,15 @@ describe('NFT Details', () => {
     });
 
     it('should open tab to polygon opensea url with nft info', async () => {
+      useParams.mockReturnValue({ chainId: CHAIN_IDS.POLYGON });
       const polygonState = {
         ...mockState,
         metamask: {
           ...mockState.metamask,
-          ...mockNetworkState({ chainId: CHAIN_IDS.POLYGON }),
+          ...mockNetworkState({
+            chainId: CHAIN_IDS.POLYGON,
+            nickname: 'polygon',
+          }),
         },
       };
       const polygonMockStore = configureMockStore([thunk])(polygonState);
@@ -296,11 +312,15 @@ describe('NFT Details', () => {
     });
 
     it('should open tab to sepolia opensea url with nft info', async () => {
+      useParams.mockReturnValue({ chainId: CHAIN_IDS.SEPOLIA });
       const sepoliaState = {
         ...mockState,
         metamask: {
           ...mockState.metamask,
-          ...mockNetworkState({ chainId: CHAIN_IDS.SEPOLIA }),
+          ...mockNetworkState({
+            chainId: CHAIN_IDS.SEPOLIA,
+            nickname: 'sepolia',
+          }),
         },
       };
       const sepoliaMockStore = configureMockStore([thunk])(sepoliaState);
@@ -326,6 +346,7 @@ describe('NFT Details', () => {
     });
 
     it('should not render opensea redirect button', async () => {
+      useParams.mockReturnValue({ chainId: '0x99' });
       const randomNetworkState = {
         ...mockState,
         metamask: {

diff --git a/ui/components/app/assets/nfts/nft-details/nft-details.tsx b/ui/components/app/assets/nfts/nft-details/nft-details.tsx
@@ -1,9 +1,10 @@
 import React, { useEffect, useContext } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
-import { useHistory } from 'react-router-dom';
+import { useHistory, useParams } from 'react-router-dom';
 import { isEqual } from 'lodash';
 import { getTokenTrackerLink, getAccountLink } from '@metamask/etherscan-link';
 import { Nft } from '@metamask/assets-controllers';
+import { Hex } from '@metamask/utils';
 import {
   TextColor,
   IconColor,
@@ -19,8 +20,15 @@ import {
 import { useI18nContext } from '../../../../../hooks/useI18nContext';
 import { shortenAddress } from '../../../../../helpers/utils/util';
 import { getNftImageAlt } from '../../../../../helpers/utils/nfts';
-import { getCurrentChainId } from '../../../../../../shared/modules/selectors/networks';
-import { getCurrentNetwork, getIpfsGateway } from '../../../../../selectors';
+import {
+  getCurrentChainId,
+  getNetworkConfigurationsByChainId,
+} from '../../../../../../shared/modules/selectors/networks';
+import {
+  getCurrentNetwork,
+  getIpfsGateway,
+  getNetworkConfigurationIdByChainId,
+} from '../../../../../selectors';
 import {
   ASSET_ROUTE,
   DEFAULT_ROUTE,
@@ -31,6 +39,8 @@ import {
   removeAndIgnoreNft,
   setRemoveNftMessage,
   setNewNftAddedMessage,
+  setActiveNetworkWithError,
+  setSwitchedNetworkDetails,
 } from '../../../../../store/actions';
 import { CHAIN_IDS } from '../../../../../../shared/constants/network';
 import NftOptions from '../nft-options/nft-options';
@@ -71,13 +81,20 @@ import { Numeric } from '../../../../../../shared/modules/Numeric';
 // eslint-disable-next-line import/no-restricted-paths
 import { addUrlProtocolPrefix } from '../../../../../../app/scripts/lib/util';
 import useGetAssetImageUrl from '../../../../../hooks/useGetAssetImageUrl';
+import { getImageForChainId } from '../../../../../selectors/multichain';
 import NftDetailInformationRow from './nft-detail-information-row';
 import NftDetailInformationFrame from './nft-detail-information-frame';
 import NftDetailDescription from './nft-detail-description';
 
 const MAX_TOKEN_ID_LENGTH = 15;
 
-export default function NftDetails({ nft }: { nft: Nft }) {
+export function NftDetailsComponent({
+  nft,
+  nftChainId,
+}: {
+  nft: Nft;
+  nftChainId: string;
+}) {
   const {
     image,
     imageOriginal,
@@ -104,6 +121,14 @@ export default function NftDetails({ nft }: { nft: Nft }) {
   const currency = useSelector(getCurrentCurrency);
   const selectedNativeConversionRate = useSelector(getConversionRate);
 
+  const nftNetworkConfigs = useSelector(getNetworkConfigurationsByChainId);
+  const nftChainNetwork = nftNetworkConfigs[nftChainId as Hex];
+  const nftChainImage = getImageForChainId(nftChainId as string);
+  const networks = useSelector(getNetworkConfigurationIdByChainId) as Record<
+    string,
+    string
+  >;
+
   const [addressCopied, handleAddressCopy] = useCopyToClipboard();
 
   const nftImageAlt = getNftImageAlt(nft);
@@ -240,7 +265,28 @@ export default function NftDetails({ nft }: { nft: Nft }) {
   const sendDisabled =
     standard !== TokenStandard.ERC721 && standard !== TokenStandard.ERC1155;
 
+  const setCorrectChain = async () => {
+    // If we aren't presently on the chain of the nft, change to it
+    if (nftChainId !== currentChain.chainId) {
+      try {
+        const networkConfigurationId = networks[nftChainId as Hex];
+        await dispatch(setActiveNetworkWithError(networkConfigurationId));
+        await dispatch(
+          setSwitchedNetworkDetails({
+            networkClientId: networkConfigurationId,
+          }),
+        );
+      } catch (err) {
+        console.error(`Failed to switch chains for NFT.
+          Target chainId: ${nftChainId}, Current chainId: ${currentChain.chainId}.
+          ${err}`);
+        throw err;
+      }
+    }
+  };
+
   const onSend = async () => {
+    await setCorrectChain();
     await dispatch(
       startNewDraftTransaction({
         type: AssetType.NFT,
@@ -350,8 +396,8 @@ export default function NftDetails({ nft }: { nft: Nft }) {
             <NftItem
               src={nftItemSrc as string | undefined}
               alt={nftImageAlt}
-              networkName={currentChain.nickname ?? ''}
-              networkSrc={currentChain.rpcPrefs?.imageUrl}
+              networkName={nftChainNetwork.name ?? ''}
+              networkSrc={nftChainImage}
               isIpfsURL={isIpfsURL}
               onClick={handleImageClick}
               detailView
@@ -851,3 +897,11 @@ export default function NftDetails({ nft }: { nft: Nft }) {
     </Page>
   );
 }
+
+function NftDetails({ nft }: { nft: Nft }) {
+  const { chainId } = useParams();
+
+  return <NftDetailsComponent nft={nft} nftChainId={chainId ?? ''} />;
+}
+
+export default NftDetails;