Skip to content

Commit

Permalink
fix: display msg if tokenId already exists (#20940)
Browse files Browse the repository at this point in the history
## 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 :
#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

<!--
How should reviewers and QA manually test your changes? For instance:

- Mint nft exp id 577
- Import the nft with id 577
- Try importing again with id 577
- Try importing again with 0x241
-->

## 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
  • Loading branch information
sahar-fehri authored Sep 29, 2023
1 parent 77b3b80 commit 707b2f4
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 3 deletions.
3 changes: 3 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 23 additions & 2 deletions ui/components/multichain/import-nfts-modal/import-nfts-modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -67,14 +69,15 @@ 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);
const [nftAddFailed, setNftAddFailed] = useState(false);
const trackEvent = useContext(MetaMetricsContext);
const [nftAddressValidationError, setNftAddressValidationError] =
useState(null);
const [duplicateTokenIdError, setDuplicateTokenIdError] = useState(null);

const handleAddNft = async () => {
try {
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -250,6 +269,8 @@ export const ImportNftsModal = ({ onClose }) => {
validateAndSetTokenId(e.target.value);
setNftAddFailed(false);
}}
helpText={duplicateTokenIdError}
error={duplicateTokenIdError}
/>
</Box>
</Box>
Expand Down
38 changes: 37 additions & 1 deletion ui/helpers/utils/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -30,8 +33,10 @@ import {
SNAPS_METADATA,
} from '../../../shared/constants/snaps';
///: END:ONLY_INCLUDE_IN

// formatData :: ( date: <Unix Timestamp> ) -> 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 '';
Expand Down Expand Up @@ -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;
};
91 changes: 91 additions & 0 deletions ui/helpers/utils/util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});

0 comments on commit 707b2f4

Please sign in to comment.