Skip to content

Commit 4812116

Browse files
chore(runway): cherry-pick fix: cp-7.63.0 Fix mm_pay_quote_* metrics (#25306)
- fix: cp-7.63.0 Fix `mm_pay_quote_*` metrics (#25159) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This PR aims to fix `mm_pay_quote_requested` and `mm_pay_quote_loaded` on certain `Transaction*` events. After this PR metrics will be setlled as noted below: 1. On `TransactionAdded, both `mm_pay_quote_requested` and `mm_pay_quote_loaded` are initialized to `false`. 2. If no quote is requested and the user rejects the transaction, both values stay `false`. 3. If the user views the quote and then rejects the transaction, both values are true in the `TransactionRejected` event. 4. If the user requests a quote but it hasn’t finished loading before they reject, `mm_pay_quote_requested` is true and `mm_pay_quote_loaded` is false. 5. If the user requests a quote, let it load then change the amount to get new quote but leave it while loading, both values remain `true` as they request and load before. 6. If the user views the quote and approves the transaction, both values are true in the finalized `TransactionConfirmed` event. ## **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: #25112 ## **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** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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 - [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. ## **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] > Aligns `mm_pay_quote_*` metrics and centralizes source-amount logic. > > - New `useTransactionPayHasSourceAmount` hook (with tests) to detect when quotes are required; integrated into `custom-amount-info` and `useTransactionCustomAmount` > - `useTransactionCustomAmount` now sets `mm_pay_quote_requested` when updating amount only if a source amount exists > - `useTransactionPayMetrics` refactored to read `mm_pay_quote_requested` from stored metrics and set `mm_pay_quote_loaded` via an internal ref once quotes are present; removes dependency on quote-loading flag > - Expanded/updated tests for metrics, custom amount behavior, and quote flags > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e1b4bd1. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> [5f3d172](5f3d172) Co-authored-by: OGPoyraz <omergoktugpoyraz@gmail.com>
1 parent cbf4e7b commit 4812116

7 files changed

Lines changed: 241 additions & 56 deletions

File tree

app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ import {
2828
useIsTransactionPayLoading,
2929
useTransactionPayQuotes,
3030
useTransactionPayRequiredTokens,
31-
useTransactionPaySourceAmounts,
3231
} from '../../../hooks/pay/useTransactionPayData';
32+
import { useTransactionPayHasSourceAmount } from '../../../hooks/pay/useTransactionPayHasSourceAmount';
3333
import { useTransactionPayMetrics } from '../../../hooks/pay/useTransactionPayMetrics';
3434
import { useTransactionPayAvailableTokens } from '../../../hooks/pay/useTransactionPayAvailableTokens';
3535
import Text, {
@@ -290,16 +290,7 @@ function useIsResultReady({
290290
}) {
291291
const quotes = useTransactionPayQuotes();
292292
const isQuotesLoading = useIsTransactionPayLoading();
293-
const requiredTokens = useTransactionPayRequiredTokens();
294-
const sourceAmounts = useTransactionPaySourceAmounts();
295-
296-
const hasSourceAmount = sourceAmounts?.some((a) =>
297-
requiredTokens.some(
298-
(rt) =>
299-
rt.address.toLowerCase() === a.targetTokenAddress.toLowerCase() &&
300-
!rt.skipIfBalance,
301-
),
302-
);
293+
const hasSourceAmount = useTransactionPayHasSourceAmount();
303294

304295
return (
305296
!isKeyboardVisible &&
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import {
2+
TransactionPayRequiredToken,
3+
TransactionPaySourceAmount,
4+
} from '@metamask/transaction-pay-controller';
5+
import { renderHookWithProvider } from '../../../../../util/test/renderWithProvider';
6+
import { useTransactionPayHasSourceAmount } from './useTransactionPayHasSourceAmount';
7+
import { merge } from 'lodash';
8+
import {
9+
simpleSendTransactionControllerMock,
10+
transactionIdMock,
11+
} from '../../__mocks__/controllers/transaction-controller-mock';
12+
import { transactionApprovalControllerMock } from '../../__mocks__/controllers/approval-controller-mock';
13+
import { Hex } from '@metamask/utils';
14+
import {
15+
ConfirmationContextParams,
16+
useConfirmationContext,
17+
} from '../../context/confirmation-context';
18+
19+
jest.mock('../../context/confirmation-context');
20+
21+
const TOKEN_ADDRESS_MOCK = '0x123' as Hex;
22+
const OTHER_TOKEN_ADDRESS_MOCK = '0x456' as Hex;
23+
24+
function createSourceAmount(
25+
targetTokenAddress: Hex,
26+
): TransactionPaySourceAmount {
27+
return {
28+
targetTokenAddress,
29+
} as unknown as TransactionPaySourceAmount;
30+
}
31+
32+
function createRequiredToken(
33+
address: Hex,
34+
skipIfBalance: boolean,
35+
): TransactionPayRequiredToken {
36+
return {
37+
address,
38+
skipIfBalance,
39+
} as unknown as TransactionPayRequiredToken;
40+
}
41+
42+
function runHook({
43+
sourceAmounts = [],
44+
tokens = [],
45+
}: {
46+
sourceAmounts?: TransactionPaySourceAmount[];
47+
tokens?: TransactionPayRequiredToken[];
48+
} = {}) {
49+
const state = merge(
50+
{},
51+
simpleSendTransactionControllerMock,
52+
transactionApprovalControllerMock,
53+
{
54+
engine: {
55+
backgroundState: {
56+
TransactionPayController: {
57+
transactionData: {
58+
[transactionIdMock]: {
59+
isLoading: false,
60+
sourceAmounts,
61+
tokens,
62+
},
63+
},
64+
},
65+
},
66+
},
67+
},
68+
);
69+
70+
return renderHookWithProvider(useTransactionPayHasSourceAmount, { state });
71+
}
72+
73+
describe('useTransactionPayHasSourceAmount', () => {
74+
const useConfirmationContextMock = jest.mocked(useConfirmationContext);
75+
76+
beforeEach(() => {
77+
jest.resetAllMocks();
78+
79+
useConfirmationContextMock.mockReturnValue({
80+
isTransactionDataUpdating: false,
81+
} as ConfirmationContextParams);
82+
});
83+
84+
it('returns true when there are non-optional source amounts', () => {
85+
const { result } = runHook({
86+
sourceAmounts: [createSourceAmount(TOKEN_ADDRESS_MOCK)],
87+
tokens: [createRequiredToken(TOKEN_ADDRESS_MOCK, false)],
88+
});
89+
90+
expect(result.current).toBe(true);
91+
});
92+
93+
it('returns false when source amounts are empty', () => {
94+
const { result } = runHook({
95+
sourceAmounts: [],
96+
tokens: [createRequiredToken(TOKEN_ADDRESS_MOCK, false)],
97+
});
98+
99+
expect(result.current).toBe(false);
100+
});
101+
102+
it('returns false when all source amounts are optional', () => {
103+
const { result } = runHook({
104+
sourceAmounts: [createSourceAmount(TOKEN_ADDRESS_MOCK)],
105+
tokens: [createRequiredToken(TOKEN_ADDRESS_MOCK, true)],
106+
});
107+
108+
expect(result.current).toBe(false);
109+
});
110+
111+
it('returns false when source amounts do not match required tokens', () => {
112+
const { result } = runHook({
113+
sourceAmounts: [createSourceAmount(OTHER_TOKEN_ADDRESS_MOCK)],
114+
tokens: [createRequiredToken(TOKEN_ADDRESS_MOCK, false)],
115+
});
116+
117+
expect(result.current).toBe(false);
118+
});
119+
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { useMemo } from 'react';
2+
import {
3+
useTransactionPayRequiredTokens,
4+
useTransactionPaySourceAmounts,
5+
} from './useTransactionPayData';
6+
7+
/**
8+
* Returns whether there are non-optional source amounts that require a quote.
9+
* This is true when the user needs to bridge/swap tokens to complete the transaction.
10+
*/
11+
export function useTransactionPayHasSourceAmount() {
12+
const sourceAmounts = useTransactionPaySourceAmounts();
13+
const requiredTokens = useTransactionPayRequiredTokens();
14+
15+
return useMemo(
16+
() =>
17+
sourceAmounts?.some((a) =>
18+
requiredTokens.some(
19+
(rt) =>
20+
rt.address.toLowerCase() === a.targetTokenAddress.toLowerCase() &&
21+
!rt.skipIfBalance,
22+
),
23+
) ?? false,
24+
[sourceAmounts, requiredTokens],
25+
);
26+
}

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

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
} from '@metamask/transaction-pay-controller';
2222
import { Json } from '@metamask/utils';
2323
import {
24-
useIsTransactionPayQuoteLoading,
2524
useTransactionPayQuotes,
2625
useTransactionPayRequiredTokens,
2726
useTransactionPayTotals,
@@ -37,9 +36,13 @@ jest.mock('../pay/useTransactionPayData');
3736
jest.mock('./useTransactionPayAvailableTokens');
3837
jest.mock('../send/useAccountTokens');
3938

39+
const mockSelectConfirmationMetricsById = jest.fn();
40+
4041
jest.mock('../../../../../core/redux/slices/confirmationMetrics', () => ({
4142
...jest.requireActual('../../../../../core/redux/slices/confirmationMetrics'),
4243
updateConfirmationMetric: jest.fn(),
44+
selectConfirmationMetricsById: (...args: unknown[]) =>
45+
mockSelectConfirmationMetricsById(...args),
4346
}));
4447

4548
const CHAIN_ID_MOCK = '0x1';
@@ -89,10 +92,6 @@ describe('useTransactionPayMetrics', () => {
8992
useTransactionPayAvailableTokens,
9093
);
9194

92-
const useIsTransactionPayQuoteLoadingMock = jest.mocked(
93-
useIsTransactionPayQuoteLoading,
94-
);
95-
9695
const useAccountTokensMock = jest.mocked(useAccountTokens);
9796

9897
beforeEach(() => {
@@ -114,8 +113,8 @@ describe('useTransactionPayMetrics', () => {
114113
} as never);
115114

116115
useTransactionPayQuotesMock.mockReturnValue([]);
117-
useIsTransactionPayQuoteLoadingMock.mockReturnValue(false);
118116
useAccountTokensMock.mockReturnValue([]);
117+
mockSelectConfirmationMetricsById.mockReturnValue(undefined);
119118

120119
useTransactionPayAvailableTokensMock.mockReturnValue([
121120
{},
@@ -343,12 +342,14 @@ describe('useTransactionPayMetrics', () => {
343342
});
344343
});
345344

346-
it('is true when loading starts', async () => {
345+
it('is true when stored in metrics', async () => {
347346
useTransactionPayTokenMock.mockReturnValue({
348347
payToken: PAY_TOKEN_MOCK,
349348
setPayToken: noop,
350349
} as ReturnType<typeof useTransactionPayToken>);
351-
useIsTransactionPayQuoteLoadingMock.mockReturnValue(true);
350+
mockSelectConfirmationMetricsById.mockReturnValue({
351+
properties: { mm_pay_quote_requested: true },
352+
});
352353

353354
runHook();
354355

@@ -389,29 +390,6 @@ describe('useTransactionPayMetrics', () => {
389390
});
390391
});
391392

392-
it('is true when has quotes even while loading new quotes', async () => {
393-
useTransactionPayTokenMock.mockReturnValue({
394-
payToken: PAY_TOKEN_MOCK,
395-
setPayToken: noop,
396-
} as ReturnType<typeof useTransactionPayToken>);
397-
useIsTransactionPayQuoteLoadingMock.mockReturnValue(true);
398-
useTransactionPayQuotesMock.mockReturnValue([QUOTE_MOCK]);
399-
400-
runHook();
401-
402-
await act(async () => noop());
403-
404-
expect(updateConfirmationMetricMock).toHaveBeenCalledWith({
405-
id: transactionIdMock,
406-
params: {
407-
properties: expect.objectContaining({
408-
mm_pay_quote_loaded: true,
409-
}),
410-
sensitiveProperties: {},
411-
},
412-
});
413-
});
414-
415393
it('is false when no quotes', async () => {
416394
useTransactionPayTokenMock.mockReturnValue({
417395
payToken: PAY_TOKEN_MOCK,

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

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { useEffect, useMemo, useRef } from 'react';
2-
import { useDispatch } from 'react-redux';
3-
import { updateConfirmationMetric } from '../../../../../core/redux/slices/confirmationMetrics';
2+
import { useDispatch, useSelector } from 'react-redux';
3+
import {
4+
selectConfirmationMetricsById,
5+
updateConfirmationMetric,
6+
} from '../../../../../core/redux/slices/confirmationMetrics';
7+
import { RootState } from '../../../../../reducers';
48
import { useTransactionMetadataRequest } from '../transactions/useTransactionMetadataRequest';
59
import { useDeepMemo } from '../useDeepMemo';
610
import { Hex, Json, isCaipChainId, isHexString } from '@metamask/utils';
@@ -10,7 +14,6 @@ import { useTransactionPayToken } from './useTransactionPayToken';
1014
import { BridgeToken } from '../../../../UI/Bridge/types';
1115
import { hasTransactionType } from '../../utils/transaction';
1216
import {
13-
useIsTransactionPayQuoteLoading,
1417
useTransactionPayQuotes,
1518
useTransactionPayRequiredTokens,
1619
useTransactionPayTotals,
@@ -28,22 +31,26 @@ export function useTransactionPayMetrics() {
2831
const requiredTokens = useTransactionPayRequiredTokens();
2932
const highestBalanceChainId = useHighestBalanceCaipChainId();
3033
const automaticPayToken = useRef<BridgeToken>();
31-
const hasRequestedQuoteRef = useRef(false);
34+
const hasLoadedQuoteRef = useRef(false);
3235
const quotes = useTransactionPayQuotes();
33-
const isQuotesLoading = useIsTransactionPayQuoteLoading();
3436
const totals = useTransactionPayTotals();
3537
const tokens = useTransactionPayAvailableTokens();
3638

37-
if (isQuotesLoading && !hasRequestedQuoteRef.current) {
38-
hasRequestedQuoteRef.current = true;
39+
const transactionId = transactionMeta?.id ?? '';
40+
const storedMetrics = useSelector((state: RootState) =>
41+
selectConfirmationMetricsById(state, transactionId),
42+
);
43+
44+
const hasQuotes = (quotes?.length ?? 0) > 0;
45+
46+
if (hasQuotes && !hasLoadedQuoteRef.current) {
47+
hasLoadedQuoteRef.current = true;
3948
}
4049

4150
const availableTokens = useMemo(
4251
() => tokens.filter((t) => !t.disabled),
4352
[tokens],
4453
);
45-
46-
const transactionId = transactionMeta?.id ?? '';
4754
const { chainId, type } = transactionMeta ?? {};
4855
const primaryRequiredToken = requiredTokens.find((t) => !t.skipIfBalance);
4956
const sendingValue = Number(primaryRequiredToken?.amountHuman ?? '0');
@@ -55,8 +62,6 @@ export function useTransactionPayMetrics() {
5562
const properties: Json = {};
5663
const sensitiveProperties: Json = {};
5764

58-
const hasQuotes = (quotes?.length ?? 0) > 0;
59-
6065
if (payToken) {
6166
properties.mm_pay = true;
6267
properties.mm_pay_token_selected = payToken.symbol;
@@ -74,8 +79,9 @@ export function useTransactionPayMetrics() {
7479

7580
properties.mm_pay_payment_token_list_size = availableTokens.length;
7681

77-
properties.mm_pay_quote_requested = hasRequestedQuoteRef.current;
78-
properties.mm_pay_quote_loaded = hasQuotes;
82+
properties.mm_pay_quote_requested =
83+
(storedMetrics?.properties?.mm_pay_quote_requested as boolean) ?? false;
84+
properties.mm_pay_quote_loaded = hasLoadedQuoteRef.current;
7985
properties.mm_pay_chain_highest_balance_caip =
8086
highestBalanceChainId ?? null;
8187
}

0 commit comments

Comments
 (0)