From b5c1de900de7fc83e3238930f3c6ae2fe2ca486c Mon Sep 17 00:00:00 2001 From: Ariella Vu <20778143+digiwand@users.noreply.github.com> Date: Wed, 8 Feb 2023 18:00:25 +0700 Subject: [PATCH] Refactor ConfirmTransaction to Functional Component (#17473) * ConfirmTransaction: rm unused metricEvent context * ConfirmTransaction: rm unused mapToProps props: - unapprovedTxs - id * ConfirmTransaction: convert to FC * ConfirmTransaction: use const ORIGIN_METAMASK * ConfirmTransaction: rm mapStateToProps * ConfirmTransaction: mv confirm-transaction ducks * ConfirmTransaction: mv getContractMethodData duck * ConfirmTransaction: rm container file * ConfirmTransaction: use ORIGIN_METAMASK const * ConfirmationTransaction: add tests * ConfirmTransaction: update jest mock after merge * tests: update snapshot after mock-state updates --- test/data/mock-state.json | 4 +- .../signature-request-original.test.js.snap | 4 +- .../signature-request.component.test.js.snap | 4 +- .../confirm-transaction.test.js.snap | 279 ++++++++++++++++ .../confirm-transaction.component.js | 307 +++++++++--------- .../confirm-transaction.container.js | 68 ---- .../confirm-transaction.test.js | 290 +++++++++++++++++ ui/pages/confirm-transaction/index.js | 2 +- 8 files changed, 736 insertions(+), 222 deletions(-) create mode 100644 ui/pages/confirm-transaction/__snapshots__/confirm-transaction.test.js.snap delete mode 100644 ui/pages/confirm-transaction/confirm-transaction.container.js create mode 100644 ui/pages/confirm-transaction/confirm-transaction.test.js diff --git a/test/data/mock-state.json b/test/data/mock-state.json index f68548098..e2eb7d9fd 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -140,12 +140,14 @@ "incomingTransactions": {}, "unapprovedTxs": { "8393540981007587": { + "chainId": "0x5", "id": 8393540981007587, "time": 1536268017676, "status": "unapproved", "metamaskNetworkId": "4", "loadingDefaults": false, "txParams": { + "data": "0xa9059cbb000000000000000000000000b19ac54efa18cc3a14a5b821bfec73d284bf0c5e0000000000000000000000000000000000000000000000003782dace9d900000", "from": "0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc", "to": "0xc42edfcc21ed14dda456aa0756c153f7985d8813", "value": "0x0", @@ -185,7 +187,7 @@ } ] ], - "origin": "MetaMask" + "origin": "metamask" } }, "selectedAddress": "0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc", diff --git a/ui/components/app/signature-request-original/__snapshots__/signature-request-original.test.js.snap b/ui/components/app/signature-request-original/__snapshots__/signature-request-original.test.js.snap index 1c9b73e90..c225cfb88 100644 --- a/ui/components/app/signature-request-original/__snapshots__/signature-request-original.test.js.snap +++ b/ui/components/app/signature-request-original/__snapshots__/signature-request-original.test.js.snap @@ -44,7 +44,7 @@ exports[`SignatureRequestOriginal should match snapshot 1`] = ` of - 0 + 1
+`; diff --git a/ui/pages/confirm-transaction/confirm-transaction.component.js b/ui/pages/confirm-transaction/confirm-transaction.component.js index ac4a577c7..cfa563c5d 100644 --- a/ui/pages/confirm-transaction/confirm-transaction.component.js +++ b/ui/pages/confirm-transaction/confirm-transaction.component.js @@ -1,6 +1,6 @@ -import React, { Component } from 'react'; -import PropTypes from 'prop-types'; -import { Switch, Route } from 'react-router-dom'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { Switch, Route, useHistory, useParams } from 'react-router-dom'; import Loading from '../../components/ui/loading-screen'; import ConfirmTransactionSwitch from '../confirm-transaction-switch'; import ConfirmContractInteraction from '../confirm-contract-interaction'; @@ -9,6 +9,14 @@ import ConfirmDeployContract from '../confirm-deploy-contract'; import ConfirmDecryptMessage from '../confirm-decrypt-message'; import ConfirmEncryptionPublicKey from '../confirm-encryption-public-key'; +import { ORIGIN_METAMASK } from '../../../shared/constants/app'; + +import { + clearConfirmTransaction, + setTransactionToConfirm, +} from '../../ducks/confirm-transaction/confirm-transaction.duck'; +import { getMostRecentOverviewPage } from '../../ducks/history/history'; +import { getSendTo } from '../../ducks/send'; import { CONFIRM_TRANSACTION_ROUTE, CONFIRM_DEPLOY_CONTRACT_PATH, @@ -19,188 +27,191 @@ import { ENCRYPTION_PUBLIC_KEY_REQUEST_PATH, DEFAULT_ROUTE, } from '../../helpers/constants/routes'; +import { isTokenMethodAction } from '../../helpers/utils/transactions.util'; +import { usePrevious } from '../../hooks/usePrevious'; +import { + getUnapprovedTransactions, + unconfirmedTransactionsListSelector, + unconfirmedTransactionsHashSelector, +} from '../../selectors'; import { disconnectGasFeeEstimatePoller, + getContractMethodData, getGasFeeEstimatesAndStartPolling, addPollingTokenToAppState, removePollingTokenFromAppState, + setDefaultHomeActiveTabName, } from '../../store/actions'; import ConfirmSignatureRequest from '../confirm-signature-request'; import ConfirmTokenTransactionSwitch from './confirm-token-transaction-switch'; -export default class ConfirmTransaction extends Component { - static contextTypes = { - metricsEvent: PropTypes.func, - }; +const ConfirmTransaction = () => { + const dispatch = useDispatch(); + const history = useHistory(); + const { id: paramsTransactionId } = useParams(); - static propTypes = { - history: PropTypes.object.isRequired, - totalUnapprovedCount: PropTypes.number.isRequired, - sendTo: PropTypes.string, - setTransactionToConfirm: PropTypes.func, - clearConfirmTransaction: PropTypes.func, - mostRecentOverviewPage: PropTypes.string.isRequired, - transaction: PropTypes.object, - getContractMethodData: PropTypes.func, - transactionId: PropTypes.string, - paramsTransactionId: PropTypes.string, - isTokenMethodAction: PropTypes.bool, - setDefaultHomeActiveTabName: PropTypes.func, - }; + const [isMounted, setIsMounted] = useState(false); + const [pollingToken, setPollingToken] = useState(); - constructor(props) { - super(props); - this.state = {}; - } + const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage); + const sendTo = useSelector(getSendTo); + const unapprovedTxs = useSelector(getUnapprovedTransactions); + const unconfirmedTransactions = useSelector( + unconfirmedTransactionsListSelector, + ); + const unconfirmedMessages = useSelector(unconfirmedTransactionsHashSelector); - _beforeUnload = () => { - this._isMounted = false; - if (this.state.pollingToken) { - disconnectGasFeeEstimatePoller(this.state.pollingToken); - removePollingTokenFromAppState(this.state.pollingToken); + const totalUnapprovedCount = unconfirmedTransactions.length || 0; + const transaction = useMemo(() => { + return totalUnapprovedCount + ? unapprovedTxs[paramsTransactionId] || + unconfirmedMessages[paramsTransactionId] || + unconfirmedTransactions[0] + : {}; + }, [ + paramsTransactionId, + totalUnapprovedCount, + unapprovedTxs, + unconfirmedMessages, + unconfirmedTransactions, + ]); + + const { id, type } = transaction; + const transactionId = id && String(id); + const isValidERC20TokenMethod = isTokenMethodAction(type); + + const prevParamsTransactionId = usePrevious(paramsTransactionId); + const prevTransactionId = usePrevious(transactionId); + + const _beforeUnload = useCallback(() => { + setIsMounted(false); + + if (pollingToken) { + disconnectGasFeeEstimatePoller(pollingToken); + removePollingTokenFromAppState(pollingToken); } - }; + }, [pollingToken]); - componentDidMount() { - this._isMounted = true; - const { - totalUnapprovedCount = 0, - sendTo, - history, - mostRecentOverviewPage, - transaction: { txParams: { data } = {}, origin } = {}, - getContractMethodData, - transactionId, - paramsTransactionId, - } = this.props; + useEffect(() => { + setIsMounted(true); - getGasFeeEstimatesAndStartPolling().then((pollingToken) => { - if (this._isMounted) { - this.setState({ pollingToken }); - addPollingTokenToAppState(pollingToken); + const { txParams: { data } = {}, origin } = transaction; + + getGasFeeEstimatesAndStartPolling().then((_pollingToken) => { + if (isMounted) { + setPollingToken(_pollingToken); + addPollingTokenToAppState(_pollingToken); } else { - disconnectGasFeeEstimatePoller(pollingToken); - removePollingTokenFromAppState(pollingToken); + disconnectGasFeeEstimatePoller(_pollingToken); + removePollingTokenFromAppState(_pollingToken); } }); - window.addEventListener('beforeunload', this._beforeUnload); + window.addEventListener('beforeunload', _beforeUnload); if (!totalUnapprovedCount && !sendTo) { history.replace(mostRecentOverviewPage); - return; + } else { + if (origin !== ORIGIN_METAMASK) { + dispatch(getContractMethodData(data)); + } + + const txId = transactionId || paramsTransactionId; + if (txId) { + dispatch(setTransactionToConfirm(txId)); + } } - if (origin !== 'metamask') { - getContractMethodData(data); - } + return () => { + _beforeUnload(); + window.removeEventListener('beforeunload', _beforeUnload); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); - const txId = transactionId || paramsTransactionId; - if (txId) { - this.props.setTransactionToConfirm(txId); - } - } - - componentWillUnmount() { - this._beforeUnload(); - window.removeEventListener('beforeunload', this._beforeUnload); - } - - componentDidUpdate(prevProps) { - const { - setTransactionToConfirm, - transaction: { txData: { txParams: { data } = {}, origin } = {} }, - clearConfirmTransaction, - getContractMethodData, - paramsTransactionId, - transactionId, - history, - mostRecentOverviewPage, - totalUnapprovedCount, - setDefaultHomeActiveTabName, - } = this.props; + useEffect(() => { + const { txData: { txParams: { data } = {}, origin } = {} } = transaction; if ( paramsTransactionId && transactionId && - prevProps.paramsTransactionId !== paramsTransactionId + prevParamsTransactionId !== paramsTransactionId ) { - clearConfirmTransaction(); - setTransactionToConfirm(paramsTransactionId); - if (origin !== 'metamask') { - getContractMethodData(data); + dispatch(clearConfirmTransaction()); + dispatch(setTransactionToConfirm(paramsTransactionId)); + if (origin !== ORIGIN_METAMASK) { + dispatch(getContractMethodData(data)); } - } else if ( - prevProps.transactionId && - !transactionId && - !totalUnapprovedCount - ) { - setDefaultHomeActiveTabName('activity').then(() => { + } else if (prevTransactionId && !transactionId && !totalUnapprovedCount) { + dispatch(setDefaultHomeActiveTabName('activity')).then(() => { history.replace(DEFAULT_ROUTE); }); } else if ( - prevProps.transactionId && + prevTransactionId && transactionId && - prevProps.transactionId !== transactionId + prevTransactionId !== transactionId ) { history.replace(mostRecentOverviewPage); } + }, [ + dispatch, + transaction, + paramsTransactionId, + transactionId, + history, + mostRecentOverviewPage, + prevParamsTransactionId, + prevTransactionId, + totalUnapprovedCount, + ]); + + const validTransactionId = + transactionId && + (!paramsTransactionId || paramsTransactionId === transactionId); + + if (isValidERC20TokenMethod && validTransactionId) { + return ; } + // Show routes when state.confirmTransaction has been set and when either the ID in the params + // isn't specified or is specified and matches the ID in state.confirmTransaction in order to + // support URLs of /confirm-transaction or /confirm-transaction/ + return validTransactionId ? ( + + + + + + + + + + ) : ( + + ); +}; - render() { - const { - transactionId, - paramsTransactionId, - isTokenMethodAction, - transaction, - } = this.props; - - const validTransactionId = - transactionId && - (!paramsTransactionId || paramsTransactionId === transactionId); - - if (isTokenMethodAction && validTransactionId) { - return ; - } - // Show routes when state.confirmTransaction has been set and when either the ID in the params - // isn't specified or is specified and matches the ID in state.confirmTransaction in order to - // support URLs of /confirm-transaction or /confirm-transaction/ - return validTransactionId ? ( - - - - - - - - - - ) : ( - - ); - } -} +export default ConfirmTransaction; diff --git a/ui/pages/confirm-transaction/confirm-transaction.container.js b/ui/pages/confirm-transaction/confirm-transaction.container.js deleted file mode 100644 index 28a1c6a5a..000000000 --- a/ui/pages/confirm-transaction/confirm-transaction.container.js +++ /dev/null @@ -1,68 +0,0 @@ -import { connect } from 'react-redux'; -import { compose } from 'redux'; -import { withRouter } from 'react-router-dom'; -import { - setTransactionToConfirm, - clearConfirmTransaction, -} from '../../ducks/confirm-transaction/confirm-transaction.duck'; -import { isTokenMethodAction } from '../../helpers/utils/transactions.util'; - -import { - getContractMethodData, - setDefaultHomeActiveTabName, -} from '../../store/actions'; -import { - unconfirmedTransactionsListSelector, - unconfirmedTransactionsHashSelector, -} from '../../selectors'; -import { getMostRecentOverviewPage } from '../../ducks/history/history'; -import { getSendTo } from '../../ducks/send'; -import ConfirmTransaction from './confirm-transaction.component'; - -const mapStateToProps = (state, ownProps) => { - const { - metamask: { unapprovedTxs }, - } = state; - const { - match: { params = {} }, - } = ownProps; - const { id } = params; - const sendTo = getSendTo(state); - - const unconfirmedTransactions = unconfirmedTransactionsListSelector(state); - const unconfirmedMessages = unconfirmedTransactionsHashSelector(state); - const totalUnconfirmed = unconfirmedTransactions.length; - const transaction = totalUnconfirmed - ? unapprovedTxs[id] || unconfirmedMessages[id] || unconfirmedTransactions[0] - : {}; - const { id: transactionId, type } = transaction; - - return { - totalUnapprovedCount: totalUnconfirmed, - sendTo, - unapprovedTxs, - id, - mostRecentOverviewPage: getMostRecentOverviewPage(state), - paramsTransactionId: id && String(id), - transactionId: transactionId && String(transactionId), - transaction, - isTokenMethodAction: isTokenMethodAction(type), - }; -}; - -const mapDispatchToProps = (dispatch) => { - return { - setTransactionToConfirm: (transactionId) => { - dispatch(setTransactionToConfirm(transactionId)); - }, - clearConfirmTransaction: () => dispatch(clearConfirmTransaction()), - getContractMethodData: (data) => dispatch(getContractMethodData(data)), - setDefaultHomeActiveTabName: (tabName) => - dispatch(setDefaultHomeActiveTabName(tabName)), - }; -}; - -export default compose( - withRouter, - connect(mapStateToProps, mapDispatchToProps), -)(ConfirmTransaction); diff --git a/ui/pages/confirm-transaction/confirm-transaction.test.js b/ui/pages/confirm-transaction/confirm-transaction.test.js new file mode 100644 index 000000000..996f9da75 --- /dev/null +++ b/ui/pages/confirm-transaction/confirm-transaction.test.js @@ -0,0 +1,290 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; + +import ReactRouterDOM from 'react-router-dom'; + +import * as ConfirmTransactionDucks from '../../ducks/confirm-transaction/confirm-transaction.duck'; +import * as Actions from '../../store/actions'; +import _mockState from '../../../test/data/mock-state.json'; +import { renderWithProvider } from '../../../test/lib/render-helpers'; +import { setBackgroundConnection } from '../../../test/jest'; + +import { + CONFIRM_TRANSACTION_ROUTE, + CONFIRM_DEPLOY_CONTRACT_PATH, + CONFIRM_SEND_ETHER_PATH, + CONFIRM_TOKEN_METHOD_PATH, + SIGNATURE_REQUEST_PATH, + DECRYPT_MESSAGE_REQUEST_PATH, + ENCRYPTION_PUBLIC_KEY_REQUEST_PATH, +} from '../../helpers/constants/routes'; + +import ConfirmTransaction from '.'; + +const mockUnapprovedTx = Object.values(_mockState.metamask.unapprovedTxs)[0]; + +const middleware = [thunk]; + +const mockState = { + metamask: { + ..._mockState.metamask, + }, + appState: { + gasLoadingAnimationIsShowing: false, + }, + history: { + mostRecentOverviewPage: '/', + }, + send: { + draftTransactions: {}, + }, +}; + +setBackgroundConnection({ + addPollingTokenToAppState: jest.fn(), + disconnectGasFeeEstimatePoller: jest.fn(), + getContractMethodData: jest.fn(), + getGasFeeEstimatesAndStartPolling: jest.fn(), + removePollingTokenFromAppState: jest.fn(), + setDefaultHomeActiveTabName: jest.fn(), +}); + +jest.mock('../../ducks/confirm-transaction/confirm-transaction.duck', () => ({ + setTransactionToConfirm: jest.fn().mockImplementation((txId) => { + return { type: 'mock-set-transaction-to-confirm', value: txId }; + }), +})); + +jest.mock('react-router-dom', () => { + const original = jest.requireActual('react-router-dom'); + return { + ...original, + useHistory: () => ({ + replace: jest.fn(), + }), + }; +}); + +jest.mock('../confirm-contract-interaction', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('../confirm-decrypt-message', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('../confirm-deploy-contract', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('../confirm-encryption-public-key', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('../confirm-send-ether', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('../confirm-signature-request', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('./confirm-token-transaction-switch', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); +jest.mock('../confirm-transaction-switch', () => { + return { + __esModule: true, + default: () => { + return
; + }, + }; +}); + +describe('Confirmation Transaction Page', () => { + it('should display the Loading component when the transaction is invalid', () => { + const mockStore = configureMockStore(middleware)({ + ...mockState, + metamask: { + ...mockState.metamask, + unapprovedTxs: {}, + }, + }); + const { container } = renderWithProvider(, mockStore); + + expect(container.querySelector('.loading-overlay')).toBeInTheDocument(); + expect(container.querySelector('.loading-overlay')).toMatchSnapshot(); + }); + + it('should not display the Loading component when the transaction is valid', () => { + const mockStore = configureMockStore(middleware)({ ...mockState }); + const { container } = renderWithProvider(, mockStore); + expect(container.querySelector('.loading-overlay')).toBeNull(); + }); + + [ + [CONFIRM_DEPLOY_CONTRACT_PATH, '.mock-confirm-deploy-contract'], + [CONFIRM_SEND_ETHER_PATH, '.mock-confirm-send-ether'], + [CONFIRM_TOKEN_METHOD_PATH, '.mock-confirm-contract-interaction'], + [DECRYPT_MESSAGE_REQUEST_PATH, '.mock-confirm-decrypt-message'], + [ENCRYPTION_PUBLIC_KEY_REQUEST_PATH, '.mock-confirm-encryption-public-key'], + [SIGNATURE_REQUEST_PATH, '.mock-confirm-signature-request'], + ].forEach(([componentPath, mockClassNameMatch]) => { + it(`should render "${componentPath}" route`, () => { + const mockStore = configureMockStore(middleware)(mockState); + const { container } = renderWithProvider( + , + mockStore, + `${CONFIRM_TRANSACTION_ROUTE}/${mockUnapprovedTx.id}${componentPath}`, + ); + + expect(container.querySelector(mockClassNameMatch)).toBeInTheDocument(); + }); + }); + + it(`should render ConfirmTokenTransactionSwitch component if it's a valid ERC20 token method`, () => { + const mockStore = configureMockStore(middleware)({ + ...mockState, + metamask: { + ...mockState.metamask, + unapprovedTxs: { + [mockUnapprovedTx.id]: { + ...mockUnapprovedTx, + type: 'transfer', + }, + }, + }, + }); + const { container } = renderWithProvider( + , + mockStore, + // use valid matched route path to check against ConfirmTokenTransactionSwitch + `${CONFIRM_TRANSACTION_ROUTE}/${mockUnapprovedTx.id}${CONFIRM_DEPLOY_CONTRACT_PATH}`, + ); + + expect( + container.querySelector('.mock-confirm-token-transaction-switch'), + ).toBeInTheDocument(); + }); + + it(`should render ConfirmTransactionSwitch component if the route path is unmatched and the transaction is valid`, () => { + const mockStore = configureMockStore(middleware)(mockState); + const { container } = renderWithProvider( + , + mockStore, + `/unknown-path`, + ); + + expect( + container.querySelector('.mock-confirm-transaction-switch'), + ).toBeInTheDocument(); + }); + + describe('initialization', () => { + it('should poll for gas estimates', () => { + const mockStore = configureMockStore(middleware)(mockState); + const gasEstimationPollingSpy = jest.spyOn( + Actions, + 'getGasFeeEstimatesAndStartPolling', + ); + + renderWithProvider(, mockStore); + + expect(gasEstimationPollingSpy).toHaveBeenCalled(); + }); + + it('should call setTransactionToConfirm if transaction id is provided', () => { + const mockStore = configureMockStore(middleware)({ ...mockState }); + ConfirmTransactionDucks.setTransactionToConfirm.mockClear(); + + renderWithProvider(, mockStore); + + expect( + ConfirmTransactionDucks.setTransactionToConfirm, + ).toHaveBeenCalled(); + }); + + it('should not call setTransactionToConfirm when transaction id is not provided', () => { + const mockStore = configureMockStore(middleware)({ + ...mockState, + metamask: { ...mockState.metamask, unapprovedTxs: {} }, + }); + jest.spyOn(ReactRouterDOM, 'useParams').mockImplementation(() => { + return { id: null }; + }); + ConfirmTransactionDucks.setTransactionToConfirm.mockClear(); + + renderWithProvider(, mockStore); + + expect( + ConfirmTransactionDucks.setTransactionToConfirm, + ).not.toHaveBeenCalled(); + }); + + describe('when unapproved transactions exist or a sendTo recipient exists', () => { + it('should not call history.replace(mostRecentOverviewPage)', () => { + const mockStore = configureMockStore(middleware)(mockState); + const replaceSpy = jest.fn(); + jest.spyOn(ReactRouterDOM, 'useHistory').mockImplementation(() => { + return { + replace: replaceSpy, + }; + }); + + renderWithProvider(, mockStore); + expect(replaceSpy).not.toHaveBeenCalled(); + }); + }); + + describe('when no unapproved transactions and no sendTo recipient exist', () => { + it('should call history.replace(mostRecentOverviewPage)', () => { + const mockStore = configureMockStore(middleware)({ + ...mockState, + metamask: { + ...mockState.metamask, + unapprovedTxs: {}, + }, + }); + const replaceSpy = jest.fn(); + jest.spyOn(ReactRouterDOM, 'useHistory').mockImplementation(() => { + return { + replace: replaceSpy, + }; + }); + + renderWithProvider(, mockStore, '/asdfb'); + expect(replaceSpy).toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/ui/pages/confirm-transaction/index.js b/ui/pages/confirm-transaction/index.js index 108e7e887..c5fd32ca3 100644 --- a/ui/pages/confirm-transaction/index.js +++ b/ui/pages/confirm-transaction/index.js @@ -1,3 +1,3 @@ -import ConfirmTransaction from './confirm-transaction.container'; +import ConfirmTransaction from './confirm-transaction.component'; export default ConfirmTransaction;