fix: add updateNft fct

[sahar-fehri]

Explanation

When a user imports an NFT when the display NFT media toggle is off and IPFS toggle is off, then enables the toggles, the image of the NFT will not be displayed.

This PR adds a new fct that fetches the NFT metadata and udpates the state. This fct will be called from extension based on the toggles and if the name/description/image attributes are null

References

• Related to extension PR #67890

Changelog

@metamask/assets-controllers

• ADDED: Added new fct updateNftMetadata that fetches nft metadata and updates the state

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

[MajorLift]

Hi great work here! I had a few suggestions for improving typing.

[MajorLift]

This type assertion is avoidable by using a type guard as the predicate for the filter operation.

This allows the compiler to narrow down to PromiseFulfilledResult at the type level.

Suggested change

	// lib.es2020.promise.d.ts does not export its types so we're using a simple type.
	const success = nftMetadataResults.filter(
	(promise) => promise.status === 'fulfilled',
	) as { status: 'fulfilled'; value: NftUpdate }[];
	const success = nftMetadataResults.filter(
	(result): result is PromiseFulfilledResult<NftUpdate> =>
	result.status === 'fulfilled',
	);

[sahar-fehri]

That's nice thnx! 🙏

[MajorLift]

any is unnecessary here since the compiler tells us exactly what the as assertion target type should be

Argument of type '"getNftInformation"' is not assignable to parameter of type 'keyof NftController'.ts(2345)

Suggested change

	// TODO: Replace `any` with type
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	.stub(nftController, 'getNftInformation' as any)
	.stub(nftController, 'getNftInformation' as keyof typeof nftController)

In general, shared libraries is trying to avoid introducing new any types except under a few very limited circumstances. An as assertion even to an approximate type is always preferred.

[MajorLift]

Nit: Type annotations should be avoided unless they narrow an inferred type. There's an entry in the new TypeScript guidelines that covers this topic.

Suggested change

	const nftsWithChecksumAdr = nfts.map((nft: Nft) => {
	const nftsWithChecksumAdr = nfts.map((nft) => {

[MajorLift]

(Follow-up from #4008 (comment))

Nit: This could be refactored to be more concise.

I also prefer forEach here instead of map to prevent confusion, since the return value is not being used and the updateNft operation relies on side effects.

Suggested change

	// lib.es2020.promise.d.ts does not export its types so we're using a simple type.
	const success = nftMetadataResults.filter(
	(promise) => promise.status === 'fulfilled',
	) as { status: 'fulfilled'; value: NftUpdate }[];
	if (success.length !== 0) {
	success.map((elm) =>
	this.updateNft(
	elm.value.nft,
	elm.value.newMetadata,
	userAddress,
	chainId,
	),
	);
	}
	nftMetadataResults
	.filter(
	(result): result is PromiseFulfilledResult<NftUpdate> =>
	result.status === 'fulfilled',
	)
	.forEach((elm) =>
	this.updateNft(
	elm.value.nft,
	elm.value.newMetadata,
	userAddress,
	chainId,
	),
	);

[MajorLift]

LGTM!