Skip to content

Commit

Permalink
Hide 'Add snap account' button behind an experimental setting (#20974)
Browse files Browse the repository at this point in the history
* feat: hide 'Add Snap Account' button behind an experimental setting

* test: add e2e test to hide the "Add snap account" button in settings
  • Loading branch information
danroc authored Sep 21, 2023
1 parent 7dbb8c8 commit 0bc3aa9
Show file tree
Hide file tree
Showing 14 changed files with 379 additions and 41 deletions.
12 changes: 12 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.

17 changes: 17 additions & 0 deletions app/scripts/controllers/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
*
Expand Down
6 changes: 6 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
Expand Down
3 changes: 3 additions & 0 deletions shared/constants/metametrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -1380,6 +1380,7 @@
}
],
"desktopEnabled": false,
"addSnapAccountEnabled": false,
"pendingApprovals": {
"testApprovalId": {
"id": "testApprovalId",
Expand Down
29 changes: 15 additions & 14 deletions test/e2e/run-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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];
Expand Down
52 changes: 52 additions & 0 deletions test/e2e/tests/settings-add-snap-account-toggle.spec.js
Original file line number Diff line number Diff line change
@@ -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,
);
},
);
});
});
18 changes: 12 additions & 6 deletions ui/components/multichain/account-list-menu/account-list-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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('');
Expand Down Expand Up @@ -198,7 +204,7 @@ export const AccountListMenu = ({ onClose }) => {
</Box>
{/* Add / Import / Hardware */}
<Box padding={4}>
<Box marginBottom={4}>
<Box>
<ButtonLink
size={Size.SM}
startIconName={IconName.Add}
Expand All @@ -218,7 +224,7 @@ export const AccountListMenu = ({ onClose }) => {
{t('addAccount')}
</ButtonLink>
</Box>
<Box marginBottom={4}>
<Box marginTop={4}>
<ButtonLink
size={Size.SM}
startIconName={IconName.Import}
Expand All @@ -237,7 +243,7 @@ export const AccountListMenu = ({ onClose }) => {
{t('importAccount')}
</ButtonLink>
</Box>
<Box marginBottom={4}>
<Box marginTop={4}>
<ButtonLink
size={Size.SM}
startIconName={IconName.Hardware}
Expand Down Expand Up @@ -265,7 +271,7 @@ export const AccountListMenu = ({ onClose }) => {
</Box>
{
///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps)
<>
addSnapAccountEnabled && (
<Box marginTop={4}>
<ButtonLink
size={Size.SM}
Expand All @@ -284,12 +290,12 @@ export const AccountListMenu = ({ onClose }) => {
{t('settingAddSnapAccount')}
</ButtonLink>
</Box>
</>
)
///: END:ONLY_INCLUDE_IN
}
{
///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
<Box>
<Box marginTop={4}>
<ButtonLink
size={Size.SM}
startIconName={IconName.Custody}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,29 +158,58 @@ describe('AccountListMenu', () => {
});

///: 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(<AccountListMenu {...props} />, 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
Expand Down
Loading

0 comments on commit 0bc3aa9

Please sign in to comment.