From 44ccf3b548dc8ef6d077d02c0a6098dd48d58ff2 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Mon, 3 Nov 2025 11:54:28 +0000 Subject: [PATCH] chore: Refactor to remove warnings in confirmations' --- .../modules/selectors/smart-transactions.ts | 11 +- test/jest/setup.js | 27 ++ .../account-list-menu.test.tsx | 22 +- .../__snapshots__/confirm.test.tsx.snap | 171 +++++++-- .../confirmations/confirm/confirm.test.tsx | 102 ++--- .../useNonContractAddressAlerts.test.ts | 352 ++++++------------ .../useAutomaticGasFeeTokenSelect.test.ts | 50 ++- .../hooks/useCurrentConfirmation.ts | 37 +- ui/pages/confirmations/selectors/confirm.ts | 99 ++++- .../onboarding-flow/welcome/welcome.test.js | 11 +- ui/selectors/accounts.ts | 11 +- ui/selectors/approvals.ts | 20 +- ui/selectors/selectors.js | 28 +- ui/selectors/signatures.ts | 6 +- ui/selectors/transactions.js | 83 +++-- 15 files changed, 559 insertions(+), 471 deletions(-) diff --git a/shared/modules/selectors/smart-transactions.ts b/shared/modules/selectors/smart-transactions.ts index e13ed3911d1a..5fc180cab91d 100644 --- a/shared/modules/selectors/smart-transactions.ts +++ b/shared/modules/selectors/smart-transactions.ts @@ -111,7 +111,10 @@ export const getSmartTransactionsMigrationAppliedInternal = createSelector( // @ts-expect-error TODO: Fix types for `getSmartTransactionsOptInStatusInternal` once `getPreferences is converted to TypeScript export const getSmartTransactionsOptInStatusForMetrics = createSelector( getSmartTransactionsOptInStatusInternal, - (optInStatus: boolean): boolean => optInStatus, + (optInStatus: boolean | null | undefined): boolean | null => + optInStatus === undefined || optInStatus === null + ? null + : Boolean(optInStatus), ); /** @@ -124,11 +127,13 @@ export const getSmartTransactionsOptInStatusForMetrics = createSelector( // @ts-expect-error TODO: Fix types for `getSmartTransactionsOptInStatusInternal` once `getPreferences is converted to TypeScript export const getSmartTransactionsPreferenceEnabled = createSelector( getSmartTransactionsOptInStatusInternal, - (optInStatus: boolean): boolean => { + (optInStatus: boolean | null | undefined): boolean => { // In the absence of an explicit opt-in or opt-out, // the Smart Transactions toggle is enabled. const DEFAULT_SMART_TRANSACTIONS_ENABLED = true; - return optInStatus ?? DEFAULT_SMART_TRANSACTIONS_ENABLED; + return optInStatus === undefined || optInStatus === null + ? DEFAULT_SMART_TRANSACTIONS_ENABLED + : Boolean(optInStatus); }, ); diff --git a/test/jest/setup.js b/test/jest/setup.js index c73c088a6091..9854e1ee2137 100644 --- a/test/jest/setup.js +++ b/test/jest/setup.js @@ -1,6 +1,33 @@ // This file is for Jest-specific setup only and runs before our Jest tests. +import { setBackgroundConnection } from '../../ui/store/background-connection'; import '../helpers/setup-after-helper'; +const backgroundMethodCache = new Map(); + +const backgroundStub = new Proxy( + {}, + { + get: (_target, prop) => { + if (prop === '__esModule') { + return undefined; + } + + const cacheKey = String(prop); + + if (!backgroundMethodCache.has(cacheKey)) { + backgroundMethodCache.set( + cacheKey, + jest.fn().mockResolvedValue(undefined), + ); + } + + return backgroundMethodCache.get(cacheKey); + }, + }, +); + +setBackgroundConnection(backgroundStub); + jest.mock('webextension-polyfill', () => { return { runtime: { diff --git a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx index 2981cb346809..36028f80ef14 100644 --- a/ui/components/multichain/account-list-menu/account-list-menu.test.tsx +++ b/ui/components/multichain/account-list-menu/account-list-menu.test.tsx @@ -98,7 +98,21 @@ const render = ( state: 'OPEN', }, }; - const store = configureStore(merge(defaultState, state)); + const combinedState = merge({}, defaultState, state); + if (state?.metamask?.internalAccounts) { + if ('accounts' in state.metamask.internalAccounts) { + combinedState.metamask.internalAccounts.accounts = + state.metamask.internalAccounts.accounts; + } + if ('selectedAccount' in state.metamask.internalAccounts) { + combinedState.metamask.internalAccounts.selectedAccount = + state.metamask.internalAccounts.selectedAccount; + } + } + if (state?.metamask && 'keyrings' in state.metamask) { + combinedState.metamask.keyrings = state.metamask.keyrings; + } + const store = configureStore(combinedState); return renderWithProvider(, store, location); }; @@ -395,10 +409,16 @@ describe('AccountListMenu', () => { { type: 'HD Key Tree', accounts: [mockAccount.address], + metadata: { + id: 'mock-hd-keyring-id', + }, }, { type: 'Snap Keyring', accounts: [mockBtcAccount.address], + metadata: { + id: 'mock-snap-keyring-id', + }, }, ], }, diff --git a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap index 1d9ade407617..337a3e4e4460 100644 --- a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap +++ b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap @@ -284,15 +284,48 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitB >
+ > +
+ + + + + +
+
- Goerli logo + G
+

+ Spending cap request +

+

+ This site wants permission to spend your tokens. +

+ > +
+ + + + + +
+
- Goerli logo + G
+

+ Spending cap request +

+

+ This site wants permission to spend your tokens. +

+ > +
+ + + + + +
+
- Goerli logo + G
+

+ Signature request +

+

+ Review request details before you confirm. +

{ const middleware = [thunk]; const mockedAssetDetails = jest.mocked(useAssetDetails); +const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); + +const renderConfirm = async ( + mockStore: MockStoreEnhanced, +): Promise => { + let renderResult: RenderResult | undefined; + await act(async () => { + renderResult = renderWithConfirmContextProvider(, mockStore); + await flushPromises(); + }); + if (!renderResult) { + throw new Error('Failed to render Confirm component'); + } + return renderResult; +}; describe('Confirm', () => { afterEach(() => { @@ -70,16 +87,11 @@ describe('Confirm', () => { })); }); - it('should render', () => { + it('should render', async () => { const mockStore = configureMockStore(middleware)(mockState); - act(() => { - const { container } = renderWithConfirmContextProvider( - , - mockStore, - ); - expect(container).toBeDefined(); - }); + const { container } = await renderConfirm(mockStore); + expect(container).toBeDefined(); }); it('should match snapshot for signature - typed sign - permit', async () => { @@ -96,15 +108,7 @@ describe('Confirm', () => { }); const mockStore = configureMockStore(middleware)(mockStateTypedSign); - let container; - - await act(async () => { - const { container: renderContainer } = renderWithConfirmContextProvider( - , - mockStore, - ); - container = renderContainer; - }); + const { container } = await renderConfirm(mockStore); expect(container).toMatchSnapshot(); }); @@ -112,14 +116,7 @@ describe('Confirm', () => { it('matches snapshot for signature - personal sign type', async () => { const mockStatePersonalSign = getMockPersonalSignConfirmState(); const mockStore = configureMockStore(middleware)(mockStatePersonalSign); - - let container; - await act(async () => { - const { container: renderContainer } = - await renderWithConfirmContextProvider(, mockStore); - - container = renderContainer; - }); + const { container } = await renderConfirm(mockStore); expect(container).toMatchSnapshot(); }); @@ -140,15 +137,7 @@ describe('Confirm', () => { }); const mockStore = configureMockStore(middleware)(mockStateTypedSign); - - let container; - await act(async () => { - const { container: renderContainer } = renderWithConfirmContextProvider( - , - mockStore, - ); - container = renderContainer; - }); + const { container } = await renderConfirm(mockStore); expect(container).toMatchSnapshot(); }); @@ -156,14 +145,7 @@ describe('Confirm', () => { it('should match snapshot for signature - typed sign - V4', async () => { const mockStateTypedSign = getMockTypedSignConfirmState(); const mockStore = configureMockStore(middleware)(mockStateTypedSign); - - let container; - await act(async () => { - const { container: renderContainer } = - await renderWithConfirmContextProvider(, mockStore); - - container = renderContainer; - }); + const { container } = await renderConfirm(mockStore); expect(container).toMatchSnapshot(); }); @@ -182,14 +164,9 @@ describe('Confirm', () => { standard: 'ERC20', }); - await act(async () => { - const { container } = await renderWithConfirmContextProvider( - , - mockStore, - ); + const renderResult = await renderConfirm(mockStore); - expect(container).toMatchSnapshot(); - }); + expect(renderResult.container).toMatchSnapshot(); }); it('should match snapshot for signature - typed sign - V4 - PermitBatch', async () => { @@ -206,14 +183,9 @@ describe('Confirm', () => { standard: 'ERC20', }); - await act(async () => { - const { container } = await renderWithConfirmContextProvider( - , - mockStore, - ); + const renderResult = await renderConfirm(mockStore); - expect(container).toMatchSnapshot(); - }); + expect(renderResult.container).toMatchSnapshot(); }); it('should render SmartTransactionsBannerAlert for transaction types but not signature types', async () => { @@ -235,24 +207,14 @@ describe('Confirm', () => { const mockStoreTransaction = configureMockStore(middleware)(mockStateTransaction); - await act(async () => { - const { container } = renderWithConfirmContextProvider( - , - mockStoreTransaction, - ); - expect(container).toMatchSnapshot(); - }); + const transactionRender = await renderConfirm(mockStoreTransaction); + expect(transactionRender.container).toMatchSnapshot(); // Test with a signature type (reuse existing mock) const mockStateTypedSign = getMockTypedSignConfirmState(); const mockStoreSign = configureMockStore(middleware)(mockStateTypedSign); - await act(async () => { - const { container } = renderWithConfirmContextProvider( - , - mockStoreSign, - ); - expect(container).toMatchSnapshot(); - }); + const signatureRender = await renderConfirm(mockStoreSign); + expect(signatureRender.container).toMatchSnapshot(); }); }); diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useNonContractAddressAlerts.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useNonContractAddressAlerts.test.ts index 32ca6b58dde3..370ab29282fc 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useNonContractAddressAlerts.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useNonContractAddressAlerts.test.ts @@ -5,28 +5,36 @@ import { TransactionType, } from '@metamask/transaction-controller'; import { waitFor } from '@testing-library/react'; -import { useContext } from 'react'; -import { useSelector } from 'react-redux'; +import { createElement, type ReactNode } from 'react'; import { useLocation } from 'react-router-dom'; -import { getNetworkConfigurationsByChainId } from '../../../../../../shared/modules/selectors/networks'; -import { getMockConfirmStateForTransaction } from '../../../../../../test/data/confirmations/helper'; +import { + getMockConfirmState, + getMockConfirmStateForTransaction, +} from '../../../../../../test/data/confirmations/helper'; import { genUnapprovedContractInteractionConfirmation } from '../../../../../../test/data/confirmations/contract-interaction'; -import { renderHookWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; +import { renderHookWithProvider } from '../../../../../../test/lib/render-helpers'; import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants'; -import { I18nContext } from '../../../../../contexts/i18n'; import { Severity } from '../../../../../helpers/constants/design-system'; -import { selectPendingApprovalsForNavigation } from '../../../../../selectors'; +import { useI18nContext } from '../../../../../hooks/useI18nContext'; +import { useIsUpgradeTransaction } from '../../../components/confirm/info/hooks/useIsUpgradeTransaction'; import { ConfirmContext } from '../../../context/confirm'; +import type { ConfirmContextType } from '../../../context/confirm'; +import type { Confirmation } from '../../../types/confirm'; import { useNonContractAddressAlerts } from './useNonContractAddressAlerts'; import { useContractCode } from './useContractCode'; -jest.mock('react-redux', () => ({ - ...jest.requireActual('react-redux'), - useSelector: jest.fn(), - useDispatch: jest.fn(), +jest.mock('../../../../../hooks/useI18nContext', () => ({ + useI18nContext: jest.fn(), })); +jest.mock( + '../../../components/confirm/info/hooks/useIsUpgradeTransaction', + () => ({ + useIsUpgradeTransaction: jest.fn(), + }), +); + const messageIdMock = '12345'; jest.mock('react-router-dom', () => ({ @@ -37,11 +45,6 @@ jest.mock('react-router-dom', () => ({ useLocation: jest.fn(), })); -jest.mock('react', () => ({ - ...jest.requireActual('react'), - useContext: jest.fn(), -})); - jest.mock('./useContractCode', () => ({ useContractCode: jest.fn(), })); @@ -54,6 +57,8 @@ const TRANSACTION_ID_MOCK = '123-456'; const ACCOUNT_ADDRESS_MOCK = '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'; const ACCOUNT_ADDRESS_2_MOCK = '0x2e0d7e8c45221fca00d74a3609a0f7097035d09b'; +type MockState = Parameters[0]; + const TRANSACTION_META_MOCK = { id: TRANSACTION_ID_MOCK, chainId: '0x5', @@ -67,74 +72,82 @@ const TRANSACTION_META_MOCK = { time: new Date().getTime() - 10000, } as TransactionMeta; +function isTransactionMetaConfirmation( + confirmation: Confirmation, +): confirmation is TransactionMeta { + return 'txParams' in confirmation; +} + function runHook({ currentConfirmation, + stateOverrides, }: { - currentConfirmation?: TransactionMeta; + currentConfirmation?: Confirmation; + stateOverrides?: MockState; } = {}) { - const state = currentConfirmation - ? getMockConfirmStateForTransaction(currentConfirmation as TransactionMeta) - : {}; + const confirmContextValue = { + currentConfirmation, + isScrollToBottomCompleted: true, + setIsScrollToBottomCompleted: jest.fn(), + }; + + const Container = ({ children }: { children: ReactNode }) => + createElement( + ConfirmContext.Provider, + { + value: confirmContextValue as unknown as ConfirmContextType, + }, + children, + ); - const response = renderHookWithConfirmContextProvider( + const state = + currentConfirmation && isTransactionMetaConfirmation(currentConfirmation) + ? getMockConfirmStateForTransaction(currentConfirmation, stateOverrides) + : getMockConfirmState(stateOverrides); + + return renderHookWithProvider( useNonContractAddressAlerts, state, + '/', + Container, ); - - return response.result.current; } describe('useNonContractAddressAlerts', () => { - const useContextMock = useContext as jest.Mock; - const useSelectorMock = useSelector as jest.Mock; + const mockUseI18nContext = jest.mocked(useI18nContext); + const mockUseIsUpgradeTransaction = jest.mocked(useIsUpgradeTransaction); const mockUseContractCode = jest.mocked(useContractCode); const useLocationMock = jest.mocked(useLocation); beforeEach(() => { jest.resetAllMocks(); + mockUseI18nContext.mockReturnValue( + (translationKey: string) => translationKey, + ); + + mockUseIsUpgradeTransaction.mockReturnValue({ + isUpgrade: false, + isUpgradeOnly: false, + }); + useLocationMock.mockReturnValue({ search: '', } as unknown as ReturnType); - mockUseContractCode.mockImplementation( - () => - ({ - pending: false, - value: { - isContractAddress: false, - contractCode: '', - }, - }) as ReturnType, - ); + mockUseContractCode.mockReturnValue({ + pending: false, + value: { + isContractAddress: false, + contractCode: '', + }, + } as ReturnType); }); it('returns no alerts if no confirmation', () => { - const confirmation = TRANSACTION_META_MOCK; - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: confirmation }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; - }); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [confirmation]; - } - - return undefined; - }); + const { result } = runHook(); - expect(runHook()).toEqual([]); + expect(result.current).toEqual([]); }); it('returns no alerts if the transaction has no data', () => { @@ -146,34 +159,11 @@ describe('useNonContractAddressAlerts', () => { }, }; - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: transactionWithNoData }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; - }); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [transactionWithNoData]; - } - - return undefined; + const { result } = runHook({ + currentConfirmation: transactionWithNoData, }); - expect( - runHook({ - currentConfirmation: transactionWithNoData, - }), - ).toEqual([]); + expect(result.current).toEqual([]); }); it('returns no alerts if the transaction has data but the recipient is a contract', () => { @@ -185,84 +175,36 @@ describe('useNonContractAddressAlerts', () => { }, }; - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: transactionWithData }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; - }); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [transactionWithData]; - } - - return undefined; - }); + mockUseContractCode.mockReturnValue({ + pending: false, + value: { + isContractAddress: true, + contractCode: '', + }, + } as ReturnType); - mockUseContractCode.mockImplementation( - () => - ({ - pending: false, - value: { - isContractAddress: true, - contractCode: '', - }, - }) as ReturnType, - ); + const { result } = runHook({ + currentConfirmation: transactionWithData, + }); - expect( - runHook({ - currentConfirmation: transactionWithData, - }), - ).toEqual([]); + expect(result.current).toEqual([]); }); - it('returns no alert for authorization request', async () => { + it('returns no alert for authorization request', () => { const authorizationList = [{ address: '0x123' as Hex }]; const transaction = genUnapprovedContractInteractionConfirmation({ authorizationList, }); - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: transaction }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; + mockUseIsUpgradeTransaction.mockReturnValueOnce({ + isUpgrade: true, + isUpgradeOnly: false, }); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [transaction]; - } - - return undefined; + const { result } = runHook({ + currentConfirmation: transaction, }); - const { result } = renderHookWithConfirmContextProvider( - useNonContractAddressAlerts, - { - currentConfirmation: transaction, - }, - ); - expect(result.current).toEqual([]); }); @@ -274,35 +216,10 @@ describe('useNonContractAddressAlerts', () => { data: '0xabcdef', }, }; - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: transactionWithData }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; - }); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [transactionWithData]; - } - - return undefined; - }); - const { result } = renderHookWithConfirmContextProvider( - useNonContractAddressAlerts, - { - currentConfirmation: transactionWithData, - }, - ); + const { result } = runHook({ + currentConfirmation: transactionWithData, + }); await waitFor(() => { expect(result.current).toEqual([ @@ -327,35 +244,10 @@ describe('useNonContractAddressAlerts', () => { data: '0xabcdef', }, }; - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: transactionWithData }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; - }); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [transactionWithData]; - } - - return undefined; - }); - const { result } = renderHookWithConfirmContextProvider( - useNonContractAddressAlerts, - { - currentConfirmation: transactionWithData, - }, - ); + const { result } = runHook({ + currentConfirmation: transactionWithData, + }); await waitFor(() => { expect(result.current).toEqual([]); @@ -371,48 +263,18 @@ describe('useNonContractAddressAlerts', () => { }, }; - useContextMock.mockImplementation((context) => { - if (context === ConfirmContext) { - return { currentConfirmation: transactionWithData }; - } else if (context === I18nContext) { - return (translationKey: string) => translationKey; - } - return undefined; - }); + mockUseContractCode.mockReturnValue({ + pending: false, + value: { + isContractAddress: false, + contractCode: null, + }, + } as ReturnType); - useSelectorMock.mockImplementation((selector) => { - if (selector === getNetworkConfigurationsByChainId) { - return { - '0x5': { - chainId: '0x5', - name: 'Mainnet', - }, - }; - } else if (selector === selectPendingApprovalsForNavigation) { - return [transactionWithData]; - } - - return undefined; + const { result } = runHook({ + currentConfirmation: transactionWithData, }); - mockUseContractCode.mockImplementation( - () => - ({ - pending: false, - value: { - isContractAddress: false, - contractCode: null, // simulate failure - }, - }) as ReturnType, - ); - - const { result } = renderHookWithConfirmContextProvider( - useNonContractAddressAlerts, - { - currentConfirmation: transactionWithData, - }, - ); - await waitFor(() => { expect(result.current).toEqual([]); }); diff --git a/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts b/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts index a0f9851baaf6..5e19457a1839 100644 --- a/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts +++ b/ui/pages/confirmations/hooks/useAutomaticGasFeeTokenSelect.test.ts @@ -11,6 +11,7 @@ import { updateSelectedGasFeeToken } from '../../../store/controller-actions/tra import { Alert } from '../../../ducks/confirm-alerts/confirm-alerts'; import { forceUpdateMetamaskState } from '../../../store/actions'; import { GAS_FEE_TOKEN_MOCK } from '../../../../test/data/confirmations/gas'; +import * as actionConstants from '../../../store/actionConstants'; import { useInsufficientBalanceAlerts } from './alerts/transactions/useInsufficientBalanceAlerts'; import { useAutomaticGasFeeTokenSelect } from './useAutomaticGasFeeTokenSelect'; import { useIsGaslessSupported } from './gas/useIsGaslessSupported'; @@ -53,6 +54,35 @@ async function flushAsyncUpdates() { }); } +type ConfirmHookRenderResult = ReturnType< + typeof renderHookWithConfirmContextProvider +>; + +function updateTransactionInStore( + store: ConfirmHookRenderResult['store'], + transactionId: string, + updates: Partial, +) { + if (!store) { + throw new Error('Expected store to be defined'); + } + + act(() => { + const currentTransactions = store + .getState() + .metamask.transactions.map((transaction: TransactionMeta) => + transaction.id === transactionId + ? { ...transaction, ...updates } + : transaction, + ); + + store.dispatch({ + type: actionConstants.UPDATE_METAMASK_STATE, + value: { transactions: currentTransactions }, + }); + }); +} + describe('useAutomaticGasFeeTokenSelect', () => { const updateSelectedGasFeeTokenMock = jest.mocked(updateSelectedGasFeeToken); const forceUpdateMetamaskStateMock = jest.mocked(forceUpdateMetamaskState); @@ -111,7 +141,7 @@ describe('useAutomaticGasFeeTokenSelect', () => { }); it('selects first gas fee token on rerender when selection becomes eligible', async () => { - const { rerender, state, store } = runHook({ + const { rerender, store } = runHook({ selectedGasFeeToken: GAS_FEE_TOKEN_MOCK.tokenAddress, }); @@ -124,11 +154,11 @@ describe('useAutomaticGasFeeTokenSelect', () => { expect(updateSelectedGasFeeTokenMock).toHaveBeenCalledTimes(0); expect(forceUpdateMetamaskStateMock).toHaveBeenCalledTimes(0); - const transactionMeta = state.metamask - .transactions[0] as unknown as TransactionMeta; + const transactionMeta = store.getState().metamask + .transactions[0] as TransactionMeta; - act(() => { - transactionMeta.selectedGasFeeToken = undefined; + updateTransactionInStore(store, transactionMeta.id, { + selectedGasFeeToken: undefined, }); rerender(); @@ -202,7 +232,7 @@ describe('useAutomaticGasFeeTokenSelect', () => { }); it('does not select first gas fee token after firstCheck is set to false', async () => { - const { rerender, state, store } = runHook(); + const { rerender, store } = runHook(); if (!store) { throw new Error('Expected store to be defined'); @@ -211,10 +241,10 @@ describe('useAutomaticGasFeeTokenSelect', () => { await flushAsyncUpdates(); // Simulate a rerender with new state that would otherwise trigger selection - act(() => { - ( - state.metamask.transactions[0] as unknown as TransactionMeta - ).selectedGasFeeToken = undefined; + const transactionMeta = store.getState().metamask + .transactions[0] as TransactionMeta; + updateTransactionInStore(store, transactionMeta.id, { + selectedGasFeeToken: undefined, }); rerender(); diff --git a/ui/pages/confirmations/hooks/useCurrentConfirmation.ts b/ui/pages/confirmations/hooks/useCurrentConfirmation.ts index 4a1a2734fc0b..a9173a963ddb 100644 --- a/ui/pages/confirmations/hooks/useCurrentConfirmation.ts +++ b/ui/pages/confirmations/hooks/useCurrentConfirmation.ts @@ -1,15 +1,10 @@ import { ApprovalType } from '@metamask/controller-utils'; -import { TransactionMeta } from '@metamask/transaction-controller'; import { useMemo } from 'react'; import { useSelector } from 'react-redux'; import { useParams } from 'react-router-dom'; -import { - ApprovalsMetaMaskState, - getUnapprovedTransaction, - oldestPendingConfirmationSelector, - selectPendingApproval, -} from '../../../selectors'; -import { selectUnapprovedMessage } from '../../../selectors/signatures'; +import { selectConfirmationData } from '../selectors/confirm'; +import type { ConfirmationSelection } from '../selectors/confirm'; +import { ConfirmMetamaskState } from '../types/confirm'; import { shouldUseRedesignForSignatures, shouldUseRedesignForTransactions, @@ -25,25 +20,18 @@ import { */ const useCurrentConfirmation = () => { const { id: paramsConfirmationId } = useParams<{ id: string }>(); - const oldestPendingApproval = useSelector(oldestPendingConfirmationSelector); - const confirmationId = paramsConfirmationId ?? oldestPendingApproval?.id; - - const pendingApproval = useSelector((state) => - selectPendingApproval(state as ApprovalsMetaMaskState, confirmationId), - ); - const transactionMetadata = useSelector((state) => - // TODO: Fix in https://github.com/MetaMask/metamask-extension/issues/31973 - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (getUnapprovedTransaction as any)(state, confirmationId), - ) as TransactionMeta | undefined; - - const signatureMessage = useSelector((state) => - selectUnapprovedMessage(state, confirmationId), + const { + pendingApproval, + transactionMeta: transactionMetadata, + signatureMessage, + } = useSelector((state) => + selectConfirmationData(state, paramsConfirmationId), ); + const pendingApprovalType = pendingApproval?.type as ApprovalType | undefined; const useRedesignedForSignatures = shouldUseRedesignForSignatures({ - approvalType: pendingApproval?.type as ApprovalType, + approvalType: pendingApprovalType, }); const useRedesignedForTransaction = shouldUseRedesignForTransactions({ @@ -54,7 +42,7 @@ const useCurrentConfirmation = () => { useRedesignedForSignatures || useRedesignedForTransaction; return useMemo(() => { - if (pendingApproval?.type === ApprovalType.AddEthereumChain) { + if (pendingApprovalType === ApprovalType.AddEthereumChain) { return { currentConfirmation: pendingApproval }; } @@ -71,6 +59,7 @@ const useCurrentConfirmation = () => { signatureMessage, shouldUseRedesign, pendingApproval, + pendingApprovalType, ]); }; diff --git a/ui/pages/confirmations/selectors/confirm.ts b/ui/pages/confirmations/selectors/confirm.ts index 3898da957a9a..93d3272f0bae 100644 --- a/ui/pages/confirmations/selectors/confirm.ts +++ b/ui/pages/confirmations/selectors/confirm.ts @@ -1,9 +1,16 @@ +import type { ApprovalControllerState } from '@metamask/approval-controller'; import { ApprovalType } from '@metamask/controller-utils'; - +import type { TransactionMeta } from '@metamask/transaction-controller'; import { createSelector } from 'reselect'; -import { getPendingApprovals } from '../../../selectors/approvals'; import { createDeepEqualSelector } from '../../../../shared/modules/selectors/util'; -import { ConfirmMetamaskState } from '../types/confirm'; +import { + ApprovalsMetaMaskState, + getPendingApprovals, + selectPendingApproval, +} from '../../../selectors/approvals'; +import { selectUnapprovedMessage } from '../../../selectors/signatures'; +import { getUnapprovedTransactions } from '../../../selectors/transactions'; +import { ConfirmMetamaskState, SignatureRequestType } from '../types/confirm'; const ConfirmationApprovalTypes = [ ApprovalType.PersonalSign, @@ -11,30 +18,82 @@ const ConfirmationApprovalTypes = [ ApprovalType.Transaction, ]; -export function pendingConfirmationsSelector(state: ConfirmMetamaskState) { - return getPendingApprovals(state).filter(({ type }) => - ConfirmationApprovalTypes.includes(type as ApprovalType), - ); -} - -export function pendingConfirmationsSortedSelector( - state: ConfirmMetamaskState, -) { - return getPendingApprovals(state) - .filter(({ type }) => +export const pendingConfirmationsSelector = createSelector( + getPendingApprovals, + (pendingApprovals) => + pendingApprovals.filter(({ type }) => ConfirmationApprovalTypes.includes(type as ApprovalType), - ) - .sort((a1, a2) => a1.time - a2.time); -} + ), +); + +export const pendingConfirmationsSortedSelector = createSelector( + pendingConfirmationsSelector, + (pendingConfirmations) => + [...pendingConfirmations].sort((a1, a2) => a1.time - a2.time), +); const firstPendingConfirmationSelector = createSelector( pendingConfirmationsSortedSelector, (pendingConfirmations) => pendingConfirmations[0], ); -export const oldestPendingConfirmationSelector = createDeepEqualSelector( - firstPendingConfirmationSelector, - (firstPendingConfirmation) => firstPendingConfirmation, +export const oldestPendingConfirmationSelector = + firstPendingConfirmationSelector; + +const selectUnapprovedTransaction = createDeepEqualSelector( + getUnapprovedTransactions, + (_state: ConfirmMetamaskState, transactionId?: string) => transactionId, + (unapprovedTxs, transactionId): TransactionMeta | undefined => { + if (!unapprovedTxs || typeof unapprovedTxs !== 'object' || !transactionId) { + return undefined; + } + + const typedUnapprovedTxs = unapprovedTxs as Record; + const match = Object.values(typedUnapprovedTxs).find( + ({ id }) => id === transactionId, + ); + + return match ? { ...match } : undefined; + }, +); + +export type ConfirmationSelection = { + id?: string; + pendingApproval?: ApprovalControllerState['pendingApprovals'][string]; + transactionMeta?: TransactionMeta; + signatureMessage?: SignatureRequestType; +}; + +export const selectConfirmationData = createDeepEqualSelector( + (state: ConfirmMetamaskState, confirmationId?: string) => + confirmationId ?? oldestPendingConfirmationSelector(state)?.id, + (state: ConfirmMetamaskState, _confirmationId?: string) => state, + (effectiveId, state): ConfirmationSelection => { + if (!effectiveId) { + return { + id: undefined, + pendingApproval: undefined, + transactionMeta: undefined, + signatureMessage: undefined, + }; + } + + const pendingApproval = selectPendingApproval( + state as ApprovalsMetaMaskState, + effectiveId, + ) as ApprovalControllerState['pendingApprovals'][string] | undefined; + const transactionMeta = selectUnapprovedTransaction(state, effectiveId); + const signatureMessage = selectUnapprovedMessage(state, effectiveId) as + | SignatureRequestType + | undefined; + + return { + id: effectiveId, + pendingApproval, + transactionMeta, + signatureMessage, + }; + }, ); export function selectEnableEnforcedSimulations( diff --git a/ui/pages/onboarding-flow/welcome/welcome.test.js b/ui/pages/onboarding-flow/welcome/welcome.test.js index e9e356853929..bdf864126615 100644 --- a/ui/pages/onboarding-flow/welcome/welcome.test.js +++ b/ui/pages/onboarding-flow/welcome/welcome.test.js @@ -49,7 +49,7 @@ describe('Welcome Page', () => { let enabledMetricsSpy; beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); startOAuthLoginSpy = jest .spyOn(Actions, 'startOAuthLogin') .mockReturnValueOnce(jest.fn().mockResolvedValueOnce(true)); @@ -89,10 +89,11 @@ describe('Welcome Page', () => { }); it('should show the error modal when the error thrown in login', async () => { - jest.resetAllMocks(); - jest - .spyOn(Actions, 'startOAuthLogin') - .mockReturnValueOnce(jest.fn().mockRejectedValueOnce(new Error('test'))); + jest.clearAllMocks(); + startOAuthLoginSpy.mockReset(); + startOAuthLoginSpy.mockReturnValueOnce( + jest.fn().mockRejectedValueOnce(new Error('test')), + ); const { getByText, getByTestId } = renderWithProvider( , diff --git a/ui/selectors/accounts.ts b/ui/selectors/accounts.ts index 6d9c8d5123e6..cb910cb43af9 100644 --- a/ui/selectors/accounts.ts +++ b/ui/selectors/accounts.ts @@ -33,16 +33,13 @@ export function isNonEvmAccount(account: InternalAccount) { } export const getInternalAccounts = createSelector( - (state: AccountsState) => - Object.values(state.metamask.internalAccounts.accounts), - (accounts) => accounts, -); - -export const getInternalAccountsObject = createSelector( (state: AccountsState) => state.metamask.internalAccounts.accounts, - (internalAccounts) => internalAccounts, + (accounts) => Object.values(accounts), ); +export const getInternalAccountsObject = (state: AccountsState) => + state.metamask.internalAccounts.accounts; + export const getMemoizedInternalAccountByAddress = createDeepEqualSelector( [getInternalAccounts, (_state, address) => address], (internalAccounts, address) => { diff --git a/ui/selectors/approvals.ts b/ui/selectors/approvals.ts index edb74c00313f..81e67a570b67 100644 --- a/ui/selectors/approvals.ts +++ b/ui/selectors/approvals.ts @@ -54,13 +54,16 @@ export function getApprovalFlows(state: ApprovalsMetaMaskState) { return state.metamask.approvalFlows; } -export function getPendingApprovals(state: ApprovalsMetaMaskState) { - return Object.values(state.metamask.pendingApprovals ?? {}); -} +export const getPendingApprovals = createSelector( + (state: ApprovalsMetaMaskState) => state.metamask.pendingApprovals ?? {}, + (pendingApprovals) => Object.values(pendingApprovals), +); -export function pendingApprovalsSortedSelector(state: ApprovalsMetaMaskState) { - return getPendingApprovals(state).sort((a1, a2) => a1.time - a2.time); -} +export const pendingApprovalsSortedSelector = createSelector( + getPendingApprovals, + (pendingApprovals) => + [...pendingApprovals].sort((a1, a2) => a1.time - a2.time), +); /** * Returns pending approvals sorted by time for use in confirmation navigation. @@ -94,10 +97,7 @@ const internalSelectPendingApproval = createSelector( (approvals, id) => approvals.find(({ id: approvalId }) => approvalId === id), ); -export const selectPendingApproval = createDeepEqualSelector( - internalSelectPendingApproval, - (approval) => approval, -); +export const selectPendingApproval = internalSelectPendingApproval; export const getApprovalsByOrigin = ( state: ApprovalsMetaMaskState, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index b735468da5ea..0a63c1faa078 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -452,7 +452,7 @@ export const getMetaMaskAccounts = createDeepEqualSelector( currentChainId, chainId, ) => - Object.values(internalAccounts).reduce((accounts, internalAccount) => { + internalAccounts.reduce((accounts, internalAccount) => { // TODO: mix in the identity state here as well, consolidating this // selector with `accountsWithSendEtherInfoSelector` let account = internalAccount; @@ -2033,15 +2033,10 @@ export function getNativeCurrencyForChain(chainId) { * @param state - The Redux store state. * @returns {Array} An array of internal accounts. */ -export const getMemoizedMetaMaskInternalAccounts = createDeepEqualSelector( - getInternalAccounts, - (internalAccounts) => internalAccounts, -); +export const getMemoizedMetaMaskInternalAccounts = getInternalAccounts; -export const selectERC20TokensByChain = createDeepEqualSelector( - (state) => state.metamask.tokensChainsCache, - (erc20TokensByChain) => erc20TokensByChain, -); +export const selectERC20TokensByChain = (state) => + state.metamask.tokensChainsCache; export const selectERC20Tokens = createDeepEqualSelector( getCurrentChainId, @@ -2082,10 +2077,19 @@ export const getMetadataContractName = createSelector( export const getTxData = (state) => state.confirmTransaction.txData; export const getUnapprovedTransaction = createDeepEqualSelector( - (state) => getUnapprovedTransactions(state), + getUnapprovedTransactions, (_, transactionId) => transactionId, - (unapprovedTxs, transactionId) => - Object.values(unapprovedTxs).find(({ id }) => id === transactionId), + (unapprovedTxs, transactionId) => { + if (!unapprovedTxs || typeof unapprovedTxs !== 'object') { + return undefined; + } + + const match = Object.values(unapprovedTxs).find( + ({ id }) => id === transactionId, + ); + + return match ? { ...match } : undefined; + }, ); export const getTransaction = createDeepEqualSelector( diff --git a/ui/selectors/signatures.ts b/ui/selectors/signatures.ts index 0fd99eb4361a..92d1372e5048 100644 --- a/ui/selectors/signatures.ts +++ b/ui/selectors/signatures.ts @@ -1,6 +1,5 @@ import { createSelector } from 'reselect'; import { DefaultRootState } from 'react-redux'; -import { createDeepEqualSelector } from '../../shared/modules/selectors/util'; import { unapprovedPersonalMsgsSelector, unapprovedTypedMessagesSelector, @@ -21,7 +20,4 @@ const internalSelectUnapprovedMessage = createSelector( (messages, messageId) => messages[messageId], ); -export const selectUnapprovedMessage = createDeepEqualSelector( - internalSelectUnapprovedMessage, - (message) => message, -); +export const selectUnapprovedMessage = internalSelectUnapprovedMessage; diff --git a/ui/selectors/transactions.js b/ui/selectors/transactions.js index dd6ac06739f2..f19a0aad4c66 100644 --- a/ui/selectors/transactions.js +++ b/ui/selectors/transactions.js @@ -50,20 +50,10 @@ export const getTransactions = createDeepEqualSelector( return [...transactions].sort((a, b) => a.time - b.time); // Ascending }, - (transactions) => transactions, + (transactions) => [...transactions], ); -export const getAllNetworkTransactions = createDeepEqualSelector( - // Input Selector: Retrieve all transactions from the state. - getTransactions, - // Output Selector: Filter transactions by popular networks. - (transactions) => { - if (!transactions.length) { - return []; - } - return transactions; - }, -); +export const getAllNetworkTransactions = getTransactions; export const getCurrentNetworkTransactions = createDeepEqualSelector( (state) => { @@ -79,7 +69,7 @@ export const getCurrentNetworkTransactions = createDeepEqualSelector( (transaction) => transaction.chainId === chainId, ); }, - (transactions) => transactions, + (transactions) => [...transactions], ); export const incomingTxListSelectorAllChains = createDeepEqualSelector( @@ -93,23 +83,20 @@ export const incomingTxListSelectorAllChains = createDeepEqualSelector( tx.txParams.to === selectedAddress, ); }, - (transactions) => transactions, + (transactions) => [...transactions], ); export const getUnapprovedTransactions = createDeepEqualSelector( - (state) => { - const transactions = getTransactions(state); - return filterAndShapeUnapprovedTransactions(transactions); - }, - (transactions) => transactions, + getTransactions, + (transactions) => filterAndShapeUnapprovedTransactions(transactions), ); // Unlike `getUnapprovedTransactions` and `getCurrentNetworkTransactions` // returns the total number of unapproved transactions on all networks export const getAllUnapprovedTransactions = createDeepEqualSelector( - (state) => { - const { transactions } = state.metamask || []; - if (!transactions?.length) { + (state) => state.metamask?.transactions ?? [], + (transactions) => { + if (!transactions.length) { return []; } @@ -119,22 +106,30 @@ export const getAllUnapprovedTransactions = createDeepEqualSelector( return filterAndShapeUnapprovedTransactions(sortedTransactions); }, - (transactions) => transactions, ); export const getApprovedAndSignedTransactions = createDeepEqualSelector( - (state) => { - // Fetch transactions across all networks to address a nonce management limitation. - // This issue arises when a pending transaction exists on one network, and the user initiates another transaction on a different network. - const transactions = getTransactions(state); + getTransactions, + (transactions) => { + if (!Array.isArray(transactions)) { + return []; + } - return transactions.filter((transaction) => - [TransactionStatus.approved, TransactionStatus.signed].includes( - transaction.status, - ), - ); + if (transactions.length === 0) { + return []; + } + + return transactions.reduce((result, transaction) => { + if ( + [TransactionStatus.approved, TransactionStatus.signed].includes( + transaction.status, + ) + ) { + result.push({ ...transaction }); + } + return result; + }, []); }, - (transactions) => transactions, ); export const incomingTxListSelector = createDeepEqualSelector( @@ -148,7 +143,7 @@ export const incomingTxListSelector = createDeepEqualSelector( tx.txParams.to === selectedAddress, ); }, - (transactions) => transactions, + (transactions) => [...transactions], ); export const unapprovedPersonalMsgsSelector = (state) => @@ -679,10 +674,22 @@ export const nonceSortedCompletedTransactionsSelector = createSelector( export const submittedPendingTransactionsSelector = createSelector( transactionsSelector, - (transactions = []) => - transactions.filter( - (transaction) => transaction.status === TransactionStatus.submitted, - ), + (transactions = []) => { + if (!Array.isArray(transactions)) { + return []; + } + + if (transactions.length === 0) { + return []; + } + + return transactions.reduce((result, transaction) => { + if (transaction.status === TransactionStatus.submitted) { + result.push({ ...transaction }); + } + return result; + }, []); + }, ); const TRANSACTION_APPROVAL_TYPES = [