From 707b2f400f3f21a39e19ceff5dafee69463b3bea Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Fri, 29 Sep 2023 17:59:25 +0200 Subject: [PATCH] fix: display msg if tokenId already exists (#20940) ## Explanation This PR adds displaying an error msg when the nft has been already imported. This avoids seeing the same nft twice when the user first imports tokenId(decimal value) then imports it again (hex value). * Fixes #MMASSETS-27 ## Screenshots/Screencaps ### Before Github issue : https://github.com/MetaMask/metamask-extension/issues/18957 ### After 1- Mint and import nft with id 577 ![image](https://github.com/MetaMask/metamask-extension/assets/10994169/229a2ffb-7ab0-4bed-82fc-9ff8f0a8c69e) 2- Try importing Nft again with id 577 ![image](https://github.com/MetaMask/metamask-extension/assets/10994169/16106948-2bc2-4e1a-837d-d5073417a2a9) 3- Try importing Nft again with hex value: 0x241 ![image](https://github.com/MetaMask/metamask-extension/assets/10994169/f957585d-9b0d-47be-9964-39dfe639170a) ## Manual Testing Steps ## Pre-merge author checklist - [ ] I've clearly explained: - [x ] What problem this PR is solving - [ x] How this problem was solved - [x ] How reviewers can test my changes - [ ] Sufficient automated test coverage has been added ## Pre-merge reviewer checklist - [ ] Manual testing (e.g. pull and build branch, run in browser, test code being changed) - [ ] PR is linked to the appropriate GitHub issue - [ ] **IF** this PR fixes a bug in the release milestone, add this PR to the release milestone --- app/_locales/en/messages.json | 3 + .../import-nfts-modal/import-nfts-modal.js | 25 ++++- ui/helpers/utils/util.js | 38 +++++++- ui/helpers/utils/util.test.js | 91 +++++++++++++++++++ 4 files changed, 154 insertions(+), 3 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 739cc7ef2495..91a2dde78727 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -2586,6 +2586,9 @@ "message": "This token is an NFT. Add on the $1", "description": "$1 is a clickable link with text defined by the 'importNFTPage' key" }, + "nftAlreadyAdded": { + "message": "NFT has already been added." + }, "nftDisclaimer": { "message": "Disclaimer: MetaMask pulls the media file from the source url. This url sometimes is changed by the marketplace the NFT was minted on." }, diff --git a/ui/components/multichain/import-nfts-modal/import-nfts-modal.js b/ui/components/multichain/import-nfts-modal/import-nfts-modal.js index 2d86856aea17..78b36e9ca2fe 100644 --- a/ui/components/multichain/import-nfts-modal/import-nfts-modal.js +++ b/ui/components/multichain/import-nfts-modal/import-nfts-modal.js @@ -52,6 +52,8 @@ import { ModalOverlay, } from '../../component-library'; import Tooltip from '../../ui/tooltip'; +import { useNftsCollections } from '../../../hooks/useNftsCollections'; +import { checkTokenIdExists } from '../../../helpers/utils/util'; export const ImportNftsModal = ({ onClose }) => { const t = useI18nContext(); @@ -67,7 +69,7 @@ export const ImportNftsModal = ({ onClose }) => { tokenId: initialTokenId, ignoreErc20Token, } = useSelector((state) => state.appState.importNftsModal); - + const existingNfts = useNftsCollections(); const [nftAddress, setNftAddress] = useState(initialTokenAddress ?? ''); const [tokenId, setTokenId] = useState(initialTokenId ?? ''); const [disabled, setDisabled] = useState(true); @@ -75,6 +77,7 @@ export const ImportNftsModal = ({ onClose }) => { const trackEvent = useContext(MetaMetricsContext); const [nftAddressValidationError, setNftAddressValidationError] = useState(null); + const [duplicateTokenIdError, setDuplicateTokenIdError] = useState(null); const handleAddNft = async () => { try { @@ -140,7 +143,23 @@ export const ImportNftsModal = ({ onClose }) => { }; const validateAndSetTokenId = (val) => { - setDisabled(!isValidHexAddress(nftAddress) || !val || isNaN(Number(val))); + setDuplicateTokenIdError(null); + // Check if tokenId is already imported + const tokenIdExists = checkTokenIdExists( + nftAddress, + val, + existingNfts.collections, + ); + if (tokenIdExists) { + setDuplicateTokenIdError(t('nftAlreadyAdded')); + } + setDisabled( + !isValidHexAddress(nftAddress) || + !val || + isNaN(Number(val)) || + tokenIdExists, + ); + setTokenId(val); }; @@ -250,6 +269,8 @@ export const ImportNftsModal = ({ onClose }) => { validateAndSetTokenId(e.target.value); setNftAddFailed(false); }} + helpText={duplicateTokenIdError} + error={duplicateTokenIdError} /> diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index 20ba3656b39d..7552a0e6e6f9 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -10,8 +10,11 @@ import bowser from 'bowser'; ///: BEGIN:ONLY_INCLUDE_IN(snaps) import { getSnapPrefix } from '@metamask/snaps-utils'; import { WALLET_SNAP_PERMISSION_KEY } from '@metamask/rpc-methods'; +// eslint-disable-next-line import/no-duplicates import { isObject } from '@metamask/utils'; ///: END:ONLY_INCLUDE_IN +// eslint-disable-next-line import/no-duplicates +import { isStrictHexString } from '@metamask/utils'; import { CHAIN_IDS, NETWORK_TYPES } from '../../../shared/constants/network'; import { toChecksumHexAddress, @@ -30,8 +33,10 @@ import { SNAPS_METADATA, } from '../../../shared/constants/snaps'; ///: END:ONLY_INCLUDE_IN - // formatData :: ( date: ) -> String +import { isEqualCaseInsensitive } from '../../../shared/modules/string-utils'; +import { hexToDecimal } from '../../../shared/modules/conversion.utils'; + export function formatDate(date, format = "M/d/y 'at' T") { if (!date) { return ''; @@ -631,3 +636,34 @@ export const getNetworkNameFromProviderType = (providerName) => { export const isAbleToExportAccount = (keyringType = '') => { return !keyringType.includes('Hardware') && !keyringType.includes('Snap'); }; + +/** + * Checks if a tokenId in Hex or decimal format already exists in an object. + * + * @param {string} address - collection address. + * @param {string} tokenId - tokenId to search for + * @param {*} obj - object to look into + * @returns {boolean} `false` if tokenId does not already exist. + */ +export const checkTokenIdExists = (address, tokenId, obj) => { + // check if input tokenId is hexadecimal + // If it is convert to decimal and compare with existing tokens + const isHex = isStrictHexString(tokenId); + let convertedTokenId = tokenId; + if (isHex) { + // Convert to decimal + convertedTokenId = hexToDecimal(tokenId); + } + + if (obj[address]) { + const value = obj[address]; + return lodash.some(value.nfts, (nft) => { + return ( + nft.address === address && + (isEqualCaseInsensitive(nft.tokenId, tokenId) || + isEqualCaseInsensitive(nft.tokenId, convertedTokenId.toString())) + ); + }); + } + return false; +}; diff --git a/ui/helpers/utils/util.test.js b/ui/helpers/utils/util.test.js index 77ac796c2d9d..652efcd9e7f2 100644 --- a/ui/helpers/utils/util.test.js +++ b/ui/helpers/utils/util.test.js @@ -929,4 +929,95 @@ describe('util', () => { expect(util.getNetworkNameFromProviderType('rpc')).toStrictEqual(''); }); }); + + describe('checkTokenIdExists()', () => { + const data = { + '0x2df920B180c58766951395c26ecF1EC2063490Fa': { + collectionName: 'Numbers', + nfts: [ + { + address: '0x2df920B180c58766951395c26ecF1EC2063490Fa', + description: 'Numbers', + favorite: false, + name: 'Numbers #132', + tokenId: '132', + }, + ], + }, + '0x2df920B180c58766951395c26ecF1EC206343334': { + collectionName: 'toto', + nfts: [ + { + address: '0x2df920B180c58766951395c26ecF1EC206343334', + description: 'toto', + favorite: false, + name: 'toto#3453', + tokenId: '3453', + }, + ], + }, + '0xf4910C763eD4e47A585E2D34baA9A4b611aE448C': { + collectionName: 'foo', + nfts: [ + { + address: '0xf4910C763eD4e47A585E2D34baA9A4b611aE448C', + description: 'foo', + favorite: false, + name: 'toto#111486581076844052489180254627234340268504869259922513413248833349282110111749', + tokenId: + '111486581076844052489180254627234340268504869259922513413248833349282110111749', + }, + ], + }, + }; + it('should return true if it exists', () => { + expect( + util.checkTokenIdExists( + '0x2df920B180c58766951395c26ecF1EC206343334', + '3453', + data, + ), + ).toBeTruthy(); + }); + + it('should return true if it exists in decimal format', () => { + expect( + util.checkTokenIdExists( + '0x2df920B180c58766951395c26ecF1EC206343334', + '0xD7D', + data, + ), + ).toBeTruthy(); + }); + + it('should return true if is exists but input is not decimal nor hex', () => { + expect( + util.checkTokenIdExists( + '0xf4910C763eD4e47A585E2D34baA9A4b611aE448C', + '111486581076844052489180254627234340268504869259922513413248833349282110111749', + data, + ), + ).toBeTruthy(); + }); + + it('should return false if it does not exists', () => { + expect( + util.checkTokenIdExists( + '0x2df920B180c58766951395c26ecF1EC206343334', + '1122', + data, + ), + ).toBeFalsy(); + }); + + it('should return false if it address does not exists', () => { + expect( + util.checkTokenIdExists( + '0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57', + '1122', + data, + ), + ).toBeFalsy(); + }); + }); });