Skip to content

Commit 0da8069

Browse files
authored
chore: money account related code refactor (#29228)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** Code cleanup and refactoring. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/CONF-1228 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** NA ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [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 - [X] 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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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] > **Medium Risk** > Changes confirmation flow UI/enablement and how `accountOverride` is set for money-account transactions; miswiring could block confirmations or select the wrong funding account. > > **Overview** > Extracts money-account deposit/withdraw account-picking into a dedicated `PayAccountSelector` component that reads the current `accountOverride` and writes updates via `TransactionPayController.setTransactionConfig`. > > Updates `CustomAmountInfo` to optionally show this selector via a new `supportAccountSelection` prop, and **blocks pay-token display and confirm** until an account has been selected (i.e., no `accountOverride`). Money-account deposit/withdraw info components now enable this flag, and unit tests are updated/added to cover the new selector behavior and wiring. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d7a71aa. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 262d190 commit 0da8069

10 files changed

Lines changed: 358 additions & 76 deletions

File tree

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
import React, { act } from 'react';
2+
import { fireEvent } from '@testing-library/react-native';
3+
import { TransactionType } from '@metamask/transaction-controller';
4+
import { Hex } from '@metamask/utils';
5+
import { merge } from 'lodash';
6+
7+
import Engine from '../../../../../core/Engine';
8+
import renderWithProvider from '../../../../../util/test/renderWithProvider';
9+
import { transactionApprovalControllerMock } from '../../__mocks__/controllers/approval-controller-mock';
10+
import { otherControllersMock } from '../../__mocks__/controllers/other-controllers-mock';
11+
import { simpleSendTransactionControllerMock } from '../../__mocks__/controllers/transaction-controller-mock';
12+
import { useTransactionMetadataRequest } from '../../hooks/transactions/useTransactionMetadataRequest';
13+
import { useTransactionAccountOverride } from '../../hooks/transactions/useTransactionAccountOverride';
14+
import PayAccountSelector from './PayAccountSelector';
15+
16+
jest.mock('../../hooks/transactions/useTransactionMetadataRequest');
17+
jest.mock('../../hooks/transactions/useTransactionAccountOverride');
18+
19+
jest.mock('../../../../../core/Engine', () => ({
20+
context: {
21+
TransactionPayController: {
22+
setTransactionConfig: jest.fn(),
23+
},
24+
},
25+
}));
26+
27+
jest.mock('../AccountSelector', () => {
28+
const { TouchableOpacity, Text } = jest.requireActual('react-native');
29+
return {
30+
__esModule: true,
31+
default: ({
32+
onAccountSelected,
33+
selectedAddress,
34+
label,
35+
}: {
36+
onAccountSelected: (address: string) => void;
37+
selectedAddress?: string;
38+
label?: string;
39+
}) => (
40+
<TouchableOpacity
41+
testID="account-selector"
42+
onPress={() => onAccountSelected('0xSelectedAddress')}
43+
>
44+
<Text testID="account-selector-label">{label ?? 'To'}</Text>
45+
<Text testID="account-selector-address">
46+
{selectedAddress ?? 'No selection'}
47+
</Text>
48+
</TouchableOpacity>
49+
),
50+
};
51+
});
52+
53+
const useTransactionMetadataRequestMock = jest.mocked(
54+
useTransactionMetadataRequest,
55+
);
56+
const useTransactionAccountOverrideMock = jest.mocked(
57+
useTransactionAccountOverride,
58+
);
59+
60+
const setTransactionConfigMock = jest.mocked(
61+
Engine.context.TransactionPayController.setTransactionConfig,
62+
);
63+
64+
function render() {
65+
return renderWithProvider(<PayAccountSelector />, {
66+
state: merge(
67+
{},
68+
simpleSendTransactionControllerMock,
69+
transactionApprovalControllerMock,
70+
otherControllersMock,
71+
),
72+
});
73+
}
74+
75+
describe('PayAccountSelector', () => {
76+
beforeEach(() => {
77+
jest.resetAllMocks();
78+
79+
useTransactionMetadataRequestMock.mockReturnValue({
80+
id: 'mock-tx-id',
81+
type: TransactionType.moneyAccountDeposit,
82+
txParams: { from: '0x123' },
83+
} as never);
84+
85+
useTransactionAccountOverrideMock.mockReturnValue(undefined);
86+
});
87+
88+
it('returns null for non-money-account transactions', () => {
89+
useTransactionMetadataRequestMock.mockReturnValue({
90+
id: 'mock-tx-id',
91+
type: TransactionType.simpleSend,
92+
txParams: { from: '0x123' },
93+
} as never);
94+
95+
const { queryByTestId } = render();
96+
expect(queryByTestId('account-selector')).toBeNull();
97+
});
98+
99+
it('renders with no pre-selected address when accountOverride is undefined', () => {
100+
const { getByTestId } = render();
101+
102+
expect(getByTestId('account-selector-address')).toHaveTextContent(
103+
'No selection',
104+
);
105+
});
106+
107+
it('renders with pre-selected address from accountOverride', () => {
108+
useTransactionAccountOverrideMock.mockReturnValue(
109+
'0xOverrideAddress' as Hex,
110+
);
111+
112+
const { getByTestId } = render();
113+
114+
expect(getByTestId('account-selector-address')).toHaveTextContent(
115+
'0xOverrideAddress',
116+
);
117+
});
118+
119+
it('shows "From" label for deposit transactions', () => {
120+
const { getByTestId } = render();
121+
122+
expect(getByTestId('account-selector-label')).toHaveTextContent('From');
123+
});
124+
125+
it('shows default "To" label for withdraw transactions', () => {
126+
useTransactionMetadataRequestMock.mockReturnValue({
127+
id: 'mock-tx-id',
128+
type: TransactionType.moneyAccountWithdraw,
129+
txParams: { from: '0x123' },
130+
} as never);
131+
132+
const { getByTestId } = render();
133+
134+
expect(getByTestId('account-selector-label')).toHaveTextContent('To');
135+
});
136+
137+
it('calls setTransactionConfig with accountOverride on account selection', async () => {
138+
const { getByTestId } = render();
139+
140+
await act(async () => {
141+
fireEvent.press(getByTestId('account-selector'));
142+
});
143+
144+
expect(setTransactionConfigMock).toHaveBeenCalledWith(
145+
'mock-tx-id',
146+
expect.any(Function),
147+
);
148+
149+
const configCallback = setTransactionConfigMock.mock.calls[0][1];
150+
const config = {} as { accountOverride?: Hex; isPostQuote?: boolean };
151+
configCallback(config as never);
152+
153+
expect(config.accountOverride).toBe('0xSelectedAddress');
154+
});
155+
156+
it('sets accountOverride for withdraw transactions', async () => {
157+
useTransactionMetadataRequestMock.mockReturnValue({
158+
id: 'mock-tx-id',
159+
type: TransactionType.moneyAccountWithdraw,
160+
txParams: { from: '0x123' },
161+
} as never);
162+
163+
const { getByTestId } = render();
164+
165+
await act(async () => {
166+
fireEvent.press(getByTestId('account-selector'));
167+
});
168+
169+
const configCallback = setTransactionConfigMock.mock.calls[0][1];
170+
const config = {} as { accountOverride?: Hex; isPostQuote?: boolean };
171+
configCallback(config as never);
172+
173+
expect(config.accountOverride).toBe('0xSelectedAddress');
174+
expect(config.isPostQuote).toBeUndefined();
175+
});
176+
177+
it('does not set isPostQuote for deposit transactions', async () => {
178+
const { getByTestId } = render();
179+
180+
await act(async () => {
181+
fireEvent.press(getByTestId('account-selector'));
182+
});
183+
184+
const configCallback = setTransactionConfigMock.mock.calls[0][1];
185+
const config = {} as { accountOverride?: Hex; isPostQuote?: boolean };
186+
configCallback(config as never);
187+
188+
expect(config.accountOverride).toBe('0xSelectedAddress');
189+
expect(config.isPostQuote).toBeUndefined();
190+
});
191+
192+
it('does not call setTransactionConfig when transactionId is missing', async () => {
193+
useTransactionMetadataRequestMock.mockReturnValue({
194+
type: TransactionType.moneyAccountDeposit,
195+
txParams: { from: '0x123' },
196+
} as never);
197+
198+
const { getByTestId } = render();
199+
200+
await act(async () => {
201+
fireEvent.press(getByTestId('account-selector'));
202+
});
203+
204+
expect(setTransactionConfigMock).not.toHaveBeenCalled();
205+
});
206+
});
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import React, { useCallback } from 'react';
2+
import { TransactionType } from '@metamask/transaction-controller';
3+
import { Hex } from '@metamask/utils';
4+
5+
import { strings } from '../../../../../../locales/i18n';
6+
import Engine from '../../../../../core/Engine';
7+
import { useTransactionMetadataRequest } from '../../hooks/transactions/useTransactionMetadataRequest';
8+
import { useTransactionAccountOverride } from '../../hooks/transactions/useTransactionAccountOverride';
9+
import { hasTransactionType } from '../../utils/transaction';
10+
import AccountSelector from '../AccountSelector';
11+
12+
const PayAccountSelector: React.FC = () => {
13+
const transactionMeta = useTransactionMetadataRequest();
14+
const transactionId = transactionMeta?.id;
15+
const accountOverride = useTransactionAccountOverride();
16+
17+
const isMoneyAccountWithdraw = hasTransactionType(transactionMeta, [
18+
TransactionType.moneyAccountWithdraw,
19+
]);
20+
const isMoneyAccountDeposit = hasTransactionType(transactionMeta, [
21+
TransactionType.moneyAccountDeposit,
22+
]);
23+
24+
const handleAccountSelected = useCallback(
25+
(address: string) => {
26+
if (transactionId) {
27+
Engine.context.TransactionPayController.setTransactionConfig(
28+
transactionId,
29+
(config) => {
30+
config.accountOverride = address as Hex;
31+
},
32+
);
33+
}
34+
},
35+
[transactionId],
36+
);
37+
38+
if (!isMoneyAccountDeposit && !isMoneyAccountWithdraw) {
39+
return null;
40+
}
41+
42+
const label = isMoneyAccountDeposit
43+
? strings('confirm.label.from')
44+
: undefined;
45+
46+
return (
47+
<AccountSelector
48+
label={label}
49+
selectedAddress={accountOverride}
50+
onAccountSelected={handleAccountSelected}
51+
/>
52+
);
53+
};
54+
55+
export default PayAccountSelector;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { default } from './PayAccountSelector';

0 commit comments

Comments
 (0)