Skip to content

Commit 3ec4b1e

Browse files
authored
feat: Migration #122 set redesignedConfirmationsEnabled to true (#25769)
<!-- 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** This migration sets redesignedConfirmationsEnabled to true. Some users may have explicitly turned off the experimental setting, which this migration will reset to true. This is intentional as we also plan to remove the setting in an upcoming release. I also added the redesigned confirmation prop to the Sentry state log. Needed to add or not add it to support the setting in the tests. I went with adding it. --- Getting the tests to pass were a bit tricky. It turns out the migrations run after the fixtures are set. The withPreferencesController fixture method is no help here. One way we discussed to set the desired test state is to set the previous migration data to the state and setting the fixture migration version to the current version: ``` meta: { version: 122 } ``` This would require opening a live version, extracting the latest migration state, and adding the mock state to the tests. Instead, we manually toggle the setting off for each test that requires the old signature pages. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25768?quickstart=1) ## **Related issues** Fixes: #24614 ## **Manual testing steps** 1. Turn off the Experimental > Improved signature redesign setting 2. Run newest version with migration 3. Observe setting has been turned on ## **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/develop/.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/develop/.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.
1 parent 821c3bd commit 3ec4b1e

19 files changed

+218
-14
lines changed

app/scripts/lib/setupSentry.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ export const SENTRY_BACKGROUND_STATE = {
246246
preferences: {
247247
autoLockTimeLimit: true,
248248
hideZeroBalanceTokens: true,
249+
redesignedConfirmationsEnabled: true,
249250
isRedesignedConfirmationsDeveloperEnabled: false,
250251
showExtensionInFullSizeView: true,
251252
showFiatInTestnets: true,

app/scripts/migrations/122.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { migrate, version } from './122';
2+
3+
const oldVersion = 121;
4+
5+
describe('migration #122', () => {
6+
afterEach(() => {
7+
jest.resetAllMocks();
8+
});
9+
10+
it('updates the version metadata', async () => {
11+
const oldStorage = {
12+
meta: {
13+
version: oldVersion,
14+
},
15+
data: {},
16+
};
17+
18+
const newStorage = await migrate(oldStorage);
19+
20+
expect(newStorage.meta).toStrictEqual({ version });
21+
});
22+
23+
describe('set redesignedConfirmationsEnabled to true in PreferencesController', () => {
24+
it('sets redesignedConfirmationsEnabled to true', async () => {
25+
const oldStorage = {
26+
PreferencesController: {
27+
preferences: {
28+
redesignedConfirmationsEnabled: false,
29+
},
30+
},
31+
};
32+
33+
const expectedState = {
34+
PreferencesController: {
35+
preferences: {
36+
redesignedConfirmationsEnabled: true,
37+
},
38+
},
39+
};
40+
41+
const transformedState = await migrate({
42+
meta: { version: oldVersion },
43+
data: oldStorage,
44+
});
45+
46+
expect(transformedState.data).toEqual(expectedState);
47+
});
48+
49+
it(
50+
'sets redesignedConfirmationsEnabled to true even with original preferences object in the' +
51+
'state',
52+
async () => {
53+
const oldStorage = {
54+
PreferencesController: {},
55+
};
56+
57+
const expectedState = {
58+
PreferencesController: {
59+
preferences: {
60+
redesignedConfirmationsEnabled: true,
61+
},
62+
},
63+
};
64+
65+
const transformedState = await migrate({
66+
meta: { version: oldVersion },
67+
data: oldStorage,
68+
});
69+
70+
expect(transformedState.data).toEqual(expectedState);
71+
},
72+
);
73+
});
74+
});

app/scripts/migrations/122.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { cloneDeep } from 'lodash';
2+
import { hasProperty, isObject } from '@metamask/utils';
3+
4+
type VersionedData = {
5+
meta: { version: number };
6+
data: Record<string, unknown>;
7+
};
8+
9+
export const version = 122;
10+
11+
/**
12+
* This migration sets preference redesignedConfirmationsEnabled to true
13+
*
14+
* @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist.
15+
* @param originalVersionedData.meta - State metadata.
16+
* @param originalVersionedData.meta.version - The current state version.
17+
* @param originalVersionedData.data - The persisted MetaMask state, keyed by controller.
18+
* @returns Updated versioned MetaMask extension state.
19+
*/
20+
export async function migrate(
21+
originalVersionedData: VersionedData,
22+
): Promise<VersionedData> {
23+
const versionedData = cloneDeep(originalVersionedData);
24+
versionedData.meta.version = version;
25+
transformState(versionedData.data);
26+
return versionedData;
27+
}
28+
29+
// TODO: Replace `any` with specific type
30+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
31+
function transformState(state: Record<string, any>) {
32+
if (!hasProperty(state, 'PreferencesController')) {
33+
return;
34+
}
35+
36+
if (!isObject(state.PreferencesController)) {
37+
const controllerType = typeof state.PreferencesController;
38+
global.sentry?.captureException?.(
39+
new Error(`state.PreferencesController is type: ${controllerType}`),
40+
);
41+
}
42+
43+
if (!isObject(state.PreferencesController?.preferences)) {
44+
state.PreferencesController = {
45+
preferences: {},
46+
};
47+
}
48+
49+
state.PreferencesController.preferences.redesignedConfirmationsEnabled = true;
50+
}

app/scripts/migrations/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ const migrations = [
133133
require('./120'),
134134
require('./120.1'),
135135
require('./121'),
136-
136+
require('./122'),
137137
require('./123'),
138138
];
139139

test/e2e/accounts/snap-account-signatures-and-disconnects.spec.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { Suite } from 'mocha';
22
import FixtureBuilder from '../fixture-builder';
3-
import { withFixtures, multipleGanacheOptions } from '../helpers';
3+
import {
4+
withFixtures,
5+
multipleGanacheOptions,
6+
tempToggleSettingRedesignedConfirmations,
7+
} from '../helpers';
48
import { Driver } from '../webdriver/driver';
59
import {
610
installSnapSimpleKeyring,
@@ -27,6 +31,8 @@ describe('Snap Account Signatures and Disconnects', function (this: Suite) {
2731

2832
const newPublicKey = await makeNewAccountAndSwitch(driver);
2933

34+
await tempToggleSettingRedesignedConfirmations(driver);
35+
3036
// open the Test Dapp and connect Account 2 to it
3137
await connectAccountToTestDapp(driver);
3238

test/e2e/accounts/snap-account-signatures.spec.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { Suite } from 'mocha';
2-
import { openDapp, withFixtures } from '../helpers';
2+
import {
3+
openDapp,
4+
tempToggleSettingRedesignedConfirmations,
5+
withFixtures,
6+
} from '../helpers';
37
import { Driver } from '../webdriver/driver';
48
import {
59
accountSnapFixtures,
@@ -27,6 +31,8 @@ describe('Snap Account Signatures', function (this: Suite) {
2731

2832
const newPublicKey = await makeNewAccountAndSwitch(driver);
2933

34+
await tempToggleSettingRedesignedConfirmations(driver);
35+
3036
await openDapp(driver);
3137

3238
// Run all 6 signature types

test/e2e/helpers.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,39 @@ async function getSelectedAccountAddress(driver) {
11601160
return accountAddress;
11611161
}
11621162

1163+
/**
1164+
* Rather than using the FixtureBuilder#withPreferencesController to set the setting
1165+
* we need to manually set the setting because the migration #122 overrides this.
1166+
* We should be able to remove this when we delete the redesignedConfirmationsEnabled setting.
1167+
*
1168+
* @param driver
1169+
*/
1170+
async function tempToggleSettingRedesignedConfirmations(driver) {
1171+
// Ensure we are on the extension window
1172+
await driver.switchToWindowWithTitle(WINDOW_TITLES.ExtensionInFullScreenView);
1173+
1174+
// Open settings menu button
1175+
const accountOptionsMenuSelector =
1176+
'[data-testid="account-options-menu-button"]';
1177+
await driver.waitForSelector(accountOptionsMenuSelector);
1178+
await driver.clickElement(accountOptionsMenuSelector);
1179+
1180+
// Click settings from dropdown menu
1181+
await driver.clickElement('[data-testid="global-menu-settings"]');
1182+
1183+
// Click Experimental tab
1184+
const experimentalTabRawLocator = {
1185+
text: 'Experimental',
1186+
tag: 'div',
1187+
};
1188+
await driver.clickElement(experimentalTabRawLocator);
1189+
1190+
// Click redesignedConfirmationsEnabled toggle
1191+
await driver.clickElement(
1192+
'[data-testid="toggle-redesigned-confirmations-container"]',
1193+
);
1194+
}
1195+
11631196
module.exports = {
11641197
DAPP_HOST_ADDRESS,
11651198
DAPP_URL,
@@ -1229,4 +1262,5 @@ module.exports = {
12291262
defaultGanacheOptionsForType2Transactions,
12301263
removeSelectedAccount,
12311264
getSelectedAccountAddress,
1265+
tempToggleSettingRedesignedConfirmations,
12321266
};

test/e2e/snaps/test-snap-siginsights.spec.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const {
55
openDapp,
66
unlockWallet,
77
switchToNotificationWindow,
8+
tempToggleSettingRedesignedConfirmations,
89
WINDOW_TITLES,
910
} = require('../helpers');
1011
const FixtureBuilder = require('../fixture-builder');
@@ -29,6 +30,7 @@ describe('Test Snap Signature Insights', function () {
2930
},
3031
async ({ driver }) => {
3132
await unlockWallet(driver);
33+
await tempToggleSettingRedesignedConfirmations(driver);
3234

3335
// navigate to test snaps page and connect
3436
await driver.openNewPage(TEST_SNAPS_WEBSITE_URL);

test/e2e/tests/dapp-interactions/signin-with-ethereum.spec.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const {
44
withFixtures,
55
openDapp,
66
DAPP_URL,
7+
tempToggleSettingRedesignedConfirmations,
78
unlockWallet,
89
WINDOW_TITLES,
910
} = require('../../helpers');
@@ -28,6 +29,7 @@ describe('Sign in with ethereum', function () {
2829
},
2930
async ({ driver }) => {
3031
await unlockWallet(driver);
32+
await tempToggleSettingRedesignedConfirmations(driver);
3133

3234
// Create a signin with ethereum request in test dapp
3335
await openDapp(driver);

test/e2e/tests/metrics/signature-approved.spec.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
unlockWallet,
99
getEventPayloads,
1010
clickSignOnSignatureConfirmation,
11+
tempToggleSettingRedesignedConfirmations,
1112
validateContractDetails,
1213
} = require('../../helpers');
1314
const FixtureBuilder = require('../../fixture-builder');
@@ -65,6 +66,7 @@ describe('Signature Approved Event @no-mmi', function () {
6566
},
6667
async ({ driver, mockedEndpoint: mockedEndpoints }) => {
6768
await unlockWallet(driver);
69+
await tempToggleSettingRedesignedConfirmations(driver);
6870
await openDapp(driver);
6971

7072
// creates a sign typed data signature request
@@ -116,6 +118,7 @@ describe('Signature Approved Event @no-mmi', function () {
116118
},
117119
async ({ driver, mockedEndpoint: mockedEndpoints }) => {
118120
await unlockWallet(driver);
121+
await tempToggleSettingRedesignedConfirmations(driver);
119122
await openDapp(driver);
120123

121124
// creates a sign typed data signature request
@@ -163,6 +166,7 @@ describe('Signature Approved Event @no-mmi', function () {
163166
},
164167
async ({ driver, mockedEndpoint: mockedEndpoints }) => {
165168
await unlockWallet(driver);
169+
await tempToggleSettingRedesignedConfirmations(driver);
166170
await openDapp(driver);
167171

168172
// creates a sign typed data signature request
@@ -209,6 +213,7 @@ describe('Signature Approved Event @no-mmi', function () {
209213
},
210214
async ({ driver, mockedEndpoint: mockedEndpoints }) => {
211215
await unlockWallet(driver);
216+
await tempToggleSettingRedesignedConfirmations(driver);
212217
await openDapp(driver);
213218

214219
// creates a sign typed data signature request
@@ -260,6 +265,7 @@ describe('Signature Approved Event @no-mmi', function () {
260265
},
261266
async ({ driver, mockedEndpoint: mockedEndpoints }) => {
262267
await unlockWallet(driver);
268+
await tempToggleSettingRedesignedConfirmations(driver);
263269
await openDapp(driver);
264270

265271
// creates a sign typed data signature request

0 commit comments

Comments
 (0)