Skip to content

Commit 0e12141

Browse files
davidmurdochdavidmurdoch
authored
chore(migrations): remove nulls from root state (#39471)
<!-- 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** Follow up to the workaround in 3345f3e [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/39471?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Related to: #39464 <!-- ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** --> ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds a targeted cleanup migration and aligns tests/snapshots. > > - Introduces `migrations/190` that sets version to `190` and removes `null` `seedWords` and `forgottenPassword` from `data`, recording changed keys > - Registers the migration in `migrations/index.js` > - Adds comprehensive unit tests `migrations/190.test.ts` covering presence/absence and null/non-null cases > - Updates e2e state snapshot meta version from `189` to `190` > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5a2e246. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f5bd62e commit 0e12141

File tree

4 files changed

+207
-1
lines changed

4 files changed

+207
-1
lines changed

‎app/scripts/migrations/190.test.ts‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { cloneDeep } from 'lodash';
2+
import { migrate, version } from './190';
3+
4+
const VERSION = version;
5+
const OLD_VERSION = VERSION - 1;
6+
7+
describe(`migration #${VERSION}`, () => {
8+
it('removes seedWords when value is null', async () => {
9+
const oldStorage = {
10+
meta: { version: OLD_VERSION },
11+
data: {
12+
seedWords: null,
13+
PreferencesController: {
14+
showTestNetworks: true,
15+
},
16+
},
17+
};
18+
19+
const versionedData = cloneDeep(oldStorage);
20+
const changedKeys = new Set<string>();
21+
22+
await migrate(versionedData, changedKeys);
23+
24+
expect(versionedData.meta.version).toBe(VERSION);
25+
expect('seedWords' in versionedData.data).toBe(false);
26+
expect(versionedData.data).toStrictEqual({
27+
PreferencesController: {
28+
showTestNetworks: true,
29+
},
30+
});
31+
expect(changedKeys).toStrictEqual(new Set(['seedWords']));
32+
});
33+
34+
it('keeps seedWords when value is not null', async () => {
35+
const oldStorage = {
36+
meta: { version: OLD_VERSION },
37+
data: {
38+
seedWords: 'mock-seed-phrase',
39+
},
40+
};
41+
42+
const versionedData = cloneDeep(oldStorage);
43+
const changedKeys = new Set<string>();
44+
45+
await migrate(versionedData, changedKeys);
46+
47+
expect(versionedData.meta.version).toBe(VERSION);
48+
expect(versionedData.data).toStrictEqual(oldStorage.data);
49+
expect(changedKeys.size).toBe(0);
50+
});
51+
52+
it('handles state when seedWords property does not exist', async () => {
53+
const oldStorage = {
54+
meta: { version: OLD_VERSION },
55+
data: {
56+
PreferencesController: {
57+
showTestNetworks: true,
58+
},
59+
},
60+
};
61+
62+
const versionedData = cloneDeep(oldStorage);
63+
const changedKeys = new Set<string>();
64+
65+
await migrate(versionedData, changedKeys);
66+
67+
expect(versionedData.meta.version).toBe(VERSION);
68+
expect('seedWords' in versionedData.data).toBe(false);
69+
expect(versionedData.data).toStrictEqual(oldStorage.data);
70+
expect(changedKeys.size).toBe(0);
71+
});
72+
73+
it('removes forgottenPassword when value is null', async () => {
74+
const oldStorage = {
75+
meta: { version: OLD_VERSION },
76+
data: {
77+
forgottenPassword: null,
78+
PreferencesController: {
79+
showTestNetworks: true,
80+
},
81+
},
82+
};
83+
84+
const versionedData = cloneDeep(oldStorage);
85+
const changedKeys = new Set<string>();
86+
87+
await migrate(versionedData, changedKeys);
88+
89+
expect(versionedData.meta.version).toBe(VERSION);
90+
expect('forgottenPassword' in versionedData.data).toBe(false);
91+
expect(versionedData.data).toStrictEqual({
92+
PreferencesController: {
93+
showTestNetworks: true,
94+
},
95+
});
96+
expect(changedKeys).toStrictEqual(new Set(['forgottenPassword']));
97+
});
98+
99+
it('keeps forgottenPassword when value is not null', async () => {
100+
const oldStorage = {
101+
meta: { version: OLD_VERSION },
102+
data: {
103+
forgottenPassword: true,
104+
},
105+
};
106+
107+
const versionedData = cloneDeep(oldStorage);
108+
const changedKeys = new Set<string>();
109+
110+
await migrate(versionedData, changedKeys);
111+
112+
expect(versionedData.meta.version).toBe(VERSION);
113+
expect(versionedData.data).toStrictEqual(oldStorage.data);
114+
expect(changedKeys.size).toBe(0);
115+
});
116+
117+
it('handles state when forgottenPassword property does not exist', async () => {
118+
const oldStorage = {
119+
meta: { version: OLD_VERSION },
120+
data: {
121+
PreferencesController: {
122+
showTestNetworks: true,
123+
},
124+
},
125+
};
126+
127+
const versionedData = cloneDeep(oldStorage);
128+
const changedKeys = new Set<string>();
129+
130+
await migrate(versionedData, changedKeys);
131+
132+
expect(versionedData.meta.version).toBe(VERSION);
133+
expect('forgottenPassword' in versionedData.data).toBe(false);
134+
expect(versionedData.data).toStrictEqual(oldStorage.data);
135+
expect(changedKeys.size).toBe(0);
136+
});
137+
138+
it('removes both seedWords and forgottenPassword when both are null', async () => {
139+
const oldStorage = {
140+
meta: { version: OLD_VERSION },
141+
data: {
142+
seedWords: null,
143+
forgottenPassword: null,
144+
PreferencesController: {
145+
showTestNetworks: true,
146+
},
147+
},
148+
};
149+
150+
const versionedData = cloneDeep(oldStorage);
151+
const changedKeys = new Set<string>();
152+
153+
await migrate(versionedData, changedKeys);
154+
155+
expect(versionedData.meta.version).toBe(VERSION);
156+
expect('seedWords' in versionedData.data).toBe(false);
157+
expect('forgottenPassword' in versionedData.data).toBe(false);
158+
expect(versionedData.data).toStrictEqual({
159+
PreferencesController: {
160+
showTestNetworks: true,
161+
},
162+
});
163+
expect(changedKeys).toStrictEqual(
164+
new Set(['seedWords', 'forgottenPassword']),
165+
);
166+
});
167+
});

‎app/scripts/migrations/190.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { hasProperty } from '@metamask/utils';
2+
import type { Migrate } from './types';
3+
4+
export const version = 190;
5+
6+
/**
7+
* Migration that removes `null` seedWords and forgottenPassword values from the persisted state.
8+
*
9+
* If the `seedWords` property exists on the data object and its value is
10+
* `null`, this migration deletes the property and records `seedWords` in
11+
* the set of changed keys.
12+
*
13+
* If the `forgottenPassword` property exists on the data object and its value is
14+
* `null`, this migration deletes the property and records `forgottenPassword` in
15+
* the set of changed keys.
16+
*
17+
* @param versionedData - The versioned data object to migrate.
18+
* @param changedKeys - A set used to record keys that were modified.
19+
*/
20+
export const migrate = (async (versionedData, changedKeys) => {
21+
versionedData.meta.version = version;
22+
23+
if (
24+
hasProperty(versionedData.data, 'seedWords') &&
25+
versionedData.data.seedWords === null
26+
) {
27+
delete versionedData.data.seedWords;
28+
changedKeys.add('seedWords');
29+
}
30+
31+
if (
32+
hasProperty(versionedData.data, 'forgottenPassword') &&
33+
versionedData.data.forgottenPassword === null
34+
) {
35+
delete versionedData.data.forgottenPassword;
36+
changedKeys.add('forgottenPassword');
37+
}
38+
}) satisfies Migrate;

‎app/scripts/migrations/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ const migrations = [
225225
require('./187'),
226226
require('./188'),
227227
require('./189'),
228+
require('./190'),
228229
];
229230

230231
export default migrations;

‎test/e2e/tests/metrics/state-snapshots/errors-before-init-opt-in-ui-state.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,5 +316,5 @@
316316
"config": "object",
317317
"firstTimeInfo": "object"
318318
},
319-
"meta": { "version": 189 }
319+
"meta": { "version": 190 }
320320
}

0 commit comments

Comments
 (0)