Skip to content

Commit

Permalink
refactor: Replace usages of the deprecated setProviderType (#22619)
Browse files Browse the repository at this point in the history
## **Description**

All usages of the deprecated `NetworkController` method
`setProviderType` have been replaced. The method `setActiveNetwork` is
now used instead; it was updated in a recent release to accept types as
well as IDs.

## **Related issues**

N/A

## **Manual testing steps**

No functional changes

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
  • Loading branch information
Gudahtt committed Aug 22, 2024
1 parent ed44d49 commit d2d48d2
Show file tree
Hide file tree
Showing 11 changed files with 12 additions and 83 deletions.
2 changes: 1 addition & 1 deletion app/scripts/controllers/mmi-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ export default class MMIController extends EventEmitter {
16,
);
if (selectedChainId !== chainId && chainId === 1) {
await this.networkController.setProviderType('mainnet');
await this.networkController.setActiveNetwork('mainnet');
} else if (selectedChainId !== chainId) {
const { networkConfigurations } = this.networkController.state;

Expand Down
3 changes: 0 additions & 3 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3254,9 +3254,6 @@ export default class MetamaskController extends EventEmitter {
verifyPassword: this.verifyPassword.bind(this),

// network management
setProviderType: (type) => {
return this.networkController.setProviderType(type);
},
setActiveNetwork: (networkConfigurationId) => {
return this.networkController.setActiveNetwork(networkConfigurationId);
},
Expand Down
3 changes: 1 addition & 2 deletions shared/constants/network.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { capitalize, pick } from 'lodash';
/**
* A type representing any valid value for 'type' for setProviderType and other
* methods that add or manipulate networks in MetaMask state.
* A type representing built-in network types, used as an identifier.
*/
export type NetworkType = (typeof NETWORK_TYPES)[keyof typeof NETWORK_TYPES];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default class LoadingNetworkScreen extends PureComponent {
providerId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
showNetworkDropdown: PropTypes.func,
setProviderArgs: PropTypes.array,
setProviderType: PropTypes.func,
setActiveNetwork: PropTypes.func,
rollbackToPreviousProvider: PropTypes.func,
isNetworkLoading: PropTypes.bool,
showDeprecatedRpcUrlWarning: PropTypes.bool,
Expand Down Expand Up @@ -87,7 +87,7 @@ export default class LoadingNetworkScreen extends PureComponent {
};

renderConnectionFailureNotification = (message, showTryAgain = false) => {
const { showNetworkDropdown, setProviderArgs, setProviderType } =
const { showNetworkDropdown, setProviderArgs, setActiveNetwork } =
this.props;

return (
Expand Down Expand Up @@ -128,7 +128,7 @@ export default class LoadingNetworkScreen extends PureComponent {
<ButtonPrimary
onClick={() => {
this.setState({ showErrorScreen: false });
setProviderType(...setProviderArgs);
setActiveNetwork(...setProviderArgs);
window.clearTimeout(this.cancelCallTimeout);
this.cancelCallTimeout = setTimeout(
this.cancelCall,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ const mapStateToProps = (state) => {

const mapDispatchToProps = (dispatch) => {
return {
setProviderType: (type) => {
dispatch(actions.setProviderType(type));
setActiveNetwork: (type) => {
dispatch(actions.setActiveNetwork(type));
},
rollbackToPreviousProvider: () =>
dispatch(actions.rollbackToPreviousProvider()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { NetworkListItem } from '../network-list-item';
import {
hideNetworkBanner,
setActiveNetwork,
setProviderType,
setShowTestNetworks,
showModal,
toggleNetworkMenu,
Expand Down Expand Up @@ -303,11 +302,7 @@ export const NetworkListMenu = ({ onClose }) => {
focus={isCurrentNetwork && !focusSearch}
onClick={() => {
dispatch(toggleNetworkMenu());
if (network.providerType) {
dispatch(setProviderType(network.providerType));
} else {
dispatch(setActiveNetwork(network.id));
}
dispatch(setActiveNetwork(network.providerType || network.id));

// If presently on and connected to a dapp, communicate a change to
// the dapp via silent switchEthereumChain that the network has
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ import {
import { NetworkListMenu } from '.';

const mockSetShowTestNetworks = jest.fn();
const mockSetProviderType = jest.fn();
const mockToggleNetworkMenu = jest.fn();
const mockSetNetworkClientIdForDomain = jest.fn();
const mockSetActiveNetwork = jest.fn();

jest.mock('../../../store/actions.ts', () => ({
setShowTestNetworks: () => mockSetShowTestNetworks,
setProviderType: () => mockSetProviderType,
setActiveNetwork: () => mockSetActiveNetwork,
toggleNetworkMenu: () => mockToggleNetworkMenu,
setNetworkClientIdForDomain: (network, id) =>
Expand Down Expand Up @@ -108,7 +106,7 @@ describe('NetworkListMenu', () => {
const { getByText } = render();
fireEvent.click(getByText(MAINNET_DISPLAY_NAME));
expect(mockToggleNetworkMenu).toHaveBeenCalled();
expect(mockSetProviderType).toHaveBeenCalled();
expect(mockSetActiveNetwork).toHaveBeenCalled();
});

it('shows the correct selected network when networks share the same chain ID', () => {
Expand Down
3 changes: 0 additions & 3 deletions ui/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,6 @@ async function startApp(metamaskState, backgroundConnection, opts) {
updateCurrentLocale: (code) => {
store.dispatch(actions.updateCurrentLocale(code));
},
setProviderType: (type) => {
store.dispatch(actions.setProviderType(type));
},
setFeatureFlag: (key, value) => {
store.dispatch(actions.setFeatureFlag(key, value));
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
import { useI18nContext } from '../../../hooks/useI18nContext';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import { getMostRecentOverviewPage } from '../../../ducks/history/history';
import { setProviderType } from '../../../store/actions';
import { setActiveNetwork } from '../../../store/actions';
import { mmiActionsFactory } from '../../../store/institutional/institution-background';
import { getMMIConfiguration } from '../../../selectors/institutional/selectors';
import {
Expand Down Expand Up @@ -100,7 +100,7 @@ const ConfirmAddCustodianToken: React.FC = () => {
) as NetworkType | undefined;

if (networkType) {
await dispatch(setProviderType(networkType));
await dispatch(setActiveNetwork(networkType));
}
}
}
Expand Down
43 changes: 0 additions & 43 deletions ui/store/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1025,49 +1025,6 @@ describe('Actions', () => {
});
});

describe('#setProviderType', () => {
afterEach(() => {
sinon.restore();
});

it('calls setProviderType', async () => {
const store = mockStore();

const setProviderTypeStub = sinon.stub().callsFake((_, cb) => cb());

background.getApi.returns({
setProviderType: setProviderTypeStub,
});

setBackgroundConnection(background.getApi());

await store.dispatch(actions.setProviderType());
expect(setProviderTypeStub.callCount).toStrictEqual(1);
});

it('displays warning when setProviderType throws', async () => {
const store = mockStore();

background.getApi.returns({
setProviderType: sinon
.stub()
.callsFake((_, cb) => cb(new Error('error'))),
});

setBackgroundConnection(background.getApi());

const expectedActions = [
{
type: 'DISPLAY_WARNING',
payload: 'Had a problem changing networks!',
},
];

await store.dispatch(actions.setProviderType());
expect(store.getActions()).toStrictEqual(expectedActions);
});
});

describe('#setActiveNetwork', () => {
afterEach(() => {
sinon.restore();
Expand Down
16 changes: 1 addition & 15 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ import {
} from '../../shared/modules/i18n';
import { decimalToHex } from '../../shared/modules/conversion.utils';
import { TxGasFees, PriorityLevels } from '../../shared/constants/gas';
import { NetworkType, RPCDefinition } from '../../shared/constants/network';
import { RPCDefinition } from '../../shared/constants/network';
import { EtherDenomination } from '../../shared/constants/common';
import {
isErrorWithMessage,
Expand Down Expand Up @@ -2370,20 +2370,6 @@ export function createRetryTransaction(
// config
//

export function setProviderType(
type: NetworkType,
): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
return async (dispatch: MetaMaskReduxDispatch) => {
log.debug(`background.setProviderType`, type);
try {
await submitRequestToBackground('setProviderType', [type]);
} catch (error) {
logErrorWithMessage(error);
dispatch(displayWarning('Had a problem changing networks!'));
}
};
}

export function upsertNetworkConfiguration(
{
id,
Expand Down

0 comments on commit d2d48d2

Please sign in to comment.