feat: adds "data collection for marketing" toggles #24605

jonybur · 2024-05-20T09:37:20Z

Adds data collection for marketing toggles (and toasts/warnings) on:

Onboarding
Toast in Wallet
Settings page

Description

Related issues

Fixes:
https://github.com/MetaMask/MetaMask-planning/issues/2437
https://github.com/MetaMask/MetaMask-planning/issues/2438
https://github.com/MetaMask/MetaMask-planning/issues/2526

Manual testing steps

Onboarding checkbox:
Make the metametrics.js to return renderOnboarding instead of renderLegacyOnboarding

Start a new account
There should be a new checkbox that asks for marketing consent
Checking it should set the marketing consent to true (check at Settings, Securty tab page)

Security tab:

Go to Security tab
When checking the "Data collection for marketing" to true, the "Participate in MetaMetrics" toggle should turn to true
When checking "Participate in MetaMetrics" to false, "Data collection for marketing" should be set to false
When "Participate in Metametrics" is true and "Data collection for marketing" is true, and the latter is set to false, a warning message should appear.

Toast:
An already onboarded user will have the "dataCollectionForMarketing" value as null (neither true or false). This will trigger the toast.

By clicking on "I accept", it should set the "Data collection for marketing" to true.
By closing the toast or clicking on "No thanks", it should set the "Data collection for marketing" to false.

All of these actions should trigger subsequent Segment events.

Screenshots/Recordings

Before

After

demo2.mp4

Pre-merge author checklist

I’ve followed MetaMask Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

…sk/metamask-extension into jb-opt-in-metrics

…k-extension into jb-opt-in-metrics

…mask-extension into jb-opt-in-metrics

github-actions · 2024-05-20T09:37:32Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

NidhiKJha

Really good start but the checkbox don't show up on this page when I try to create a new account. Also, left some comments

ui/pages/home/home.component.js

ui/pages/settings/security-tab/security-tab.component.js

jonybur · 2024-05-21T16:26:50Z

@NidhiKJha you will need to render the new onboarding screen, right now it's hidden through the privacy policy date. Just make the metametrics.js to return renderOnboarding instead of renderLegacyOnboarding

Gudahtt · 2024-06-07T21:15:42Z

ui/store/actions.ts

+  return (dispatch: MetaMaskReduxDispatch) => {
+    log.debug(`background.setDataCollectionForMarketing`);
+    return new Promise((resolve, reject) => {
+      callBackgroundMethod<string>(


This function is deprecated, please use submitRequestToBackground instead

Gudahtt · 2024-06-07T21:21:37Z

ui/store/actions.ts

+        (err, metaMetricsId) => {
+          log.debug(err);
+          if (err) {
+            dispatch(displayWarning(err));


displayWarning is deprecated, please find some other way of handling this error. This method will display the error totally out of context in random places in the app, and it'll stick around for a seemingly arbitrary amount of time. Bad UX.

Once this is converted to use submitRequestToBackground, this error variable won't be here unless we add a try...catch block to catch it. We may or may not want to do that, depending on when we expect this to fail.

If it should never fail, I'd recommend not catching the error and letting it bubble up to the global error handler to be captured by Sentry.

If we'd expect it to fail sometimes, then we should catch it. If the failure condition is relevant to the user (e.g. if an operation failed and the user should be informed), then we'll have to think about where in the UI to best display this error. Maybe easiest to throw here and catch it in a UI component where we can show the error.

If we expect it to fail sometimes in a way that isn't important to a user, then we can just log the error to the console and move on.

Gudahtt · 2024-06-07T21:25:21Z

ui/store/actions.ts

+           * changed. The logic to determine which way to toggle is in the
+           * toggleSession handler in setupSentry.js.
+           */
+          window.sentry?.toggleSession();


This doesn't seem necessary; this toggle doesn't impact Sentry. Only the overall meta metrics toggle impacts Sentry.

Gudahtt · 2024-06-07T21:38:14Z

ui/ducks/metamask/metamask.js

@@ -162,6 +163,12 @@ export default function reduceMetamask(state = initialState, action) {
        participateInMetaMetrics: action.value,
      };

+    case actionConstants.SET_DATA_COLLECTION_FOR_MARKETING:


What is this for? Generally we shouldn't adding anything to this Redux slice, it's meant as a mirror of the background state. The actions here other than the background state update action are all examples of technical debt that we have not cleaned up yet.

This is to mirror the implementation of SET_PARTICIPATE_IN_METAMETRICS. We are adding the marketing toggle flag to the state as well (dataCollectionForMarketing)

This seems like a misunderstanding. SET_PARTICIPATE_IN_METAMETRICS shouldn't be here either

These are good points, and these changes will be postponed for a later date given our time constraints. Both SET_PARTICIPATE_IN_METAMETRICS and SET_DATA_COLLECTION_FOR_MARKETING should be deleted from the redux state.

metamaskbot · 2024-06-10T17:11:24Z

Builds ready [e27aef2]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (49 ± 4 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	61	100	80	10	5
		domContentLoaded	9	26	11	3	2
		load	40	70	49	7	4
		domInteractive	9	26	11	3	2

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 406 Bytes (0.01%)
ui: 6.51 KiB (0.10%)
common: 2.45 KiB (0.04%)

codecov · 2024-06-10T17:11:30Z

Codecov Report

Attention: Patch coverage is 48.31461% with 46 lines in your changes missing coverage. Please review.

Project coverage is 65.60%. Comparing base (54461ab) to head (e27aef2).
Report is 23 commits behind head on develop.

❗ Current head e27aef2 differs from pull request most recent head 0dc9878

Please upload reports for the commit 0dc9878 to get more accurate results.

Files	Patch %	Lines
...es/settings/security-tab/security-tab.component.js	44.00%	14 Missing ⚠️
ui/pages/home/home.component.js	18.75%	13 Missing ⚠️
ui/store/actions.ts	14.29%	6 Missing ⚠️
ui/ducks/metamask/metamask.js	0.00%	4 Missing ⚠️
...i/pages/onboarding-flow/metametrics/metametrics.js	55.56%	4 Missing ⚠️
app/scripts/controllers/metametrics.js	66.67%	2 Missing ⚠️
ui/pages/home/home.container.js	0.00%	1 Missing ⚠️
...rity-tab/metametrics-toggle/metametrics-toggle.tsx	80.00%	1 Missing ⚠️
...es/settings/security-tab/security-tab.container.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #24605      +/-   ##
===========================================
- Coverage    65.64%   65.60%   -0.04%     
===========================================
  Files         1362     1362              
  Lines        54189    54265      +76     
  Branches     14112    14127      +15     
===========================================
+ Hits         35572    35600      +28     
- Misses       18617    18665      +48

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

darkwing

Marking as RC until Mark's item is addressed: #24605 (review)

jonybur · 2024-06-11T11:58:50Z

@darkwing comments by @Gudahtt have been addressed

Resolved

app/_locales/en/messages.json

builds.yml

ui/pages/home/home.component.js

metamaskbot · 2024-06-12T00:39:55Z

Builds ready [0dc9878]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (46 ± 5 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	63	118	79	12	6
		domContentLoaded	9	13	10	1	1
		load	39	86	46	10	5
		domInteractive	9	13	10	1	1

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 406 Bytes (0.01%)
ui: 6.51 KiB (0.10%)
common: 2.53 KiB (0.04%)

bergeron

I think I tested all combinations successfully.

New user:
- metrics off, marketing off
- metrics on, marketing off
- metrics on, marketing on
existing user:
- metrics off (don't see modal)
- metrics on, marketing off
- metrics on, marketing on

Just make sure we resolve this slack thread to be sure nothing is broken:

https://consensys.slack.com/archives/C06VBTL358A/p1718150795110559

jonybur and others added 10 commits May 16, 2024 16:17

Add metametrics toast when onboarded

86cde15

Add link

288b56c

Add toggle

581a2ce

Add popover

a91a256

feat: add consent checkbox to onboarding metametrics page

1413cb8

Merge branch 'develop' into feat/2526-update-consent-checkbox

f726ff5

test: add cleanup to timer dependent test

2322921

Merge branch 'feat/2526-update-consent-checkbox' of github.com:MetaMa…

a0cef9f

…sk/metamask-extension into jb-opt-in-metrics

Merge branch 'jb-metametrics-id-toast' of github.com:MetaMask/metamas…

879d4d3

…k-extension into jb-opt-in-metrics

Merge branch 'jb-metametrics-id-settings' of github.com:MetaMask/meta…

e971a28

…mask-extension into jb-opt-in-metrics

jonybur added 5 commits May 20, 2024 13:38

Tie all to state

a5b9333

Add warning message to settings

3b8afb1

Dont trigger wallet message after onboarding

f739f46

Only show popover if participateInMetaMetrics is true

cfc0c66

When toggling marketing on, also toggle participate in metametrics to on

cf3f7c2

jonybur changed the title ~~feat: Add metametrics toggle~~ feat: adds "data collection for marketing" toggles May 20, 2024

jonybur added 2 commits May 21, 2024 00:58

Add segment events

a353506

Refactor tracking event

5b7f1a8

hesterbruikman added the marketing-consent label May 21, 2024

jonybur marked this pull request as ready for review May 21, 2024 12:30

jonybur requested a review from a team as a code owner May 21, 2024 12:30

NidhiKJha added the team-wallet-ux label May 21, 2024

jonybur and others added 2 commits May 21, 2024 13:54

Fix lint

ddeb213

Merge branch 'develop' into jb-opt-in-metrics

702bc6e

NidhiKJha requested changes May 21, 2024

View reviewed changes

vpintorico assigned jonybur May 21, 2024

jonybur mentioned this pull request May 21, 2024

feat: adds strings for the opt in toggles #24686

Closed

7 tasks

Add const

a798922

Gudahtt reviewed Jun 7, 2024

View reviewed changes

Update ui store action method

085e482

jonybur requested a review from Gudahtt June 10, 2024 06:56

Merge branch 'develop' into jb-opt-in-metrics

6be0d81

NidhiKJha previously approved these changes Jun 10, 2024

View reviewed changes

Merge branch 'develop' into jb-opt-in-metrics

e27aef2

darkwing approved these changes Jun 10, 2024

View reviewed changes

darkwing previously requested changes Jun 10, 2024

View reviewed changes

jonybur requested a review from darkwing June 11, 2024 11:58

bergeron reviewed Jun 11, 2024

View reviewed changes

app/_locales/en/messages.json Show resolved Hide resolved

bergeron reviewed Jun 11, 2024

View reviewed changes

app/_locales/en/messages.json Outdated Show resolved Hide resolved

bergeron reviewed Jun 11, 2024

View reviewed changes

builds.yml Outdated Show resolved Hide resolved

bergeron reviewed Jun 11, 2024

View reviewed changes

ui/pages/home/home.component.js Show resolved Hide resolved

Embed link, add localization description

0dc9878

darkwing dismissed NidhiKJha’s stale review via 0dc9878 June 12, 2024 00:05

bergeron approved these changes Jun 12, 2024

View reviewed changes

darkwing approved these changes Jun 12, 2024

View reviewed changes

NidhiKJha approved these changes Jun 12, 2024

View reviewed changes

jonybur merged commit ab695aa into develop Jun 12, 2024
74 of 75 checks passed

jonybur deleted the jb-opt-in-metrics branch June 12, 2024 14:38

github-actions bot locked and limited conversation to collaborators Jun 12, 2024

metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds "data collection for marketing" toggles #24605

feat: adds "data collection for marketing" toggles #24605

jonybur commented May 20, 2024 •

edited

github-actions bot commented May 20, 2024

NidhiKJha left a comment •

edited

jonybur commented May 21, 2024

Gudahtt Jun 7, 2024

jonybur Jun 10, 2024

Gudahtt Jun 7, 2024

jonybur Jun 10, 2024

Gudahtt Jun 7, 2024

jonybur Jun 10, 2024

Gudahtt Jun 7, 2024

jonybur Jun 10, 2024

Gudahtt Jun 10, 2024

jonybur Jun 11, 2024

metamaskbot commented Jun 10, 2024

codecov bot commented Jun 10, 2024 •

edited

darkwing left a comment

jonybur commented Jun 11, 2024

metamaskbot commented Jun 12, 2024

bergeron left a comment

feat: adds "data collection for marketing" toggles #24605

feat: adds "data collection for marketing" toggles #24605

Conversation

jonybur commented May 20, 2024 • edited

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented May 20, 2024

NidhiKJha left a comment • edited

Choose a reason for hiding this comment

jonybur commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metamaskbot commented Jun 10, 2024

codecov bot commented Jun 10, 2024 • edited

Codecov Report

darkwing left a comment

Choose a reason for hiding this comment

jonybur commented Jun 11, 2024

metamaskbot commented Jun 12, 2024

bergeron left a comment

Choose a reason for hiding this comment

jonybur commented May 20, 2024 •

edited

NidhiKJha left a comment •

edited

codecov bot commented Jun 10, 2024 •

edited