From 0bc3aa9a928c8835cc3fad7d2579d55ec96c90d7 Mon Sep 17 00:00:00 2001 From: Daniel Rocha Date: Fri, 22 Sep 2023 00:02:00 +0200 Subject: [PATCH] Hide 'Add snap account' button behind an experimental setting (#20974) * feat: hide 'Add Snap Account' button behind an experimental setting * test: add e2e test to hide the "Add snap account" button in settings --- app/_locales/en/messages.json | 12 +++ app/scripts/controllers/preferences.js | 17 ++++ app/scripts/metamask-controller.js | 6 ++ shared/constants/metametrics.ts | 3 + test/data/mock-state.json | 1 + test/e2e/run-all.js | 29 +++--- .../settings-add-snap-account-toggle.spec.js | 52 +++++++++++ .../account-list-menu/account-list-menu.js | 18 ++-- .../account-list-menu.test.js | 69 +++++++++----- .../experimental-tab.test.js.snap | 85 ++++++++++++++++++ .../experimental-tab.component.js | 90 ++++++++++++++++++- .../experimental-tab.container.js | 16 ++++ ui/selectors/selectors.js | 12 +++ ui/store/actions.ts | 10 +++ 14 files changed, 379 insertions(+), 41 deletions(-) create mode 100644 test/e2e/tests/settings-add-snap-account-toggle.spec.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index ba42ad30d41a..342649dbb644 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -268,6 +268,12 @@ "addSnapAccountModalDescription": { "message": "Discover options to keep your account secure with MetaMask Snaps" }, + "addSnapAccountToggle": { + "message": "Enable \"Add Snap account\"" + }, + "addSnapAccountsDescription": { + "message": "Turning on this feature will give you the option to add a Snap account right from your account list. If you install a Snap account, remember that it is a third-party service." + }, "addSuggestedNFTs": { "message": "Add suggested NFTs" }, @@ -4024,6 +4030,12 @@ "smartSwapsSubDescription": { "message": "* Smart Swaps will attempt to submit your transaction privately, multiple times. If all attempts fail, the transaction will be broadcast publicly to ensure your Swap successfully goes through." }, + "snapAccounts": { + "message": "Snap accounts" + }, + "snapAccountsDescription": { + "message": "Accounts controlled by third-party Snaps." + }, "snapConfigure": { "message": "Configure" }, diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 5c1f169834ab..e396ae3b41f0 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -66,6 +66,9 @@ export default class PreferencesController { ///: BEGIN:ONLY_INCLUDE_IN(blockaid) securityAlertsEnabled: false, ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + addSnapAccountEnabled: false, + ///: END:ONLY_INCLUDE_IN advancedGasFee: {}, // WARNING: Do not use feature flags for security-sensitive things. @@ -240,6 +243,20 @@ export default class PreferencesController { } ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + /** + * Setter for the `addSnapAccountEnabled` property. + * + * @param {boolean} addSnapAccountEnabled - Whether or not the user wants to + * enable the "Add Snap accounts" button. + */ + setAddSnapAccountEnabled(addSnapAccountEnabled) { + this.store.updateState({ + addSnapAccountEnabled, + }); + } + ///: END:ONLY_INCLUDE_IN + /** * Setter for the `advancedGasFee` property * diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 70ec00c59921..33d16f66b0f7 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -2372,6 +2372,12 @@ export default class MetamaskController extends EventEmitter { preferencesController, ), ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + setAddSnapAccountEnabled: + preferencesController.setAddSnapAccountEnabled.bind( + preferencesController, + ), + ///: END:ONLY_INCLUDE_IN setIpfsGateway: preferencesController.setIpfsGateway.bind( preferencesController, ), diff --git a/shared/constants/metametrics.ts b/shared/constants/metametrics.ts index d510db5e7a38..b17c67e63038 100644 --- a/shared/constants/metametrics.ts +++ b/shared/constants/metametrics.ts @@ -631,6 +631,9 @@ export enum MetaMetricsEventName { SnapUpdated = 'Snap Updated', SnapExportUsed = 'Snap Export Used', ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + AddSnapAccountEnabled = 'Add Snap Account Enabled', + ///: END:ONLY_INCLUDE_IN } export enum MetaMetricsEventAccountType { diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 7c42087b90fc..be994991cb49 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -1380,6 +1380,7 @@ } ], "desktopEnabled": false, + "addSnapAccountEnabled": false, "pendingApprovals": { "testApprovalId": { "id": "testApprovalId", diff --git a/test/e2e/run-all.js b/test/e2e/run-all.js index 4fa986aa4f98..d8094187a018 100644 --- a/test/e2e/run-all.js +++ b/test/e2e/run-all.js @@ -101,20 +101,6 @@ async function main() { if (snaps) { const testDir = path.join(__dirname, 'snaps'); testPaths = await getTestPathsForTestDir(testDir); - - if (buildType && buildType !== 'flask') { - // These tests should only be ran on Flask for now - const filteredTests = [ - 'test-snap-manageAccount.spec.js', - 'test-snap-rpc.spec.js', - 'test-snap-lifecycle.spec.js', - 'ppom-toggle-settings.spec.js', - 'petnames.spec.js', - ]; - testPaths = testPaths.filter((p) => - filteredTests.every((filteredTest) => !p.endsWith(filteredTest)), - ); - } } else if (rpc) { const testDir = path.join(__dirname, 'json-rpc'); testPaths = await getTestPathsForTestDir(testDir); @@ -135,6 +121,21 @@ async function main() { } } + // These tests should only be ran on Flask for now. + if (buildType !== 'flask') { + const filteredTests = [ + 'settings-add-snap-account-toggle.spec.js', + 'test-snap-manageAccount.spec.js', + 'test-snap-rpc.spec.js', + 'test-snap-lifecycle.spec.js', + 'ppom-toggle-settings.spec.js', + 'petnames.spec.js', + ]; + testPaths = testPaths.filter((p) => + filteredTests.every((filteredTest) => !p.endsWith(filteredTest)), + ); + } + const runE2eTestPath = path.join(__dirname, 'run-e2e-test.js'); const args = [runE2eTestPath]; diff --git a/test/e2e/tests/settings-add-snap-account-toggle.spec.js b/test/e2e/tests/settings-add-snap-account-toggle.spec.js new file mode 100644 index 000000000000..54a32f5d1662 --- /dev/null +++ b/test/e2e/tests/settings-add-snap-account-toggle.spec.js @@ -0,0 +1,52 @@ +const { strict: assert } = require('assert'); +const { withFixtures } = require('../helpers'); +const FixtureBuilder = require('../fixture-builder'); + +describe('Add snap account experimental settings', function () { + it('switch "Enable Add snap account" to on', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + title: this.test.title, + failOnConsoleError: false, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + // Make sure the "Add snap account" button is not visible. + await driver.clickElement('[data-testid="account-menu-icon"]'); + await driver.assertElementNotPresent({ + text: 'Add snap account', + tag: 'button', + }); + await driver.clickElement('.mm-box button[aria-label="Close"]'); + + // Navigate to experimental settings. + await driver.clickElement( + '[data-testid="account-options-menu-button"]', + ); + await driver.clickElement({ text: 'Settings', tag: 'div' }); + await driver.clickElement({ text: 'Experimental', tag: 'div' }); + + // Switch "Enable Add snap account" to on. + const toggle = await driver.findElement( + '[data-testid="add-snap-account-toggle"]', + ); + await driver.scrollToElement(toggle); + await driver.clickElement('[data-testid="add-snap-account-toggle"]'); + + // Make sure the "Add snap account" button is visible. + await driver.clickElement('[data-testid="account-menu-icon"]'); + assert.equal( + await driver.isElementPresentAndVisible({ + text: 'Add snap account', + tag: 'button', + }), + true, + ); + }, + ); + }); +}); diff --git a/ui/components/multichain/account-list-menu/account-list-menu.js b/ui/components/multichain/account-list-menu/account-list-menu.js index c44b6e5b59bf..a51ba2e0d612 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.js +++ b/ui/components/multichain/account-list-menu/account-list-menu.js @@ -29,6 +29,9 @@ import { getMetaMaskAccountsOrdered, getConnectedSubjectsForAllAddresses, getOriginOfCurrentTab, + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + getIsAddSnapAccountEnabled, + ///: END:ONLY_INCLUDE_IN } from '../../../selectors'; import { toggleAccountMenu, setSelectedAccount } from '../../../store/actions'; import { @@ -57,6 +60,9 @@ export const AccountListMenu = ({ onClose }) => { const currentTabOrigin = useSelector(getOriginOfCurrentTab); const history = useHistory(); const dispatch = useDispatch(); + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + const addSnapAccountEnabled = useSelector(getIsAddSnapAccountEnabled); + ///: END:ONLY_INCLUDE_IN const [searchQuery, setSearchQuery] = useState(''); const [actionMode, setActionMode] = useState(''); @@ -198,7 +204,7 @@ export const AccountListMenu = ({ onClose }) => { {/* Add / Import / Hardware */} - + { {t('addAccount')} - + { {t('importAccount')} - + { { ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) - <> + addSnapAccountEnabled && ( { {t('settingAddSnapAccount')} - + ) ///: END:ONLY_INCLUDE_IN } { ///: BEGIN:ONLY_INCLUDE_IN(build-mmi) - + { }); ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) - it('renders the add snap account button', async () => { - const { getByText } = render(); - const addSnapAccountButton = getByText( - messages.settingAddSnapAccount.message, - ); - expect(addSnapAccountButton).toBeInTheDocument(); - - fireEvent.click(addSnapAccountButton); + describe('addSnapAccountButton', () => { + const renderWithState = (state, props = { onClose: () => jest.fn() }) => { + const store = configureStore({ + ...mockState, + ...{ + metamask: { + ...mockState.metamask, + ...state, + }, + }, + activeTab: { + id: 113, + title: 'E2E Test Dapp', + origin: 'https://metamask.github.io', + protocol: 'https:', + url: 'https://metamask.github.io/test-dapp/', + }, + }); + return renderWithProvider(, store); + }; + + it("doesn't render the add snap account button if it's disabled", async () => { + const { getByText } = renderWithState({ addSnapAccountEnabled: false }); + expect(() => getByText(messages.settingAddSnapAccount.message)).toThrow( + `Unable to find an element with the text: ${messages.settingAddSnapAccount.message}`, + ); + }); - await waitFor(() => { - expect(mockToggleAccountMenu).toHaveBeenCalled(); + it("renders the add snap account button if it's enabled", async () => { + const { getByText } = renderWithState({ addSnapAccountEnabled: true }); + const addSnapAccountButton = getByText( + messages.settingAddSnapAccount.message, + ); + expect(addSnapAccountButton).toBeInTheDocument(); + + fireEvent.click(addSnapAccountButton); + await waitFor(() => { + expect(mockToggleAccountMenu).toHaveBeenCalled(); + }); }); - }); - it('pushes history when clicking add snap account from extended view', async () => { - const { getByText } = render(); - mockGetEnvironmentType.mockReturnValueOnce('fullscreen'); - const addSnapAccountButton = getByText( - messages.settingAddSnapAccount.message, - ); - fireEvent.click(addSnapAccountButton); - await waitFor(() => { - expect(historyPushMock).toHaveBeenCalledWith(ADD_SNAP_ACCOUNT_ROUTE); + it('pushes history when clicking add snap account from extended view', async () => { + const { getByText } = renderWithState({ addSnapAccountEnabled: true }); + mockGetEnvironmentType.mockReturnValueOnce('fullscreen'); + const addSnapAccountButton = getByText( + messages.settingAddSnapAccount.message, + ); + + fireEvent.click(addSnapAccountButton); + await waitFor(() => { + expect(historyPushMock).toHaveBeenCalledWith(ADD_SNAP_ACCOUNT_ROUTE); + }); }); }); ///: END:ONLY_INCLUDE_IN diff --git a/ui/pages/settings/experimental-tab/__snapshots__/experimental-tab.test.js.snap b/ui/pages/settings/experimental-tab/__snapshots__/experimental-tab.test.js.snap index c0c14c5068f2..e8ea928d052b 100644 --- a/ui/pages/settings/experimental-tab/__snapshots__/experimental-tab.test.js.snap +++ b/ui/pages/settings/experimental-tab/__snapshots__/experimental-tab.test.js.snap @@ -173,6 +173,91 @@ exports[`ExperimentalTab with desktop enabled renders ExperimentalTab component +

+ Snaps +

+
+
+ + Snap accounts + +
+
+ Accounts controlled by third-party Snaps. +
+
+
+ Enable "Add Snap account" +
+
+
+

diff --git a/ui/pages/settings/experimental-tab/experimental-tab.component.js b/ui/pages/settings/experimental-tab/experimental-tab.component.js index 9e6fcec56e2f..e89a90231757 100644 --- a/ui/pages/settings/experimental-tab/experimental-tab.component.js +++ b/ui/pages/settings/experimental-tab/experimental-tab.component.js @@ -5,7 +5,12 @@ import { getNumberOfSettingsInSection, handleSettingsRefs, } from '../../../helpers/utils/settings-search'; -import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics'; +import { + MetaMetricsEventCategory, + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + MetaMetricsEventName, + ///: END:ONLY_INCLUDE_IN +} from '../../../../shared/constants/metametrics'; import { Text, Box } from '../../../components/component-library'; import { @@ -37,6 +42,10 @@ export default class ExperimentalTab extends PureComponent { securityAlertsEnabled: PropTypes.bool, setSecurityAlertsEnabled: PropTypes.func, ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + addSnapAccountEnabled: PropTypes.bool, + setAddSnapAccountEnabled: PropTypes.func, + ///: END:ONLY_INCLUDE_IN }; settingsRefs = Array( @@ -238,6 +247,80 @@ export default class ExperimentalTab extends PureComponent { } ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + renderKeyringSnapsToggle() { + const { t, trackEvent } = this.context; + const { addSnapAccountEnabled, setAddSnapAccountEnabled } = this.props; + + return ( + <> + + {t('snaps')} + + +
+ {t('snapAccounts')} +
+ + {t('snapAccountsDescription')} + + +
+ + {t('addSnapAccountToggle')} + + + { + trackEvent({ + event: MetaMetricsEventName.AddSnapAccountEnabled, + category: MetaMetricsEventCategory.Settings, + properties: { + enabled: !value, + }, + }); + setAddSnapAccountEnabled(!value); + }} + /> + +
+ + {t('addSnapAccountsDescription')} + +
+
+ + + ); + } + ///: END:ONLY_INCLUDE_IN + render() { return (
@@ -247,6 +330,11 @@ export default class ExperimentalTab extends PureComponent { ///: END:ONLY_INCLUDE_IN } {this.renderTransactionSecurityCheckToggle()} + { + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + this.renderKeyringSnapsToggle() + ///: END:ONLY_INCLUDE_IN + } { ///: BEGIN:ONLY_INCLUDE_IN(desktop) this.renderDesktopEnableButton() diff --git a/ui/pages/settings/experimental-tab/experimental-tab.container.js b/ui/pages/settings/experimental-tab/experimental-tab.container.js index dadf718ec2e4..567a88759cd9 100644 --- a/ui/pages/settings/experimental-tab/experimental-tab.container.js +++ b/ui/pages/settings/experimental-tab/experimental-tab.container.js @@ -6,12 +6,18 @@ import { ///: BEGIN:ONLY_INCLUDE_IN(blockaid) setSecurityAlertsEnabled, ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + setAddSnapAccountEnabled, + ///: END:ONLY_INCLUDE_IN } from '../../../store/actions'; import { getIsTransactionSecurityCheckEnabled, ///: BEGIN:ONLY_INCLUDE_IN(blockaid) getIsSecurityAlertsEnabled, ///: END:ONLY_INCLUDE_IN + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + getIsAddSnapAccountEnabled, + ///: END:ONLY_INCLUDE_IN } from '../../../selectors'; import ExperimentalTab from './experimental-tab.component'; @@ -19,9 +25,14 @@ const mapStateToProps = (state) => { return { transactionSecurityCheckEnabled: getIsTransactionSecurityCheckEnabled(state), + ///: BEGIN:ONLY_INCLUDE_IN(blockaid) securityAlertsEnabled: getIsSecurityAlertsEnabled(state), ///: END:ONLY_INCLUDE_IN + + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + addSnapAccountEnabled: getIsAddSnapAccountEnabled(state), + ///: END:ONLY_INCLUDE_IN }; }; @@ -29,9 +40,14 @@ const mapDispatchToProps = (dispatch) => { return { setTransactionSecurityCheckEnabled: (val) => dispatch(setTransactionSecurityCheckEnabled(val)), + ///: BEGIN:ONLY_INCLUDE_IN(blockaid) setSecurityAlertsEnabled: (val) => setSecurityAlertsEnabled(val), ///: END:ONLY_INCLUDE_IN + + ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) + setAddSnapAccountEnabled: (val) => setAddSnapAccountEnabled(val), + ///: END:ONLY_INCLUDE_IN }; }; diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 5f554f54fbbd..69e6c4cb1e4c 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -1533,6 +1533,18 @@ export function getIsSecurityAlertsEnabled(state) { } ///: END:ONLY_INCLUDE_IN +///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) +/** + * Get the state of the `addSnapAccountEnabled` flag. + * + * @param {*} state + * @returns The state of the `addSnapAccountEnabled` flag. + */ +export function getIsAddSnapAccountEnabled(state) { + return state.metamask.addSnapAccountEnabled; +} +///: END:ONLY_INCLUDE_IN + export function getIsCustomNetwork(state) { const chainId = getCurrentChainId(state); diff --git a/ui/store/actions.ts b/ui/store/actions.ts index 23016882516c..4dfefa608109 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -4436,6 +4436,16 @@ export function setSecurityAlertsEnabled(val: boolean): void { } ///: END:ONLY_INCLUDE_IN +///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps) +export async function setAddSnapAccountEnabled(value: boolean): Promise { + try { + await submitRequestToBackground('setAddSnapAccountEnabled', [value]); + } catch (error) { + logErrorWithMessage(error); + } +} +///: END:ONLY_INCLUDE_IN + export function setFirstTimeUsedNetwork(chainId: string) { return submitRequestToBackground('setFirstTimeUsedNetwork', [chainId]); }