-
-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: fix update nft metadata when toggles off #4096
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 tasks
sahar-fehri
commented
Mar 21, 2024
mcmire
reviewed
Mar 22, 2024
bergeron
reviewed
Mar 26, 2024
bergeron
approved these changes
Mar 26, 2024
mcmire
approved these changes
Mar 26, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
montelaidev
added a commit
that referenced
this pull request
Apr 17, 2024
## Explanation ## References ## Changelog ## [13.0.0] Accounts Controller ### Changed - Fix update setSelectedAccount to throw if the id is not found ([#4167](#4167)) - Fix normal account indexing naming with index gap ([#4089](#4089)) - **BREAKING** Bump peer dependency `@metamask/snaps-controllers` to `^6.0.3` and dependencies `@metamask/snaps-sdk` to `^3.1.1`, `@metamask/eth-snap-keyring` to `^3.0.0`([#4090](#4090)) ## [28.0.0] Assets Controller ### Added - Add reservoir migration ([#4030](#4030)) ### Changed - Fix getting nft tokenURI ([#4136](#4136)) - **BREAKING** Bump peer dependency on `@metamask/keyring-controller` ([#4090](#4090)) - Fix token detection during account change ([#4133](#4133)) - Fix update nft metadata when toggles off ([#4096](#4096)) - Adds `tokenMethodIncreaseAllowance` ([#4069](#4069)) - Fix mantle token mispriced ([#4045](#4045)) ## [15.0.0] Keyring Controller ### Changed - **BREAKING** use getAccounts on HD Keyring when calling addNewAccount ([#4158](#4158)) - Pass CAIP-2 scope to execution context ([#4090](#4090)) - Allow gas limits to be changed during #addPaymasterData ([#3942](#3942)) ## [10.0.0] Preferences Controller ### Changed - **BREAKING** Bump peer dependency on `@metamask/keyring-controller` to `^15.0.0` ([#4090](#4090)) - Restore previous behavior of toChecksumHexAddress ([#4046](#4046)) ## [15.0.0] Signature Controller ### Changed - **BREAKING** Bump peer dependency on `@metamask/keyring-controller` to `^15.0.0` ([#4090](#4090)) ## [8.0.0] User Operation Controller ### Changed - **BREAKING** Bump peer dependency on `@metamask/keyring-controller` to `^15.0.0` and Pass CAIP-2 scope to execution context ([#4090](#4090)) - Allow gas limits to be changed during #addPaymasterData ([#3942](#3942)) ## 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Screen.Recording.2024-03-04.at.14.31.31.mov
After
videoAfter.mov
References
Changelog
@metamask/assets-controllers
onPreferencesStateChange
to check if IPFS toggle or display media toggle is ON. If so i update the users nft metadata.updateNftMetadata
function.Checklist