Skip to content

Commit 955fd0a

Browse files
authored
fix: show realized PnL on closed perp feed rows (#33356)
<!-- 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. --> <!-- mms-check directive vocabulary — read by .github/scripts/shared/pr-template-checks.ts at module load to build the validation plan. Directives are invisible in rendered markdown and must NOT be removed or edited without updating the validator registry. type=text Section must contain non-placeholder prose. type=changelog Section must have a valid CHANGELOG entry: line. type=issue-link Section must have a Fixes:/Closes:/Refs: line with a value. type=manual-testing Section must have real testing steps or an explicit N/A. type=screenshot Section must have evidence (image/URL) or an explicit N/A. type=checklist Section must have all checkboxes consciously checked. required=true|false Whether a missing/invalid section runs the validator at all. blocking=true|false Whether a failure of this check fails the CI workflow. Default: false — failures are shown as warnings in the sticky comment but do not block the PR. Sections without a directive are checked for structural presence only. --> ## **Description** <!-- mms-check: type=text required=true --> This PR fixes empty PnL column on closed perp feed rows. Root cause: the action label used the exit trade (`intent === 'exit'`), but PnL presentation used `isClosedPosition()`, which can misclassify closed perps when API still reports a non-zero `marginUsd`. That left the action reading as a close while the PnL path treated the position as open (empty column). Also includes the Leaderboard "All types" dropdown (shared with feed). <img height="190" alt="Screenshot 2026-07-15 at 21 12 38" src="https://github.com/user-attachments/assets/b9143105-cde3-4044-827f-ea8b28122e8a" /> ## **Changelog** <!-- mms-check: type=changelog required=true blocking=true --> <!-- 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** <!-- mms-check: type=issue-link required=true --> Fixes: https://consensyssoftware.atlassian.net/browse/TSA-893 ## **Manual testing steps** <!-- mms-check: type=manual-testing required=true --> ```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** <!-- mms-check: type=screenshot required=true --> <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** NA ### **After** NA ## **Pre-merge author checklist** <!-- mms-check: type=checklist required=true --> <!-- 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. --> - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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** > Feed row labeling and PnL depend on trade intent vs snapshot heuristics, and leaderboard filtering UX changed (dropdown + perps-off behavior), so regressions could affect social surfaces users see often. > > **Overview** > **Feed mapping** now treats the triggering trade’s `enter`/`exit` intent as the source of truth for whether a row is open or closed, so action labels and PnL/value presentation stay aligned. That fixes closed perp rows that still showed an empty PnL column when Clicker left stale non-zero `marginUsd` while the exit fill was clearly an `exit`. > > **Leaderboard + feed** share a new `TypeFilter` module (`TypeFilterSelector` + `TypeFilterSheet`) under Social Leaderboard. `TopTradersView` drops inline All/Tokens/Perps pills in favor of the same “All types” dropdown as `FeedView`; when perps are disabled the selector is hidden instead of showing only All. Copy moves to `social_leaderboard.type_filter.*` and titles are sentence-cased in `en.json`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fb7f086. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 38e16cd commit 955fd0a

20 files changed

Lines changed: 402 additions & 358 deletions

app/components/Views/SocialLeaderboard/FeedView/FeedView.test.tsx

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ import {
1010
getFeedTradeButtonTestId,
1111
getFeedTradeCardTestId,
1212
getFeedTraderTestId,
13-
getFeedTypeOptionTestId,
1413
} from './FeedView.testIds';
14+
import {
15+
TypeFilterSelectorsIDs,
16+
getTypeFilterOptionTestId,
17+
} from '../components/TypeFilter';
1518
import type { FeedItem, FeedSection, FeedTypeFilter } from './types';
1619
import type { UseTraderFeedResult } from './hooks/useTraderFeed';
1720

@@ -152,12 +155,17 @@ jest.mock('../TraderPositionView/components/QuickBuy', () => {
152155

153156
let handleTypeFilterChange: ((value: FeedTypeFilter) => void) | undefined;
154157

155-
jest.mock('./components/FeedTypeSheet', () => {
158+
jest.mock('../components/TypeFilter', () => {
156159
const ReactActual = jest.requireActual('react');
157-
const Actual = jest.requireActual('./components/FeedTypeSheet').default;
158-
return (props: React.ComponentProps<typeof Actual>) => {
159-
handleTypeFilterChange = props.onChange;
160-
return ReactActual.createElement(Actual, props);
160+
const Actual = jest.requireActual('../components/TypeFilter');
161+
return {
162+
...Actual,
163+
TypeFilterSheet: (
164+
props: React.ComponentProps<typeof Actual.TypeFilterSheet>,
165+
) => {
166+
handleTypeFilterChange = props.onChange;
167+
return ReactActual.createElement(Actual.TypeFilterSheet, props);
168+
},
161169
};
162170
});
163171

@@ -173,7 +181,7 @@ describe('FeedView', () => {
173181
renderWithProvider(<FeedView />);
174182

175183
expect(
176-
screen.getByTestId(FeedViewSelectorsIDs.TYPE_SELECTOR),
184+
screen.getByTestId(TypeFilterSelectorsIDs.SELECTOR),
177185
).toBeOnTheScreen();
178186
expect(
179187
screen.getByTestId(FeedViewSelectorsIDs.AUDIENCE_TOGGLE),
@@ -288,8 +296,8 @@ describe('FeedView', () => {
288296
it('tracks type filter changes via Trader Feed Interaction', () => {
289297
renderWithProvider(<FeedView />);
290298

291-
fireEvent.press(screen.getByTestId(FeedViewSelectorsIDs.TYPE_SELECTOR));
292-
fireEvent.press(screen.getByTestId(getFeedTypeOptionTestId('tokens')));
299+
fireEvent.press(screen.getByTestId(TypeFilterSelectorsIDs.SELECTOR));
300+
fireEvent.press(screen.getByTestId(getTypeFilterOptionTestId('tokens')));
293301

294302
expect(mockTrack).toHaveBeenCalledWith(
295303
MetaMetricsEvents.SOCIAL_TRADER_FEED_INTERACTION,
@@ -434,8 +442,8 @@ describe('FeedView', () => {
434442

435443
renderWithProvider(<FeedView />);
436444

437-
fireEvent.press(screen.getByTestId(FeedViewSelectorsIDs.TYPE_SELECTOR));
438-
fireEvent.press(screen.getByTestId(getFeedTypeOptionTestId('tokens')));
445+
fireEvent.press(screen.getByTestId(TypeFilterSelectorsIDs.SELECTOR));
446+
fireEvent.press(screen.getByTestId(getTypeFilterOptionTestId('tokens')));
439447

440448
expect(
441449
screen.getByTestId(FeedViewSelectorsIDs.TYPE_EMPTY_STATE),
@@ -459,8 +467,8 @@ describe('FeedView', () => {
459467

460468
renderWithProvider(<FeedView />);
461469

462-
fireEvent.press(screen.getByTestId(FeedViewSelectorsIDs.TYPE_SELECTOR));
463-
fireEvent.press(screen.getByTestId(getFeedTypeOptionTestId('perps')));
470+
fireEvent.press(screen.getByTestId(TypeFilterSelectorsIDs.SELECTOR));
471+
fireEvent.press(screen.getByTestId(getTypeFilterOptionTestId('perps')));
464472

465473
expect(
466474
screen.getByTestId(FeedViewSelectorsIDs.TYPE_EMPTY_STATE),
@@ -480,8 +488,8 @@ describe('FeedView', () => {
480488

481489
renderWithProvider(<FeedView />);
482490

483-
fireEvent.press(screen.getByTestId(FeedViewSelectorsIDs.TYPE_SELECTOR));
484-
fireEvent.press(screen.getByTestId(getFeedTypeOptionTestId('perps')));
491+
fireEvent.press(screen.getByTestId(TypeFilterSelectorsIDs.SELECTOR));
492+
fireEvent.press(screen.getByTestId(getTypeFilterOptionTestId('perps')));
485493

486494
expect(
487495
screen.getByTestId(FeedViewSelectorsIDs.TYPE_EMPTY_STATE),

app/components/Views/SocialLeaderboard/FeedView/FeedView.testIds.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,6 @@ export const FeedViewSelectorsIDs = {
99
ERROR_STATE: 'feed-view-error-state',
1010
RETRY_BUTTON: 'feed-view-retry-button',
1111
AUDIENCE_TOGGLE: 'feed-view-audience-toggle',
12-
TYPE_SELECTOR: 'feed-view-type-selector',
13-
TYPE_SELECTOR_SHEET: 'feed-view-type-selector-sheet',
14-
TYPE_SELECTOR_BACKDROP: 'feed-view-type-selector-backdrop',
1512
TRADE_BUTTON: 'feed-view-trade-button',
1613
TRADE_CARD: 'feed-view-trade-card',
1714
} as const;
@@ -24,5 +21,3 @@ export const getFeedTradeCardTestId = (id: string) =>
2421
export const getFeedTraderTestId = (id: string) => `feed-item-trader-${id}`;
2522
export const getFeedAudienceOptionTestId = (audience: string) =>
2623
`${FeedViewSelectorsIDs.AUDIENCE_TOGGLE}-${audience}`;
27-
export const getFeedTypeOptionTestId = (type: string) =>
28-
`feed-view-type-option-${type}`;

app/components/Views/SocialLeaderboard/FeedView/FeedView.tsx

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ import FeedAudienceToggle from './components/FeedAudienceToggle';
4848
import FeedItemRow from './components/FeedItemRow';
4949
import FeedItemRowSkeleton from './components/FeedItemRowSkeleton';
5050
import FeedTypeEmptyState from './components/FeedTypeEmptyState';
51-
import FeedTypeSelector from './components/FeedTypeSelector';
52-
import FeedTypeSheet from './components/FeedTypeSheet';
51+
import { TypeFilterSelector, TypeFilterSheet } from '../components/TypeFilter';
5352
import FollowingEmptyState from './components/FollowingEmptyState';
5453
import { useTraderFeed } from './hooks/useTraderFeed';
5554
import type {
@@ -446,7 +445,7 @@ const FeedView: React.FC<FeedViewProps> = ({ isActive = true }) => {
446445
twClassName="px-4 py-3"
447446
gap={3}
448447
>
449-
<FeedTypeSelector
448+
<TypeFilterSelector
450449
value={typeFilter}
451450
onPress={() => setIsTypeSheetOpen(true)}
452451
/>
@@ -455,7 +454,7 @@ const FeedView: React.FC<FeedViewProps> = ({ isActive = true }) => {
455454

456455
{content}
457456

458-
<FeedTypeSheet
457+
<TypeFilterSheet
459458
isOpen={isTypeSheetOpen}
460459
value={typeFilter}
461460
onChange={handleTypeFilterChange}

app/components/Views/SocialLeaderboard/FeedView/components/FeedTypeSelector.test.tsx

Lines changed: 0 additions & 32 deletions
This file was deleted.

app/components/Views/SocialLeaderboard/FeedView/components/feedTypeOptions.ts

Lines changed: 0 additions & 9 deletions
This file was deleted.

app/components/Views/SocialLeaderboard/FeedView/types.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import type { PositionTokenAvatarData } from '../components/PositionTokenAvatar'
88
export type FeedAudience = 'all' | 'following';
99

1010
/**
11-
* Feed type filter. Mirrors the leaderboard chain filters: everything, spot
12-
* tokens, or perps. Applied client-side over the loaded feed pages.
11+
* Feed type filter. Alias of the shared {@link SocialTypeFilter} used across the
12+
* leaderboard and feed: everything, spot tokens, or perps. Applied client-side
13+
* over the loaded feed pages.
1314
*/
14-
export type FeedTypeFilter = 'all' | 'tokens' | 'perps';
15+
export type { SocialTypeFilter as FeedTypeFilter } from '../components/TypeFilter/types';
1516

1617
/** Trade action verb, shown after the trader username. */
1718
export type FeedAction = 'bought' | 'sold' | 'opened' | 'closed';

app/components/Views/SocialLeaderboard/FeedView/utils/mapFeedItem.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,55 @@ describe('mapFeedItem', () => {
170170
expect(result?.action).toBe('sold');
171171
});
172172

173+
it('keeps a perp enter fill as "opened" even when the snapshot looks closed', () => {
174+
const result = mapFeedItem(
175+
mockPerpFeedItem({
176+
// A closed-looking snapshot (Clicker reports currentValueUSD === 0) must
177+
// not override the triggering trade's `enter` intent.
178+
currentValueUSD: 0,
179+
trades: [
180+
{
181+
direction: 'buy',
182+
intent: 'enter',
183+
tokenAmount: 5,
184+
usdCost: 50600,
185+
timestamp: 1_700_000_500,
186+
transactionHash: '0xhash',
187+
classification: 'perp',
188+
perpPositionType: 'long',
189+
perpLeverage: 8,
190+
},
191+
],
192+
}),
193+
);
194+
195+
expect(result?.action).toBe('opened');
196+
});
197+
198+
it('keeps a spot enter fill as "bought" even when the snapshot looks closed', () => {
199+
const result = mapFeedItem(
200+
mockSpotFeedItem({
201+
// Closed-looking spot snapshot (fully sold out) must still defer to the
202+
// triggering trade's `enter` intent.
203+
positionAmount: 0,
204+
soldUsd: 100_000,
205+
trades: [
206+
{
207+
direction: 'buy',
208+
intent: 'enter',
209+
tokenAmount: 1000,
210+
usdCost: 120000,
211+
timestamp: 1_700_000_000,
212+
transactionHash: '0xhash',
213+
classification: 'spot',
214+
},
215+
],
216+
}),
217+
);
218+
219+
expect(result?.action).toBe('bought');
220+
});
221+
173222
it('returns null for a spot trade on an unsupported chain', () => {
174223
const result = mapFeedItem(mockSpotFeedItem({ chain: 'fantom' }));
175224

@@ -280,6 +329,40 @@ describe('mapFeedItem', () => {
280329
expect(result?.valueLabel).toContain('4,659');
281330
});
282331

332+
it('shows realized PnL for a closed perp exit when marginUsd is still non-zero', () => {
333+
const result = mapFeedItem(
334+
mockPerpFeedItem({
335+
tokenSymbol: 'BTC',
336+
currentValueUSD: undefined,
337+
pnlValueUsd: undefined,
338+
pnlPercent: undefined,
339+
realizedPnl: 12_500,
340+
marginUsd: 151_400,
341+
positionAmount: 2.5,
342+
perpPositionType: 'short',
343+
perpLeverage: 9,
344+
trades: [
345+
{
346+
direction: 'sell',
347+
intent: 'exit',
348+
tokenAmount: -2.5,
349+
usdCost: -151_400,
350+
timestamp: 1_700_000_500,
351+
transactionHash: '0xhash',
352+
classification: 'perp',
353+
perpPositionType: 'short',
354+
perpLeverage: 9,
355+
},
356+
],
357+
}),
358+
);
359+
360+
expect(result?.action).toBe('closed');
361+
expect(result?.hasValueData).toBe(true);
362+
expect(result?.hasPnlData).toBe(true);
363+
expect(result?.valueLabel).toContain('12,500');
364+
});
365+
283366
it('marks negative PnL as not positive', () => {
284367
const result = mapFeedItem(
285368
mockPerpFeedItem({ pnlValueUsd: -500, realizedPnl: -500 }),

app/components/Views/SocialLeaderboard/FeedView/utils/mapFeedItem.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,32 @@ function findTriggeringTrade(
6868
}
6969

7070
/**
71-
* Resolves the action verb for a feed row. Perp fills read as "opened"/"closed",
72-
* spot as "bought"/"sold" (mirrors `TradeRow`). When the position has no trade
73-
* to key off, falls back to the position's open/closed state.
71+
* Feed rows are keyed off a triggering trade when one exists, and the trade's
72+
* intent is authoritative in both directions: an `exit` fill reads as closed
73+
* (even when {@link isClosedPosition} misclassifies a perp that still carries
74+
* stale non-zero margin in the Clicker payload), and an `enter` fill reads as
75+
* open (even when the position snapshot looks closed). We only fall back to the
76+
* {@link isClosedPosition} snapshot heuristic when there is no triggering trade.
7477
*/
75-
function resolveAction(
78+
function isFeedItemClosed(
79+
coreItem: CoreFeedItem,
7680
trade: Trade | undefined,
77-
isPerp: boolean,
78-
isClosed: boolean,
79-
): FeedAction {
80-
const isExit = trade ? trade.intent === 'exit' : isClosed;
81+
): boolean {
82+
if (trade) {
83+
return trade.intent === 'exit';
84+
}
85+
return isClosedPosition(coreItem);
86+
}
87+
88+
/**
89+
* Resolves the action verb for a feed row. Perp fills read as "opened"/"closed",
90+
* spot as "bought"/"sold" (mirrors `TradeRow`).
91+
*/
92+
function resolveAction(isPerp: boolean, isClosed: boolean): FeedAction {
8193
if (isPerp) {
82-
return isExit ? 'closed' : 'opened';
94+
return isClosed ? 'closed' : 'opened';
8395
}
84-
return isExit ? 'sold' : 'bought';
96+
return isClosed ? 'sold' : 'bought';
8597
}
8698

8799
/**
@@ -286,9 +298,9 @@ export function mapFeedItem(coreItem: CoreFeedItem): FeedItem | null {
286298

287299
const timestampMs = toMs(timestamp);
288300
const isPerp = isPerpPosition(coreItem);
289-
const isClosed = isClosedPosition(coreItem);
290301
const trade = findTriggeringTrade(trades ?? [], timestampMs);
291-
const action = resolveAction(trade, isPerp, isClosed);
302+
const isClosed = isFeedItemClosed(coreItem, trade);
303+
const action = resolveAction(isPerp, isClosed);
292304
const subHeader = buildSubHeader(trade);
293305
const presentation = buildFeedItemPresentation(coreItem, isClosed);
294306

0 commit comments

Comments
 (0)