Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure has_marketing_consent user trait are correctly set and sent after participateInMetaMetrics is set to true #29871

Merged
merged 14 commits into from
Feb 6, 2025
Merged
12 changes: 8 additions & 4 deletions app/scripts/controllers/metametrics-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ export default class MetaMetricsController extends BaseController<
),
};

if (!previousUserTraits) {
if (!previousUserTraits && metamaskState.participateInMetaMetrics) {
this.update((state) => {
state.previousUserTraits = currentTraits;
});
Expand All @@ -1252,9 +1252,13 @@ export default class MetaMetricsController extends BaseController<
const previous = previousUserTraits[k];
return !isEqual(previous, v);
});
this.update((state) => {
state.previousUserTraits = currentTraits;
});

if (metamaskState.participateInMetaMetrics) {
this.update((state) => {
state.previousUserTraits = currentTraits;
});
}

return updates;
}

Expand Down
10 changes: 10 additions & 0 deletions test/e2e/page-objects/flows/onboarding.flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,20 @@ import { E2E_SRP } from '../../default-fixture';
* @param [options.password] - The password to create. Defaults to WALLET_PASSWORD.
* @param [options.participateInMetaMetrics] - Whether to participate in MetaMetrics. Defaults to false.
* @param [options.needNavigateToNewPage] - Indicates whether to navigate to a new page before starting the onboarding flow. Defaults to true.
* @param [options.dataCollectionForMarketing] - Whether to opt in to data collection for marketing. Defaults to false.
*/
export const createNewWalletOnboardingFlow = async ({
driver,
password = WALLET_PASSWORD,
participateInMetaMetrics = false,
needNavigateToNewPage = true,
dataCollectionForMarketing = false,
}: {
driver: Driver;
password?: string;
participateInMetaMetrics?: boolean;
needNavigateToNewPage?: boolean;
dataCollectionForMarketing?: boolean;
}): Promise<void> => {
console.log('Starting the creation of a new wallet onboarding flow');
if (needNavigateToNewPage) {
Expand All @@ -40,6 +43,9 @@ export const createNewWalletOnboardingFlow = async ({

const onboardingMetricsPage = new OnboardingMetricsPage(driver);
await onboardingMetricsPage.check_pageIsLoaded();
if (dataCollectionForMarketing) {
await onboardingMetricsPage.clickDataCollectionForMarketingCheckbox();
}
if (participateInMetaMetrics) {
await onboardingMetricsPage.clickIAgreeButton();
} else {
Expand Down Expand Up @@ -109,24 +115,28 @@ export const importSRPOnboardingFlow = async ({
* @param [options.password] - The password to use. Defaults to WALLET_PASSWORD.
* @param [options.participateInMetaMetrics] - Whether to participate in MetaMetrics. Defaults to false.
* @param [options.needNavigateToNewPage] - Indicates whether to navigate to a new page before starting the onboarding flow. Defaults to true.
* @param [options.dataCollectionForMarketing] - Whether to opt in to data collection for marketing. Defaults to false.
*/
export const completeCreateNewWalletOnboardingFlow = async ({
driver,
password = WALLET_PASSWORD,
participateInMetaMetrics = false,
needNavigateToNewPage = true,
dataCollectionForMarketing = false,
}: {
driver: Driver;
password?: string;
participateInMetaMetrics?: boolean;
needNavigateToNewPage?: boolean;
dataCollectionForMarketing?: boolean;
}): Promise<void> => {
console.log('start to complete create new wallet onboarding flow ');
await createNewWalletOnboardingFlow({
driver,
password,
participateInMetaMetrics,
needNavigateToNewPage,
dataCollectionForMarketing,
});
const onboardingCompletePage = new OnboardingCompletePage(driver);
await onboardingCompletePage.check_pageIsLoaded();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ class OnboardingMetricsPage {

private readonly iAgreeButton = '[data-testid="metametrics-i-agree"]';

private readonly dataCollectionForMarketingCheckbox =
'[data-testid="metametrics-data-collection-checkbox"]';

private readonly metametricsMessage = {
text: 'Help us improve MetaMask',
tag: 'h2',
Expand Down Expand Up @@ -39,6 +42,10 @@ class OnboardingMetricsPage {
async clickIAgreeButton(): Promise<void> {
await this.driver.clickElementAndWaitToDisappear(this.iAgreeButton);
}

async clickDataCollectionForMarketingCheckbox(): Promise<void> {
await this.driver.clickElement(this.dataCollectionForMarketingCheckbox);
}
}

export default OnboardingMetricsPage;
20 changes: 20 additions & 0 deletions test/e2e/page-objects/pages/settings/privacy-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ class PrivacySettings {

private readonly revealSrpWrongPasswordMessage = '.mm-help-text';

private readonly participateInMetaMetricsToggle =
'[data-testid="participate-in-meta-metrics-toggle"] .toggle-button';

private readonly dataCollectionForMarketingToggle =
'[data-testid="data-collection-for-marketing-toggle"] .toggle-button';

constructor(driver: Driver) {
this.driver = driver;
}
Expand Down Expand Up @@ -212,6 +218,20 @@ class PrivacySettings {
text: expectedSrpText,
});
}

async toggleParticipateInMetaMetrics(): Promise<void> {
console.log(
'Toggle participate in meta metrics in Security and Privacy settings page',
);
await this.driver.clickElement(this.participateInMetaMetricsToggle);
}

async toggleDataCollectionForMarketing(): Promise<void> {
console.log(
'Toggle data collection for marketing in Security and Privacy settings page',
);
await this.driver.clickElement(this.dataCollectionForMarketingToggle);
}
}

export default PrivacySettings;
2 changes: 1 addition & 1 deletion test/e2e/tests/metrics/marketing-cookieid.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const selectors = {
globalMenuSettingsButton: '[data-testid="global-menu-settings"]',
securityAndPrivacySettings: { text: 'Security & privacy', tag: 'div' },
dataCollectionForMarketingToggle:
'[data-testid="dataCollectionForMarketing"] .toggle-button',
'[data-testid="data-collection-for-marketing-toggle"] .toggle-button',
dataCollectionWarningAckButton: { text: 'Okay', tag: 'Button' },
};

Expand Down
4 changes: 2 additions & 2 deletions test/e2e/tests/metrics/metametrics-persistence.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('MetaMetrics ID persistence', function () {

// toggle off
await driver.clickElement(
'[data-testid="participateInMetaMetrics"] .toggle-button',
'[data-testid="participate-in-meta-metrics-toggle"] .toggle-button',
);

// wait for state to update
Expand All @@ -65,7 +65,7 @@ describe('MetaMetrics ID persistence', function () {

// toggle back on
await driver.clickElement(
'[data-testid="participateInMetaMetrics"] .toggle-button',
'[data-testid="participate-in-meta-metrics-toggle"] .toggle-button',
);

// wait for state to update
Expand Down
176 changes: 176 additions & 0 deletions test/e2e/tests/metrics/segment-user-traits.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
import { strict as assert } from 'assert';
import { Mockttp } from 'mockttp';
import { getEventPayloads, withFixtures } from '../../helpers';
import FixtureBuilder from '../../fixture-builder';
import {
completeCreateNewWalletOnboardingFlow,
createNewWalletOnboardingFlow,
} from '../../page-objects/flows/onboarding.flow';
import { MOCK_META_METRICS_ID } from '../../constants';
import HeaderNavbar from "../../page-objects/pages/header-navbar";

Check failure on line 10 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `"../../page-objects/pages/header-navbar"` with `'../../page-objects/pages/header-navbar'`
import SettingsPage from "../../page-objects/pages/settings/settings-page";

Check failure on line 11 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `"../../page-objects/pages/settings/settings-page"` with `'../../page-objects/pages/settings/settings-page'`
import PrivacySettings from "../../page-objects/pages/settings/privacy-settings";

Check failure on line 12 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `"../../page-objects/pages/settings/privacy-settings"` with `'../../page-objects/pages/settings/privacy-settings'`

async function mockSegment(mockServer: Mockttp) {
return [
await mockServer
.forPost('https://api.segment.io/v1/batch')
.withJsonBodyIncluding({
batch: [{ type: 'identify' }],
})
.thenCallback(() => {
return {
statusCode: 200,
};
}),
];
}

describe('Segment User Traits', function () {
it('sends identify event when user opts in both metrics and data collection during onboarding', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder({onboarding: true})

Check failure on line 33 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `onboarding:·true` with `·onboarding:·true·`
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
})
.build(),
title: this.test?.fullTitle(),
testSpecificMock: mockSegment,
},
async ({driver, mockedEndpoint: mockedEndpoints}) => {

Check failure on line 41 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `driver,·mockedEndpoint:·mockedEndpoints` with `·driver,·mockedEndpoint:·mockedEndpoints·`
await createNewWalletOnboardingFlow({
driver,
participateInMetaMetrics: true,
dataCollectionForMarketing: true,
});
const events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 1);
assert.deepStrictEqual(events[0].traits.is_metrics_opted_in, true);
assert.deepStrictEqual(events[0].traits.has_marketing_consent, true);
},
);
});

it('sends identify event when user opts into metrics but not data collection during onboarding', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder({onboarding: true})

Check failure on line 58 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `onboarding:·true` with `·onboarding:·true·`
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
})
.build(),
title: this.test?.fullTitle(),
testSpecificMock: mockSegment,
},
async ({driver, mockedEndpoint: mockedEndpoints}) => {

Check failure on line 66 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `driver,·mockedEndpoint:·mockedEndpoints` with `·driver,·mockedEndpoint:·mockedEndpoints·`
await createNewWalletOnboardingFlow({
driver,
participateInMetaMetrics: true,
dataCollectionForMarketing: false,
});
const events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 1);
assert.deepStrictEqual(events[0].traits.is_metrics_opted_in, true);
assert.deepStrictEqual(events[0].traits.has_marketing_consent, false);
},
);
});

it('will not send identify event when user opts out of both metrics and data collection during onboarding', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder({onboarding: true})

Check failure on line 83 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `onboarding:·true` with `·onboarding:·true·`
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
participateInMetaMetrics: true,
})
.build(),
title: this.test?.fullTitle(),
testSpecificMock: mockSegment,
},
async ({driver, mockedEndpoint: mockedEndpoints}) => {

Check failure on line 92 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `driver,·mockedEndpoint:·mockedEndpoints` with `·driver,·mockedEndpoint:·mockedEndpoints·`
await createNewWalletOnboardingFlow({
driver,
participateInMetaMetrics: false,
dataCollectionForMarketing: false,
});
const events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 0);
},
);
});

it('sends identify event when user enables metrics in privacy settings after opting out during onboarding', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder({onboarding: true})

Check failure on line 107 in test/e2e/tests/metrics/segment-user-traits.spec.ts

View workflow job for this annotation

GitHub Actions / Test lint / Test lint

Replace `onboarding:·true` with `·onboarding:·true·`
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
participateInMetaMetrics: false,
})
.build(),
title: this.test?.fullTitle(),
testSpecificMock: mockSegment,
},
async ({driver, mockedEndpoint: mockedEndpoints}) => {
let events = [];
await completeCreateNewWalletOnboardingFlow({
driver,
participateInMetaMetrics: false,
});
events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 0);
await new HeaderNavbar(driver).openSettingsPage();
const settingsPage = new SettingsPage(driver);
await settingsPage.check_pageIsLoaded();
await settingsPage.goToPrivacySettings();

const privacySettings = new PrivacySettings(driver);
await privacySettings.check_pageIsLoaded();
await privacySettings.toggleParticipateInMetaMetrics();
events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 1);
assert.deepStrictEqual(events[0].traits.is_metrics_opted_in, true);
assert.deepStrictEqual(events[0].traits.has_marketing_consent, false);
},
);
});

it('sends identify event when user opts in both metrics and data in privacy settings after opting out during onboarding', async function () {
await withFixtures(
{
fixtures: new FixtureBuilder({onboarding: true})
.withMetaMetricsController({
metaMetricsId: MOCK_META_METRICS_ID,
participateInMetaMetrics: false,
})
.build(),
title: this.test?.fullTitle(),
testSpecificMock: mockSegment,
},
async ({driver, mockedEndpoint: mockedEndpoints}) => {
let events = [];
await completeCreateNewWalletOnboardingFlow({
driver,
participateInMetaMetrics: false,
});
events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 0);
await new HeaderNavbar(driver).openSettingsPage();
const settingsPage = new SettingsPage(driver);
await settingsPage.check_pageIsLoaded();
await settingsPage.goToPrivacySettings();

const privacySettings = new PrivacySettings(driver);
await privacySettings.check_pageIsLoaded();
await privacySettings.toggleParticipateInMetaMetrics();
await privacySettings.toggleDataCollectionForMarketing();
events = await getEventPayloads(driver, mockedEndpoints);
assert.equal(events.length, 1);
assert.deepStrictEqual(events[0].traits.is_metrics_opted_in, true);
assert.deepStrictEqual(events[0].traits.has_marketing_consent, true);
},
);
});
})
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ exports[`Onboarding Metametrics Component should match snapshot 1`] = `
</ul>
<label
class="mm-box mm-text mm-checkbox mm-text--body-md mm-box--padding-bottom-3 mm-box--display-inline-flex mm-box--align-items-center mm-box--color-text-default"
data-testid="metametrics-data-collection-checkbox"
for="metametrics-opt-in"
>
<span
Expand Down Expand Up @@ -256,6 +257,7 @@ exports[`Onboarding Metametrics Component should match snapshot after new policy
</ul>
<label
class="mm-box mm-text mm-checkbox mm-text--body-md mm-box--padding-bottom-3 mm-box--display-inline-flex mm-box--align-items-center mm-box--color-text-default"
data-testid="metametrics-data-collection-checkbox"
for="metametrics-opt-in"
>
<span
Expand Down
1 change: 1 addition & 0 deletions ui/pages/onboarding-flow/metametrics/metametrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export default function OnboardingMetametrics() {
</ul>
<Checkbox
id="metametrics-opt-in"
data-testid="metametrics-data-collection-checkbox"
isChecked={dataCollectionForMarketing}
onClick={() =>
dispatch(setDataCollectionForMarketing(!dataCollectionForMarketing))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ exports[`Security Tab should match snapshot 1`] = `
</div>
<div
class="settings-page__content-item-col"
data-testid="participateInMetaMetrics"
data-testid="participate-in-meta-metrics-toggle"
>
<label
class="toggle-button toggle-button--off"
Expand All @@ -1469,7 +1469,7 @@ exports[`Security Tab should match snapshot 1`] = `
/>
</div>
<input
data-testid="toggleButton"
data-testid="participate-in-meta-metrics-toggle-button"
style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
type="checkbox"
value="false"
Expand Down Expand Up @@ -1512,7 +1512,7 @@ exports[`Security Tab should match snapshot 1`] = `
</div>
<div
class="settings-page__content-item-col"
data-testid="dataCollectionForMarketing"
data-testid="data-collection-for-marketing-toggle"
>
<label
class="toggle-button toggle-button--off"
Expand Down
Loading
Loading