Skip to content

Commit e4c7d07

Browse files
fix(perps): pass position in TP/SL onConfirm to avoid No position found errors (MetaMask#27409)
<!-- 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** When editing Take Profit / Stop Loss (TP/SL) for an existing Perps position, the `onConfirm` callback relied on a ref (`currentPositionRef.current`) to get the position at execution time. If the position was updated during navigation (e.g. via WebSocket), the ref could be stale, causing "No position found" errors when the user confirmed TP/SL. This change extends the TP/SL `onConfirm` signature to accept an optional first argument `position`. The TPSL view now passes the position from route params into `onConfirm`, and the market-details flow uses that passed position when present, falling back to the ref. Order flow and market-tabs callbacks were updated to match the new signature (they ignore the position argument). This ensures the callback always receives the correct position and avoids the stale-ref error. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2369 ## **Manual testing steps** ```gherkin Feature: Perps TP/SL confirmation Scenario: user confirms TP/SL for an existing position without "No position found" error Given user has an open Perps position and navigates to edit TP/SL (market details or market tabs) And the position data may have been updated (e.g. via WebSocket) before user taps Confirm When user sets Take Profit and/or Stop Loss and taps Confirm Then TP/SL is updated successfully and no "No position found" error is shown ``` ## **Screenshots/Recordings** N/A (behavioral fix; no UI 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] > **Medium Risk** > Changes the `PerpsTPSL` route `onConfirm` callback signature/return type and updates multiple callers, so mismatches could break TP/SL confirmation flows if any usage is missed. > > **Overview** > Fixes a TP/SL confirmation race by **passing the `position` from `PerpsTPSLView` into the route `onConfirm` callback**, and having `PerpsMarketDetailsView` prefer that position (falling back to `currentPositionRef`) before calling `handleUpdateTPSL`. > > Updates the `PerpsTPSL` navigation type and all known callers (`PerpsOrderView`, `PerpsMarketTabs`) to match the new `(position?, tp?, sl?, trackingData?)` signature, adjusts TPSL view tests accordingly, and adds a market-details test asserting `{ success: false }` when confirm runs after the position is cleared. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ee47f9c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 1fc916e commit e4c7d07

7 files changed

Lines changed: 124 additions & 33 deletions

File tree

app/components/UI/Perps/Views/PerpsMarketDetailsView/PerpsMarketDetailsView.test.tsx

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,6 +1939,69 @@ describe('PerpsMarketDetailsView', () => {
19391939

19401940
expect(getByText('Geo Block Tooltip')).toBeOnTheScreen();
19411941
});
1942+
1943+
it('returns failure when TPSL onConfirm runs after position is cleared', async () => {
1944+
mockNavigate.mockClear();
1945+
1946+
const existingPosition = {
1947+
symbol: 'BTC',
1948+
size: '0.5',
1949+
entryPrice: '50000',
1950+
leverage: { value: 10, type: 'isolated' as const },
1951+
marginUsed: '5000',
1952+
unrealizedPnl: '100',
1953+
returnOnEquity: '0.02',
1954+
liquidationPrice: '45000',
1955+
};
1956+
1957+
mockUseHasExistingPosition.mockReturnValue({
1958+
hasPosition: true,
1959+
isLoading: false,
1960+
error: null,
1961+
existingPosition,
1962+
refreshPosition: jest.fn(),
1963+
positionOpenedTimestamp: undefined,
1964+
});
1965+
1966+
const viewTree = (
1967+
<PerpsConnectionProvider>
1968+
<PerpsMarketDetailsView />
1969+
</PerpsConnectionProvider>
1970+
);
1971+
1972+
const { getByTestId, rerender } = renderWithProvider(viewTree, {
1973+
state: initialState,
1974+
});
1975+
1976+
fireEvent.press(getByTestId('perps-position-card-auto-close-button'));
1977+
1978+
const tpslNavigateCall = mockNavigate.mock.calls.find(
1979+
(call) => call[0] === 'PerpsTPSL',
1980+
);
1981+
expect(tpslNavigateCall).toBeTruthy();
1982+
const onConfirm = (
1983+
tpslNavigateCall as [
1984+
string,
1985+
{ onConfirm: (...args: unknown[]) => Promise<{ success: boolean }> },
1986+
]
1987+
)[1].onConfirm;
1988+
1989+
mockUseHasExistingPosition.mockReturnValue({
1990+
hasPosition: false,
1991+
isLoading: false,
1992+
error: null,
1993+
existingPosition: null,
1994+
refreshPosition: jest.fn(),
1995+
positionOpenedTimestamp: undefined,
1996+
});
1997+
1998+
rerender(viewTree);
1999+
2000+
await act(async () => {
2001+
const result = await onConfirm(undefined, undefined, undefined);
2002+
expect(result).toEqual({ success: false });
2003+
});
2004+
});
19422005
});
19432006

19442007
describe('Notification tooltip functionality', () => {

app/components/UI/Perps/Views/PerpsMarketDetailsView/PerpsMarketDetailsView.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -776,19 +776,18 @@ const PerpsMarketDetailsView: React.FC<PerpsMarketDetailsViewProps> = () => {
776776
initialTakeProfitPrice: existingPosition.takeProfitPrice,
777777
initialStopLossPrice: existingPosition.stopLossPrice,
778778
onConfirm: async (
779+
positionFromRoute?: Position,
779780
takeProfitPrice?: string,
780781
stopLossPrice?: string,
781782
trackingData?: TPSLTrackingData,
782783
) => {
783-
// Use ref to get CURRENT position at execution time, not the closure-captured position
784-
// This prevents "No position found" errors when the position updates during navigation
785-
const currentPosition = currentPositionRef.current;
786-
if (!currentPosition) {
784+
// Prefer position passed from TPSL view (from route params); fallback to ref to avoid "No position found" when ref is stale
785+
const positionToUse = positionFromRoute ?? currentPositionRef.current;
786+
if (!positionToUse) {
787787
return { success: false };
788788
}
789-
// Return value checked for consistency - error toast is shown internally by hook
790789
const result = await handleUpdateTPSL(
791-
currentPosition,
790+
positionToUse,
792791
takeProfitPrice,
793792
stopLossPrice,
794793
trackingData,

app/components/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -761,9 +761,12 @@ const PerpsOrderViewContentBase: React.FC<PerpsOrderViewContentProps> = ({
761761
initialStopLossPrice: orderForm.stopLossPrice,
762762
amount: orderForm.amount,
763763
szDecimals: marketData?.szDecimals,
764-
onConfirm: async (takeProfitPrice?: string, stopLossPrice?: string) => {
765-
// Use the same clearing approach as the "Off" button
766-
// If values are undefined or empty, ensure they're cleared properly
764+
onConfirm: async (
765+
_position?: Position,
766+
takeProfitPrice?: string,
767+
stopLossPrice?: string,
768+
) => {
769+
// Order flow: no position; just persist TP/SL in form state
767770
const tpToSet = takeProfitPrice || undefined;
768771
const slToSet = stopLossPrice || undefined;
769772

app/components/UI/Perps/Views/PerpsTPSLView/PerpsTPSLView.test.tsx

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -489,15 +489,20 @@ describe('PerpsTPSLView', () => {
489489
fireEvent.press(setButton);
490490
});
491491

492-
expect(mockOnConfirm).toHaveBeenCalledWith('3150.00', '2850.00', {
493-
direction: 'long',
494-
source: PERPS_EVENT_VALUE.RISK_MANAGEMENT_SOURCE.TRADE_SCREEN,
495-
positionSize: 0,
496-
takeProfitPercentage: undefined,
497-
stopLossPercentage: undefined,
498-
isEditingExistingPosition: false,
499-
entryPrice: 3000,
500-
});
492+
expect(mockOnConfirm).toHaveBeenCalledWith(
493+
undefined,
494+
'3150.00',
495+
'2850.00',
496+
{
497+
direction: 'long',
498+
source: PERPS_EVENT_VALUE.RISK_MANAGEMENT_SOURCE.TRADE_SCREEN,
499+
positionSize: 0,
500+
takeProfitPercentage: undefined,
501+
stopLossPercentage: undefined,
502+
isEditingExistingPosition: false,
503+
entryPrice: 3000,
504+
},
505+
);
501506
});
502507

503508
it('calls onConfirm with undefined when values are empty', async () => {
@@ -510,15 +515,20 @@ describe('PerpsTPSLView', () => {
510515
fireEvent.press(setButton);
511516
});
512517

513-
expect(mockOnConfirm).toHaveBeenCalledWith(undefined, undefined, {
514-
direction: 'long',
515-
source: PERPS_EVENT_VALUE.RISK_MANAGEMENT_SOURCE.TRADE_SCREEN,
516-
positionSize: 0,
517-
takeProfitPercentage: undefined,
518-
stopLossPercentage: undefined,
519-
isEditingExistingPosition: false,
520-
entryPrice: 3000,
521-
});
518+
expect(mockOnConfirm).toHaveBeenCalledWith(
519+
undefined,
520+
undefined,
521+
undefined,
522+
{
523+
direction: 'long',
524+
source: PERPS_EVENT_VALUE.RISK_MANAGEMENT_SOURCE.TRADE_SCREEN,
525+
positionSize: 0,
526+
takeProfitPercentage: undefined,
527+
stopLossPercentage: undefined,
528+
isEditingExistingPosition: false,
529+
entryPrice: 3000,
530+
},
531+
);
522532
});
523533

524534
it('dismisses keypad before confirming when input is focused', async () => {

app/components/UI/Perps/Views/PerpsTPSLView/PerpsTPSLView.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,13 @@ const PerpsTPSLView: React.FC = () => {
400400
isEditingExistingPosition,
401401
entryPrice: effectiveEntryPrice,
402402
};
403-
await onConfirm(parseTakeProfitPrice, parseStopLossPrice, trackingData);
403+
// Pass position from route params so the callback always has the correct position (avoids "No position found" when parent ref is stale)
404+
await onConfirm(
405+
position,
406+
parseTakeProfitPrice,
407+
parseStopLossPrice,
408+
trackingData,
409+
);
404410
navigation.goBack();
405411
} finally {
406412
setIsUpdating(false);

app/components/UI/Perps/components/PerpsMarketTabs/PerpsMarketTabs.tsx

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ import styleSheet from './PerpsMarketTabs.styles';
3030
import {
3131
OrderDirection,
3232
PERPS_CONSTANTS,
33-
type Position,
3433
type Order,
34+
type Position,
35+
type TPSLTrackingData,
3536
} from '@metamask/perps-controller';
3637
import { usePerpsMarketStats } from '../../hooks/usePerpsMarketStats';
3738
import {
@@ -329,9 +330,13 @@ const PerpsMarketTabs: React.FC<PerpsMarketTabsProps> = ({
329330
position,
330331
initialTakeProfitPrice: position.takeProfitPrice,
331332
initialStopLossPrice: position.stopLossPrice,
332-
onConfirm: async () => {
333-
// TP/SL is set directly on the position, no need to handle here
334-
// The position will update via WebSocket
333+
onConfirm: async (
334+
_position?: Position,
335+
_takeProfitPrice?: string,
336+
_stopLossPrice?: string,
337+
_trackingData?: TPSLTrackingData,
338+
) => {
339+
// TP/SL is set directly on the position via WebSocket; callback signature matches PerpsTPSL route type
335340
},
336341
});
337342
}, [position, currentPrice, navigation]);

app/components/UI/Perps/types/navigation.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,16 @@ export interface PerpsNavigationParamList extends ParamListBase {
180180
limitPrice?: string;
181181
amount?: string; // For new orders - USD amount to calculate position size for P&L
182182
szDecimals?: number; // For new orders - asset decimal precision for P&L
183+
/**
184+
* Called when user confirms TP/SL. First arg is position when editing existing position (avoids "No position found" from stale ref).
185+
* Signature: (position?, takeProfitPrice?, stopLossPrice?, trackingData?) so both edit-flow and order-flow can use it.
186+
*/
183187
onConfirm: (
188+
position?: Position,
184189
takeProfitPrice?: string,
185190
stopLossPrice?: string,
186191
trackingData?: TPSLTrackingData,
187-
) => Promise<void>;
192+
) => Promise<{ success: boolean } | void>;
188193
};
189194

190195
// PnL Hero Card screen

0 commit comments

Comments
 (0)