Skip to content

Commit e249678

Browse files
authored
fix: for postquote payments payment token for MM Pay transaction should not be reset when accountOverride is changed (#30094)
<!-- 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** Money account withdraw, selected token should not be reset when account selection is changed. ## **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** Related to: https://consensyssoftware.atlassian.net/browse/CONF-1381 ## **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** TODO ## **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 pay-token auto-selection behavior for post-quote withdraw transaction types, which can affect what asset is used in payment/withdraw flows if the gating condition is wrong. Scope is limited and covered by updated unit tests. > > **Overview** > Prevents `useAutomaticTransactionPayToken` from re-selecting/resetting the payment token when `accountOverride` or `from` changes for *post-quote* withdraw transaction types, preserving the user/quote-selected token. > > Simplifies `PayWithRow` by removing `useMoneyAccountPayToken`-driven display/disabled states and deletes the `useMoneyAccountPayToken` hook and its tests. Refactors the money-account override event handler to use the shared `hasTransactionType` utility, and updates/extends tests to cover the new post-quote no-reselect behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d8212c9. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0612809 commit e249678

7 files changed

Lines changed: 74 additions & 507 deletions

File tree

app/components/Views/confirmations/components/rows/pay-with-row/pay-with-row.test.tsx

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { useTransactionPayToken } from '../../../hooks/pay/useTransactionPayToke
55
import { useTransactionPayWithdraw } from '../../../hooks/pay/useTransactionPayWithdraw';
66
import { useTransactionPayRequiredTokens } from '../../../hooks/pay/useTransactionPayData';
77
import { useTransactionPaySelectedFiatPaymentMethod } from '../../../hooks/pay/useTransactionPaySelectedFiatPaymentMethod';
8-
import { useMoneyAccountPayToken } from '../../../hooks/pay/useMoneyAccountPayToken';
98
import { type PaymentMethod } from '@metamask/ramps-controller';
109
import { useNavigation } from '@react-navigation/native';
1110
import { act, fireEvent } from '@testing-library/react-native';
@@ -29,7 +28,6 @@ jest.mock('../../../hooks/pay/useTransactionPayToken');
2928
jest.mock('../../../hooks/pay/useTransactionPayWithdraw');
3029
jest.mock('../../../hooks/pay/useTransactionPayData');
3130
jest.mock('../../../hooks/pay/useTransactionPaySelectedFiatPaymentMethod');
32-
jest.mock('../../../hooks/pay/useMoneyAccountPayToken');
3331
jest.mock('../../../../../../util/address');
3432
jest.mock('../../../hooks/metrics/useConfirmationMetricEvents');
3533
jest.mock('@react-navigation/native', () => ({
@@ -87,10 +85,6 @@ describe('PayWithRow', () => {
8785
const useTransactionPayRequiredTokensMock = jest.mocked(
8886
useTransactionPayRequiredTokens,
8987
);
90-
const useTransactionMetadataRequestMock = jest.mocked(
91-
useTransactionMetadataRequest,
92-
);
93-
const useMoneyAccountPayTokenMock = jest.mocked(useMoneyAccountPayToken);
9488
const useParamsMock = jest.mocked(useParams);
9589
const mockSetConfirmationMetric = jest.fn();
9690

@@ -110,13 +104,6 @@ describe('PayWithRow', () => {
110104

111105
useTransactionPayRequiredTokensMock.mockReturnValue(undefined as never);
112106

113-
useMoneyAccountPayTokenMock.mockReturnValue({
114-
displayToken: undefined,
115-
isAwaitingAccountSelection: false,
116-
isMoneyAccountDeposit: false,
117-
isMoneyAccountWithdraw: false,
118-
});
119-
120107
jest
121108
.mocked(useTransactionPaySelectedFiatPaymentMethod)
122109
.mockReturnValue(undefined);
@@ -140,13 +127,6 @@ describe('PayWithRow', () => {
140127
} as never);
141128

142129
isHardwareAccountMock.mockReturnValue(false);
143-
144-
useMoneyAccountPayTokenMock.mockReturnValue({
145-
displayToken: undefined,
146-
isAwaitingAccountSelection: false,
147-
isMoneyAccountDeposit: false,
148-
isMoneyAccountWithdraw: false,
149-
});
150130
});
151131

152132
it('renders selected pay token', async () => {

app/components/Views/confirmations/components/rows/pay-with-row/pay-with-row.tsx

Lines changed: 2 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import React, {
2-
useCallback,
3-
useEffect,
4-
useMemo,
5-
useRef,
6-
useState,
7-
} from 'react';
1+
import React, { useCallback, useMemo } from 'react';
82
import { useNavigation } from '@react-navigation/native';
93
import { PaymentType } from '@consensys/on-ramp-sdk';
104
import Routes from '../../../../../../constants/navigation/Routes';
@@ -46,7 +40,6 @@ import {
4640
} from '../../../ConfirmationView.testIds';
4741
import { useConfirmationMetricEvents } from '../../../hooks/metrics/useConfirmationMetricEvents';
4842
import { type PaymentMethod } from '@metamask/ramps-controller';
49-
import { useMoneyAccountPayToken } from '../../../hooks/pay/useMoneyAccountPayToken';
5043
import { useParams } from '../../../../../../util/navigation/navUtils';
5144
import { SetPayTokenRequest } from '../../../hooks/pay/useAutomaticTransactionPayToken';
5245
import { useConfirmationContext } from '../../../context/confirmation-context';
@@ -59,8 +52,6 @@ interface PayWithRouteParams {
5952
export function PayWithRow() {
6053
const navigation = useNavigation();
6154
const { payToken } = useTransactionPayToken();
62-
const { displayToken: moneyAccountDisplayToken, isAwaitingAccountSelection } =
63-
useMoneyAccountPayToken();
6455
const { isWithdraw } = useTransactionPayWithdraw();
6556
const requiredTokens = useTransactionPayRequiredTokens();
6657
const selectedFiatPaymentMethod =
@@ -116,14 +107,11 @@ export function PayWithRow() {
116107
(token) => !token.skipIfBalance && !token.allowUnderMinimum,
117108
);
118109
const displayToken = useMemo(() => {
119-
if (moneyAccountDisplayToken) {
120-
return moneyAccountDisplayToken;
121-
}
122110
if (isWithdraw) {
123111
return payToken ?? defaultWithdrawToken ?? null;
124112
}
125113
return payToken ?? null;
126-
}, [isWithdraw, payToken, defaultWithdrawToken, moneyAccountDisplayToken]);
114+
}, [isWithdraw, payToken, defaultWithdrawToken]);
127115

128116
// For deposits, show the user's balance of the selected pay token
129117
const balanceUsdFormatted = useMemo(
@@ -143,30 +131,6 @@ export function PayWithRow() {
143131
);
144132
}
145133

146-
if (isAwaitingAccountSelection) {
147-
return (
148-
<Box
149-
flexDirection={FlexDirection.Row}
150-
alignItems={AlignItems.center}
151-
justifyContent={JustifyContent.spaceBetween}
152-
style={[styles.container, styles.disabled]}
153-
testID={ConfirmationRowComponentIDs.PAY_WITH}
154-
>
155-
<Text variant={TextVariant.BodyMd} color={TextColor.TextAlternative}>
156-
{label}
157-
</Text>
158-
<Text
159-
variant={TextVariant.BodyMd}
160-
fontWeight={FontWeight.Medium}
161-
color={TextColor.TextAlternative}
162-
testID={TransactionPayComponentIDs.PAY_WITH_SYMBOL}
163-
>
164-
{strings('confirm.label.payment_method')}
165-
</Text>
166-
</Box>
167-
);
168-
}
169-
170134
if (!displayToken) {
171135
return <PayWithRowSkeleton />;
172136
}

app/components/Views/confirmations/hooks/pay/useAutomaticTransactionPayToken.test.ts

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,45 @@ describe('useAutomaticTransactionPayToken', () => {
964964
});
965965
});
966966

967-
it('re-selects pay token when accountOverride changes', () => {
967+
it('does not re-select pay token when accountOverride changes for post-quote withdraws', () => {
968+
useTransactionPayAvailableTokensMock.mockReturnValue({
969+
availableTokens: [
970+
{
971+
address: TOKEN_ADDRESS_1_MOCK,
972+
chainId: CHAIN_ID_1_MOCK,
973+
},
974+
{
975+
address: PREFERRED_TOKEN_ADDRESS_MOCK,
976+
chainId: PREFERRED_CHAIN_ID_MOCK,
977+
},
978+
] as AssetType[],
979+
hasTokens: true,
980+
});
981+
982+
useTransactionMetadataRequestMock.mockReturnValue({
983+
id: transactionIdMock,
984+
type: TransactionType.moneyAccountWithdraw,
985+
txParams: { from: '0xAddress1' },
986+
} as never);
987+
988+
const { rerender } = runHook({
989+
preferredToken: {
990+
address: PREFERRED_TOKEN_ADDRESS_MOCK as Hex,
991+
chainId: PREFERRED_CHAIN_ID_MOCK as Hex,
992+
},
993+
});
994+
995+
expect(setPayTokenMock).toHaveBeenCalledTimes(1);
996+
setPayTokenMock.mockClear();
997+
998+
useTransactionAccountOverrideMock.mockReturnValue('0xOverrideA' as Hex);
999+
1000+
rerender(undefined);
1001+
1002+
expect(setPayTokenMock).not.toHaveBeenCalled();
1003+
});
1004+
1005+
it('does not re-select pay token on from change for post-quote withdraws', () => {
9681006
useTransactionPayAvailableTokensMock.mockReturnValue({
9691007
availableTokens: [
9701008
{
@@ -979,25 +1017,33 @@ describe('useAutomaticTransactionPayToken', () => {
9791017
hasTokens: true,
9801018
});
9811019

1020+
useTransactionPayTokenMock.mockReturnValue({
1021+
payToken: {
1022+
address: TOKEN_ADDRESS_1_MOCK,
1023+
chainId: CHAIN_ID_1_MOCK,
1024+
} as unknown as ReturnType<typeof useTransactionPayToken>['payToken'],
1025+
setPayToken: setPayTokenMock,
1026+
});
1027+
9821028
useTransactionMetadataRequestMock.mockReturnValue({
9831029
id: transactionIdMock,
984-
type: TransactionType.moneyAccountDeposit,
1030+
type: TransactionType.predictWithdraw,
9851031
txParams: { from: '0xAddress1' },
9861032
} as never);
9871033

9881034
const { rerender } = runHook();
9891035

990-
expect(setPayTokenMock).toHaveBeenCalledTimes(1);
991-
setPayTokenMock.mockClear();
1036+
expect(setPayTokenMock).not.toHaveBeenCalled();
9921037

993-
useTransactionAccountOverrideMock.mockReturnValue('0xOverrideA' as Hex);
1038+
useTransactionMetadataRequestMock.mockReturnValue({
1039+
id: transactionIdMock,
1040+
type: TransactionType.predictWithdraw,
1041+
txParams: { from: '0xAddress2' },
1042+
} as never);
9941043

9951044
rerender(undefined);
9961045

997-
expect(setPayTokenMock).toHaveBeenCalledWith({
998-
address: TOKEN_ADDRESS_2_MOCK,
999-
chainId: CHAIN_ID_2_MOCK,
1000-
});
1046+
expect(setPayTokenMock).not.toHaveBeenCalled();
10011047
});
10021048

10031049
it('does not re-select on from change when disabled', () => {

app/components/Views/confirmations/hooks/pay/useAutomaticTransactionPayToken.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,12 @@ export function useAutomaticTransactionPayToken({
177177
const prevAccountKeyRef = useRef(`${from ?? ''}:${accountOverride ?? ''}`);
178178
useEffect(() => {
179179
const accountKey = `${from ?? ''}:${accountOverride ?? ''}`;
180-
if (disable || !from || prevAccountKeyRef.current === accountKey) {
180+
if (
181+
disable ||
182+
!from ||
183+
prevAccountKeyRef.current === accountKey ||
184+
postQuoteTransactionType
185+
) {
181186
return;
182187
}
183188
prevAccountKeyRef.current = accountKey;
@@ -189,7 +194,14 @@ export function useAutomaticTransactionPayToken({
189194
});
190195
log('Re-selected pay token after account change', automaticToken);
191196
}
192-
}, [accountOverride, automaticToken, disable, from, setPayToken]);
197+
}, [
198+
accountOverride,
199+
automaticToken,
200+
disable,
201+
from,
202+
postQuoteTransactionType,
203+
setPayToken,
204+
]);
193205

194206
return automaticToken;
195207
}

0 commit comments

Comments
 (0)