fix: remove unecessary calls to third party apis

[sahar-fehri]

Description

The CollectibleContracts component was making more calls to the NFT API than it should.

Users could either have the nft detection enabled or disabled.

When the nft detection is enabled, the component was:
1- Executing detectNfts function which triggers one call to get all nfts for a user
2- Calling updateNftMetadata which triggers N number of calls to the NFT-API assuming the user has N NFT.

The second call is unnecessary if the user already have the nftDetection enabled.

In this fix, i added a check in the useEffect to see if the user has the nftDetection enabled or not

    if (updatableCollectibles.length !== 0 && !useNftDetection) {
      updateAllCollectibleMetadata(updatableCollectibles);
    }

If it is already enabled, calling the detectNfts function should be enough to refresh metadata.

When the user has the nftDetection Disabled, in this case the updateNftMetadata will be executed.
I made an update in the updateNftMetadata fct to not execute the calls every time the user pulls down t refresh. A 10 mins interval should be enough.

Note: We will have an update on the nftDetectionController in the future to update the polling strategy.

Related issues

• Compare url on core: https://github.com/MetaMask/core/compare/patch/mobile-assets-controllers-26...patch/mobile-assets-controllers-26-optimize-nft-refresh-calls?expand=1
• fix: avoid multiple calls to refresh nft metadata core#4325

Manual testing steps

To check the unecessary calls:

1. Build on main
2. Go to app/components/UI/CollectibleContracts/index.js and add a log inside useEffect
3. Go to node-modules, nftDetectionController.js function getOwnerNfts and add a console.log
4. Go to node-modules, nftController.js function getNftInformationFromApi and add a log
5. When you have nft detection disabled, go to nft tab, and pull down to refresh and check the calls made in your terminal
6. Enable nft detection, go to nft tab, pull down to refresh and check your logs

(See before video, notice on that wallet i have 5 nfts, it should be making only 1 call per nft => 5 calls in total but its making more than that. Every state update will result in the useEffect being executed which we attempt to avoid in this fix)

To check the update on this PR:

1- Build on this branch
2. Go to app/components/UI/CollectibleContracts/index.js and add a log inside useEffect
3. Go to node-modules, nftDetectionController.js function getOwnerNfts and add a console.log
4. Go to node-modules, nftController.js function getNftInformationFromApi and add a log
5. When you have nft detection disabled, go to nft tab, and pull down to refresh and check the calls made in your terminal
6. Enable nft detection, go to nft tab, pull down to refresh and check your logs

Screenshots/Recordings

Before

before4.mov

After

after.mov

Pre-merge author checklist

• I’ve followed MetaMask Coding Standards.
• 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 format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). 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.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[legobeat]

Great catch!

1. The patches/@metamask+assets-controllers+26.0.0.patch diff is big. Do you have a reference for the actual intended commit(s) on https://github.com/MetaMask/core?
2. Would it be possible to construct a test or two to verify correct behavior and prevent future regressions?

[github-actions]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 32e4d97
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/835e83ad-b480-4e6d-b617-26a5753e898d

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[sahar-fehri]

Great catch!

1. The patches/@metamask+assets-controllers+26.0.0.patch diff is big. Do you have a reference for the actual intended commit(s) on https://github.com/MetaMask/core?
2. Would it be possible to construct a test or two to verify correct behavior and prevent future regressions?

Hey @legobeat ! I have this core PR for these updates MetaMask/core#4325
Will work on the unit tests 🙏

[github-actions]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 01c07f3
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cf6f97fb-f62f-42ac-b5ad-1ba87edcc571

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 1256597
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/4015f973-f6ae-426f-9303-955f3edc83dd

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 8458ea6
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/29a12e7e-db17-46d0-8685-e89b264e563b

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0550366
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/18bc5b18-584c-40d3-aeb4-2727b1f2c629

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 131fbe8
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d6e13f21-5c89-4931-b56a-0b5d10fbb4ee

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[github-actions]

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 3d201ce
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/de74d422-431f-496d-8075-d2f4fa4ffe7b

Note

• You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[tommasini]

LGTM!