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
2024-12-22 09:23:21 +01:00 · 2023-03-13 14:29:37 -05:00 · 2023-03-13 14:29:37 -05:00 · 5b1b5dc03b
commit 5b1b5dc03b
parent e7527b65ee
19 changed files with 208 additions and 178 deletions
--- 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
--- 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=
--- 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:
--- 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',
--- 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({
--- 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'",
--- 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',
--- 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,
--- 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',
--- 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;
      });
    });

--- 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'),
--- 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,
      );
    });

--- a/ui/pages/home/home.component.js
+++ b/ui/pages/home/home.component.js
@ -666,21 +666,19 @@ export default class Home extends PureComponent {
                  }
                />
              </Tab>
-              {process.env.NFTS_V1 ? (
-                <Tab
-                  activeClassName="home__tab--active"
-                  className="home__tab"
-                  data-testid="home__nfts-tab"
-                  name={this.context.t('nfts')}
-                  tabKey="nfts"
-                >
-                  <NftsTab
-                    onAddNFT={() => {
-                      history.push(ADD_NFT_ROUTE);
-                    }}
-                  />
-                </Tab>
-              ) : null}
+              <Tab
+                activeClassName="home__tab--active"
+                className="home__tab"
+                data-testid="home__nfts-tab"
+                name={this.context.t('nfts')}
+                tabKey="nfts"
+              >
+                <NftsTab
+                  onAddNFT={() => {
+                    history.push(ADD_NFT_ROUTE);
+                  }}
+                />
+              </Tab>
              <Tab
                activeClassName="home__tab--active"
                className="home__tab"
--- a/ui/pages/import-token/import-token.component.js
+++ b/ui/pages/import-token/import-token.component.js
@ -309,8 +309,7 @@ class ImportToken extends Component {
        });

        break;
-      case process.env.NFTS_V1 &&
-        (standard === 'ERC1155' || standard === 'ERC721'):
+      case standard === 'ERC1155' || standard === 'ERC721':
        this.setState({
          nftAddressError: this.context.t('nftAddressError', [
            <a
--- a/ui/pages/import-token/import-token.test.js
+++ b/ui/pages/import-token/import-token.test.js
@ -152,7 +152,6 @@ describe('Import Token', () => {
    });

    it('sets and error when a token is an NFT', async () => {
-      process.env.NFTS_V1 = true;
      getTokenStandardAndDetails.mockImplementation(() =>
        Promise.resolve({ standard: 'ERC721' }),
      );
--- 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 ? (
-          <Authenticated path={ADD_NFT_ROUTE} component={AddNftPage} exact />
-        ) : null}
+        <Authenticated path={ADD_NFT_ROUTE} component={AddNftPage} exact />
        <Authenticated
          path={CONFIRM_IMPORT_TOKEN_ROUTE}
          component={ConfirmImportTokenPage}
--- a/ui/pages/settings/experimental-tab/snapshots/experimental-tab.test.js.snap
+++ b/ui/pages/settings/experimental-tab/snapshots/experimental-tab.test.js.snap
@ -95,6 +95,166 @@ exports[`ExperimentalTab with desktop enabled renders ExperimentalTab component
        </div>
      </div>
    </div>
+    <div
+      class="settings-page__content-row"
+    >
+      <div
+        class="settings-page__content-item"
+      >
+        <span>
+          Enable OpenSea API
+        </span>
+        <div
+          class="settings-page__content-description"
+        >
+          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.
+        </div>
+      </div>
+      <div
+        class="settings-page__content-item"
+      >
+        <div
+          class="settings-page__content-item-col"
+        >
+          <label
+            class="toggle-button toggle-button--off"
+            tabindex="0"
+          >
+            <div
+              style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
+            >
+              <div
+                style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(242, 244, 246);"
+              >
+                <div
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
+                />
+                <div
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 1;"
+                />
+              </div>
+              <div
+                style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
+              >
+                <div
+                  style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: none; border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(106, 115, 125); left: 3px;"
+                />
+              </div>
+              <input
+                style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
+                type="checkbox"
+                value="false"
+              />
+            </div>
+            <div
+              class="toggle-button__status"
+            >
+              <span
+                class="toggle-button__label-off"
+              >
+                Off
+              </span>
+              <span
+                class="toggle-button__label-on"
+              >
+                On
+              </span>
+            </div>
+          </label>
+        </div>
+      </div>
+    </div>
+    <div
+      class="settings-page__content-row"
+    >
+      <div
+        class="settings-page__content-item"
+      >
+        <span>
+          Autodetect NFTs
+        </span>
+        <div
+          class="settings-page__content-description"
+        >
+          <p
+            class="box mm-text mm-text--body-md mm-text--color-text-alternative box--flex-direction-row"
+          >
+            We use third-party APIs to detect NFTs in your wallet, which means your IP address may be exposed to centralized servers. There are a few things to be cautious about when enabling this feature.
+          </p>
+          <ul
+            class="settings-page__content-unordered-list"
+          >
+            <li>
+              Your account address will be viewable to third-party APIs.
+            </li>
+            <li>
+              NFT metadata may contain links to scams or phishing sites.
+            </li>
+            <li>
+              Anyone can airdrop NFTs to your account. This can include offensive content that might be automatically displayed in your wallet.
+            </li>
+          </ul>
+          <p
+            class="box mm-text mm-text--body-md mm-text--color-text-alternative box--padding-top-4 box--flex-direction-row"
+          >
+            Leave this feature off if you don't want the app to pull data from those services.
+          </p>
+        </div>
+      </div>
+      <div
+        class="settings-page__content-item"
+      >
+        <div
+          class="settings-page__content-item-col"
+        >
+          <label
+            class="toggle-button toggle-button--off"
+            tabindex="0"
+          >
+            <div
+              style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
+            >
+              <div
+                style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(242, 244, 246);"
+              >
+                <div
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
+                />
+                <div
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 1;"
+                />
+              </div>
+              <div
+                style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
+              >
+                <div
+                  style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: none; border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(106, 115, 125); left: 3px;"
+                />
+              </div>
+              <input
+                style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
+                type="checkbox"
+                value="false"
+              />
+            </div>
+            <div
+              class="toggle-button__status"
+            >
+              <span
+                class="toggle-button__label-off"
+              >
+                Off
+              </span>
+              <span
+                class="toggle-button__label-on"
+              >
+                On
+              </span>
+            </div>
+          </label>
+        </div>
+      </div>
+    </div>
    <div
      class="settings-page__content-row"
      data-testid="advanced-setting-desktop-pairing"
--- a/ui/pages/settings/experimental-tab/experimental-tab.component.js
+++ b/ui/pages/settings/experimental-tab/experimental-tab.component.js
@ -253,7 +253,7 @@ export default class ExperimentalTab extends PureComponent {
    return (
      <div className="settings-page__body">
        {this.renderTransactionSecurityCheckToggle()}
-        {process.env.NFTS_V1 && this.renderOpenSeaEnabledToggle()}
+        {this.renderOpenSeaEnabledToggle()}
        {
          ///: BEGIN:ONLY_INCLUDE_IN(flask)
          this.renderDesktopEnableButton()
--- a/ui/pages/settings/security-tab/security-tab.test.js
+++ b/ui/pages/settings/security-tab/security-tab.test.js
@ -76,10 +76,7 @@ describe('Security Tab', () => {

    const checkboxes = queryAllByRole('checkbox');

-    let index = 4;
-    if (process.env.NFTS_V1) {
-      index = 5;
-    }
+    const index = 5;
    const toggleMetaMetricsCheckbox = checkboxes[index];

    expect(toggleMetaMetricsCheckbox).toHaveAttribute('value', 'false');