Skip to content

Commit

Permalink
feat(3744): Implement feature Flag Values with Scope Based on thresho…
Browse files Browse the repository at this point in the history
…ld (#29081)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
A more advanced version of remote rollout feature flags are object
flags, described in ADR
[here](https://github.com/MetaMask/decisions/blob/b3094d47a568ac1e076a44fa704c2d29d1b59f35/decisions/wallet-platform/0001-remote-rollout-feature-flags.md#use-case-2a-feature-flag-values-with-scope-based-on-threshold),
we would like to address the use case allowing different values for
random subsets of the user base.

After the implementation of
MetaMask/MetaMask-planning#3742 in
@metamask/remote-feature-flag-controller, we can now pass in
metaMetricsId in controller when registering and further retrieve value
based on threshold.

In terms of metaMetricsId, it can be found in app state when onboarding
a user, deleting metametrics data would not remove metaMetricsId
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29081?quickstart=1)

## **Related issues**
Controller: MetaMask/MetaMask-planning#3742
Fixes: MetaMask/MetaMask-planning#3744

## **Manual testing steps**
Load the extension
Option 1: input chrome.storage.local.get(console.log) in dev tools
console, and you'll find in data => RemoteFeatureFlagController =>
contains 2 dataset, cacheTimestamp and remoteFeatureFlags with value,
where `remoteFeatureFlags` has 3 elements, the `testFlagForThreshold` is
based on `metametricsId`


<img width="408" alt="Screenshot 2024-12-12 at 21 58 26"
src="https://github.com/user-attachments/assets/f095a066-2513-4eb5-b034-9102f34fbfc7"
/>

Option 2: go to settings => advanced, and click download stage logs
button, you'll find testFlagForThreshold within the remoteFeatureFlags
state.
Option 3: Settings => about page, if you open console in dev tools,
there's a log after Feature flag fetched successfully, which will be
removed after we implement 1st feature flag in production, it looks like
```
Fetch remote feature flag success, eg: testFlagForThreshold has value {"name":"groupC","value":"valueC"}
```

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: MetaMask Bot <[email protected]>
  • Loading branch information
DDDDDanica and metamaskbot authored Jan 15, 2025
1 parent 7a78bf2 commit 5b5c04a
Show file tree
Hide file tree
Showing 19 changed files with 121 additions and 100 deletions.
1 change: 1 addition & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2439,6 +2439,7 @@ export default class MetamaskController extends EventEmitter {
allowedEvents: [],
}),
disabled: !this.preferencesController.state.useExternalServices,
getMetaMetricsId: () => this.metaMetricsController.getMetaMetricsId(),
clientConfigApiService: new ClientConfigApiService({
fetch: globalThis.fetch.bind(globalThis),
config: {
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,8 @@
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
"cockatiel": true,
"uuid": true
}
},
"@metamask/rpc-errors": {
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,8 @@
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
"cockatiel": true,
"uuid": true
}
},
"@metamask/rpc-errors": {
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1645,7 +1645,8 @@
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
"cockatiel": true,
"uuid": true
}
},
"@metamask/rpc-errors": {
Expand Down
3 changes: 2 additions & 1 deletion lavamoat/browserify/mmi/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1737,7 +1737,8 @@
"@metamask/remote-feature-flag-controller": {
"packages": {
"@metamask/base-controller": true,
"cockatiel": true
"cockatiel": true,
"uuid": true
}
},
"@metamask/rpc-errors": {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@
"@metamask/providers": "^18.2.0",
"@metamask/queued-request-controller": "^7.0.1",
"@metamask/rate-limit-controller": "^6.0.0",
"@metamask/remote-feature-flag-controller": "^1.1.0",
"@metamask/remote-feature-flag-controller": "^1.3.0",
"@metamask/rpc-errors": "^7.0.0",
"@metamask/safe-event-emitter": "^3.1.1",
"@metamask/scure-bip39": "^2.0.3",
Expand Down
8 changes: 6 additions & 2 deletions test/e2e/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,16 @@ export const DEFAULT_SOLANA_ACCOUNT =
/* Default (mocked) SOLANA balance used by the Solana RPC provider */
export const DEFAULT_SOLANA_BALANCE = 1; // SOL

/* Title of the mocked E2E test empty HTML page */
export const EMPTY_E2E_TEST_PAGE_TITLE = 'E2E Test Page';
/* Title of Portfolio page */
export const PORTFOLIO_PAGE_TITLE = 'MetaMask Portfolio';

/* Account types */
export enum ACCOUNT_TYPE {
Ethereum,
Bitcoin,
Solana,
}

/* Meta metricsId generated by generateMetaMetricsId */
export const MOCK_META_METRICS_ID =
'0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420';
24 changes: 23 additions & 1 deletion test/e2e/mock-e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,29 @@ async function setupMocking(
return {
ok: true,
statusCode: 200,
json: [{ feature1: true }, { feature2: false }],
json: [
{ feature1: true },
{ feature2: false },
{
feature3: [
{
value: 'valueA',
name: 'groupA',
scope: { type: 'threshold', value: 0.3 },
},
{
value: 'valueB',
name: 'groupB',
scope: { type: 'threshold', value: 0.5 },
},
{
scope: { type: 'threshold', value: 1 },
value: 'valueC',
name: 'groupC',
},
],
},
],
};
});

Expand Down
44 changes: 5 additions & 39 deletions test/e2e/tests/metrics/delete-metametrics-data.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ const selectors = {
globalMenuSettingsButton: '[data-testid="global-menu-settings"]',
securityAndPrivacySettings: { text: 'Security & privacy', tag: 'div' },
experimentalSettings: { text: 'Experimental', tag: 'div' },
deletMetaMetricsSettings: '[data-testid="delete-metametrics-data-button"]',
deleteMetaMetricsDataButton: {
text: 'Delete MetaMetrics data',
tag: 'button',
},
deleteMetaMetricsDataButton: '[data-testid="delete-metametrics-data-button"]',
clearButton: { text: 'Clear', tag: 'button' },
backButton: '[data-testid="settings-back-button"]',
};
Expand Down Expand Up @@ -111,7 +107,6 @@ describe('Delete MetaMetrics Data @no-mmi', function (this: Suite) {
await driver.clickElement(selectors.globalMenuSettingsButton);
await driver.clickElement(selectors.securityAndPrivacySettings);

await driver.findElement(selectors.deletMetaMetricsSettings);
await driver.clickElement(selectors.deleteMetaMetricsDataButton);

// there is a race condition, where we need to wait before clicking clear button otherwise an error is thrown in the background
Expand Down Expand Up @@ -157,65 +152,37 @@ describe('Delete MetaMetrics Data @no-mmi', function (this: Suite) {
},
);
});

it('while user has opted out for metrics tracking', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder()
.withMetaMetricsController({
metaMetricsId: 'fake-metrics-id',
participateInMetaMetrics: false,
})
.build(),
defaultGanacheOptions,
title: this.test?.fullTitle(),
testSpecificMock: mockSegment,
},
async ({
driver,
mockedEndpoint: mockedEndpoints,
}: TestSuiteArguments) => {
async ({ driver }: TestSuiteArguments) => {
await unlockWallet(driver);

await driver.clickElement(selectors.accountOptionsMenuButton);
await driver.clickElement(selectors.globalMenuSettingsButton);
await driver.clickElement(selectors.securityAndPrivacySettings);

await driver.findElement(selectors.deletMetaMetricsSettings);
await driver.clickElement(selectors.deleteMetaMetricsDataButton);

// there is a race condition, where we need to wait before clicking clear button otherwise an error is thrown in the background
// we cannot wait for a UI conditon, so we a delay to mitigate this until another solution is found
await driver.delay(3000);
await driver.clickElementAndWaitToDisappear(selectors.clearButton);

const deleteMetaMetricsDataButton = await driver.findElement(
selectors.deleteMetaMetricsDataButton,
);
await (
deleteMetaMetricsDataButton as WebElementWithWaitForElementState
).waitForElementState('disabled');

const events = await getEventPayloads(
driver,
mockedEndpoints as MockedEndpoint[],
);
assert.equal(events.length, 2);

await driver.clickElementAndWaitToDisappear(
'.mm-box button[aria-label="Close"]',
);
await driver.clickElement(selectors.accountOptionsMenuButton);
await driver.clickElement(selectors.globalMenuSettingsButton);
await driver.clickElement(selectors.securityAndPrivacySettings);

const deleteMetaMetricsDataButtonRefreshed = await driver.findElement(
selectors.deleteMetaMetricsDataButton,
);
await (
deleteMetaMetricsDataButtonRefreshed as WebElementWithWaitForElementState
).waitForElementState('disabled');
},
);
});

it('when the user has never opted in for metrics', async function () {
await withFixtures(
{
Expand All @@ -230,7 +197,6 @@ describe('Delete MetaMetrics Data @no-mmi', function (this: Suite) {
await driver.clickElement(selectors.accountOptionsMenuButton);
await driver.clickElement(selectors.globalMenuSettingsButton);
await driver.clickElement(selectors.securityAndPrivacySettings);
await driver.findElement(selectors.deletMetaMetricsSettings);

const deleteMetaMetricsDataButton = await driver.findElement(
selectors.deleteMetaMetricsDataButton,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@
},
"QueuedRequestController": { "queuedRequestCount": 0 },
"RemoteFeatureFlagController": {
"remoteFeatureFlags": { "feature1": true, "feature2": false },
"remoteFeatureFlags": {},
"cacheTimestamp": "number"
},
"SelectedNetworkController": { "domains": "object" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,65 +6,65 @@
"isAccountMenuOpen": false,
"isNetworkMenuOpen": false,
"nextNonce": null,
"pendingTokens": "object",
"welcomeScreenSeen": false,
"confirmationExchangeRates": {},
"ledgerTransportStatus": "string",
"ledgerWebHidConnectedStatus": "string",
"loadingMessage": "undefined",
"shouldClose": "boolean",
"menuOpen": "boolean",
"modal": "object",
"alertOpen": "boolean",
"alertMessage": null,
"qrCodeData": null,
"networkDropdownOpen": "boolean",
"importNftsModal": "object",
"showPermittedNetworkToastOpen": "boolean",
"showIpfsModalOpen": "boolean",
"showBasicFunctionalityModal": "boolean",
"externalServicesOnboardingToggleState": "boolean",
"keyringRemovalSnapModal": "object",
"showKeyringRemovalSnapModal": "boolean",
"importTokensModalOpen": "boolean",
"deprecatedNetworkModalOpen": "boolean",
"accountDetail": "object",
"isLoading": "boolean",
"isNftStillFetchingIndication": "boolean",
"showNftDetectionEnablementToast": "boolean",
"loadingMessage": "undefined",
"warning": "string",
"buyView": "object",
"defaultHdPaths": "object",
"networksTabSelectedRpcUrl": "string",
"requestAccountTabs": "object",
"openMetaMaskTabs": "object",
"currentWindowTab": "object",
"showWhatsNewPopup": "boolean",
"showTermsOfUsePopup": "boolean",
"singleExceptions": "object",
"gasLoadingAnimationIsShowing": "boolean",
"externalServicesOnboardingToggleState": "boolean",
"smartTransactionsError": null,
"smartTransactionsErrorMessageDismissed": "boolean",
"ledgerWebHidConnectedStatus": "string",
"ledgerTransportStatus": "string",
"newNftAddedMessage": "string",
"removeNftMessage": "string",
"newNetworkAddedName": "string",
"editedNetwork": "undefined",
"newNetworkAddedConfigurationId": "string",
"selectedNetworkConfigurationId": "string",
"sendInputCurrencySwitched": "boolean",
"newTokensImported": "string",
"newTokensImportedError": "string",
"onboardedInThisUISession": "boolean",
"customTokenAmount": "string",
"accountDetailsAddress": "string",
"isAddingNewNetwork": "boolean",
"isMultiRpcOnboarding": "boolean",
"errorInSettings": null,
"openMetaMaskTabs": "object",
"pendingTokens": "object",
"qrCodeData": null,
"selectedNetworkConfigurationId": "string",
"scrollToBottom": "boolean",
"sendInputCurrencySwitched": "boolean",
"shouldClose": "boolean",
"showBasicFunctionalityModal": "boolean",
"showDataDeletionErrorModal": "boolean",
"txId": null,
"accountDetailsAddress": "string",
"showDeleteMetaMetricsDataModal": "boolean",
"showIpfsModalOpen": "boolean",
"showKeyringRemovalSnapModal": "boolean",
"showNftDetectionEnablementToast": "boolean",
"showPermittedNetworkToastOpen": "boolean",
"showTermsOfUsePopup": "boolean",
"showWhatsNewPopup": "boolean",
"singleExceptions": "object",
"smartTransactionsError": null,
"smartTransactionsErrorMessageDismissed": "boolean",
"showDataDeletionErrorModal": "boolean",
"snapsInstallPrivacyWarningShown": "boolean",
"txId": null,
"warning": "string",
"welcomeScreenSeen": false
"isAddingNewNetwork": "boolean",
"isMultiRpcOnboarding": "boolean",
"errorInSettings": null
},
"bridge": "object",
"confirmAlerts": "object",
Expand Down Expand Up @@ -289,7 +289,7 @@
"isCheckingAccountsPresence": "boolean",
"queuedRequestCount": 0,
"fcmToken": "string",
"remoteFeatureFlags": { "feature1": true, "feature2": false },
"remoteFeatureFlags": {},
"cacheTimestamp": "number",
"accounts": "object",
"accountsByChainId": "object",
Expand Down
15 changes: 9 additions & 6 deletions test/e2e/tests/portfolio/portfolio-site.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { MockttpServer } from 'mockttp';
import { withFixtures } from '../../helpers';
import { EMPTY_E2E_TEST_PAGE_TITLE } from '../../constants';
import { PORTFOLIO_PAGE_TITLE, MOCK_META_METRICS_ID } from '../../constants';
import FixtureBuilder from '../../fixture-builder';
import { emptyHtmlPage } from '../../mock-e2e';
import HomePage from '../../page-objects/pages/home/homepage';
Expand All @@ -26,20 +26,23 @@ describe('Portfolio site', function () {
await withFixtures(
{
dapp: true,
fixtures: new FixtureBuilder().build(),
fixtures: new FixtureBuilder()
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
participateInMetaMetrics: true,
})
.build(),
title: this.test?.fullTitle(),
testSpecificMock: mockPortfolioSite,
},
async ({ driver }) => {
await loginWithBalanceValidation(driver);
await new HomePage(driver).openPortfolioPage();

// Click Portfolio site
await driver.switchToWindowWithTitle(EMPTY_E2E_TEST_PAGE_TITLE);
await driver.switchToWindowWithTitle(PORTFOLIO_PAGE_TITLE);

// Verify site
await driver.waitForUrl({
url: 'https://portfolio.metamask.io/?metamaskEntry=ext_portfolio_button&metametricsId=null&metricsEnabled=false&marketingEnabled=false',
url: `https://portfolio.metamask.io/?metamaskEntry=ext_portfolio_button&metametricsId=${MOCK_META_METRICS_ID}&metricsEnabled=true&marketingEnabled=false`,
});
},
);
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/tests/remote-feature-flag/mock-data.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
export const MOCK_REMOTE_FEATURE_FLAGS_RESPONSE = {
feature1: true,
feature2: false,
feature3: {
name: 'groupC',
value: 'valueC',
},
};
10 changes: 8 additions & 2 deletions test/e2e/tests/remote-feature-flag/remote-feature-flag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@ import { getCleanAppState, withFixtures } from '../../helpers';
import FixtureBuilder from '../../fixture-builder';
import { TestSuiteArguments } from '../confirmations/transactions/shared';
import { loginWithBalanceValidation } from '../../page-objects/flows/login.flow';
import { MOCK_META_METRICS_ID } from '../../constants';
import { MOCK_REMOTE_FEATURE_FLAGS_RESPONSE } from './mock-data';

describe('Remote feature flag', function (this: Suite) {
it('should be fetched when basic functionality toggle is on', async function () {
it('should be fetched with threshold value when basic functionality toggle is on', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder().build(),
fixtures: new FixtureBuilder()
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
participateInMetaMetrics: true,
})
.build(),
title: this.test?.fullTitle(),
},
async ({ driver }: TestSuiteArguments) => {
Expand Down
Loading

0 comments on commit 5b5c04a

Please sign in to comment.