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

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Jan 23, 2025

Description

It was observed in production metrics that the has_marketing_consent user trait was not set (either null or undefined) for 40-50% of users. Meanwhile, another ~40% of users had it set to false, and the remainder had it set to true.

The problem was in the metametrics controller. The controller sets state.previousUserTraits in the _buildUserTraitsObject function, which gets called when there are state updates. That function also uses state.previousUserTraits to determine which values need to be returned, by comparing it to the new currentTraits.

The problem is that if a trait is set while participateInMetaMetrics is false, then the trait will not be sent in a request to Segment, BUT the trait will be saved in state.previousUserTraits. Later, if participateInMetaMetrics is set to true, _buildUserTraitsObject will not return a trait currently matches state.previousUserTraits

The has_marketing_consent trait is set to true when the user checks the associated checkbox on the metametrics screen of the onboarding flow, but then if the user clicks confirm/yes on that screen to opt-in to metametrics, the aforementioned problem is hit.

To solve this, we just ensure that state.previousUserTraits is not saved if participateInMetaMetrics is false.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3932

Manual testing steps

  1. Install a build from this branch
  2. Open the background console and the network tab
  3. Start onboarding. On the metametrics screen, click the checkbox and then "I agree"
  4. The network tab should now include a request to segment with user traits, where has_marketing_consent is set to true

--

  1. Follow the above steps on the v12.9.3 build (step 4 will fail)
  2. Update the version of that install to the build from this branch
  3. Log in.
  4. The network tab should now include a request to segment with user traits, where has_marketing_consent is set to true

--

  1. Install a build from this branch
  2. Open the background console and the network tab
  3. Start onboarding. On the metametrics screen, DO NOT click the checkbox and then click "No thanks"
  4. The network tab should not include a request to segment
  5. Complete onboarding and go to settings and toggle "Participate in MetaMetrics" to true
  6. The network tab tab should now include a request to segment with user traits, where has_marketing_consent is set to false. If the marketing consent toggle is turned on, there should then be a network request where the user trait is true

Important

This requires manual test from QA.

  • Create a new user and click both "I agree" and "we'll use this data... " when user is on "help us improve Metamask" page
  • There should be 1 request to segment with user traits, where both has_marketing_consent and participate_in_metametrics set to true.
  • Proceed on onboarding
  • Head to privacy page in settings
  • Opt out "participate in metametrics"
  • There's no more request to segment after

Screenshots/Recordings

Copy.text.Untitled.Project.mp4

Pre-merge author checklist

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.

Copy link
Contributor

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.

@metamaskbot metamaskbot added the team-extension-platform Extension Platform team label Jan 23, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [a7887e3]
Page Load Metrics (1721 ± 86 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14922010172217684
domContentLoaded14761994169817584
load14882017172117986
domInteractive246939136
backgroundConnect86622136
firstReactRender1576382311
getState55818199
initialActions01000
loadScripts10281506124714670
setupStore55612147
uiStartup16672296197520196
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 56 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@danjm danjm changed the title Ensure user traits set before opting in to participate in metametrics… fix: Ensure user traits set before opting in to participate in metametrics… Jan 29, 2025
@DDDDDanica DDDDDanica force-pushed the user-trait-fix branch 6 times, most recently from 5ff1ff4 to 2d5dd49 Compare January 30, 2025 02:53
@metamaskbot
Copy link
Collaborator

Builds ready [2d5dd49]
Page Load Metrics (1794 ± 71 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15312147179014871
domContentLoaded15192055176313766
load15312148179414971
domInteractive25142422813
backgroundConnect1086302210
firstReactRender16100452713
getState496192512
initialActions01000
loadScripts10691517128411857
setupStore884302512
uiStartup175728872089257123
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 56 Bytes (0.00%)
  • ui: 102 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@danjm danjm changed the title fix: Ensure user traits set before opting in to participate in metametrics… fix: Ensure has_marketing_consent user traits are correctly set and sent Jan 30, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [a4a3cb0]
Page Load Metrics (1847 ± 94 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint40324391777377181
domContentLoaded15802338182418689
load15852354184719694
domInteractive25103562813
backgroundConnect981252110
firstReactRender1996512412
getState478252612
initialActions01000
loadScripts11441827134416881
setupStore889172010
uiStartup183926992190257124
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 56 Bytes (0.00%)
  • ui: 102 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@DDDDDanica DDDDDanica changed the title fix: Ensure has_marketing_consent user traits are correctly set and sent fix: Ensure has_marketing_consent user traits are correctly set and sent get properly updated upon participate in metametrics Jan 30, 2025
@danjm danjm changed the title fix: Ensure has_marketing_consent user traits are correctly set and sent get properly updated upon participate in metametrics fix: Ensure has_marketing_consent user trait are correctly set and sent after participateInMetaMetrics is set to true Jan 30, 2025
@DDDDDanica DDDDDanica marked this pull request as ready for review January 30, 2025 15:14
@DDDDDanica DDDDDanica requested a review from a team as a code owner January 30, 2025 15:14
DDDDDanica
DDDDDanica previously approved these changes Jan 30, 2025
NidhiKJha
NidhiKJha previously approved these changes Jan 31, 2025
@danjm danjm added this pull request to the merge queue Jan 31, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 31, 2025
@danjm danjm added this pull request to the merge queue Jan 31, 2025
@danjm danjm removed this pull request from the merge queue due to a manual request Jan 31, 2025
@DDDDDanica DDDDDanica dismissed stale reviews from NidhiKJha and themself via 6ca1707 February 3, 2025 22:19
@DDDDDanica DDDDDanica force-pushed the user-trait-fix branch 8 times, most recently from 51e0c13 to 89a899e Compare February 5, 2025 15:40
DDDDDanica
DDDDDanica previously approved these changes Feb 5, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [e7f67d5]
Page Load Metrics (1749 ± 97 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint42222401687355170
domContentLoaded14262207171220498
load14872240174920397
domInteractive2592422110
backgroundConnect1065382110
firstReactRender1696382713
getState65415157
initialActions01000
loadScripts10031711124416579
setupStore85414136
uiStartup172825582011235113
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 56 Bytes (0.00%)
  • ui: 102 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [95087c2]
Page Load Metrics (1497 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint13571756149810349
domContentLoaded1348166714769244
load13561725149710249
domInteractive227228115
backgroundConnect86420188
firstReactRender1497252210
getState37115199
initialActions01000
loadScripts920124110438541
setupStore65216167
uiStartup15122218170515876
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 56 Bytes (0.00%)
  • ui: 102 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@chloeYue
Copy link
Contributor

chloeYue commented Feb 6, 2025

QA manual testing passed !

@DDDDDanica DDDDDanica added this pull request to the merge queue Feb 6, 2025
Merged via the queue into main with commit c4aa6a9 Feb 6, 2025
74 checks passed
@DDDDDanica DDDDDanica deleted the user-trait-fix branch February 6, 2025 12:59
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2025
@metamaskbot metamaskbot added the release-12.13.0 Issue or pull request that will be included in release 12.13.0 label Feb 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-attribution QA Passed release-12.13.0 Issue or pull request that will be included in release 12.13.0 team-extension-platform Extension Platform team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants