-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Remove previousUserTraits from metametrics controller state (#30621
) <!-- 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** <!-- 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? --> Truly solving MetaMask/MetaMask-planning#3932 required us to clear `previousUserTraits` from metametrics controller state. That property just functions as a cache, and we could actually maintain it in memory. The reason we need to clear it is that there are many users who had the `has_marketing_consent` trait populated to `true` when the application had a bug that prevented that trait from being submitted to segment. That bug is fixed, so new users don't hit that problem. However, to correctly track existing users that have `has_marketing_consent` set to true, we need the comparison of previous and current user traits in the `_buildUserTraitsObject` function to fail its equality check. To do that, we are clearing `previousUserTraits` so that, upon update of the extension and when the first metrics event is set to segment, the current user trait values will compare to undefined, and they will all be submitted to segment, including `has_marketing_consent`. [](https://codespaces.new/MetaMask/metamask-extension/pull/30621?quickstart=1) ## **Related issues** Fixes: ## **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 ## **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.
- Loading branch information
](https://codespaces.new/MetaMask/metamask-extension/pull/30621?quickstart=1){kind=link}
Showing
9 changed files
with
106 additions
and
41 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { migrate, version } from './143.1'; | ||
|
|
||
| const oldVersion = 143; | ||
|
|
||
| describe(`migration #${version}`, () => { | ||
| it('updates the version metadata', async () => { | ||
| const oldStorage = { | ||
| meta: { version: oldVersion }, | ||
| data: {}, | ||
| }; | ||
| const newStorage = await migrate(oldStorage); | ||
| expect(newStorage.meta).toStrictEqual({ version }); | ||
| }); | ||
|
|
||
| describe(`migration #${version}`, () => { | ||
| it('removes the previousUserTraits property from MetaMetricsController state and does not remove other properties', async () => { | ||
| const oldStorage = { | ||
| meta: { version: oldVersion }, | ||
| data: { | ||
| MetaMetricsController: { | ||
| previousUserTraits: { test: 123 }, | ||
| foo: 'bar', | ||
| }, | ||
| }, | ||
| }; | ||
| const expectedData = { | ||
| MetaMetricsController: { | ||
| foo: 'bar', | ||
| }, | ||
| }; | ||
| const newStorage = await migrate(oldStorage); | ||
|
|
||
| expect(newStorage.data).toStrictEqual(expectedData); | ||
| }); | ||
|
|
||
| it('has no effect if the previousUserTraits property does not exist', async () => { | ||
| const oldStorage = { | ||
| meta: { version: oldVersion }, | ||
| data: { | ||
| MetaMetricsController: { | ||
| foo: 'bar', | ||
| }, | ||
| }, | ||
| }; | ||
| const expectedData = { | ||
| MetaMetricsController: { | ||
| foo: 'bar', | ||
| }, | ||
| }; | ||
| const newStorage = await migrate(oldStorage); | ||
|
|
||
| expect(newStorage.data).toStrictEqual(expectedData); | ||
| }); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { cloneDeep } from 'lodash'; | ||
|
|
||
| type VersionedData = { | ||
| meta: { version: number }; | ||
| data: Record<string, unknown>; | ||
| }; | ||
|
|
||
| export const version = 143.1; | ||
|
|
||
| /** | ||
| * This migration deletes the `previousUserTraits` property from the MetaMetrics Controller state | ||
| * | ||
| * @param originalVersionedData - Versioned MetaMask extension state, exactly | ||
| * what we persist to dist. | ||
| * @param originalVersionedData.meta - State metadata. | ||
| * @param originalVersionedData.meta.version - The current state version. | ||
| * @param originalVersionedData.data - The persisted MetaMask state, keyed by | ||
| * controller. | ||
| * @returns Updated versioned MetaMask extension state. | ||
| */ | ||
| export async function migrate( | ||
| originalVersionedData: VersionedData, | ||
| ): Promise<VersionedData> { | ||
| const versionedData = cloneDeep(originalVersionedData); | ||
| versionedData.meta.version = version; | ||
| transformState(versionedData.data); | ||
| return versionedData; | ||
| } | ||
|
|
||
| function transformState(state: Record<string, unknown>) { | ||
| const metaMetricsControllerState = state?.MetaMetricsController as | ||
| | Record<string, unknown> | ||
| | undefined; | ||
|
|
||
| delete metaMetricsControllerState?.previousUserTraits; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters