Skip to content

Commit

Permalink
Fix tokenIds larger than MAX_SAFE_INTEGER converted to scientific not…
Browse files Browse the repository at this point in the history
…ation and failing to import (#15016)

* fix tokenIds larger than MAX_SAFE_INTEGER breaking

* add tests
  • Loading branch information
adonesky1 authored Jun 28, 2022
1 parent bf9140a commit 5290402
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 8 deletions.
12 changes: 4 additions & 8 deletions ui/pages/add-collectible/add-collectible.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { useDispatch, useSelector } from 'react-redux';
import { util } from '@metamask/controllers';
import { useI18nContext } from '../../hooks/useI18nContext';
import { DEFAULT_ROUTE } from '../../helpers/constants/routes';

import {
DISPLAY,
FONT_WEIGHT,
Expand Down Expand Up @@ -55,9 +54,7 @@ export default function AddCollectible() {

const handleAddCollectible = async () => {
try {
await dispatch(
addCollectibleVerifyOwnership(address, tokenId.toString()),
);
await dispatch(addCollectibleVerifyOwnership(address, tokenId));
} catch (error) {
const { message } = error;
dispatch(setNewCollectibleAddedMessage(message));
Expand Down Expand Up @@ -99,7 +96,7 @@ export default function AddCollectible() {
};

const validateAndSetTokenId = (val) => {
setDisabled(!util.isValidHexAddress(address) || !val);
setDisabled(!util.isValidHexAddress(address) || !val || isNaN(Number(val)));
setTokenId(val);
};

Expand Down Expand Up @@ -149,7 +146,7 @@ export default function AddCollectible() {
)}
<Box margin={4}>
<FormField
id="address"
dataTestId="address"
titleText={t('address')}
placeholder="0x..."
value={address}
Expand All @@ -161,7 +158,7 @@ export default function AddCollectible() {
autoFocus
/>
<FormField
id="token-id"
dataTestId="token-id"
titleText={t('tokenId')}
placeholder={t('nftTokenIdPlaceholder')}
value={tokenId}
Expand All @@ -170,7 +167,6 @@ export default function AddCollectible() {
setCollectibleAddFailed(false);
}}
tooltipText={t('importNFTTokenIdToolTip')}
numeric
/>
</Box>
</Box>
Expand Down
79 changes: 79 additions & 0 deletions ui/pages/add-collectible/add-collectible.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import React from 'react';
import { fireEvent } from '@testing-library/react';
import configureMockStore from 'redux-mock-store';
import { renderWithProvider } from '../../../test/jest/rendering';
import * as Actions from '../../store/actions';
import AddCollectible from '.';

const VALID_ADDRESS = '0x312BE6a98441F9F6e3F6246B13CA19701e0AC3B9';
const INVALID_ADDRESS = 'aoinsafasdfa';
const VALID_TOKENID = '1201';
const INVALID_TOKENID = 'abcde';

describe('AddCollectible', () => {
const store = configureMockStore([])({
metamask: { provider: { chainId: '0x1' } },
});

it('should enable the "Add" button when valid entries are input into both Address and TokenId fields', () => {
const { getByTestId, getByText } = renderWithProvider(
<AddCollectible />,
store,
);
expect(getByText('Add')).not.toBeEnabled();
fireEvent.change(getByTestId('address'), {
target: { value: VALID_ADDRESS },
});
fireEvent.change(getByTestId('token-id'), {
target: { value: VALID_TOKENID },
});
expect(getByText('Add')).toBeEnabled();
});

it('should not enable the "Add" button when an invalid entry is input into one or both Address and TokenId fields', () => {
const { getByTestId, getByText } = renderWithProvider(
<AddCollectible />,
store,
);
expect(getByText('Add')).not.toBeEnabled();
fireEvent.change(getByTestId('address'), {
target: { value: INVALID_ADDRESS },
});
fireEvent.change(getByTestId('token-id'), {
target: { value: VALID_TOKENID },
});
expect(getByText('Add')).not.toBeEnabled();
fireEvent.change(getByTestId('address'), {
target: { value: VALID_ADDRESS },
});
expect(getByText('Add')).toBeEnabled();
fireEvent.change(getByTestId('token-id'), {
target: { value: INVALID_TOKENID },
});
expect(getByText('Add')).not.toBeEnabled();
});

it('should call addCollectibleVerifyOwnership action with correct values (tokenId should not be in scientific notation)', () => {
const { getByTestId, getByText } = renderWithProvider(
<AddCollectible />,
store,
);
fireEvent.change(getByTestId('address'), {
target: { value: VALID_ADDRESS },
});
const LARGE_TOKEN_ID = Number.MAX_SAFE_INTEGER + 1;
fireEvent.change(getByTestId('token-id'), {
target: { value: LARGE_TOKEN_ID },
});
const addCollectibleVerifyOwnershipSpy = jest.spyOn(
Actions,
'addCollectibleVerifyOwnership',
);

fireEvent.click(getByText('Add'));
expect(addCollectibleVerifyOwnershipSpy).toHaveBeenCalledWith(
'0x312BE6a98441F9F6e3F6246B13CA19701e0AC3B9',
'9007199254740992',
);
});
});

0 comments on commit 5290402

Please sign in to comment.