Skip to content

Commit 6ed72f0

Browse files
authored
fix: name being out of sync in account list (#26542)
## **Description** This PR fixes the issue where the account name being created in the connect account flow is out of sync. <!-- 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/26542?quickstart=1) ## **Related issues** Fixes: [25846](#25846) ## **Manual testing steps** 1. Create a snap/hw account 2. Go to test dapp 3. Click connect 4. Click on create new account 5. See that the suggested name is Account 3 6. Create the new account 7. See in the list that there is `Account 3` in the list ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** https://github.com/user-attachments/assets/9c0d511a-a84a-4da8-80e5-4dbadbb7cee6 <!-- [screenshots/recordings] --> ### **After** https://github.com/user-attachments/assets/daf22aed-593f-4e81-8535-11a26cd5e4fc <!-- [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 084768a commit 6ed72f0

File tree

4 files changed

+143
-20
lines changed

4 files changed

+143
-20
lines changed

ui/components/app/modals/new-account-modal/new-account-modal.component.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ export default class NewAccountModal extends Component {
2626
});
2727
};
2828

29-
onSubmit = () => {
30-
this.props.onSave(this.state.alias).then(this.props.hideModal);
29+
onSubmit = async () => {
30+
await this.props.onSave(this.state.alias).then(this.props.hideModal);
3131
};
3232

3333
onKeyPress = (e) => {

ui/components/app/modals/new-account-modal/new-account-modal.container.js

+17-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { connect } from 'react-redux';
2-
import * as actions from '../../../../store/actions';
2+
import {
3+
addNewAccount,
4+
setAccountLabel,
5+
forceUpdateMetamaskState,
6+
hideModal,
7+
} from '../../../../store/actions';
38
import NewAccountModal from './new-account-modal.component';
49

510
function mapStateToProps(state) {
@@ -10,14 +15,14 @@ function mapStateToProps(state) {
1015

1116
function mapDispatchToProps(dispatch) {
1217
return {
13-
hideModal: () => dispatch(actions.hideModal()),
14-
createAccount: (newAccountName) => {
15-
return dispatch(actions.addNewAccount()).then((newAccountAddress) => {
16-
if (newAccountName) {
17-
dispatch(actions.setAccountLabel(newAccountAddress, newAccountName));
18-
}
19-
return newAccountAddress;
20-
});
18+
hideModal: () => dispatch(hideModal()),
19+
createAccount: async (newAccountName) => {
20+
const newAccountAddress = await dispatch(addNewAccount());
21+
if (newAccountName) {
22+
dispatch(setAccountLabel(newAccountAddress, newAccountName));
23+
}
24+
await forceUpdateMetamaskState(dispatch);
25+
return newAccountAddress;
2126
},
2227
};
2328
}
@@ -30,9 +35,9 @@ function mergeProps(stateProps, dispatchProps) {
3035
...stateProps,
3136
...dispatchProps,
3237
onSave: (newAccountName) => {
33-
return createAccount(newAccountName).then((newAccountAddress) =>
34-
onCreateNewAccount(newAccountAddress),
35-
);
38+
return createAccount(newAccountName).then((newAccountAddress) => {
39+
onCreateNewAccount(newAccountAddress);
40+
});
3641
},
3742
};
3843
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import React from 'react';
2+
import thunk from 'redux-thunk';
3+
import { waitFor, fireEvent } from '@testing-library/react';
4+
import configureStore from 'redux-mock-store';
5+
import { renderWithProvider } from '../../../../../test/jest';
6+
import mockState from '../../../../../test/data/mock-state.json';
7+
import messages from '../../../../../app/_locales/en/messages.json';
8+
import NewAccountModal from './new-account-modal.container';
9+
10+
const mockOnCreateNewAccount = jest.fn();
11+
const mockNewAccountNumber = 2;
12+
const mockNewMetamaskState = {
13+
...mockState.metamask,
14+
currentLocale: 'en',
15+
};
16+
const mockAddress = '0x1234567890';
17+
18+
const mockSubmitRequestToBackground = jest.fn().mockImplementation((method) => {
19+
switch (method) {
20+
case 'addNewAccount':
21+
return mockAddress;
22+
case 'setAccountLabel':
23+
return {};
24+
case 'getState':
25+
return mockNewMetamaskState;
26+
default:
27+
return {};
28+
}
29+
});
30+
31+
jest.mock('../../../../store/background-connection', () => ({
32+
...jest.requireActual('../../../../store/background-connection'),
33+
submitRequestToBackground: (method: string, args: unknown) =>
34+
mockSubmitRequestToBackground(method, args),
35+
}));
36+
37+
const renderModal = (
38+
props = {
39+
onCreateNewAccount: mockOnCreateNewAccount,
40+
newAccountNumber: mockNewAccountNumber,
41+
},
42+
) => {
43+
const state = {
44+
...mockState,
45+
metamask: {
46+
...mockState.metamask,
47+
currentLocale: 'en',
48+
},
49+
appState: {
50+
...mockState.appState,
51+
modal: {
52+
...mockState.appState.modal,
53+
modalState: {
54+
name: 'NEW_ACCOUNT',
55+
props,
56+
},
57+
},
58+
},
59+
};
60+
const middlewares = [thunk];
61+
const mockStore = configureStore(middlewares);
62+
const store = mockStore(state);
63+
64+
return {
65+
render: renderWithProvider(<NewAccountModal />, store),
66+
store,
67+
};
68+
};
69+
70+
describe('NewAccountModal', () => {
71+
it('calls forceUpdateMetamaskState after adding account', async () => {
72+
const { render } = renderModal();
73+
const { getByText } = render;
74+
const addAccountButton = getByText(messages.save.message);
75+
expect(addAccountButton).toBeInTheDocument();
76+
77+
fireEvent.click(addAccountButton);
78+
79+
await waitFor(() => {
80+
expect(mockSubmitRequestToBackground).toHaveBeenNthCalledWith(
81+
2,
82+
'getState',
83+
undefined,
84+
);
85+
});
86+
});
87+
});

ui/pages/permissions-connect/permissions-connect.test.tsx

+37-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import thunk from 'redux-thunk';
44

55
import { ApprovalType } from '@metamask/controller-utils';
66
import { BtcAccountType } from '@metamask/keyring-api';
7+
import { fireEvent } from '@testing-library/react';
78
import messages from '../../../app/_locales/en/messages.json';
89
import { renderWithProvider } from '../../../test/lib/render-helpers';
910
import mockState from '../../../test/data/mock-state.json';
@@ -16,6 +17,7 @@ const mockPermissionRequestId = '0cbc1f26-8772-4512-8ad7-f547d6e8b72c';
1617

1718
jest.mock('../../store/actions', () => {
1819
return {
20+
...jest.requireActual('../../store/actions'),
1921
getRequestAccountTabIds: jest.fn().mockReturnValue({
2022
type: 'SET_REQUEST_ACCOUNT_TABS',
2123
payload: {},
@@ -110,24 +112,31 @@ const render = (
110112
const mockStore = configureStore(middlewares);
111113
const store = mockStore(state);
112114

113-
return renderWithProvider(
114-
<PermissionApprovalContainer {...props} />,
115+
return {
116+
render: renderWithProvider(
117+
<PermissionApprovalContainer {...props} />,
118+
store,
119+
`${CONNECT_ROUTE}/${mockPermissionRequestId}`,
120+
),
115121
store,
116-
`${CONNECT_ROUTE}/${mockPermissionRequestId}`,
117-
);
122+
};
118123
};
119124

120125
describe('PermissionApprovalContainer', () => {
121126
describe('ConnectPath', () => {
122127
it('renders correctly', () => {
123-
const { container, getByText } = render();
128+
const {
129+
render: { container, getByText },
130+
} = render();
124131
expect(getByText(messages.next.message)).toBeInTheDocument();
125132
expect(getByText(messages.cancel.message)).toBeInTheDocument();
126133
expect(container).toMatchSnapshot();
127134
});
128135

129136
it('renders the list without BTC accounts', async () => {
130-
const { getByText, queryByText } = render();
137+
const {
138+
render: { getByText, queryByText },
139+
} = render();
131140
expect(
132141
getByText(
133142
`${mockAccount.metadata.name} (${shortenAddress(
@@ -144,4 +153,26 @@ describe('PermissionApprovalContainer', () => {
144153
).not.toBeInTheDocument();
145154
});
146155
});
156+
157+
describe('Add new account', () => {
158+
it('displays the correct account number', async () => {
159+
const {
160+
render: { getByText },
161+
store,
162+
} = render();
163+
fireEvent.click(getByText(messages.newAccount.message));
164+
165+
const dispatchedActions = store.getActions();
166+
167+
expect(dispatchedActions).toHaveLength(2); // first action is 'SET_REQUEST_ACCOUNT_TABS'
168+
expect(dispatchedActions[1]).toStrictEqual({
169+
type: 'UI_MODAL_OPEN',
170+
payload: {
171+
name: 'NEW_ACCOUNT',
172+
onCreateNewAccount: expect.any(Function),
173+
newAccountNumber: 2,
174+
},
175+
});
176+
});
177+
});
147178
});

0 commit comments

Comments
 (0)