From 26222a9b271db6df6bd3ff2bf5dd94748363187d Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Wed, 1 Feb 2023 11:24:41 +0530 Subject: [PATCH] Refactoring confirm-transaction-base component (#17484) --- .../confirm-page-container.component.js | 19 +- .../currency-display.component.js | 2 +- ui/hooks/useTransactionInsights.js | 85 +++ .../confirm-transaction-base.test.js.snap | 542 ++++++++++++++++++ .../confirm-transaction-base.component.js | 91 +-- .../confirm-transaction-base.container.js | 10 - .../confirm-transaction-base.test.js | 151 +++++ 7 files changed, 795 insertions(+), 105 deletions(-) create mode 100644 ui/hooks/useTransactionInsights.js create mode 100644 ui/pages/confirm-transaction-base/__snapshots__/confirm-transaction-base.test.js.snap create mode 100644 ui/pages/confirm-transaction-base/confirm-transaction-base.test.js diff --git a/ui/components/app/confirm-page-container/confirm-page-container.component.js b/ui/components/app/confirm-page-container/confirm-page-container.component.js index 240d55444..42bcec2dc 100644 --- a/ui/components/app/confirm-page-container/confirm-page-container.component.js +++ b/ui/components/app/confirm-page-container/confirm-page-container.component.js @@ -28,6 +28,9 @@ import DepositPopover from '../deposit-popover/deposit-popover'; import { fetchTokenBalance } from '../../../pages/swaps/swaps.util'; import SetApproveForAllWarning from '../set-approval-for-all-warning'; import { useI18nContext } from '../../../hooks/useI18nContext'; +///: BEGIN:ONLY_INCLUDE_IN(flask) +import useTransactionInsights from '../../../hooks/useTransactionInsights'; +///: END:ONLY_INCLUDE_IN(flask) import { getAccountName, getAddressBookEntry, @@ -84,8 +87,8 @@ const ConfirmPageContainer = (props) => { supportsEIP1559, nativeCurrency, ///: BEGIN:ONLY_INCLUDE_IN(flask) - insightComponent, - ///: END:ONLY_INCLUDE_IN + txData, + ///: END:ONLY_INCLUDE_IN(flask) assetStandard, isApprovalOrRejection, } = props; @@ -127,6 +130,14 @@ const ConfirmPageContainer = (props) => { setCollectionBalance(tokenBalance?.balance?.words?.[0] || 0); }, [fromAddress, tokenAddress]); + ///: BEGIN:ONLY_INCLUDE_IN(flask) + // As confirm-transction-base is converted to functional component + // this code can bemoved to it. + const insightComponent = useTransactionInsights({ + txData, + }); + ///: END:ONLY_INCLUDE_IN + useEffect(() => { if (isSetApproveForAll && assetStandard === TokenStandard.ERC721) { fetchCollectionBalance(); @@ -336,8 +347,8 @@ ConfirmPageContainer.propTypes = { dataHexComponent: PropTypes.node, detailsComponent: PropTypes.node, ///: BEGIN:ONLY_INCLUDE_IN(flask) - insightComponent: PropTypes.node, - ///: END:ONLY_INCLUDE_IN + txData: PropTypes.object, + ///: END:ONLY_INCLUDE_IN(flask) tokenAddress: PropTypes.string, nonce: PropTypes.string, warning: PropTypes.string, diff --git a/ui/components/ui/currency-display/currency-display.component.js b/ui/components/ui/currency-display/currency-display.component.js index 1b12d67ad..73f32a2eb 100644 --- a/ui/components/ui/currency-display/currency-display.component.js +++ b/ui/components/ui/currency-display/currency-display.component.js @@ -66,6 +66,6 @@ CurrencyDisplay.propTypes = { prefix: PropTypes.string, prefixComponent: PropTypes.node, style: PropTypes.object, - suffix: PropTypes.string, + suffix: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]), value: PropTypes.string, }; diff --git a/ui/hooks/useTransactionInsights.js b/ui/hooks/useTransactionInsights.js new file mode 100644 index 000000000..ed7916c4f --- /dev/null +++ b/ui/hooks/useTransactionInsights.js @@ -0,0 +1,85 @@ +import React, { useEffect, useState } from 'react'; +import { useSelector } from 'react-redux'; + +import { CHAIN_ID_TO_NETWORK_ID_MAP } from '../../shared/constants/network'; +import { stripHexPrefix } from '../../shared/modules/hexstring-utils'; +import { TransactionType } from '../../shared/constants/transaction'; +import { getInsightSnaps } from '../selectors'; +import { DropdownTab, Tab } from '../components/ui/tabs'; +import { SnapInsight } from '../components/app/confirm-page-container/flask/snap-insight'; + +const isAllowedTransactionTypes = (transactionType) => + transactionType === TransactionType.contractInteraction || + transactionType === TransactionType.simpleSend || + transactionType === TransactionType.tokenMethodSafeTransferFrom || + transactionType === TransactionType.tokenMethodTransferFrom || + transactionType === TransactionType.tokenMethodTransfer; + +// A hook was needed to return JSX here as the way Tabs work JSX has to be included in +// https://github.com/MetaMask/metamask-extension/blob/develop/ui/components/app/confirm-page-container/confirm-page-container-content/confirm-page-container-content.component.js#L129 +// Thus it is not possible to use React Component here +const useTransactionInsights = ({ txData }) => { + const insightSnaps = useSelector(getInsightSnaps); + const [selectedInsightSnapId, setSelectedInsightSnapId] = useState( + insightSnaps[0]?.id, + ); + + useEffect(() => { + if (insightSnaps.length && !selectedInsightSnapId) { + setSelectedInsightSnapId(insightSnaps[0]?.id); + } + }, [insightSnaps, selectedInsightSnapId, setSelectedInsightSnapId]); + + if (!isAllowedTransactionTypes(txData.type) || !insightSnaps.length) { + return null; + } + + const selectedSnap = insightSnaps.find( + ({ id }) => id === selectedInsightSnapId, + ); + + const { txParams, chainId, origin } = txData; + const networkId = CHAIN_ID_TO_NETWORK_ID_MAP[chainId]; + const caip2ChainId = `eip155:${networkId ?? stripHexPrefix(chainId)}`; + + if (insightSnaps.length === 1) { + return ( + + + + ); + } + + const dropdownOptions = insightSnaps?.map( + ({ id, manifest: { proposedName } }) => ({ + value: id, + name: proposedName, + }), + ); + + return ( + setSelectedInsightSnapId(snapId)} + > + + + ); +}; + +export default useTransactionInsights; diff --git a/ui/pages/confirm-transaction-base/__snapshots__/confirm-transaction-base.test.js.snap b/ui/pages/confirm-transaction-base/__snapshots__/confirm-transaction-base.test.js.snap new file mode 100644 index 000000000..0dd91842e --- /dev/null +++ b/ui/pages/confirm-transaction-base/__snapshots__/confirm-transaction-base.test.js.snap @@ -0,0 +1,542 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Confirm Transaction Base should match snapshot 1`] = ` +
+
+ +
+
+
+
+
+
+ + + + + +
+
+
+
+
+
+
+
+
+
+
+
+ +
+
+
+
+
+
+
+ + + + + +
+
+
+
+
+
+
+ 0x85c...D65e +
+
+
+
+
+
+ +
+
+
+ + Sending ETH + +
+
+ #undefined +
+
+
+

+
+ + + + + 0.0001 + +
+

+
+
+
+
    +
  • + +
    • +
  • + +
    • +
  • + +
    • +
+
+
+
+
+
+ +
+
+
+
+
+ Estimated gas fee +
+
+
+ + + +
+
+
+
+
+
+
+
+ + + 0.000021 + +
+
+
+
+
+
+
+ +
+
+ + Max fee: + +
+
+ + + 0.000021 + +
+
+
+
+
+
+
+
+ Total +
+
+
+
+
+ + + 0.000121 + +
+
+
+
+
+
+
+ Amount + gas fee +
+
+
+ + Max amount: + + +
+ + + 0.000121 + +
+
+
+
+
+
+
+
+
+
+
+
+ +
+ Insufficient funds. +
+
+
+
+
+ + +
+
+
+
+
+`; 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 ecbe550d0..18b716437 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -1,8 +1,5 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; -///: BEGIN:ONLY_INCLUDE_IN(flask) -import { stripHexPrefix } from 'ethereumjs-util'; -///: END:ONLY_INCLUDE_IN import ConfirmPageContainer from '../../components/app/confirm-page-container'; import TransactionDecoding from '../../components/app/transaction-decoding'; import { isBalanceSufficient } from '../send/send.utils'; @@ -56,17 +53,7 @@ import { import { MIN_GAS_LIMIT_DEC } from '../send/send.constants'; -///: BEGIN:ONLY_INCLUDE_IN(flask) -import { SnapInsight } from '../../components/app/confirm-page-container/flask/snap-insight'; -import { DropdownTab, Tab } from '../../components/ui/tabs'; -///: END:ONLY_INCLUDE_IN - -import { - NETWORK_TO_NAME_MAP, - ///: BEGIN:ONLY_INCLUDE_IN(flask) - CHAIN_ID_TO_NETWORK_ID_MAP, - ///: END:ONLY_INCLUDE_IN -} from '../../../shared/constants/network'; +import { NETWORK_TO_NAME_MAP } from '../../../shared/constants/network'; import { addHexes, hexToDecimal, @@ -158,9 +145,6 @@ export default class ConfirmTransactionBase extends Component { isMultiLayerFeeNetwork: PropTypes.bool, isBuyableChain: PropTypes.bool, isApprovalOrRejection: PropTypes.bool, - ///: BEGIN:ONLY_INCLUDE_IN(flask) - insightSnaps: PropTypes.arrayOf(PropTypes.object), - ///: END:ONLY_INCLUDE_IN assetStandard: PropTypes.string, useCurrencyRateCheck: PropTypes.bool, }; @@ -173,9 +157,6 @@ export default class ConfirmTransactionBase extends Component { editingGas: false, userAcknowledgedGasMissing: false, showWarningModal: false, - ///: BEGIN:ONLY_INCLUDE_IN(flask) - selectedInsightSnapId: this.props.insightSnaps[0]?.id, - ///: END:ONLY_INCLUDE_IN }; componentDidUpdate(prevProps) { @@ -319,12 +300,6 @@ export default class ConfirmTransactionBase extends Component { this.setState({ editingGas: false }); } - ///: BEGIN:ONLY_INCLUDE_IN(flask) - handleSnapSelected(snapId) { - this.setState({ selectedInsightSnapId: snapId }); - } - ///: END:ONLY_INCLUDE_IN - setUserAcknowledgedGasMissing() { this.setState({ userAcknowledgedGasMissing: true }); } @@ -753,67 +728,6 @@ export default class ConfirmTransactionBase extends Component { ); } - ///: BEGIN:ONLY_INCLUDE_IN(flask) - renderInsight() { - const { txData, insightSnaps } = this.props; - const { selectedInsightSnapId } = this.state; - const { txParams, chainId, origin } = txData; - - const selectedSnap = insightSnaps.find( - ({ id }) => id === selectedInsightSnapId, - ); - - const allowedTransactionTypes = - txData.type === TransactionType.contractInteraction || - txData.type === TransactionType.simpleSend || - txData.type === TransactionType.tokenMethodSafeTransferFrom || - txData.type === TransactionType.tokenMethodTransferFrom || - txData.type === TransactionType.tokenMethodTransfer; - - const networkId = CHAIN_ID_TO_NETWORK_ID_MAP[chainId]; - const caip2ChainId = `eip155:${networkId ?? stripHexPrefix(chainId)}`; - - if (!allowedTransactionTypes || !insightSnaps.length) { - return null; - } - - const dropdownOptions = insightSnaps.map( - ({ id, manifest: { proposedName } }) => ({ - value: id, - name: proposedName, - }), - ); - - return insightSnaps.length > 1 ? ( - this.handleSnapSelected(snapId)} - > - - - ) : ( - - - - ); - } - ///: END:ONLY_INCLUDE_IN - handleEdit() { const { txData, @@ -1175,9 +1089,6 @@ export default class ConfirmTransactionBase extends Component { detailsComponent={this.renderDetails()} dataComponent={this.renderData(functionType)} dataHexComponent={this.renderDataHex(functionType)} - ///: BEGIN:ONLY_INCLUDE_IN(flask) - insightComponent={this.renderInsight()} - ///: END:ONLY_INCLUDE_IN contentComponent={contentComponent} nonce={customNonceValue || nonce} unapprovedTxCount={unapprovedTxCount} 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 2cfa8c6bc..ba552ddad 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.container.js @@ -37,9 +37,6 @@ import { getUnapprovedTransaction, getFullTxData, getUseCurrencyRateCheck, - ///: BEGIN:ONLY_INCLUDE_IN(flask) - getInsightSnaps, - ///: END:ONLY_INCLUDE_IN } from '../../selectors'; import { getMostRecentOverviewPage } from '../../ducks/history/history'; import { @@ -195,10 +192,6 @@ const mapStateToProps = (state, ownProps) => { const isMultiLayerFeeNetwork = getIsMultiLayerFeeNetwork(state); - ///: BEGIN:ONLY_INCLUDE_IN(flask) - const insightSnaps = getInsightSnaps(state); - ///: END:ONLY_INCLUDE_IN - return { balance, fromAddress, @@ -249,9 +242,6 @@ const mapStateToProps = (state, ownProps) => { isMultiLayerFeeNetwork, chainId, isBuyableChain, - ///: BEGIN:ONLY_INCLUDE_IN(flask) - insightSnaps, - ///: END:ONLY_INCLUDE_IN useCurrencyRateCheck: getUseCurrencyRateCheck(state), }; }; diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.test.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.test.js new file mode 100644 index 000000000..5029bd814 --- /dev/null +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.test.js @@ -0,0 +1,151 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; + +import { renderWithProvider } from '../../../test/lib/render-helpers'; +import { setBackgroundConnection } from '../../../test/jest'; +import { INITIAL_SEND_STATE_FOR_EXISTING_DRAFT } from '../../../test/jest/mocks'; +import { GasEstimateTypes } from '../../../shared/constants/gas'; +import { HardwareKeyringTypes } from '../../../shared/constants/hardware-wallets'; +import { CHAIN_IDS } from '../../../shared/constants/network'; +import { domainInitialState } from '../../ducks/domains'; + +import ConfirmTransactionBase from './confirm-transaction-base.container'; + +const middleware = [thunk]; + +setBackgroundConnection({ + getGasFeeTimeEstimate: jest.fn(), + getGasFeeEstimatesAndStartPolling: jest.fn(), + promisifiedBackground: jest.fn(), + tryReverseResolveAddress: jest.fn(), + getNextNonce: jest.fn(), +}); + +const baseStore = { + send: INITIAL_SEND_STATE_FOR_EXISTING_DRAFT, + DNS: domainInitialState, + gas: { + customData: { limit: null, price: null }, + }, + history: { mostRecentOverviewPage: '/' }, + metamask: { + unapprovedTxs: { + 1: { + id: 1, + txParams: { + from: '0x0', + to: '0x85c1685cfceaa5c0bdb1609fc536e9a8387dd65e', + value: '0x5af3107a4000', + gas: '0x5208', + maxFeePerGas: '0x59682f16', + maxPriorityFeePerGas: '0x59682f00', + type: '0x2', + data: 'data', + }, + }, + }, + gasEstimateType: GasEstimateTypes.legacy, + gasFeeEstimates: { + low: '0', + medium: '1', + fast: '2', + }, + selectedAddress: '0x0', + keyrings: [ + { + type: HardwareKeyringTypes.hdKeyTree, + accounts: ['0x0'], + }, + ], + networkDetails: { + EIPS: {}, + }, + tokens: [], + preferences: { + useNativeCurrencyAsPrimaryCurrency: false, + }, + currentCurrency: 'USD', + provider: { + chainId: CHAIN_IDS.GOERLI, + }, + nativeCurrency: 'ETH', + featureFlags: { + sendHexData: false, + }, + addressBook: { + [CHAIN_IDS.GOERLI]: [], + }, + cachedBalances: { + [CHAIN_IDS.GOERLI]: {}, + }, + accounts: { + '0x0': { balance: '0x0', address: '0x0' }, + }, + identities: { '0x0': { address: '0x0' } }, + tokenAddress: '0x32e6c34cd57087abbd59b5a4aecc4cb495924356', + tokenList: {}, + ensResolutionsByAddress: {}, + snaps: {}, + }, + confirmTransaction: { + txData: { + id: 1, + time: 1675012496170, + status: 'unapproved', + metamaskNetworkId: '5', + originalGasEstimate: '0x5208', + userEditedGasLimit: false, + chainId: '0x5', + loadingDefaults: false, + dappSuggestedGasFees: null, + sendFlowHistory: [], + txParams: { + from: '0x0', + to: '0x85c1685cfceaa5c0bdb1609fc536e9a8387dd65e', + value: '0x5af3107a4000', + gas: '0x5208', + maxFeePerGas: '0x59682f16', + maxPriorityFeePerGas: '0x59682f00', + type: '0x2', + }, + origin: 'metamask', + actionId: 1675012496153.2039, + type: 'simpleSend', + history: [], + userFeeLevel: 'medium', + defaultGasEstimates: { + estimateType: 'medium', + gas: '0x5208', + maxFeePerGas: '0x59682f16', + maxPriorityFeePerGas: '0x59682f00', + }, + }, + tokenData: {}, + tokenProps: {}, + fiatTransactionAmount: '0.16', + fiatTransactionFee: '0', + fiatTransactionTotal: '0.16', + ethTransactionAmount: '0.0001', + ethTransactionFee: '0', + ethTransactionTotal: '0.0001', + hexTransactionAmount: '0x5af3107a4000', + hexTransactionFee: '0x0', + hexTransactionTotal: '0x5af3107a4000', + nonce: '', + }, + appState: { + sendInputCurrencySwitched: false, + }, +}; + +describe('Confirm Transaction Base', () => { + const store = configureMockStore(middleware)(baseStore); + it('should match snapshot', () => { + const { container } = renderWithProvider( + , + store, + ); + expect(container).toMatchSnapshot(); + }); +});