Skip to content

Commit

Permalink
fix: name being out of sync in account list (#26542)
Browse files Browse the repository at this point in the history
## **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.
  • Loading branch information
montelaidev authored Aug 20, 2024
1 parent 084768a commit 6ed72f0
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export default class NewAccountModal extends Component {
});
};

onSubmit = () => {
this.props.onSave(this.state.alias).then(this.props.hideModal);
onSubmit = async () => {
await this.props.onSave(this.state.alias).then(this.props.hideModal);
};

onKeyPress = (e) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { connect } from 'react-redux';
import * as actions from '../../../../store/actions';
import {
addNewAccount,
setAccountLabel,
forceUpdateMetamaskState,
hideModal,
} from '../../../../store/actions';
import NewAccountModal from './new-account-modal.component';

function mapStateToProps(state) {
Expand All @@ -10,14 +15,14 @@ function mapStateToProps(state) {

function mapDispatchToProps(dispatch) {
return {
hideModal: () => dispatch(actions.hideModal()),
createAccount: (newAccountName) => {
return dispatch(actions.addNewAccount()).then((newAccountAddress) => {
if (newAccountName) {
dispatch(actions.setAccountLabel(newAccountAddress, newAccountName));
}
return newAccountAddress;
});
hideModal: () => dispatch(hideModal()),
createAccount: async (newAccountName) => {
const newAccountAddress = await dispatch(addNewAccount());
if (newAccountName) {
dispatch(setAccountLabel(newAccountAddress, newAccountName));
}
await forceUpdateMetamaskState(dispatch);
return newAccountAddress;
},
};
}
Expand All @@ -30,9 +35,9 @@ function mergeProps(stateProps, dispatchProps) {
...stateProps,
...dispatchProps,
onSave: (newAccountName) => {
return createAccount(newAccountName).then((newAccountAddress) =>
onCreateNewAccount(newAccountAddress),
);
return createAccount(newAccountName).then((newAccountAddress) => {
onCreateNewAccount(newAccountAddress);
});
},
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import React from 'react';
import thunk from 'redux-thunk';
import { waitFor, fireEvent } from '@testing-library/react';
import configureStore from 'redux-mock-store';
import { renderWithProvider } from '../../../../../test/jest';
import mockState from '../../../../../test/data/mock-state.json';
import messages from '../../../../../app/_locales/en/messages.json';
import NewAccountModal from './new-account-modal.container';

const mockOnCreateNewAccount = jest.fn();
const mockNewAccountNumber = 2;
const mockNewMetamaskState = {
...mockState.metamask,
currentLocale: 'en',
};
const mockAddress = '0x1234567890';

const mockSubmitRequestToBackground = jest.fn().mockImplementation((method) => {
switch (method) {
case 'addNewAccount':
return mockAddress;
case 'setAccountLabel':
return {};
case 'getState':
return mockNewMetamaskState;
default:
return {};
}
});

jest.mock('../../../../store/background-connection', () => ({
...jest.requireActual('../../../../store/background-connection'),
submitRequestToBackground: (method: string, args: unknown) =>
mockSubmitRequestToBackground(method, args),
}));

const renderModal = (
props = {
onCreateNewAccount: mockOnCreateNewAccount,
newAccountNumber: mockNewAccountNumber,
},
) => {
const state = {
...mockState,
metamask: {
...mockState.metamask,
currentLocale: 'en',
},
appState: {
...mockState.appState,
modal: {
...mockState.appState.modal,
modalState: {
name: 'NEW_ACCOUNT',
props,
},
},
},
};
const middlewares = [thunk];
const mockStore = configureStore(middlewares);
const store = mockStore(state);

return {
render: renderWithProvider(<NewAccountModal />, store),
store,
};
};

describe('NewAccountModal', () => {
it('calls forceUpdateMetamaskState after adding account', async () => {
const { render } = renderModal();
const { getByText } = render;
const addAccountButton = getByText(messages.save.message);
expect(addAccountButton).toBeInTheDocument();

fireEvent.click(addAccountButton);

await waitFor(() => {
expect(mockSubmitRequestToBackground).toHaveBeenNthCalledWith(
2,
'getState',
undefined,
);
});
});
});
43 changes: 37 additions & 6 deletions ui/pages/permissions-connect/permissions-connect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import thunk from 'redux-thunk';

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

jest.mock('../../store/actions', () => {
return {
...jest.requireActual('../../store/actions'),
getRequestAccountTabIds: jest.fn().mockReturnValue({
type: 'SET_REQUEST_ACCOUNT_TABS',
payload: {},
Expand Down Expand Up @@ -110,24 +112,31 @@ const render = (
const mockStore = configureStore(middlewares);
const store = mockStore(state);

return renderWithProvider(
<PermissionApprovalContainer {...props} />,
return {
render: renderWithProvider(
<PermissionApprovalContainer {...props} />,
store,
`${CONNECT_ROUTE}/${mockPermissionRequestId}`,
),
store,
`${CONNECT_ROUTE}/${mockPermissionRequestId}`,
);
};
};

describe('PermissionApprovalContainer', () => {
describe('ConnectPath', () => {
it('renders correctly', () => {
const { container, getByText } = render();
const {
render: { container, getByText },
} = render();
expect(getByText(messages.next.message)).toBeInTheDocument();
expect(getByText(messages.cancel.message)).toBeInTheDocument();
expect(container).toMatchSnapshot();
});

it('renders the list without BTC accounts', async () => {
const { getByText, queryByText } = render();
const {
render: { getByText, queryByText },
} = render();
expect(
getByText(
`${mockAccount.metadata.name} (${shortenAddress(
Expand All @@ -144,4 +153,26 @@ describe('PermissionApprovalContainer', () => {
).not.toBeInTheDocument();
});
});

describe('Add new account', () => {
it('displays the correct account number', async () => {
const {
render: { getByText },
store,
} = render();
fireEvent.click(getByText(messages.newAccount.message));

const dispatchedActions = store.getActions();

expect(dispatchedActions).toHaveLength(2); // first action is 'SET_REQUEST_ACCOUNT_TABS'
expect(dispatchedActions[1]).toStrictEqual({
type: 'UI_MODAL_OPEN',
payload: {
name: 'NEW_ACCOUNT',
onCreateNewAccount: expect.any(Function),
newAccountNumber: 2,
},
});
});
});
});

0 comments on commit 6ed72f0

Please sign in to comment.