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

feat: Enable redesigned transaction confirmations for all users #28321

Merged
merged 19 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions app/scripts/migrations/132.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { migrate, version } from './132';

const oldVersion = 131;

describe('migration #132', () => {
it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {},
};

const newStorage = await migrate(oldStorage);

expect(newStorage.meta).toStrictEqual({ version });
});

it('does nothing if no preferences controller state is set', async () => {
const oldState = {
OtherController: {},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toEqual(oldState);
});

it('adds preferences property to the controller if it is not set and set the preference to true if migration runs', async () => {
const oldState = { PreferencesController: {} };

const expectedState = {
PreferencesController: {
preferences: {
redesignedTransactionsEnabled: true,
},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toEqual(expectedState);
});

it('changes property to true if migration runs', async () => {
const oldState = {
PreferencesController: {
preferences: {
redesignedTransactionsEnabled: false,
},
},
};

const expectedState = {
PreferencesController: {
preferences: {
redesignedTransactionsEnabled: true,
},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toEqual(expectedState);
});
});
58 changes: 58 additions & 0 deletions app/scripts/migrations/132.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { isObject } from '@metamask/utils';
import { cloneDeep } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

export const version = 132;

/**
* This migration sets `redesignedTransactionsEnabled` as true by default in preferences in PreferencesController.
*
* @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>,
): Record<string, unknown> {
if (!isObject(state?.PreferencesController)) {
return state;
}

if (!isObject(state.PreferencesController?.preferences)) {
state.PreferencesController = {
...state.PreferencesController,
preferences: {},
};
}

const preferencesControllerState = state.PreferencesController as Record<
string,
unknown
>;

const preferences = preferencesControllerState.preferences as Record<
string,
unknown
>;

// `redesignedTransactionsEnabled` was previously set to `false` by
// default in `124.ts`
preferences.redesignedTransactionsEnabled = true;

return state;
}
1 change: 1 addition & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ const migrations = [
require('./129'),
require('./130'),
require('./131'),
require('./132'),
];

export default migrations;
41 changes: 41 additions & 0 deletions test/e2e/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,46 @@ async function tempToggleSettingRedesignedConfirmations(driver) {
);
}

/**
* Rather than using the FixtureBuilder#withPreferencesController to set the setting
* we need to manually set the setting because the migration #132 overrides this.
* We should be able to remove this when we delete the redesignedTransactionsEnabled setting.
*
* @param driver
*/
async function tempToggleSettingRedesignedTransactionConfirmations(driver) {
// Ensure we are on the extension window
await driver.switchToWindowWithTitle(WINDOW_TITLES.ExtensionInFullScreenView);

// Open settings menu button
await driver.clickElement('[data-testid="account-options-menu-button"]');

// fix race condition with mmi build
if (process.env.MMI) {
await driver.waitForSelector('[data-testid="global-menu-mmi-portfolio"]');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘πŸΌ


// Click settings from dropdown menu
await driver.clickElement('[data-testid="global-menu-settings"]');

// Click Experimental tab
const experimentalTabRawLocator = {
text: 'Experimental',
tag: 'div',
};
await driver.clickElement(experimentalTabRawLocator);

// Click redesigned transactions toggle
await driver.clickElement(
'[data-testid="toggle-redesigned-transactions-container"]',
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that after clicking the toggle, we should close the settings, so we can proceed with any test normally, without the need of doing any extra step in the test, aside from calling this function (check my comment below)


// Close settings page
await driver.clickElement(
'.settings-page__header__title-container__close-button',
);
}

/**
* Opens the account options menu safely, handling potential race conditions
* with the MMI build.
Expand Down Expand Up @@ -928,6 +968,7 @@ module.exports = {
editGasFeeForm,
clickNestedButton,
tempToggleSettingRedesignedConfirmations,
tempToggleSettingRedesignedTransactionConfirmations,
openMenuSafe,
sentryRegEx,
};
3 changes: 3 additions & 0 deletions test/e2e/json-rpc/eth_sendTransaction.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const {
unlockWallet,
WINDOW_TITLES,
generateGanacheOptions,
tempToggleSettingRedesignedTransactionConfirmations,
} = require('../helpers');
const FixtureBuilder = require('../fixture-builder');

Expand Down Expand Up @@ -68,6 +69,8 @@ describe('eth_sendTransaction', function () {
async ({ driver }) => {
await unlockWallet(driver);

await tempToggleSettingRedesignedTransactionConfirmations(driver);

// eth_sendTransaction
await driver.openNewPage(`http://127.0.0.1:8080`);
const request = JSON.stringify({
Expand Down
Loading
Loading