Skip to content

Commit 765c62e

Browse files
authored
feat: Convert mmi controller to a non-controller (#27983)
<!-- 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** We want to bring MMIController up to date with our latest controller patterns. After review, it turns out that MMIController does not have any state and therefore should not inherit from BaseController (or anything, for that matter). <!-- 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? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27983?quickstart=1) ## **Related issues** Fixes: #25926 ## **Manual testing steps** N/A ## **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** - [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/develop/.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/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 3401ef0 commit 765c62e

File tree

4 files changed

+165
-88
lines changed

4 files changed

+165
-88
lines changed

app/scripts/controllers/mmi-controller.test.ts

+59-32
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ import {
1717
NETWORK_TYPES,
1818
TEST_NETWORK_TICKER_MAP,
1919
} from '../../../shared/constants/network';
20-
import MMIController from './mmi-controller';
20+
import { MMIController, AllowedActions } from './mmi-controller';
2121
import { AppStateController } from './app-state-controller';
2222
import { ControllerMessenger } from '@metamask/base-controller';
2323
import { mmiKeyringBuilderFactory } from '../mmi-keyring-builder-factory';
2424
import MetaMetricsController from './metametrics';
2525
import { ETH_EOA_METHODS } from '../../../shared/constants/eth-methods';
2626
import { mockNetworkState } from '../../../test/stub/networks';
27+
import { InfuraNetworkType } from '@metamask/controller-utils';
2728
import { API_REQUEST_LOG_EVENT } from '@metamask-institutional/sdk';
2829

2930
jest.mock('@metamask-institutional/portfolio-dashboard', () => ({
@@ -39,6 +40,21 @@ jest.mock('./permissions', () => ({
3940
}),
4041
}));
4142

43+
export const createMockNetworkConfiguration = (
44+
override?: Partial<NetworkConfiguration>,
45+
): NetworkConfiguration => {
46+
return {
47+
chainId: CHAIN_IDS.SEPOLIA,
48+
blockExplorerUrls: [],
49+
defaultRpcEndpointIndex: 0,
50+
name: 'Mock Network',
51+
nativeCurrency: 'MOCK TOKEN',
52+
rpcEndpoints: [],
53+
defaultBlockExplorerUrlIndex: 0,
54+
...override,
55+
};
56+
};
57+
4258
const mockAccount = {
4359
address: '0x758b8178a9A4B7206d1f648c4a77C515Cbac7001',
4460
id: 'mock-id',
@@ -73,10 +89,10 @@ describe('MMIController', function () {
7389
mmiConfigurationController,
7490
controllerMessenger,
7591
accountsController,
76-
networkController,
7792
keyringController,
7893
metaMetricsController,
79-
custodyController;
94+
custodyController,
95+
mmiControllerMessenger;
8096

8197
beforeEach(async function () {
8298
const mockMessenger = {
@@ -89,22 +105,10 @@ describe('MMIController', function () {
89105
subscribe: jest.fn(),
90106
};
91107

92-
networkController = new NetworkController({
93-
messenger: new ControllerMessenger().getRestricted({
94-
name: 'NetworkController',
95-
allowedEvents: [
96-
'NetworkController:stateChange',
97-
'NetworkController:networkWillChange',
98-
'NetworkController:networkDidChange',
99-
'NetworkController:infuraIsBlocked',
100-
'NetworkController:infuraIsUnblocked',
101-
],
102-
}),
103-
state: mockNetworkState({ chainId: CHAIN_IDS.SEPOLIA }),
104-
infuraProjectId: 'mock-infura-project-id',
105-
});
106-
107-
controllerMessenger = new ControllerMessenger();
108+
const controllerMessenger = new ControllerMessenger<
109+
AllowedActions,
110+
never
111+
>();
108112

109113
accountsController = new AccountsController({
110114
messenger: controllerMessenger.getRestricted({
@@ -212,14 +216,42 @@ describe('MMIController', function () {
212216
onNetworkDidChange: jest.fn(),
213217
});
214218

215-
const mmiControllerMessenger = controllerMessenger.getRestricted({
219+
controllerMessenger.registerActionHandler(
220+
'NetworkController:getState',
221+
jest.fn().mockReturnValue(mockNetworkState({ chainId: CHAIN_IDS.SEPOLIA })),
222+
);
223+
224+
controllerMessenger.registerActionHandler(
225+
'NetworkController:setActiveNetwork',
226+
InfuraNetworkType['sepolia'],
227+
);
228+
229+
controllerMessenger.registerActionHandler(
230+
'NetworkController:getNetworkClientById',
231+
jest.fn().mockReturnValue({
232+
configuration: {
233+
chainId: CHAIN_IDS.SEPOLIA,
234+
}
235+
}),
236+
);
237+
238+
controllerMessenger.registerActionHandler(
239+
'NetworkController:getNetworkConfigurationByChainId',
240+
jest.fn().mockReturnValue(createMockNetworkConfiguration()),
241+
);
242+
243+
mmiControllerMessenger = controllerMessenger.getRestricted({
216244
name: 'MMIController',
217245
allowedActions: [
218246
'AccountsController:getAccountByAddress',
219247
'AccountsController:setAccountName',
220248
'AccountsController:listAccounts',
221249
'AccountsController:getSelectedAccount',
222250
'AccountsController:setSelectedAccount',
251+
'NetworkController:getState',
252+
'NetworkController:setActiveNetwork',
253+
'NetworkController:getNetworkClientById',
254+
'NetworkController:getNetworkConfigurationByChainId'
223255
],
224256
});
225257

@@ -253,7 +285,6 @@ describe('MMIController', function () {
253285
})
254286
}
255287
}),
256-
networkController,
257288
permissionController,
258289
custodyController,
259290
metaMetricsController,
@@ -507,9 +538,7 @@ describe('MMIController', function () {
507538
CUSTODIAN_TYPES['CUSTODIAN-TYPE'] = {
508539
keyringClass: { type: 'mock-keyring-class' },
509540
};
510-
mmiController.messenger.call = jest
511-
.fn()
512-
.mockReturnValue({ address: '0x1' });
541+
jest.spyOn(mmiControllerMessenger, 'call').mockReturnValue({ address: '0x1' });
513542
mmiController.custodyController.getCustodyTypeByAddress = jest
514543
.fn()
515544
.mockReturnValue('custodian-type');
@@ -628,9 +657,7 @@ describe('MMIController', function () {
628657
mmiController.custodyController.getAccountDetails = jest
629658
.fn()
630659
.mockReturnValue({});
631-
mmiController.messenger.call = jest
632-
.fn()
633-
.mockReturnValue([mockAccount, mockAccount2]);
660+
jest.spyOn(mmiControllerMessenger, 'call').mockReturnValue([mockAccount, mockAccount2]);
634661
mmiController.mmiConfigurationController.store.getState = jest
635662
.fn()
636663
.mockReturnValue({
@@ -701,7 +728,7 @@ describe('MMIController', function () {
701728

702729
describe('handleMmiCheckIfTokenIsPresent', () => {
703730
it('should check if a token is present', async () => {
704-
mmiController.messenger.call = jest
731+
mmiController.messagingSystem.call = jest
705732
.fn()
706733
.mockReturnValue({ address: '0x1' });
707734
mmiController.custodyController.getCustodyTypeByAddress = jest
@@ -733,7 +760,7 @@ describe('MMIController', function () {
733760

734761
describe('handleMmiDashboardData', () => {
735762
it('should return internalAccounts as identities', async () => {
736-
const controllerMessengerSpy = jest.spyOn(controllerMessenger, 'call');
763+
const controllerMessengerSpy = jest.spyOn(mmiControllerMessenger, 'call');
737764
await mmiController.handleMmiDashboardData();
738765

739766
expect(controllerMessengerSpy).toHaveBeenCalledWith(
@@ -811,7 +838,7 @@ describe('MMIController', function () {
811838

812839
describe('setAccountAndNetwork', () => {
813840
it('should set a new selected account if the selectedAddress and the address from the arguments is different', async () => {
814-
const selectedAccountSpy = jest.spyOn(controllerMessenger, 'call');
841+
const selectedAccountSpy = jest.spyOn(mmiControllerMessenger, 'call');
815842
await mmiController.setAccountAndNetwork(
816843
'mock-origin',
817844
mockAccount2.address,
@@ -829,14 +856,14 @@ describe('MMIController', function () {
829856
});
830857

831858
it('should not set a new selected account the accounts are the same', async () => {
832-
const selectedAccountSpy = jest.spyOn(controllerMessenger, 'call');
859+
const selectedAccountSpy = jest.spyOn(mmiControllerMessenger, 'call');
833860
await mmiController.setAccountAndNetwork(
834861
'mock-origin',
835862
mockAccount.address,
836863
'0x1',
837864
);
838865

839-
expect(selectedAccountSpy).toHaveBeenCalledTimes(1);
866+
expect(selectedAccountSpy).toHaveBeenCalledTimes(4);
840867
const selectedAccount = accountsController.getSelectedAccount();
841868
expect(selectedAccount.id).toBe(mockAccount.id);
842869
});

0 commit comments

Comments
 (0)