Skip to content

Commit 00bd71d

Browse files
authored
fix(activity-redesign): populate Activity transaction-details fields in the redesign (#32191)
## **Description** In the redesigned Activity screen, the transaction-details sheet left **From/To** and **Amount / gas fee / total amount** blank. The row-press handler passed a *stub* to `TransactionDetailsSheet` — `transactionElement: { actionKey, value }` plus a `transactionDetails` with empty from/to and no `summaryAmount`/`summaryFee`/`summaryTotalAmount` — so the sheet had nothing to render. The legacy list runs the tx through the shared `decodeTransaction`, which produces all of those fields. This change decodes the EVM transaction the same way before navigating, and passes the real `transactionElement`/`transactionDetails`. Because the unified list is multi-chain (can't `useSelector` per-tx-chain inside a callback), the per-chain inputs — `ticker`, `conversionRate`, `currencyRates`, `contractExchangeRates`, `primaryCurrency`, `swapsTransactions`, `tokens` — are read from the redux store for that tx's chain at press time. A `try/catch` falls back to the previous minimal view if decoding ever throws. `normalizeTransaction` already carries full `txParams` (incl. gas), so API-confirmed txs populate too. ## **Changelog** CHANGELOG entry: null <!-- Behind the Activity redesign feature flag; not yet user-facing. --> ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-933 ## **Manual testing steps** ```gherkin Feature: Activity transaction details Scenario: EVM transaction details are fully populated Given the Activity redesign is enabled And I have a confirmed EVM transaction (e.g. Sent ETH) When I tap the row to open its details Then I see the From/To addresses and the Amount, gas fee, and total amount ``` ## **Screenshots/Recordings** ### **Before** Details sheet showed blank From/To and "No fee" / empty Amount and Total amount. https://github.com/user-attachments/assets/b6f8fde8-ef04-4c61-8d68-d2f7a1ade701 ### **After** From/To addresses and Amount / gas fee / total amount are populated. https://github.com/user-attachments/assets/dd98a320-49ef-4cd3-a840-bd1eabd34a1e ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Localized Activity UI/navigation change behind the redesign; tests cover race and error paths with no auth or payment impact. > > **Overview** > Fixes blank **From/To** and **Amount / gas / total** on the redesigned Activity transaction-details sheet by running each EVM row through the shared **`decodeTransaction`** path (same as the legacy list) before navigation, instead of passing stub `transactionElement` / `transactionDetails`. > > Per-transaction chain data (`ticker`, conversion rates, tokens, swaps, etc.) is read from the Redux **`store`** at press time so multi-chain rows do not depend on a single hook chain. A **press token** ref ignores stale async decode results so rapid taps do not open the wrong tx; **`try/catch`** still navigates with the previous minimal payload if decoding fails. > > **`ActivityList.test.tsx`** adds coverage for async navigation, out-of-order decodes, and decode failure fallback. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 68f8d17. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ffdaba8 commit 00bd71d

2 files changed

Lines changed: 198 additions & 26 deletions

File tree

app/components/Views/ActivityList/ActivityList.test.tsx

Lines changed: 111 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import { useLocalActivityItems } from './hooks/useLocalActivityItems';
1414
import { useUnifiedTxActions } from './useUnifiedTxActions';
1515
import Engine from '../../../core/Engine';
1616
import { trackBlockExplorerLinkClicked } from '../../../util/analytics/externalLinkTracking';
17+
import Routes from '../../../constants/navigation/Routes';
18+
import decodeTransaction from '../../UI/TransactionElement/utils';
1719

1820
jest.mock('@react-navigation/native', () => ({
1921
useNavigation: jest.fn(),
@@ -29,6 +31,8 @@ jest.mock('../../../selectors/accountsController', () => ({
2931

3032
jest.mock('../../../selectors/currencyRateController', () => ({
3133
selectCurrentCurrency: jest.fn((state) => state.currentCurrency),
34+
selectConversionRateByChainId: jest.fn(),
35+
selectCurrencyRates: jest.fn(),
3236
}));
3337

3438
jest.mock('../../../selectors/multichain/multichain', () => ({
@@ -50,6 +54,7 @@ jest.mock('../../../selectors/networkController', () => ({
5054
selectEvmNetworkConfigurationsByChainId: jest.fn((state) => state.evmConfigs),
5155
selectProviderType: jest.fn((state) => state.providerType),
5256
selectAllConfiguredEvmChainIds: jest.fn((state) => state.enabledEvm),
57+
selectTickerByChainId: jest.fn(),
5358
}));
5459

5560
jest.mock('../../../selectors/multichainNetworkController', () => ({
@@ -58,6 +63,31 @@ jest.mock('../../../selectors/multichainNetworkController', () => ({
5863

5964
jest.mock('../../../selectors/transactionController', () => ({
6065
selectRelatedChainIdsByTransactionId: jest.fn((state) => state.related),
66+
selectSwapsTransactions: jest.fn(),
67+
}));
68+
69+
jest.mock('../../../selectors/tokenRatesController', () => ({
70+
selectContractExchangeRatesByChainId: jest.fn(),
71+
}));
72+
73+
jest.mock('../../../selectors/settings', () => ({
74+
selectPrimaryCurrency: jest.fn(),
75+
}));
76+
77+
jest.mock('../../../selectors/tokensController', () => ({
78+
selectTokensByChainIdAndWalletAddress: jest.fn(),
79+
}));
80+
81+
jest.mock('../../../store', () => ({
82+
store: { getState: jest.fn(() => ({})) },
83+
}));
84+
85+
jest.mock('../../UI/TransactionElement/utils', () => ({
86+
__esModule: true,
87+
default: jest.fn(async () => [
88+
{ actionKey: 'Sent ETH' },
89+
{ hash: '0xconfirmed', renderFrom: '0xfrom', renderTo: '0xto' },
90+
]),
6191
}));
6292

6393
jest.mock('../../../selectors/bridgeStatusController', () => ({
@@ -537,17 +567,94 @@ describe('ActivityList', () => {
537567
);
538568
});
539569

540-
it('navigates to transaction details when a confirmed row is pressed', () => {
570+
it('navigates to transaction details when a confirmed row is pressed', async () => {
571+
render(<ActivityList header={<></>} onScroll={mockOnScroll} />);
572+
573+
fireEvent.press(screen.getByTestId('row-0xconfirmed'));
574+
575+
// The press handler decodes the tx (async) before navigating.
576+
await waitFor(() =>
577+
expect(mockNavigate).toHaveBeenCalledWith(
578+
expect.any(String),
579+
expect.objectContaining({
580+
screen: expect.any(String),
581+
}),
582+
),
583+
);
584+
});
585+
586+
it('opens only the most-recently-pressed row when decodes resolve out of order', async () => {
587+
const decodeMock = jest.mocked(decodeTransaction);
588+
type DecodeResult = Awaited<ReturnType<typeof decodeTransaction>>;
589+
let resolveFirst: (value: DecodeResult) => void = () => undefined;
590+
let resolveSecond: (value: DecodeResult) => void = () => undefined;
591+
decodeMock
592+
.mockImplementationOnce(
593+
() =>
594+
new Promise((resolve) => {
595+
resolveFirst = resolve;
596+
}),
597+
)
598+
.mockImplementationOnce(
599+
() =>
600+
new Promise((resolve) => {
601+
resolveSecond = resolve;
602+
}),
603+
);
604+
541605
render(<ActivityList header={<></>} onScroll={mockOnScroll} />);
542606

543607
fireEvent.press(screen.getByTestId('row-0xconfirmed'));
608+
fireEvent.press(screen.getByTestId('row-0xlocal'));
609+
610+
resolveSecond([{ actionKey: 'Approve' }, { hash: '0xlocal' }]);
611+
resolveFirst([{ actionKey: 'Sent' }, { hash: '0xconfirmed' }]);
612+
613+
await waitFor(() => {
614+
const detailCalls = mockNavigate.mock.calls.filter(
615+
(call) => call[1]?.screen === Routes.SHEET.TRANSACTION_DETAILS,
616+
);
617+
expect(detailCalls).toHaveLength(1);
618+
});
619+
620+
const detailCalls = mockNavigate.mock.calls.filter(
621+
(call) => call[1]?.screen === Routes.SHEET.TRANSACTION_DETAILS,
622+
);
623+
expect(detailCalls[0][1].params.tx.hash).toBe('0xlocal');
624+
});
625+
626+
it('falls back to a minimal details view when decoding throws', async () => {
627+
jest
628+
.mocked(decodeTransaction)
629+
.mockRejectedValueOnce(new Error('decode failed'));
544630

545-
expect(mockNavigate).toHaveBeenCalledWith(
546-
expect.any(String),
631+
render(<ActivityList header={<></>} onScroll={mockOnScroll} />);
632+
633+
fireEvent.press(screen.getByTestId('row-0xconfirmed'));
634+
635+
await waitFor(() => {
636+
const detailCalls = mockNavigate.mock.calls.filter(
637+
(call) => call[1]?.screen === Routes.SHEET.TRANSACTION_DETAILS,
638+
);
639+
expect(detailCalls).toHaveLength(1);
640+
});
641+
642+
const call = mockNavigate.mock.calls.find(
643+
(c) => c[1]?.screen === Routes.SHEET.TRANSACTION_DETAILS,
644+
);
645+
// Minimal transactionDetails are built from the item (addresses via
646+
// getActivityFromTo) rather than the decoded data.
647+
expect(call?.[1].params.transactionDetails).toEqual(
547648
expect.objectContaining({
548-
screen: expect.any(String),
649+
hash: '0xconfirmed',
650+
renderFrom: '0xevm',
651+
renderTo: '0xto',
652+
transactionType: 'contractInteraction',
549653
}),
550654
);
655+
expect(call?.[1].params.transactionElement).toEqual(
656+
expect.objectContaining({ actionKey: expect.any(String) }),
657+
);
551658
});
552659

553660
it('uses unique chain-aware fallback keys for rows without hashes', () => {

app/components/Views/ActivityList/ActivityList.tsx

Lines changed: 87 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,23 @@ import {
2828
selectAllConfiguredEvmChainIds,
2929
selectEvmNetworkConfigurationsByChainId,
3030
selectProviderType,
31+
selectTickerByChainId,
3132
} from '../../../selectors/networkController';
3233
import { selectAllConfiguredNonEvmChainIds } from '../../../selectors/multichainNetworkController';
33-
import { selectRelatedChainIdsByTransactionId } from '../../../selectors/transactionController';
34+
import {
35+
selectRelatedChainIdsByTransactionId,
36+
selectSwapsTransactions,
37+
} from '../../../selectors/transactionController';
38+
import {
39+
selectConversionRateByChainId,
40+
selectCurrencyRates,
41+
selectCurrentCurrency,
42+
} from '../../../selectors/currencyRateController';
43+
import { selectContractExchangeRatesByChainId } from '../../../selectors/tokenRatesController';
44+
import { selectPrimaryCurrency } from '../../../selectors/settings';
45+
import { selectTokensByChainIdAndWalletAddress } from '../../../selectors/tokensController';
46+
import { store } from '../../../store';
47+
import decodeTransaction from '../../UI/TransactionElement/utils';
3448
import { baseStyles } from '../../../styles/common';
3549
import { isHardwareAccount } from '../../../util/address';
3650
import {
@@ -579,11 +593,19 @@ const ActivityList = ({
579593
}
580594
}, [refetch]);
581595

596+
// Guards against out-of-order async decodes: each press claims a token, and
597+
// only the most recent press is allowed to open the details sheet. Without
598+
// this, tapping row A then row B before A's decode resolves could navigate to
599+
// A last and show the wrong transaction.
600+
const activityPressTokenRef = useRef(0);
601+
582602
const handleActivityItemPress = useCallback(
583-
(item: ActivityListItem) => {
603+
async (item: ActivityListItem) => {
584604
const { raw } = item;
585605
if (!raw) return;
586606

607+
const pressToken = (activityPressTokenRef.current += 1);
608+
587609
const itemBridgeHistoryItem = getBridgeHistoryItemByHash(item.hash);
588610
const actionKey = resolveActivityListItemTitle(
589611
item,
@@ -649,29 +671,72 @@ const ActivityList = ({
649671
return;
650672
}
651673

652-
const { from, to } = getActivityFromTo(item);
653-
const value = getActivityValue(item);
674+
const txChainId = tx.chainId;
675+
676+
// Decode the EVM transaction the same way the legacy list does, so the
677+
// detail sheet's From/To and Amount/gas/total fields are populated.
678+
// The unified list is multi-chain, so the per-chain rates/ticker/tokens
679+
// are read from the store for this tx's chain rather than via hooks.
680+
try {
681+
const state = store.getState();
682+
const [transactionElement, transactionDetails] =
683+
await decodeTransaction({
684+
tx,
685+
selectedAddress: selectedEvmAddress,
686+
chainId: txChainId,
687+
txChainId,
688+
ticker: selectTickerByChainId(state, txChainId),
689+
conversionRate: selectConversionRateByChainId(state, txChainId),
690+
currencyRates: selectCurrencyRates(state),
691+
currentCurrency: selectCurrentCurrency(state),
692+
contractExchangeRates: selectContractExchangeRatesByChainId(
693+
state,
694+
txChainId,
695+
),
696+
primaryCurrency: selectPrimaryCurrency(state),
697+
swapsTransactions: selectSwapsTransactions(state),
698+
tokens: selectTokensByChainIdAndWalletAddress(
699+
state,
700+
txChainId,
701+
selectedEvmAddress,
702+
),
703+
selectedInternalAccount: selectSelectedInternalAccount(state),
704+
});
705+
706+
if (activityPressTokenRef.current !== pressToken) return;
654707

655-
navigation.navigate(Routes.MODAL.ROOT_MODAL_FLOW, {
656-
screen: Routes.SHEET.TRANSACTION_DETAILS,
657-
params: {
658-
tx,
659-
transactionElement: {
660-
actionKey,
661-
value,
708+
navigation.navigate(Routes.MODAL.ROOT_MODAL_FLOW, {
709+
screen: Routes.SHEET.TRANSACTION_DETAILS,
710+
params: {
711+
tx,
712+
transactionElement,
713+
transactionDetails,
714+
showSpeedUpModal: noop,
715+
showCancelModal: noop,
662716
},
663-
transactionDetails: {
664-
hash: item.hash,
665-
renderFrom: from,
666-
renderTo: to,
667-
renderValue: value,
668-
transactionType: item.type,
669-
txChainId: tx.chainId,
717+
});
718+
} catch {
719+
if (activityPressTokenRef.current !== pressToken) return;
720+
const { from, to } = getActivityFromTo(item);
721+
const value = getActivityValue(item);
722+
navigation.navigate(Routes.MODAL.ROOT_MODAL_FLOW, {
723+
screen: Routes.SHEET.TRANSACTION_DETAILS,
724+
params: {
725+
tx,
726+
transactionElement: { actionKey, value },
727+
transactionDetails: {
728+
hash: item.hash,
729+
renderFrom: from,
730+
renderTo: to,
731+
renderValue: value,
732+
transactionType: item.type,
733+
txChainId,
734+
},
735+
showSpeedUpModal: noop,
736+
showCancelModal: noop,
670737
},
671-
showSpeedUpModal: noop,
672-
showCancelModal: noop,
673-
},
674-
});
738+
});
739+
}
675740
},
676741
[
677742
bridgeHistory,

0 commit comments

Comments
 (0)