Skip to content

Commit e3dd7bd

Browse files
authored
fix(predict): hide unrealized pnl if the user has 0 positions (MetaMask#22678)
## **Description** This PR fixes an issue where unrealized P&L was being displayed even when users had no active positions. The fix ensures that unrealized P&L is only shown when the user has at least one position, preventing confusing or misleading information from being displayed. **What is the reason for the change?** Users were seeing unrealized P&L data displayed even when they had no positions, which could be confusing since P&L is meaningless without any active positions. In addition, Polymarket uPnL API endpoint seems to give us bogus information when we have 0 positions. **What is the improvement/solution?** The `useUnrealizedPnL` hook now fetches both unrealized P&L data and the user's positions in parallel. The unrealized P&L is only displayed if `positions.length > 0`, ensuring users only see relevant P&L information when they have active positions. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Unrealized P&L visibility based on positions Scenario: user has no positions Given user is logged in with an account And user has no active positions When user navigates to the Predict screen Then unrealized P&L section is not displayed Scenario: user has active positions Given user is logged in with an account And user has at least one active position When user navigates to the Predict screen Then unrealized P&L section is displayed with current P&L data ``` ## **Screenshots/Recordings** ### **Before** <!-- Unrealized P&L displayed even with 0 positions --> ### **After** https://github.com/user-attachments/assets/cba5278e-09fe-4f75-9eb3-a17771d3174d ## **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] > Only show unrealized PnL when the user has at least one position, fetching PnL and positions concurrently. > > - **Predict Hook (`app/components/UI/Predict/hooks/useUnrealizedPnL.tsx`)** > - Fetches `getUnrealizedPnL` and `getPositions` in parallel via `Promise.all`. > - Sets `unrealizedPnL` to `null` when `positions.length === 0`; otherwise uses fetched P&L. > - Calls `getPositions` with `{ providerId, limit: 1, offset: 0, claimable: false }`. > - **Tests (`app/components/UI/Predict/hooks/useUnrealizedPnL.test.tsx`)** > - Add mocks and coverage for positions-based visibility (no positions => `null`, with positions => show P&L). > - Verify `getPositions` is called with the correct params and provider-specific calls. > - Assert parallel invocation of `getUnrealizedPnL` and `getPositions`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8b77e7f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent a7e247c commit e3dd7bd

2 files changed

Lines changed: 123 additions & 5 deletions

File tree

app/components/UI/Predict/hooks/useUnrealizedPnL.test.tsx

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ jest.mock('@react-navigation/native', () => ({
1414
}));
1515

1616
const mockGetUnrealizedPnL = jest.fn();
17+
const mockGetPositions = jest.fn();
1718

1819
// Mock DevLogger
1920
jest.mock('../../../../core/SDKConnect/utils/DevLogger', () => ({
@@ -26,6 +27,7 @@ jest.mock('../../../../core/Engine', () => ({
2627
context: {
2728
PredictController: {
2829
getUnrealizedPnL: mockGetUnrealizedPnL,
30+
getPositions: mockGetPositions,
2931
},
3032
AccountTreeController: {
3133
getAccountsFromSelectedAccountGroup: jest.fn(() => [
@@ -50,6 +52,7 @@ interface MockEngine {
5052
context: {
5153
PredictController?: {
5254
getUnrealizedPnL: jest.Mock;
55+
getPositions: jest.Mock;
5356
};
5457
AccountTreeController?: {
5558
getAccountsFromSelectedAccountGroup: jest.Mock;
@@ -68,9 +71,12 @@ describe('useUnrealizedPnL', () => {
6871

6972
beforeEach(() => {
7073
jest.clearAllMocks();
74+
// Default: return positions with at least one item so tests pass by default
75+
mockGetPositions.mockResolvedValue([{ id: 'position-1' }]);
7176
engine.context = {
7277
PredictController: {
7378
getUnrealizedPnL: mockGetUnrealizedPnL,
79+
getPositions: mockGetPositions,
7480
},
7581
AccountTreeController: {
7682
getAccountsFromSelectedAccountGroup: jest.fn(() => [
@@ -260,4 +266,108 @@ describe('useUnrealizedPnL', () => {
260266
providerId: 'other-provider',
261267
});
262268
});
269+
270+
describe('positions-based visibility', () => {
271+
it('returns null when user has no positions', async () => {
272+
mockGetUnrealizedPnL.mockResolvedValue(basePnL);
273+
mockGetPositions.mockResolvedValue([]);
274+
275+
const { result } = renderHook(() => useUnrealizedPnL());
276+
277+
await waitFor(() => {
278+
expect(result.current.isLoading).toBe(false);
279+
});
280+
281+
expect(result.current.unrealizedPnL).toBeNull();
282+
expect(result.current.error).toBeNull();
283+
expect(mockGetPositions).toHaveBeenCalledWith({
284+
providerId: undefined,
285+
limit: 1,
286+
offset: 0,
287+
claimable: false,
288+
});
289+
});
290+
291+
it('returns unrealized P&L when user has positions', async () => {
292+
mockGetUnrealizedPnL.mockResolvedValue(basePnL);
293+
mockGetPositions.mockResolvedValue([
294+
{ id: 'position-1' },
295+
{ id: 'position-2' },
296+
]);
297+
298+
const { result } = renderHook(() => useUnrealizedPnL());
299+
300+
await waitFor(() => {
301+
expect(result.current.isLoading).toBe(false);
302+
});
303+
304+
expect(result.current.unrealizedPnL).toEqual(basePnL);
305+
expect(result.current.error).toBeNull();
306+
});
307+
308+
it('calls getPositions with providerId when specified', async () => {
309+
mockGetUnrealizedPnL.mockResolvedValue(basePnL);
310+
mockGetPositions.mockResolvedValue([{ id: 'position-1' }]);
311+
312+
const { result } = renderHook(() =>
313+
useUnrealizedPnL({
314+
providerId: 'polymarket',
315+
}),
316+
);
317+
318+
await waitFor(() => {
319+
expect(result.current.unrealizedPnL).toEqual(basePnL);
320+
});
321+
322+
expect(mockGetPositions).toHaveBeenCalledWith({
323+
providerId: 'polymarket',
324+
limit: 1,
325+
offset: 0,
326+
claimable: false,
327+
});
328+
});
329+
330+
it('returns null when getUnrealizedPnL returns null and user has positions', async () => {
331+
mockGetUnrealizedPnL.mockResolvedValue(null);
332+
mockGetPositions.mockResolvedValue([{ id: 'position-1' }]);
333+
334+
const { result } = renderHook(() => useUnrealizedPnL());
335+
336+
await waitFor(() => {
337+
expect(result.current.isLoading).toBe(false);
338+
});
339+
340+
expect(result.current.unrealizedPnL).toBeNull();
341+
expect(result.current.error).toBeNull();
342+
});
343+
344+
it('returns null when both getUnrealizedPnL and getPositions return empty', async () => {
345+
mockGetUnrealizedPnL.mockResolvedValue(null);
346+
mockGetPositions.mockResolvedValue([]);
347+
348+
const { result } = renderHook(() => useUnrealizedPnL());
349+
350+
await waitFor(() => {
351+
expect(result.current.isLoading).toBe(false);
352+
});
353+
354+
expect(result.current.unrealizedPnL).toBeNull();
355+
expect(result.current.error).toBeNull();
356+
});
357+
358+
it('calls both getUnrealizedPnL and getPositions in parallel', async () => {
359+
mockGetUnrealizedPnL.mockResolvedValue(basePnL);
360+
mockGetPositions.mockResolvedValue([{ id: 'position-1' }]);
361+
362+
renderHook(() => useUnrealizedPnL());
363+
364+
await waitFor(() => {
365+
expect(mockGetUnrealizedPnL).toHaveBeenCalled();
366+
expect(mockGetPositions).toHaveBeenCalled();
367+
});
368+
369+
expect(mockGetUnrealizedPnL).toHaveBeenCalledTimes(1);
370+
expect(mockGetPositions).toHaveBeenCalledTimes(1);
371+
});
372+
});
263373
});

app/components/UI/Predict/hooks/useUnrealizedPnL.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,21 @@ export const useUnrealizedPnL = (
7575
}
7676
setError(null);
7777

78-
const unrealizedPnLData =
79-
await Engine.context.PredictController.getUnrealizedPnL({
78+
const [unrealizedPnLData, positions] = await Promise.all([
79+
Engine.context.PredictController.getUnrealizedPnL({
8080
address: address ?? selectedInternalAccountAddress,
8181
providerId,
82-
});
83-
84-
setUnrealizedPnL(unrealizedPnLData ?? null);
82+
}),
83+
Engine.context.PredictController.getPositions({
84+
providerId,
85+
limit: 1,
86+
offset: 0,
87+
claimable: false,
88+
}),
89+
]);
90+
91+
const _unrealizedPnL = unrealizedPnLData ?? null;
92+
setUnrealizedPnL(positions.length > 0 ? _unrealizedPnL : null);
8593

8694
DevLogger.log('useUnrealizedPnL: Loaded unrealized P&L', {
8795
unrealizedPnL: unrealizedPnLData,

0 commit comments

Comments
 (0)