From 5b1b5dc03bfbc61fa22b72e9c9b387d7ecae9e03 Mon Sep 17 00:00:00 2001 From: David Walsh Date: Mon, 13 Mar 2023 14:29:37 -0500 Subject: [PATCH] NFTs: Remove feature flag for release (#17401) * NFTs: Remove feature flag for release * Update security tab jest test * Fix broken test * Update snapshot * Update test * Fix test * Remove last usages of flag * Update CI jobs * Fix jest tests --- .circleci/config.yml | 107 ------------ .metamaskrc.dist | 1 - app/scripts/metamask-controller.js | 58 +++---- development/build/config.js | 1 - development/build/scripts.js | 1 - package.json | 1 - .../app/wallet-overview/token-overview.js | 2 +- .../wallet-overview/token-overview.test.js | 4 - ui/ducks/send/send.js | 2 +- ui/ducks/send/send.test.js | 2 - ui/helpers/constants/settings.js | 2 - ui/helpers/utils/settings-search.test.js | 2 +- ui/pages/home/home.component.js | 28 ++- .../import-token/import-token.component.js | 3 +- ui/pages/import-token/import-token.test.js | 1 - ui/pages/routes/routes.component.js | 4 +- .../experimental-tab.test.js.snap | 160 ++++++++++++++++++ .../experimental-tab.component.js | 2 +- .../security-tab/security-tab.test.js | 5 +- 19 files changed, 208 insertions(+), 178 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 9a9fdbdbe..f17e666e8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -69,9 +69,6 @@ workflows: - prep-build-test-mv3: requires: - prep-deps - - prep-build-test-nft: - requires: - - prep-deps - prep-build-test-flask: requires: - prep-deps @@ -106,12 +103,6 @@ workflows: - test-e2e-chrome-mv3: requires: - prep-build-test-mv3 - - test-e2e-chrome-nft: - requires: - - prep-build-test-nft - - test-e2e-firefox-nft: - requires: - - prep-build-test-nft - test-unit-mocha: requires: - prep-deps @@ -190,8 +181,6 @@ workflows: - test-mozilla-lint-flask - test-e2e-chrome - test-e2e-firefox - - test-e2e-chrome-nft - - test-e2e-firefox-nft - test-e2e-chrome-snaps - test-e2e-firefox-snaps - test-storybook @@ -214,7 +203,6 @@ workflows: - prep-build-storybook - prep-build-ts-migration-dashboard - prep-build-test-mv3 - - prep-build-test-nft - benchmark - user-actions-benchmark - stats-module-load-init @@ -496,27 +484,6 @@ jobs: - dist-test-mv3 - builds-test-mv3 - prep-build-test-nft: - executor: node-browsers-medium-plus - steps: - - checkout - - attach_workspace: - at: . - - run: - name: Build extension with nft's enabled for testing - command: yarn build:test:nft - - run: - name: Move test build to 'dist-test-nft' to avoid conflict with production build - command: mv ./dist ./dist-test-nft - - run: - name: Move test zips to 'builds-test-nft' to avoid conflict with production build - command: mv ./builds ./builds-test-nft - - persist_to_workspace: - root: . - paths: - - dist-test-nft - - builds-test-nft - prep-build-test: executor: node-browsers-medium-plus steps: @@ -732,80 +699,6 @@ jobs: path: test-artifacts destination: test-artifacts - test-e2e-chrome-nft: - executor: node-browsers - parallelism: 1 - steps: - - checkout - - run: - name: Re-Install Chrome - command: ./.circleci/scripts/chrome-install.sh - - attach_workspace: - at: . - - run: - name: Move test build to dist - command: mv ./dist-test-nft ./dist - - run: - name: Move test zips to builds - command: mv ./builds-test-nft ./builds - - run: - name: test:e2e:chrome - command: | - if .circleci/scripts/test-run-e2e.sh - then - yarn test:e2e:chrome:nft --retries 2 - fi - no_output_timeout: 20m - - run: - name: Merge JUnit report - command: | - if [ "$(ls -A test/test-results/e2e)" ]; then - yarn test:e2e:report - fi - when: always - - store_artifacts: - path: test-artifacts - destination: test-artifacts - - store_test_results: - path: test/test-results/e2e.xml - - test-e2e-firefox-nft: - executor: node-browsers - parallelism: 1 - steps: - - checkout - - run: - name: Install Firefox - command: ./.circleci/scripts/firefox-install.sh - - attach_workspace: - at: . - - run: - name: Move test build to dist - command: mv ./dist-test-nft ./dist - - run: - name: Move test zips to builds - command: mv ./builds-test-nft ./builds - - run: - name: test:e2e:firefox - command: | - if .circleci/scripts/test-run-e2e.sh - then - yarn test:e2e:firefox:nft --retries 2 - fi - no_output_timeout: 20m - - run: - name: Merge JUnit report - command: | - if [ "$(ls -A test/test-results/e2e)" ]; then - yarn test:e2e:report - fi - when: always - - store_artifacts: - path: test-artifacts - destination: test-artifacts - - store_test_results: - path: test/test-results/e2e.xml - test-e2e-firefox-snaps: executor: node-browsers parallelism: 2 diff --git a/.metamaskrc.dist b/.metamaskrc.dist index 6bcb5db52..d5b8114a8 100644 --- a/.metamaskrc.dist +++ b/.metamaskrc.dist @@ -3,7 +3,6 @@ PASSWORD=METAMASK PASSWORD INFURA_PROJECT_ID=00000000000 SEGMENT_WRITE_KEY= SWAPS_USE_DEV_APIS= -NFTS_V1= PUBNUB_PUB_KEY= PUBNUB_SUB_KEY= PORTFOLIO_URL= diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 61c5c8aa5..9c496fd8b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -405,32 +405,30 @@ export default class MetamaskController extends EventEmitter { this.nftController.setApiKey(process.env.OPENSEA_KEY); - process.env.NFTS_V1 && - (this.nftDetectionController = new NftDetectionController({ - onNftsStateChange: (listener) => this.nftController.subscribe(listener), - onPreferencesStateChange: - this.preferencesController.store.subscribe.bind( - this.preferencesController.store, - ), - onNetworkStateChange: (cb) => - this.networkController.store.subscribe((networkState) => { - const modifiedNetworkState = { - ...networkState, - providerConfig: { - ...networkState.provider, - chainId: hexToDecimal(networkState.provider.chainId), - }, - }; - return cb(modifiedNetworkState); - }), - getOpenSeaApiKey: () => this.nftController.openSeaApiKey, - getBalancesInSingleCall: - this.assetsContractController.getBalancesInSingleCall.bind( - this.assetsContractController, - ), - addNft: this.nftController.addNft.bind(this.nftController), - getNftState: () => this.nftController.state, - })); + this.nftDetectionController = new NftDetectionController({ + onNftsStateChange: (listener) => this.nftController.subscribe(listener), + onPreferencesStateChange: this.preferencesController.store.subscribe.bind( + this.preferencesController.store, + ), + onNetworkStateChange: (cb) => + this.networkController.store.subscribe((networkState) => { + const modifiedNetworkState = { + ...networkState, + providerConfig: { + ...networkState.provider, + chainId: hexToDecimal(networkState.provider.chainId), + }, + }; + return cb(modifiedNetworkState); + }), + getOpenSeaApiKey: () => this.nftController.openSeaApiKey, + getBalancesInSingleCall: + this.assetsContractController.getBalancesInSingleCall.bind( + this.assetsContractController, + ), + addNft: this.nftController.addNft.bind(this.nftController), + getNftState: () => this.nftController.state, + }); this.metaMetricsController = new MetaMetricsController({ segment, @@ -2183,10 +2181,10 @@ export default class MetamaskController extends EventEmitter { detectTokensController, ), - // DetectNftController - detectNfts: process.env.NFTS_V1 - ? nftDetectionController.detectNfts.bind(nftDetectionController) - : null, + // DetectCollectibleController + detectNfts: nftDetectionController.detectNfts.bind( + nftDetectionController, + ), /** Token Detection V2 */ addDetectedTokens: diff --git a/development/build/config.js b/development/build/config.js index d3c6c3964..292a5cc0b 100644 --- a/development/build/config.js +++ b/development/build/config.js @@ -7,7 +7,6 @@ const commonConfigurationPropertyNames = ['PUBNUB_PUB_KEY', 'PUBNUB_SUB_KEY']; const configurationPropertyNames = [ ...commonConfigurationPropertyNames, - 'NFTS_V1', 'INFURA_PROJECT_ID', 'PHISHING_WARNING_PAGE_URL', 'PORTFOLIO_URL', diff --git a/development/build/scripts.js b/development/build/scripts.js index 2acb93cdf..3bb01ef66 100644 --- a/development/build/scripts.js +++ b/development/build/scripts.js @@ -1106,7 +1106,6 @@ async function getEnvironmentVariables({ buildTarget, buildType, version }) { const iconNames = await generateIconNames(); return { ICON_NAMES: iconNames, - NFTS_V1: config.NFTS_V1 === '1', CONF: devMode ? config : {}, IN_TEST: testing, INFURA_PROJECT_ID: getInfuraProjectId({ diff --git a/package.json b/package.json index 29fee6b52..43a6d6e10 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,6 @@ "build:test": "SEGMENT_HOST='https://api.segment.io' SEGMENT_WRITE_KEY='FAKE' SENTRY_DSN_DEV=https://fake@sentry.io/0000000 PORTFOLIO_URL=http://127.0.0.1:8080 yarn build test", "build:test:flask": "yarn build test --build-type flask", "build:test:mv3": "ENABLE_MV3=true SEGMENT_HOST='https://api.segment.io' SEGMENT_WRITE_KEY='FAKE' SENTRY_DSN_DEV=https://fake@sentry.io/0000000 PORTFOLIO_URL=http://127.0.0.1:8080 yarn build test", - "build:test:nft": "NFTS_V1=1 SEGMENT_HOST='https://api.segment.io' SEGMENT_WRITE_KEY='FAKE' SENTRY_DSN_DEV=https://fake@sentry.io/0000000 PORTFOLIO_URL=http://127.0.0.1:8080 yarn build test", "test": "yarn lint && yarn test:unit && yarn test:unit:jest", "dapp": "node development/static-server.js node_modules/@metamask/test-dapp/dist --port 8080", "dapp-chain": "GANACHE_ARGS='-b 2' concurrently -k -n ganache,dapp -p '[{time}][{name}]' 'yarn ganache:start' 'sleep 5 && yarn dapp'", diff --git a/ui/components/app/wallet-overview/token-overview.js b/ui/components/app/wallet-overview/token-overview.js index fe6ca79ce..4c7fc481d 100644 --- a/ui/components/app/wallet-overview/token-overview.js +++ b/ui/components/app/wallet-overview/token-overview.js @@ -60,7 +60,7 @@ const TokenOverview = ({ className, token }) => { const { openBuyCryptoInPdapp } = useRamps(); useEffect(() => { - if (token.isERC721 && process.env.NFTS_V1) { + if (token.isERC721) { dispatch( showModal({ name: 'CONVERT_TOKEN_TO_NFT', diff --git a/ui/components/app/wallet-overview/token-overview.test.js b/ui/components/app/wallet-overview/token-overview.test.js index d0266ee1c..c46e31f3b 100644 --- a/ui/components/app/wallet-overview/token-overview.test.js +++ b/ui/components/app/wallet-overview/token-overview.test.js @@ -85,7 +85,6 @@ describe('TokenOverview', () => { }); it('should show ConvertTokenToNFT modal when token passed in props is an ERC721', () => { - process.env.NFTS_V1 = true; const nftToken = { ...token, isERC721: true, @@ -99,7 +98,6 @@ describe('TokenOverview', () => { name: 'CONVERT_TOKEN_TO_NFT', tokenAddress: '0x01', }); - process.env.NFTS_V1 = false; }); it('should always show the Buy button regardless of chain Id', () => { @@ -122,7 +120,6 @@ describe('TokenOverview', () => { }); it('should always show the Buy button regardless of token type', () => { - process.env.NFTS_V1 = true; const nftToken = { ...token, isERC721: true, @@ -177,7 +174,6 @@ describe('TokenOverview', () => { }); it('should have the Buy token button disabled for ERC721 tokens', () => { - process.env.NFTS_V1 = true; const nftToken = { ...token, isERC721: true, diff --git a/ui/ducks/send/send.js b/ui/ducks/send/send.js index 360076155..a549f61f1 100644 --- a/ui/ducks/send/send.js +++ b/ui/ducks/send/send.js @@ -2100,7 +2100,7 @@ export function updateSendAsset( details.standard === TokenStandard.ERC1155 || details.standard === TokenStandard.ERC721 ) { - if (type === AssetType.token && process.env.NFTS_V1) { + if (type === AssetType.token) { dispatch( showModal({ name: 'CONVERT_TOKEN_TO_NFT', diff --git a/ui/ducks/send/send.test.js b/ui/ducks/send/send.test.js index 45fd3cd77..549bf4225 100644 --- a/ui/ducks/send/send.test.js +++ b/ui/ducks/send/send.test.js @@ -1711,7 +1711,6 @@ describe('Send Slice', () => { }); it('should show ConvertTokenToNFT modal and throw "invalidAssetType" error when token passed in props is an ERC721 or ERC1155', async () => { - process.env.NFTS_V1 = true; getTokenStandardAndDetailsStub.mockImplementation(() => Promise.resolve({ standard: 'ERC1155', balance: '0x1' }), ); @@ -1740,7 +1739,6 @@ describe('Send Slice', () => { }, type: 'UI_MODAL_OPEN', }); - process.env.NFTS_V1 = false; }); }); diff --git a/ui/helpers/constants/settings.js b/ui/helpers/constants/settings.js index 552eb5c9a..477fccab2 100644 --- a/ui/helpers/constants/settings.js +++ b/ui/helpers/constants/settings.js @@ -334,7 +334,6 @@ export const SETTINGS_CONSTANTS = [ descriptionMessage: (t) => t('enableOpenSeaAPIDescription'), route: `${EXPERIMENTAL_ROUTE}#opensea-api`, icon: 'fa fa-flask', - featureFlag: 'NFTS_V1', }, { tabMessage: (t) => t('experimental'), @@ -342,7 +341,6 @@ export const SETTINGS_CONSTANTS = [ descriptionMessage: (t) => t('useNftDetectionDescription'), route: `${EXPERIMENTAL_ROUTE}#autodetect-nfts`, icon: 'fa fa-flask', - featureFlag: 'NFTS_V1', }, { tabMessage: (t) => t('advanced'), diff --git a/ui/helpers/utils/settings-search.test.js b/ui/helpers/utils/settings-search.test.js index 2f797b347..76cc601f7 100644 --- a/ui/helpers/utils/settings-search.test.js +++ b/ui/helpers/utils/settings-search.test.js @@ -182,7 +182,7 @@ describe('Settings Search Utils', () => { it('should get good experimental section number', () => { expect(getNumberOfSettingsInSection(t, t('experimental'))).toStrictEqual( - 1, + 3, ); }); diff --git a/ui/pages/home/home.component.js b/ui/pages/home/home.component.js index bed280458..1d8c8330d 100644 --- a/ui/pages/home/home.component.js +++ b/ui/pages/home/home.component.js @@ -666,21 +666,19 @@ export default class Home extends PureComponent { } /> - {process.env.NFTS_V1 ? ( - - { - history.push(ADD_NFT_ROUTE); - }} - /> - - ) : null} + + { + history.push(ADD_NFT_ROUTE); + }} + /> + { }); it('sets and error when a token is an NFT', async () => { - process.env.NFTS_V1 = true; getTokenStandardAndDetails.mockImplementation(() => Promise.resolve({ standard: 'ERC721' }), ); diff --git a/ui/pages/routes/routes.component.js b/ui/pages/routes/routes.component.js index 7c8363eb2..8bf4d80af 100644 --- a/ui/pages/routes/routes.component.js +++ b/ui/pages/routes/routes.component.js @@ -252,9 +252,7 @@ export default class Routes extends Component { component={ImportTokenPage} exact /> - {process.env.NFTS_V1 ? ( - - ) : null} + +
+
+ + Enable OpenSea API + +
+ Use OpenSea's API to fetch NFT data. NFT auto-detection relies on OpenSea's API, and will not be available when this is turned off. +
+
+
+
+