From 0c2af508efbcfc1a1b87700fb49961a3c94b3f16 Mon Sep 17 00:00:00 2001 From: amerkadicE <97883527+amerkadicE@users.noreply.github.com> Date: Thu, 9 Feb 2023 18:45:52 +0100 Subject: [PATCH] Fix recent recipient order (#16346) --- .../confirm-transaction-base.component.js | 8 +++ .../confirm-transaction-base.container.js | 21 ++++++ .../add-recipient.component.test.js | 67 +++++++++++++++++++ .../add-recipient/add-recipient.container.js | 19 +++++- .../add-recipient.container.test.js | 1 + .../send/send-footer/send-footer.component.js | 7 +- .../send/send-footer/send-footer.container.js | 18 +---- .../send/send-footer/send-footer.stories.js | 1 - ui/pages/send/send-footer/send-footer.test.js | 3 - ui/pages/send/send.test.js | 1 + 10 files changed, 118 insertions(+), 28 deletions(-) diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js index b05a101e1..b9a956dd0 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -93,6 +93,7 @@ export default class ConfirmTransactionBase extends Component { sendTransaction: PropTypes.func, showTransactionConfirmedModal: PropTypes.func, showRejectTransactionsConfirmationModal: PropTypes.func, + toAccounts: PropTypes.object, toAddress: PropTypes.string, tokenData: PropTypes.object, tokenProps: PropTypes.object, @@ -103,6 +104,7 @@ export default class ConfirmTransactionBase extends Component { txData: PropTypes.object, unapprovedTxCount: PropTypes.number, customGas: PropTypes.object, + addToAddressBookIfNew: PropTypes.func, // Component props actionKey: PropTypes.string, contentComponent: PropTypes.node, @@ -803,10 +805,16 @@ export default class ConfirmTransactionBase extends Component { maxPriorityFeePerGas, baseFeePerGas, methodData, + addToAddressBookIfNew, + toAccounts, + toAddress, } = this.props; const { submitting } = this.state; const { name } = methodData; + if (txData.type === TransactionType.simpleSend) { + addToAddressBookIfNew(toAddress, toAccounts); + } if (submitting) { return; } diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js index ba552ddad..dc0b9c722 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -13,6 +13,7 @@ import { getNextNonce, tryReverseResolveAddress, setDefaultHomeActiveTabName, + addToAddressBook, } from '../../store/actions'; import { isBalanceSufficient } from '../send/send.utils'; import { shortenAddress, valuesFor } from '../../helpers/utils/util'; @@ -44,7 +45,9 @@ import { updateGasFees, getIsGasEstimatesLoading, getNativeCurrency, + getSendToAccounts, } from '../../ducks/metamask/metamask'; +import { addHexPrefix } from '../../../app/scripts/lib/util'; import { parseStandardTokenTransactionData, @@ -74,6 +77,14 @@ const customNonceMerge = (txData) => } : txData; +function addressIsNew(toAccounts, newAddress) { + const newAddressNormalized = newAddress.toLowerCase(); + const foundMatching = toAccounts.some( + ({ address }) => address.toLowerCase() === newAddressNormalized, + ); + return !foundMatching; +} + const mapStateToProps = (state, ownProps) => { const { toAddress: propsToAddress, @@ -122,6 +133,8 @@ const mapStateToProps = (state, ownProps) => { toAddress = propsToAddress || tokenToAddress || txParamsToAddress; } + const toAccounts = getSendToAccounts(state); + const tokenList = getTokenList(state); const toName = @@ -196,6 +209,7 @@ const mapStateToProps = (state, ownProps) => { balance, fromAddress, fromName, + toAccounts, toAddress, toEns, toName, @@ -277,6 +291,13 @@ export const mapDispatchToProps = (dispatch) => { updateTransactionGasFees: (gasFees) => { dispatch(updateGasFees({ ...gasFees, expectHexWei: true })); }, + showBuyModal: () => dispatch(showModal({ name: 'DEPOSIT_ETHER' })), + addToAddressBookIfNew: (newAddress, toAccounts, nickname = '') => { + const hexPrefixedAddress = addHexPrefix(newAddress); + if (addressIsNew(toAccounts, hexPrefixedAddress)) { + dispatch(addToAddressBook(hexPrefixedAddress, nickname)); + } + }, }; }; diff --git a/ui/pages/send/send-content/add-recipient/add-recipient.component.test.js b/ui/pages/send/send-content/add-recipient/add-recipient.component.test.js index deaabbc83..0aa31b88f 100644 --- a/ui/pages/send/send-content/add-recipient/add-recipient.component.test.js +++ b/ui/pages/send/send-content/add-recipient/add-recipient.component.test.js @@ -58,4 +58,71 @@ describe('Add Recipient Component', () => { expect(container).toMatchSnapshot(); }); }); + + describe('Recent recipient order', () => { + const recentRecipientState = { + ...mockState, + metamask: { + ...mockState.metamask, + addressBook: { + '0x5': { + '0x0000000000000000000000000000000000000001': { + address: '0x0000000000000000000000000000000000000001', + chainId: '0x5', + isEns: false, + memo: '', + name: '', + }, + '0x0000000000000000000000000000000000000002': { + address: '0x0000000000000000000000000000000000000002', + chainId: '0x5', + isEns: false, + memo: '', + name: '', + }, + '0x0000000000000000000000000000000000000003': { + address: '0x0000000000000000000000000000000000000003', + chainId: '0x5', + isEns: false, + memo: '', + name: '', + }, + }, + }, + currentNetworkTxList: [ + { + time: 1674425700001, + txParams: { + to: '0x0000000000000000000000000000000000000001', + }, + }, + { + time: 1674425700002, + txParams: { + to: '0x0000000000000000000000000000000000000002', + }, + }, + { + time: 1674425700003, + txParams: { + to: '0x0000000000000000000000000000000000000003', + }, + }, + ], + }, + }; + const mockStore = configureMockStore()(recentRecipientState); + + it('should render latest used recipient first', () => { + const { getAllByTestId } = renderWithProvider( + , + mockStore, + ); + + const recipientList = getAllByTestId('recipient'); + + expect(recipientList[0]).toHaveTextContent('0x0000...0003'); + expect(recipientList[1]).toHaveTextContent('0x0000...0002'); + }); + }); }); diff --git a/ui/pages/send/send-content/add-recipient/add-recipient.container.js b/ui/pages/send/send-content/add-recipient/add-recipient.container.js index 3f4455f78..1df1274d9 100644 --- a/ui/pages/send/send-content/add-recipient/add-recipient.container.js +++ b/ui/pages/send/send-content/add-recipient/add-recipient.container.js @@ -3,6 +3,7 @@ import { getAddressBook, getAddressBookEntry, getMetaMaskAccountsOrdered, + currentNetworkTxListSelector, } from '../../../../selectors'; import { @@ -35,6 +36,22 @@ function mapStateToProps(state) { const addressBook = getAddressBook(state); + const txList = [...currentNetworkTxListSelector(state)].reverse(); + + const nonContacts = addressBook + .filter(({ name }) => !name) + .map((nonContact) => { + const nonContactTx = txList.find( + (transaction) => + transaction.txParams.to === nonContact.address.toLowerCase(), + ); + return { ...nonContact, timestamp: nonContactTx?.time }; + }); + + nonContacts.sort((a, b) => { + return b.timestamp - a.timestamp; + }); + const ownedAccounts = getMetaMaskAccountsOrdered(state); return { @@ -44,7 +61,7 @@ function mapStateToProps(state) { domainResolution, domainError: getDomainError(state), domainWarning: getDomainWarning(state), - nonContacts: addressBook.filter(({ name }) => !name), + nonContacts, ownedAccounts, isUsingMyAccountsForRecipientSearch: getIsUsingMyAccountForRecipientSearch(state), diff --git a/ui/pages/send/send-content/add-recipient/add-recipient.container.test.js b/ui/pages/send/send-content/add-recipient/add-recipient.container.test.js index 9343a2f8a..bb836ed3c 100644 --- a/ui/pages/send/send-content/add-recipient/add-recipient.container.test.js +++ b/ui/pages/send/send-content/add-recipient/add-recipient.container.test.js @@ -16,6 +16,7 @@ jest.mock('../../../../selectors', () => ({ { name: `account1:mockState` }, { name: `account2:mockState` }, ], + currentNetworkTxListSelector: (s) => `currentNetworkTxListSelector:${s}`, })); jest.mock('../../../../ducks/domains', () => ({ diff --git a/ui/pages/send/send-footer/send-footer.component.js b/ui/pages/send/send-footer/send-footer.component.js index 87dd68295..c5a8887ee 100644 --- a/ui/pages/send/send-footer/send-footer.component.js +++ b/ui/pages/send/send-footer/send-footer.component.js @@ -11,13 +11,10 @@ import { SEND_STAGES } from '../../../ducks/send'; export default class SendFooter extends Component { static propTypes = { - addToAddressBookIfNew: PropTypes.func, resetSendState: PropTypes.func, disabled: PropTypes.bool.isRequired, history: PropTypes.object, sign: PropTypes.func, - to: PropTypes.string, - toAccounts: PropTypes.array, sendStage: PropTypes.string, sendErrors: PropTypes.object, mostRecentOverviewPage: PropTypes.string.isRequired, @@ -52,11 +49,9 @@ export default class SendFooter extends Component { async onSubmit(event) { event.preventDefault(); - const { addToAddressBookIfNew, sign, to, toAccounts, history } = this.props; + const { sign, history } = this.props; const { trackEvent } = this.context; - // TODO: add nickname functionality - await addToAddressBookIfNew(to, toAccounts); const promise = sign(); Promise.resolve(promise).then(() => { diff --git a/ui/pages/send/send-footer/send-footer.container.js b/ui/pages/send/send-footer/send-footer.container.js index 7c36ab9f3..7b0e3f341 100644 --- a/ui/pages/send/send-footer/send-footer.container.js +++ b/ui/pages/send/send-footer/send-footer.container.js @@ -1,5 +1,5 @@ import { connect } from 'react-redux'; -import { addToAddressBook, cancelTx } from '../../../store/actions'; +import { cancelTx } from '../../../store/actions'; import { resetSendState, getSendStage, @@ -10,20 +10,11 @@ import { getDraftTransactionID, } from '../../../ducks/send'; import { getMostRecentOverviewPage } from '../../../ducks/history/history'; -import { addHexPrefix } from '../../../../app/scripts/lib/util'; import { getSendToAccounts } from '../../../ducks/metamask/metamask'; import SendFooter from './send-footer.component'; export default connect(mapStateToProps, mapDispatchToProps)(SendFooter); -function addressIsNew(toAccounts, newAddress) { - const newAddressNormalized = newAddress.toLowerCase(); - const foundMatching = toAccounts.some( - ({ address }) => address.toLowerCase() === newAddressNormalized, - ); - return !foundMatching; -} - function mapStateToProps(state) { return { disabled: isSendFormInvalid(state), @@ -41,12 +32,5 @@ function mapDispatchToProps(dispatch) { resetSendState: () => dispatch(resetSendState()), cancelTx: (t) => dispatch(cancelTx(t)), sign: () => dispatch(signTransaction()), - addToAddressBookIfNew: (newAddress, toAccounts, nickname = '') => { - const hexPrefixedAddress = addHexPrefix(newAddress); - if (addressIsNew(toAccounts, hexPrefixedAddress)) { - // TODO: nickname, i.e. addToAddressBook(recipient, nickname) - dispatch(addToAddressBook(hexPrefixedAddress, nickname)); - } - }, }; } diff --git a/ui/pages/send/send-footer/send-footer.stories.js b/ui/pages/send/send-footer/send-footer.stories.js index 785b653ec..be8bd1d22 100644 --- a/ui/pages/send/send-footer/send-footer.stories.js +++ b/ui/pages/send/send-footer/send-footer.stories.js @@ -13,7 +13,6 @@ export default { mostRecentOverviewPage: { control: 'text' }, sendErrors: { control: 'object' }, history: { action: 'history' }, - addToAddressBookIfNew: { action: 'addToAddressBookIfNew' }, resetSendState: { action: 'resetSendState' }, }, }; diff --git a/ui/pages/send/send-footer/send-footer.test.js b/ui/pages/send/send-footer/send-footer.test.js index a9bc5d40d..9612449c0 100644 --- a/ui/pages/send/send-footer/send-footer.test.js +++ b/ui/pages/send/send-footer/send-footer.test.js @@ -13,7 +13,6 @@ import SendFooter from '.'; const mockResetSendState = jest.fn(); const mockSendTransaction = jest.fn(); -const mockAddtoAddressBook = jest.fn(); const mockCancelTx = jest.fn(); jest.mock('../../../ducks/send/index.js', () => ({ @@ -23,7 +22,6 @@ jest.mock('../../../ducks/send/index.js', () => ({ })); jest.mock('../../../store/actions.ts', () => ({ - addToAddressBook: () => mockAddtoAddressBook, cancelTx: () => mockCancelTx, })); @@ -108,7 +106,6 @@ describe('SendFooter Component', () => { fireEvent.click(nextText); await waitFor(() => { - expect(mockAddtoAddressBook).toHaveBeenCalled(); expect(mockSendTransaction).toHaveBeenCalled(); expect(props.history.push).toHaveBeenCalledWith( CONFIRM_TRANSACTION_ROUTE, diff --git a/ui/pages/send/send.test.js b/ui/pages/send/send.test.js index a659aae65..70a6031aa 100644 --- a/ui/pages/send/send.test.js +++ b/ui/pages/send/send.test.js @@ -103,6 +103,7 @@ const baseStore = { addressBook: { [CHAIN_IDS.GOERLI]: [], }, + currentNetworkTxList: [], cachedBalances: { [CHAIN_IDS.GOERLI]: {}, },