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: upgrade preferences controller to base controller v2 #27308

Closed
wants to merge 14 commits into from
Closed
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
7 changes: 3 additions & 4 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ function maybeDetectPhishing(theController) {
return {};
}

const prefState = theController.preferencesController.store.getState();
const prefState = theController.preferencesController.state;
if (!prefState.usePhishDetect) {
return {};
}
Expand Down Expand Up @@ -648,7 +648,7 @@ function emitDappViewedMetricEvent(origin) {
}

const preferencesState = controller.controllerMessenger.call(
'PreferencesController:getState',
'ExtensionPreferencesController:getState',
);
const numberOfTotalAccounts = Object.keys(preferencesState.identities).length;

Expand Down Expand Up @@ -758,8 +758,7 @@ export function setupController(
controller.preferencesController,
),
getUseAddressBarEnsResolution: () =>
controller.preferencesController.store.getState()
.useAddressBarEnsResolution,
controller.preferencesController.state.useAddressBarEnsResolution,
provider: controller.provider,
});

Expand Down
22 changes: 13 additions & 9 deletions app/scripts/controllers/app-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default class AppStateController extends EventEmitter {
isUnlocked,
initState,
onInactiveTimeout,
preferencesStore,
preferencesController,
messenger,
extension,
} = opts;
Expand Down Expand Up @@ -85,12 +85,15 @@ export default class AppStateController extends EventEmitter {
this.waitingForUnlock = [];
addUnlockListener(this.handleUnlock.bind(this));

preferencesStore.subscribe(({ preferences }) => {
const currentState = this.store.getState();
if (currentState.timeoutMinutes !== preferences.autoLockTimeLimit) {
this._setInactiveTimeout(preferences.autoLockTimeLimit);
}
});
messenger.subscribe(
'ExtensionPreferencesController:stateChange',
({ preferences }) => {
const currentState = this.store.getState();
if (currentState.timeoutMinutes !== preferences.autoLockTimeLimit) {
this._setInactiveTimeout(preferences.autoLockTimeLimit);
}
},
);

messenger.subscribe(
'KeyringController:qrKeyringStateChange',
Expand All @@ -100,8 +103,9 @@ export default class AppStateController extends EventEmitter {
}),
);

const { preferences } = preferencesStore.getState();
this._setInactiveTimeout(preferences.autoLockTimeLimit);
this._setInactiveTimeout(
preferencesController.state.preferences.autoLockTimeLimit,
);

this.messagingSystem = messenger;
this._approvalRequestId = null;
Expand Down
7 changes: 3 additions & 4 deletions app/scripts/controllers/app-state.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ describe('AppStateController', () => {
initState,
onInactiveTimeout: jest.fn(),
showUnlockRequest: jest.fn(),
preferencesStore: {
subscribe: jest.fn(),
getState: jest.fn(() => ({
preferencesController: {
state: {
preferences: {
autoLockTimeLimit: 0,
},
})),
},
},
messenger: {
call: jest.fn(() => ({
Expand Down
15 changes: 8 additions & 7 deletions app/scripts/controllers/metametrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ export default class MetaMetricsController {
* @param {object} options
* @param {object} options.segment - an instance of analytics for tracking
* events that conform to the new MetaMetrics tracking plan.
* @param {object} options.preferencesStore - The preferences controller store, used
* to access and subscribe to preferences that will be attached to events
* @param {object} options.preferencesControllerState - The state of preferences controller
* @param {Function} options.onPreferencesStateChange - Used to attach a listener to the
* stateChange event emitted by the PreferencesController
* @param {Function} options.onNetworkDidChange - Used to attach a listener to the
* networkDidChange event emitted by the networkController
* @param {Function} options.getCurrentChainId - Gets the current chain id from the
Expand All @@ -128,7 +129,8 @@ export default class MetaMetricsController {
*/
constructor({
segment,
preferencesStore,
preferencesControllerState,
onPreferencesStateChange,
onNetworkDidChange,
getCurrentChainId,
version,
Expand All @@ -144,16 +146,15 @@ export default class MetaMetricsController {
captureException(err);
}
};
const prefState = preferencesStore.getState();
this.chainId = getCurrentChainId();
this.locale = prefState.currentLocale.replace('_', '-');
this.locale = preferencesControllerState.currentLocale.replace('_', '-');
this.version =
environment === 'production' ? version : `${version}-${environment}`;
this.extension = extension;
this.environment = environment;

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
this.selectedAddress = prefState.selectedAddress;
this.selectedAddress = preferencesControllerState.selectedAddress;
///: END:ONLY_INCLUDE_IF

const abandonedFragments = omitBy(initState?.fragments, 'persist');
Expand All @@ -177,7 +178,7 @@ export default class MetaMetricsController {
},
});

preferencesStore.subscribe(({ currentLocale }) => {
onPreferencesStateChange(({ currentLocale }) => {
this.locale = currentLocale.replace('_', '-');
});

Expand Down
49 changes: 24 additions & 25 deletions app/scripts/controllers/metametrics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,22 +74,6 @@ const DEFAULT_PAGE_PROPERTIES = {
...DEFAULT_SHARED_PROPERTIES,
};

function getMockPreferencesStore({ currentLocale = LOCALE } = {}) {
let preferencesStore = {
currentLocale,
};
const subscribe = jest.fn();
const updateState = (newState) => {
preferencesStore = { ...preferencesStore, ...newState };
subscribe.mock.calls[0][0](preferencesStore);
};
return {
getState: jest.fn().mockReturnValue(preferencesStore),
updateState,
subscribe,
};
}

const SAMPLE_PERSISTED_EVENT = {
id: 'testid',
persist: true,
Expand Down Expand Up @@ -117,7 +101,10 @@ function getMetaMetricsController({
participateInMetaMetrics = true,
metaMetricsId = TEST_META_METRICS_ID,
marketingCampaignCookieId = null,
preferencesStore = getMockPreferencesStore(),
preferencesControllerState = { currentLocale: LOCALE },
onPreferencesStateChange = () => {
// do nothing
},
getCurrentChainId = () => FAKE_CHAIN_ID,
onNetworkDidChange = () => {
// do nothing
Expand All @@ -128,7 +115,8 @@ function getMetaMetricsController({
segment: segmentInstance || segment,
getCurrentChainId,
onNetworkDidChange,
preferencesStore,
preferencesControllerState,
onPreferencesStateChange,
version: '0.0.1',
environment: 'test',
initState: {
Expand Down Expand Up @@ -209,11 +197,16 @@ describe('MetaMetricsController', function () {
});

it('should update when preferences changes', function () {
const preferencesStore = getMockPreferencesStore();
let subscribeListener;
const onPreferencesStateChange = (listener) => {
subscribeListener = listener;
};
const metaMetricsController = getMetaMetricsController({
preferencesStore,
preferencesControllerState: { currentLocale: LOCALE },
onPreferencesStateChange,
});
preferencesStore.updateState({ currentLocale: 'en_UK' });

subscribeListener({ currentLocale: 'en_UK' });
expect(metaMetricsController.locale).toStrictEqual('en-UK');
});
});
Expand Down Expand Up @@ -732,9 +725,11 @@ describe('MetaMetricsController', function () {

it('should track a page view if isOptInPath is true and user not yet opted in', function () {
const metaMetricsController = getMetaMetricsController({
preferencesStore: getMockPreferencesStore({
preferencesControllerState: {
currentLocale: LOCALE,
participateInMetaMetrics: null,
}),
},
onPreferencesStateChange: jest.fn(),
});
const spy = jest.spyOn(segment, 'page');
metaMetricsController.trackPage(
Expand All @@ -746,6 +741,7 @@ describe('MetaMetricsController', function () {
},
{ isOptInPath: true },
);

expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenCalledWith(
{
Expand All @@ -765,9 +761,11 @@ describe('MetaMetricsController', function () {

it('multiple trackPage call with same actionId should result in same messageId being sent to segment', function () {
const metaMetricsController = getMetaMetricsController({
preferencesStore: getMockPreferencesStore({
preferencesControllerState: {
currentLocale: LOCALE,
participateInMetaMetrics: null,
}),
},
onPreferencesStateChange: jest.fn(),
});
const spy = jest.spyOn(segment, 'page');
metaMetricsController.trackPage(
Expand All @@ -790,6 +788,7 @@ describe('MetaMetricsController', function () {
},
{ isOptInPath: true },
);

expect(spy).toHaveBeenCalledTimes(2);
expect(spy).toHaveBeenCalledWith(
{
Expand Down
13 changes: 6 additions & 7 deletions app/scripts/controllers/mmi-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,10 @@ describe('MMIController', function () {
});

metaMetricsController = new MetaMetricsController({
preferencesStore: {
getState: jest.fn().mockReturnValue({ currentLocale: 'en' }),
subscribe: jest.fn(),
preferencesControllerState: {
currentLocale: 'en'
},
onPreferencesStateChange: jest.fn(),
getCurrentChainId: jest.fn(),
onNetworkDidChange: jest.fn(),
});
Expand Down Expand Up @@ -245,13 +245,12 @@ describe('MMIController', function () {
initState: {},
onInactiveTimeout: jest.fn(),
showUnlockRequest: jest.fn(),
preferencesStore: {
subscribe: jest.fn(),
getState: jest.fn(() => ({
preferencesController: {
state: {
preferences: {
autoLockTimeLimit: 0,
},
})),
},
},
messenger: mockMessenger,
}),
Expand Down
Loading