Skip to content

Commit 27d3f35

Browse files
georgeweilergeorgeweiler
committed
chore(runway): cherry-pick fix(ramps): improve external-browser callback redirection cp-7.71.0 (#27804)
<!-- 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? --> Fixes the external-browser return flow for unified ramps by moving callback resolution out of Build Quote and into Order Details. The bug was that external-browser returns were resolved too early in BuildQuote. If callback parsing or order lookup failed there, users could get bounced around or end up on a broken Order Details screen. This change fixes that by moving callback resolution into Order Details itself. BuildQuote now only hands off the callback context, and Order Details fetches the real order itself. That makes the flow more reliable: bailed callbacks return to Build Quote, and real fetch failures show a retryable error instead of a broken redirect. **What changed** - **Build Quote -> Order Details callback handoff** After a successful external-browser return, Build Quote now navigates to Order Details with the full `callbackUrl`, `providerCode`, and `walletAddress` instead of trying to resolve the order immediately. - **Order Details callback bootstrap** Order Details now supports loading from callback params, fetching the real order on first render, and updating route params once the order has been resolved. - **Bailed / invalid callback handling** If the callback resolves to a bailed order state or no usable order, the user is sent back to Build Quote instead of landing on a blank or broken Order Details screen. - **Retryable callback error state** If fetching the order from the callback URL fails, Order Details now shows a retryable error screen rather than silently resetting away. This makes transient backend/network failures recoverable. - **Navigation tests updated** Tests were updated to reflect the callback-based route shape and the new Order Details retry behavior. **What stays untouched** This PR does not change Order Content amount rendering, list display formatting, or duplicate placeholder cleanup. It is scoped only to fixing the external-browser redirection and callback-resolution path. ## **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: null ## **Related issues** Fixes: ## **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] --> Paypal Order going to build quote page first: Uploading Screen Recording 2026-03-23 at 1.00.39 PM.mov… ### **After** <!-- [screenshots/recordings] --> Native Transak Redirection https://github.com/user-attachments/assets/32d1a7f9-23c7-4df1-aba8-f639338d7a6f Bailed Paypal order (return to build quote page): https://github.com/user-attachments/assets/8ed07fa3-e7df-4b69-b2f0-9318799c8249 Paypal order going to order details page: https://github.com/user-attachments/assets/5a2e8489-a4b0-488d-8aca-7982df63c45c ## **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 - [ ] 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] > **Medium Risk** > Changes the unified ramps external-browser return flow and navigation params, which can impact users reaching the correct order state after checkout; failures may surface as new retry/error behaviors. > > **Overview** > Fixes unified ramps external-browser return handling by **moving callback URL resolution out of `BuildQuote` and into `OrderDetails`**. > > `BuildQuote` no longer calls `getOrderFromCallback`/`addOrder` on InAppBrowser success; it now resets navigation to `OrderDetails` with `callbackUrl`, `providerCode`, and `walletAddress`. `OrderDetails` bootstraps from these callback params, fetches the real order (bailing back to `BuildQuote` for invalid/bailed statuses), updates route params to the resolved `orderId`, and shows a retryable error state if the callback fetch fails. > > Updates `rampsNavigation` to support an `OrderDetails` route shaped around callback params (and makes `orderId` optional), and adjusts/adds tests to cover the new handoff and retry behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0eabfd6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent b7b8475 commit 27d3f35

6 files changed

Lines changed: 212 additions & 57 deletions

File tree

‎app/components/UI/Ramp/Views/BuildQuote/BuildQuote.test.tsx‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -476,12 +476,6 @@ describe('BuildQuote', () => {
476476
type: 'success',
477477
url: 'metamask://on-ramp/providers/moonpay?orderId=ord-123',
478478
});
479-
mockGetOrderFromCallback.mockResolvedValue({
480-
providerOrderId: 'ord-123',
481-
status: 'Pending',
482-
cryptoAmount: '0.05',
483-
cryptoCurrency: { symbol: 'ETH' },
484-
});
485479
mockGetBuyWidgetData.mockResolvedValue({
486480
url: 'https://widget.example.com/checkout',
487481
browser: 'IN_APP_OS_BROWSER',
@@ -497,14 +491,18 @@ describe('BuildQuote', () => {
497491
});
498492

499493
await waitFor(() => {
500-
expect(mockAddOrder).toHaveBeenCalled();
494+
expect(mockAddOrder).not.toHaveBeenCalled();
495+
expect(mockGetOrderFromCallback).not.toHaveBeenCalled();
501496
expect(mockNavigationReset).toHaveBeenCalledWith({
502497
index: 0,
503498
routes: [
504499
{
505500
name: Routes.RAMP.RAMPS_ORDER_DETAILS,
506501
params: {
507-
orderId: 'ord-123',
502+
callbackUrl:
503+
'metamask://on-ramp/providers/moonpay?orderId=ord-123',
504+
providerCode: 'moonpay',
505+
walletAddress: '0x1234567890123456789012345678901234567890',
508506
showCloseButton: true,
509507
},
510508
},

‎app/components/UI/Ramp/Views/BuildQuote/BuildQuote.tsx‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import {
2020
getWidgetRedirectConfig,
2121
} from '../../utils/buildQuoteWithRedirectUrl';
2222
import { computeAmountUpdate } from '../../utils/computeAmountUpdate';
23-
import { extractOrderCode } from '../../utils/extractOrderCode';
2423
import { getRampCallbackBaseUrl } from '../../utils/getRampCallbackBaseUrl';
2524
import { getNavigateAfterExternalBrowserRoutes } from '../../utils/rampsNavigation';
2625
import { reportRampsError } from '../../utils/reportRampsError';
@@ -176,8 +175,6 @@ function BuildQuote() {
176175
paymentMethods,
177176
getBuyWidgetData,
178177
addPrecreatedOrder,
179-
addOrder,
180-
getOrderFromCallback,
181178
paymentMethodsLoading,
182179
paymentMethodsFetching,
183180
paymentMethodsStatus,
@@ -683,36 +680,17 @@ function BuildQuote() {
683680
return;
684681
}
685682

686-
try {
687-
const order = await getOrderFromCallback(
688-
providerCode,
689-
result.url,
690-
effectiveWallet,
691-
);
692-
693-
if (!order || isBailedOrderStatus(order.status)) {
694-
navigateAfterExternalBrowser({ returnDestination: 'buildQuote' });
695-
return;
696-
}
697-
698-
addOrder(order);
699-
700-
const rawOrderId = order.providerOrderId ?? effectiveOrderId;
701-
if (!rawOrderId) {
702-
navigateAfterExternalBrowser({ returnDestination: 'buildQuote' });
703-
return;
704-
}
705-
706-
const orderCode = extractOrderCode(rawOrderId);
707-
navigateAfterExternalBrowser({
708-
returnDestination: 'order',
709-
orderCode,
710-
providerCode,
711-
walletAddress: effectiveWallet || undefined,
712-
});
713-
} catch {
683+
if (!effectiveWallet) {
714684
navigateAfterExternalBrowser({ returnDestination: 'buildQuote' });
685+
return;
715686
}
687+
688+
navigateAfterExternalBrowser({
689+
returnDestination: 'order',
690+
callbackUrl: result.url,
691+
providerCode,
692+
walletAddress: effectiveWallet,
693+
});
716694
} finally {
717695
InAppBrowser.closeAuth();
718696
}
@@ -757,8 +735,6 @@ function BuildQuote() {
757735
navigation,
758736
getBuyWidgetData,
759737
addPrecreatedOrder,
760-
getOrderFromCallback,
761-
addOrder,
762738
navigateAfterExternalBrowser,
763739
]);
764740

‎app/components/UI/Ramp/Views/OrderDetails/OrderDetails.test.tsx‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
22
import { ActivityIndicator } from 'react-native';
3-
import { fireEvent, waitFor } from '@testing-library/react-native';
3+
import { fireEvent, waitFor, act } from '@testing-library/react-native';
44
import OrderDetails, {
55
createRampsOrderDetailsNavDetails,
66
} from './OrderDetails';
@@ -11,21 +11,29 @@ import { RampsOrderStatus } from '@metamask/ramps-controller';
1111

1212
const mockSetOptions = jest.fn();
1313
const mockNavigate = jest.fn();
14+
const mockSetParams = jest.fn();
15+
const mockReset = jest.fn();
1416
jest.mock('@react-navigation/native', () => ({
1517
...jest.requireActual('@react-navigation/native'),
1618
useNavigation: () => ({
1719
setOptions: mockSetOptions,
1820
navigate: mockNavigate,
1921
goBack: jest.fn(),
22+
setParams: mockSetParams,
23+
reset: mockReset,
2024
}),
2125
}));
2226

2327
const mockGetOrderById = jest.fn();
2428
const mockRefreshOrder = jest.fn();
29+
const mockGetOrderFromCallback = jest.fn();
30+
const mockAddOrder = jest.fn();
2531
jest.mock('../../hooks/useRampsOrders', () => ({
2632
useRampsOrders: () => ({
2733
getOrderById: mockGetOrderById,
2834
refreshOrder: mockRefreshOrder,
35+
getOrderFromCallback: mockGetOrderFromCallback,
36+
addOrder: mockAddOrder,
2937
}),
3038
}));
3139

@@ -52,7 +60,9 @@ jest.mock('../../../../hooks/useAnalytics/useAnalytics', () => ({
5260
}),
5361
}));
5462

55-
const mockUseParams = jest.fn(() => ({ orderId: 'test-order-123' }));
63+
const mockUseParams = jest.fn<Record<string, string | undefined>, []>(() => ({
64+
orderId: 'test-order-123',
65+
}));
5666
jest.mock('../../../../../util/navigation/navUtils', () => ({
5767
...jest.requireActual('../../../../../util/navigation/navUtils'),
5868
useParams: () => mockUseParams(),
@@ -167,7 +177,9 @@ describe('OrderDetails', () => {
167177
expect(getByText('ramps_order_details.try_again')).toBeOnTheScreen();
168178
});
169179

170-
fireEvent.press(getByText('ramps_order_details.try_again'));
180+
await act(async () => {
181+
fireEvent.press(getByText('ramps_order_details.try_again'));
182+
});
171183
expect(mockRefreshOrder).toHaveBeenCalled();
172184
});
173185

@@ -187,4 +199,34 @@ describe('OrderDetails', () => {
187199
const result = createRampsOrderDetailsNavDetails();
188200
expect(result[0]).toBe(Routes.RAMP.RAMPS_ORDER_DETAILS);
189201
});
202+
203+
it('shows error state with retry when initial callback fetch fails', async () => {
204+
mockUseParams.mockReturnValue({
205+
callbackUrl: 'metamask://on-ramp/providers/paypal?orderId=abc',
206+
providerCode: 'paypal',
207+
walletAddress: '0x123',
208+
});
209+
mockGetOrderById.mockReturnValue(undefined);
210+
mockGetOrderFromCallback.mockRejectedValue(
211+
new Error('Network request failed'),
212+
);
213+
214+
const { getByText } = render();
215+
216+
await waitFor(() => {
217+
expect(getByText('Network request failed')).toBeOnTheScreen();
218+
});
219+
expect(getByText('ramps_order_details.try_again')).toBeOnTheScreen();
220+
221+
await act(async () => {
222+
fireEvent.press(getByText('ramps_order_details.try_again'));
223+
});
224+
expect(mockGetOrderFromCallback).toHaveBeenCalledTimes(2);
225+
expect(mockGetOrderFromCallback).toHaveBeenNthCalledWith(
226+
2,
227+
'paypal',
228+
'metamask://on-ramp/providers/paypal?orderId=abc',
229+
'0x123',
230+
);
231+
});
190232
});

‎app/components/UI/Ramp/Views/OrderDetails/OrderDetails.tsx‎

Lines changed: 112 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ import {
1515
normalizeProviderCode,
1616
RampsOrderStatus,
1717
} from '@metamask/ramps-controller';
18+
import { isBailedOrderStatus } from '../BuildQuote/BuildQuote';
1819
import { extractOrderCode } from '../../utils/extractOrderCode';
20+
import {
21+
getNavigateAfterExternalBrowserRoutes,
22+
type RampsOrderDetailsParams,
23+
} from '../../utils/rampsNavigation';
1924
import Button, {
2025
ButtonVariants,
2126
ButtonSize,
@@ -36,10 +41,6 @@ import { useRampsOrders } from '../../hooks/useRampsOrders';
3641
import { useAnalytics } from '../../../../hooks/useAnalytics/useAnalytics';
3742
import { MetaMetricsEvents } from '../../../../../core/Analytics';
3843
import { RampsOrderDetailsSelectorsIDs } from './OrderDetails.testIds';
39-
interface RampsOrderDetailsParams {
40-
orderId: string;
41-
showCloseButton?: boolean;
42-
}
4344

4445
export const createRampsOrderDetailsNavDetails =
4546
createNavigationDetails<RampsOrderDetailsParams>(
@@ -69,19 +70,89 @@ const styles = StyleSheet.create({
6970

7071
const OrderDetails = () => {
7172
const params = useParams<RampsOrderDetailsParams>();
72-
const { getOrderById, refreshOrder } = useRampsOrders();
73+
const { getOrderById, refreshOrder, getOrderFromCallback, addOrder } =
74+
useRampsOrders();
7375
const orderCode = params.orderId ? extractOrderCode(params.orderId) : '';
7476
const order = getOrderById(orderCode);
7577
const isPending = order ? PENDING_STATUSES.has(order.status) : false;
78+
const hasCallbackParams = Boolean(
79+
params.callbackUrl && params.providerCode && params.walletAddress,
80+
);
7681

77-
const [isLoading, setIsLoading] = useState(isPending);
82+
const [isLoading, setIsLoading] = useState(isPending || hasCallbackParams);
7883
const [error, setError] = useState<string | null>(null);
7984
const theme = useTheme();
8085
const { colors } = theme;
8186
const navigation = useNavigation();
8287
const { trackEvent, createEventBuilder } = useAnalytics();
8388

8489
const [isRefreshing, setIsRefreshing] = useState(false);
90+
const hasFetchedFromCallback = useRef(false);
91+
92+
const executeCallbackFetch = useCallback(
93+
async (
94+
providerCode: string,
95+
callbackUrl: string,
96+
walletAddress: string,
97+
logContext: string,
98+
) => {
99+
try {
100+
setError(null);
101+
const fetchedOrder = await getOrderFromCallback(
102+
providerCode,
103+
callbackUrl,
104+
walletAddress,
105+
);
106+
if (!fetchedOrder || isBailedOrderStatus(fetchedOrder.status)) {
107+
navigation.reset({
108+
index: 0,
109+
routes: getNavigateAfterExternalBrowserRoutes({
110+
returnDestination: 'buildQuote',
111+
}),
112+
});
113+
return;
114+
}
115+
addOrder(fetchedOrder);
116+
navigation.setParams({
117+
orderId: fetchedOrder.providerOrderId,
118+
callbackUrl: undefined,
119+
providerCode: undefined,
120+
walletAddress: undefined,
121+
});
122+
} catch (fetchError) {
123+
Logger.error(fetchError as Error, {
124+
message: `RampsOrderDetails: error fetching order from callback URL${logContext}`,
125+
callbackUrl,
126+
});
127+
setError(
128+
fetchError instanceof Error && fetchError.message
129+
? fetchError.message
130+
: strings('ramps_order_details.error_message'),
131+
);
132+
} finally {
133+
setIsLoading(false);
134+
}
135+
},
136+
[getOrderFromCallback, addOrder, navigation],
137+
);
138+
139+
const handleRetryCallbackFetch = useCallback(async () => {
140+
if (!params.callbackUrl || !params.providerCode || !params.walletAddress) {
141+
return;
142+
}
143+
setIsLoading(true);
144+
await executeCallbackFetch(
145+
params.providerCode,
146+
params.callbackUrl,
147+
params.walletAddress,
148+
' (retry)',
149+
);
150+
}, [
151+
params.callbackUrl,
152+
params.providerCode,
153+
params.walletAddress,
154+
executeCallbackFetch,
155+
]);
85156

86157
useEffect(() => {
87158
navigation.setOptions(
@@ -148,12 +219,38 @@ const OrderDetails = () => {
148219
}, [order, refreshOrder]);
149220

150221
useEffect(() => {
151-
if (isPending) {
222+
if (isPending && !hasCallbackParams) {
152223
handleOnRefresh();
153224
}
154225
// eslint-disable-next-line react-hooks/exhaustive-deps
155226
}, []);
156227

228+
useEffect(() => {
229+
if (
230+
!hasCallbackParams ||
231+
hasFetchedFromCallback.current ||
232+
!params.callbackUrl ||
233+
!params.providerCode ||
234+
!params.walletAddress
235+
) {
236+
return;
237+
}
238+
hasFetchedFromCallback.current = true;
239+
240+
executeCallbackFetch(
241+
params.providerCode,
242+
params.callbackUrl,
243+
params.walletAddress,
244+
'',
245+
);
246+
}, [
247+
hasCallbackParams,
248+
params.callbackUrl,
249+
params.providerCode,
250+
params.walletAddress,
251+
executeCallbackFetch,
252+
]);
253+
157254
if (isLoading) {
158255
return (
159256
<ScreenLayout>
@@ -166,11 +263,10 @@ const OrderDetails = () => {
166263
);
167264
}
168265

169-
if (!order) {
170-
return <ScreenLayout />;
171-
}
172-
173266
if (error) {
267+
const onRetry = hasCallbackParams
268+
? handleRetryCallbackFetch
269+
: handleOnRefresh;
174270
return (
175271
<ScreenLayout>
176272
<ScreenLayout.Body>
@@ -198,14 +294,18 @@ const OrderDetails = () => {
198294
size={ButtonSize.Lg}
199295
width={ButtonWidthTypes.Full}
200296
label={strings('ramps_order_details.try_again')}
201-
onPress={handleOnRefresh}
297+
onPress={onRetry}
202298
/>
203299
</Box>
204300
</ScreenLayout.Body>
205301
</ScreenLayout>
206302
);
207303
}
208304

305+
if (!order) {
306+
return <ScreenLayout />;
307+
}
308+
209309
return (
210310
<ScreenLayout testID={RampsOrderDetailsSelectorsIDs.CONTAINER}>
211311
<ScrollView

0 commit comments

Comments
 (0)