Skip to content

Commit b4af134

Browse files
sahar-fehrisahar-fehri
authored
fix: fix update nft metadata when toggles off (#4096)
## Explanation This PR fixes a bug on extension, where when a user imports an NFT when IPFS and display media toggles are off, then enables them, the metadata wont be refreshed. We added a check in NftController to see if any of the toggles are on then proceed to update nft metadata. We will update the metadata only for NFTs that are missing name/image or description. ### **Before** https://github.com/MetaMask/metamask-extension/assets/10994169/c0d09b77-1cd2-495e-aec8-a4ba965ed38f ### **After** https://github.com/MetaMask/metamask-extension/assets/10994169/06e56b4f-5c1b-4402-b6e2-154daff291d5 ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/assets-controllers` - **ADDED**: Added a check inside `onPreferencesStateChange` to check if IPFS toggle or display media toggle is ON. If so i update the users nft metadata. - **REMOVED**: Removed Nfts argument from `updateNftMetadata` function. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate
1 parent 0e0ee52 commit b4af134

File tree

2 files changed

+147
-8
lines changed

2 files changed

+147
-8
lines changed

‎packages/assets-controllers/src/NftController.test.ts‎

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3408,6 +3408,7 @@ describe('NftController', () => {
34083408
nftMetadata: { name: '', description: '', image: '', standard: '' },
34093409
networkClientId: testNetworkClientId,
34103410
});
3411+
34113412
sinon
34123413
.stub(nftController, 'getNftInformation' as keyof typeof nftController)
34133414
.returns({
@@ -3464,6 +3465,7 @@ describe('NftController', () => {
34643465
},
34653466
networkClientId: testNetworkClientId,
34663467
});
3468+
34673469
sinon
34683470
.stub(nftController, 'getNftInformation' as keyof typeof nftController)
34693471
.rejects(new Error('Error'));
@@ -3500,7 +3502,7 @@ describe('NftController', () => {
35003502
});
35013503
});
35023504

3503-
it('should update metadata when some calls to fetch metadata succeed', async () => {
3505+
it('should update metadata when some calls to fetch metadata succeed', async () => {
35043506
const { nftController } = setupController();
35053507
const { selectedAddress } = nftController.config;
35063508
const spy = jest.spyOn(nftController, 'updateNft');
@@ -3515,6 +3517,7 @@ describe('NftController', () => {
35153517
},
35163518
networkClientId: testNetworkClientId,
35173519
});
3520+
35183521
await nftController.addNft('0xtest2', '2', {
35193522
nftMetadata: {
35203523
name: '',
@@ -3524,6 +3527,7 @@ describe('NftController', () => {
35243527
},
35253528
networkClientId: testNetworkClientId,
35263529
});
3530+
35273531
await nftController.addNft('0xtest3', '3', {
35283532
nftMetadata: {
35293533
name: '',
@@ -3533,6 +3537,7 @@ describe('NftController', () => {
35333537
},
35343538
networkClientId: testNetworkClientId,
35353539
});
3540+
35363541
sinon
35373542
.stub(nftController, 'getNftInformation' as keyof typeof nftController)
35383543
.onFirstCall()
@@ -3624,5 +3629,120 @@ describe('NftController', () => {
36243629
},
36253630
]);
36263631
});
3632+
3633+
it('should not update metadata when nfts has image/name/description already', async () => {
3634+
const { nftController, triggerPreferencesStateChange } =
3635+
setupController();
3636+
const spy = jest.spyOn(nftController, 'updateNftMetadata');
3637+
const testNetworkClientId = 'sepolia';
3638+
3639+
// Add nfts
3640+
await nftController.addNft('0xtest', '3', {
3641+
nftMetadata: {
3642+
name: 'test name',
3643+
description: 'test description',
3644+
image: 'test image',
3645+
standard: 'ERC721',
3646+
},
3647+
userAddress: OWNER_ADDRESS,
3648+
networkClientId: testNetworkClientId,
3649+
});
3650+
3651+
// trigger preference change
3652+
triggerPreferencesStateChange({
3653+
...getDefaultPreferencesState(),
3654+
isIpfsGatewayEnabled: false,
3655+
openSeaEnabled: true,
3656+
selectedAddress: OWNER_ADDRESS,
3657+
});
3658+
3659+
expect(spy).toHaveBeenCalledTimes(0);
3660+
});
3661+
3662+
it('should trigger calling updateNftMetadata when preferences change - openseaEnabled', async () => {
3663+
const { nftController, triggerPreferencesStateChange, changeNetwork } =
3664+
setupController();
3665+
changeNetwork(SEPOLIA);
3666+
const spy = jest.spyOn(nftController, 'updateNftMetadata');
3667+
3668+
const testNetworkClientId = 'sepolia';
3669+
// Add nfts
3670+
await nftController.addNft('0xtest', '1', {
3671+
nftMetadata: {
3672+
name: '',
3673+
description: '',
3674+
image: '',
3675+
standard: 'ERC721',
3676+
},
3677+
userAddress: OWNER_ADDRESS,
3678+
networkClientId: testNetworkClientId,
3679+
});
3680+
3681+
expect(
3682+
nftController.state.allNfts[OWNER_ADDRESS][SEPOLIA.chainId][0]
3683+
.isCurrentlyOwned,
3684+
).toBe(true);
3685+
3686+
sinon
3687+
.stub(nftController, 'getNftInformation' as keyof typeof nftController)
3688+
.returns({
3689+
name: 'name pudgy',
3690+
image: 'url pudgy',
3691+
description: 'description pudgy',
3692+
});
3693+
3694+
// trigger preference change
3695+
triggerPreferencesStateChange({
3696+
...getDefaultPreferencesState(),
3697+
isIpfsGatewayEnabled: false,
3698+
openSeaEnabled: true,
3699+
selectedAddress: OWNER_ADDRESS,
3700+
});
3701+
3702+
expect(spy).toHaveBeenCalledTimes(1);
3703+
});
3704+
3705+
it('should trigger calling updateNftMetadata when preferences change - ipfs enabled', async () => {
3706+
const { nftController, triggerPreferencesStateChange, changeNetwork } =
3707+
setupController();
3708+
changeNetwork(SEPOLIA);
3709+
const spy = jest.spyOn(nftController, 'updateNftMetadata');
3710+
3711+
const testNetworkClientId = 'sepolia';
3712+
// Add nfts
3713+
await nftController.addNft('0xtest', '1', {
3714+
nftMetadata: {
3715+
name: '',
3716+
description: '',
3717+
image: '',
3718+
standard: 'ERC721',
3719+
},
3720+
userAddress: OWNER_ADDRESS,
3721+
networkClientId: testNetworkClientId,
3722+
});
3723+
3724+
expect(
3725+
nftController.state.allNfts[OWNER_ADDRESS][SEPOLIA.chainId][0]
3726+
.isCurrentlyOwned,
3727+
).toBe(true);
3728+
3729+
sinon
3730+
.stub(nftController, 'getNftInformation' as keyof typeof nftController)
3731+
.returns({
3732+
name: 'name pudgy',
3733+
image: 'url pudgy',
3734+
description: 'description pudgy',
3735+
});
3736+
3737+
// trigger preference change
3738+
triggerPreferencesStateChange({
3739+
...getDefaultPreferencesState(),
3740+
isIpfsGatewayEnabled: true,
3741+
openSeaEnabled: false,
3742+
selectedAddress: OWNER_ADDRESS,
3743+
});
3744+
3745+
expect(spy).toHaveBeenCalledTimes(1);
3746+
});
36273747
});
36283748
});

‎packages/assets-controllers/src/NftController.ts‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,6 @@ export class NftController extends BaseControllerV1<NftConfig, NftState> {
279279
...oldState,
280280
...{ [userAddress]: newAddressState },
281281
};
282-
283282
this.update({
284283
[baseStateKey]: newState,
285284
});
@@ -1041,7 +1040,7 @@ export class NftController extends BaseControllerV1<NftConfig, NftState> {
10411040
this.messagingSystem = messenger;
10421041

10431042
onPreferencesStateChange(
1044-
({
1043+
async ({
10451044
selectedAddress,
10461045
ipfsGateway,
10471046
openSeaEnabled,
@@ -1053,6 +1052,26 @@ export class NftController extends BaseControllerV1<NftConfig, NftState> {
10531052
openSeaEnabled,
10541053
isIpfsGatewayEnabled,
10551054
});
1055+
1056+
const needsUpdateNftMetadata =
1057+
(isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled;
1058+
1059+
if (needsUpdateNftMetadata) {
1060+
const { chainId } = this.config;
1061+
const nfts: Nft[] =
1062+
this.state.allNfts[selectedAddress]?.[chainId] ?? [];
1063+
// filter only nfts
1064+
const nftsToUpdate = nfts.filter(
1065+
(singleNft) =>
1066+
!singleNft.name && !singleNft.description && !singleNft.image,
1067+
);
1068+
if (nftsToUpdate.length !== 0) {
1069+
await this.updateNftMetadata({
1070+
nfts: nftsToUpdate,
1071+
userAddress: selectedAddress,
1072+
});
1073+
}
1074+
}
10561075
},
10571076
);
10581077

@@ -1363,20 +1382,21 @@ export class NftController extends BaseControllerV1<NftConfig, NftState> {
13631382
* Refetches NFT metadata and updates the state
13641383
*
13651384
* @param options - Options for refetching NFT metadata
1366-
* @param options.nfts - Array of nfts
1367-
* @param options.networkClientId - The networkClientId that can be used to identify the network client to use for this request.
1385+
* @param options.nfts - nfts to update metadata for.
13681386
* @param options.userAddress - The current user address
1387+
* @param options.networkClientId - The networkClientId that can be used to identify the network client to use for this request.
13691388
*/
13701389
async updateNftMetadata({
13711390
nfts,
1372-
networkClientId,
13731391
userAddress = this.config.selectedAddress,
1392+
networkClientId,
13741393
}: {
13751394
nfts: Nft[];
1376-
networkClientId?: NetworkClientId;
13771395
userAddress?: string;
1396+
networkClientId?: NetworkClientId;
13781397
}) {
13791398
const chainId = this.getCorrectChainId({ networkClientId });
1399+
13801400
const nftsWithChecksumAdr = nfts.map((nft) => {
13811401
return {
13821402
...nft,
@@ -1696,7 +1716,6 @@ export class NftController extends BaseControllerV1<NftConfig, NftState> {
16961716
updatedNft,
16971717
...nfts.slice(nftInfo.index + 1),
16981718
];
1699-
17001719
this.updateNestedNftState(newNfts, ALL_NFTS_STATE_KEY, {
17011720
chainId,
17021721
userAddress: selectedAddress,

0 commit comments

Comments
 (0)