Skip to content

Commit e186ccc

Browse files
fix: cp-12.17.0 Fix remove nft (#32102)
## **Description** PR to fix removal of NFT on a network different than the nft network. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/32102?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Make sure you have NFTs on two different networks exp (linea and ethereum) 2. Select Ethereum network 3. Go to NFTs tab 4. Remove and NFT on linea 5. It should be removed successfully ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] 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-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. --------- Co-authored-by: Nicholas Gambino <nicholas.gambino@consensys.net>
1 parent e4bd6ee commit e186ccc

File tree

5 files changed

+68
-2
lines changed

5 files changed

+68
-2
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111
- [Beta] Create Solana account automatically on wallet creation or SRP import [#32038](https://github.com/MetaMask/metamask-extension/pull/32038)
1212
- Support for Solana on Firefox ([#32104](https://github.com/MetaMask/metamask-extension/pull/32104))
1313

14+
### Fixed
15+
- Fix NFT removal on different networks ([#32102](https://github.com/MetaMask/metamask-extension/pull/32102))
16+
1417
## [12.15.2]
1518
### Added
1619
- Add icon image for Lens network ([#31638](https://github.com/MetaMask/metamask-extension/pull/31638))

test/e2e/tests/tokens/nft/remove-erc1155.spec.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import Homepage from '../../../page-objects/pages/home/homepage';
66
import NFTDetailsPage from '../../../page-objects/pages/nft-details-page';
77
import NftListPage from '../../../page-objects/pages/home/nft-list';
88
import { loginWithBalanceValidation } from '../../../page-objects/flows/login.flow';
9+
import { setupAutoDetectMocking } from './mocks';
10+
import SettingsPage from '../../../page-objects/pages/settings/settings-page';
11+
import HeaderNavbar from '../../../page-objects/pages/header-navbar';
12+
import PrivacySettings from '../../../page-objects/pages/settings/privacy-settings';
913

1014
async function mockIPFSRequest(mockServer: MockttpServer) {
1115
return [
@@ -47,4 +51,53 @@ describe('Remove ERC1155 NFT', function () {
4751
},
4852
);
4953
});
54+
55+
it('user should be able to remove an NFT while selected network is different than NFT network', async function () {
56+
const driverOptions = { mock: true };
57+
await withFixtures(
58+
{
59+
fixtures: new FixtureBuilder().withNetworkControllerOnLinea().build(),
60+
driverOptions,
61+
title: this.test?.fullTitle(),
62+
testSpecificMock: setupAutoDetectMocking,
63+
},
64+
async ({ driver }) => {
65+
await loginWithBalanceValidation(driver);
66+
67+
// navigate to security & privacy settings and toggle on NFT autodetection
68+
await new HeaderNavbar(driver).openSettingsPage();
69+
const settingsPage = new SettingsPage(driver);
70+
await settingsPage.check_pageIsLoaded();
71+
await settingsPage.goToPrivacySettings();
72+
73+
const privacySettings = new PrivacySettings(driver);
74+
await privacySettings.check_pageIsLoaded();
75+
await privacySettings.toggleAutodetectNft();
76+
await settingsPage.closeSettingsPage();
77+
78+
// check that nft is displayed
79+
const homepage = new Homepage(driver);
80+
await homepage.check_pageIsLoaded();
81+
await homepage.check_expectedBalanceIsDisplayed();
82+
await homepage.goToNftTab();
83+
const nftListPage = new NftListPage(driver);
84+
await nftListPage.check_nftNameIsDisplayed(
85+
'ENS: Ethereum Name Service',
86+
);
87+
await nftListPage.check_nftImageIsDisplayed();
88+
await nftListPage.clickNFTIconOnActivityList();
89+
90+
const nftDetailsPage = new NFTDetailsPage(driver);
91+
await nftDetailsPage.check_pageIsLoaded();
92+
93+
await nftDetailsPage.removeNFT();
94+
await driver.delay(5000);
95+
96+
// Remove NFT
97+
await nftListPage.check_successRemoveNftMessageIsDisplayed();
98+
await nftListPage.check_noNftInfoIsDisplayed();
99+
await driver.delay(5000);
100+
},
101+
);
102+
});
50103
});

ui/components/app/assets/nfts/nft-details/nft-details.test.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ describe('NFT Details', () => {
121121
await expect(removeAndIgnoreNft).toHaveBeenCalledWith(
122122
nfts[5].address,
123123
nfts[5].tokenId,
124+
'testNetworkConfigurationId',
124125
);
125126
expect(setRemoveNftMessage).toHaveBeenCalledWith('success');
126127
expect(mockHistoryPush).toHaveBeenCalledWith(DEFAULT_ROUTE);
@@ -145,6 +146,7 @@ describe('NFT Details', () => {
145146
await expect(removeAndIgnoreNft).toHaveBeenCalledWith(
146147
nfts[5].address,
147148
nfts[5].tokenId,
149+
'testNetworkConfigurationId',
148150
);
149151
expect(setRemoveNftMessage).toHaveBeenCalledWith('error');
150152
expect(mockHistoryPush).toHaveBeenCalledWith(DEFAULT_ROUTE);

ui/components/app/assets/nfts/nft-details/nft-details.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ export function NftDetailsComponent({
130130
const nftNetworkConfigs = useSelector(getNetworkConfigurationsByChainId);
131131
const nftChainNetwork = nftNetworkConfigs[nftChainId as Hex];
132132
const nftChainImage = getImageForChainId(nftChainId as string);
133+
const nftNetworkClientId =
134+
nftChainNetwork?.rpcEndpoints?.[nftChainNetwork?.defaultRpcEndpointIndex]
135+
.networkClientId;
133136
const networks = useSelector(getNetworkConfigurationIdByChainId) as Record<
134137
string,
135138
string
@@ -225,7 +228,7 @@ export function NftDetailsComponent({
225228
const onRemove = async () => {
226229
let isSuccessfulEvent = false;
227230
try {
228-
await dispatch(removeAndIgnoreNft(address, tokenId));
231+
await dispatch(removeAndIgnoreNft(address, tokenId, nftNetworkClientId));
229232
dispatch(setNewNftAddedMessage(''));
230233
dispatch(setRemoveNftMessage('success'));
231234
isSuccessfulEvent = true;

ui/store/actions.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2217,6 +2217,7 @@ export function addNftVerifyOwnership(
22172217
export function removeAndIgnoreNft(
22182218
address: string,
22192219
tokenID: string,
2220+
networkClientId: string,
22202221
shouldShowLoadingIndicator?: boolean,
22212222
): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
22222223
return async (dispatch: MetaMaskReduxDispatch) => {
@@ -2230,7 +2231,11 @@ export function removeAndIgnoreNft(
22302231
dispatch(showLoadingIndication());
22312232
}
22322233
try {
2233-
await submitRequestToBackground('removeAndIgnoreNft', [address, tokenID]);
2234+
await submitRequestToBackground('removeAndIgnoreNft', [
2235+
address,
2236+
tokenID,
2237+
{ networkClientId },
2238+
]);
22342239
} catch (error) {
22352240
logErrorWithMessage(error);
22362241
dispatch(displayWarning(error));

0 commit comments

Comments
 (0)