Skip to content

Commit 262d190

Browse files
feat: uses position id to render trader position page (#29403)
<!-- 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. --> ## **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? --> https://github.com/user-attachments/assets/5c903575-d495-49ef-8462-986d76558f5a `TraderPositionView` previously could only be reached by tapping a row in `TraderProfileView` because it required the full `Position` object as a route param. This PR makes the screen self-sufficient: given just `traderId + positionId`, it fetches the canonical position by UUID and the trader profile on its own. The existing row-tap path is unchanged — when `position` is passed it's used directly, no fetch fires. This is the foundation for deep-link support (push notifications, shared URLs); the `linking` URL config itself lands in a separate PR. ## What changed **New `useTraderPosition(positionId)` hook** Wraps `SocialService.fetchPositionById({ positionId })` via `useQuery`, mirroring the pattern used by `useTraderPositions` and `useTraderProfile`. Disabled when `positionId` is undefined, so the row-tap path pays zero network cost. **Three explicit render states in `TraderPositionView`** - **Content** — `position` resolved (from row-tap snapshot or canonical fetch). - **Skeleton** — fetch in flight and no bootstrap available (cold-start deep link). - **Fallback** — fetch failed and no bootstrap; renders an error illustration with a "back to profile" CTA. **`TraderPositionSkeleton` and `TraderPositionFallback` components** Lifted from PR #29378 (which also wanted these). They're framework-agnostic — no dependency on the alternate matching scheme used in that PR — so they drop in cleanly here. **Trader name/image self-resolution** `traderName` and `traderImageUrl` are now optional in the route contract. When absent (e.g. arriving via deep link with only IDs), the screen calls the existing `useTraderProfile(traderId)` hook to populate the header. Row-tap still passes them, so no extra fetch on that path. **Route contract update** ```ts // Before TraderPositionView: { traderId: string; traderName: string; traderImageUrl?: string; tokenSymbol: string; position?: Position; }; // After TraderPositionView: { traderId: string; tokenSymbol: string; position?: Position; // row-tap snapshot (no fetch) positionId?: string; // deep-link path (triggers fetch) traderName?: string; // resolved via useTraderProfile if absent traderImageUrl?: string; // resolved via useTraderProfile if absent }; ``` At least one of `position` or `positionId` must be passed. Old call sites (`TraderProfileView` row-tap) are unchanged. ## Why `positionId` rather than a composite key Compared to the alternative of identifying a position by `(tokenAddress, chain, positionContext)`: - `positionId` is the canonical UUID minted by the backend. Two re-entered positions on the same token never collide. - No address normalisation is needed. No EVM-vs-Solana case rules. No hardcoded chain allowlist that needs updating when a new chain ships. - Robust to position state transitions — open → closed mid-flight doesn't break URL resolution because we don't bake the open/closed flag into the identifier. - Mirrors the API: `GET /v1/traders/:traderId/positions/:positionId`. ## Out of scope (TODOs) - **Deep-link URL config.** `NavigationContainer linking` mapping `leaderboard/:traderId/positions/:positionId` → `TraderPositionView`. Lands in a follow-up. - **Background refresh / polling.** Once the screen is open, the position is a static snapshot. Future change: add `refetchInterval` to `useTraderPosition` and, on the row-tap path, also kick off a background fetch keyed by `position.positionId` to refresh + poll. Same canonical-only-after-resolve policy used elsewhere. - **Push-notification payload prefetch.** If notifications carry `traderName`, pass it through to skip the `useTraderProfile` fetch on cold-start. ## Files changed | File | Change | |---|---| | `hooks/useTraderPosition.ts` | NEW — single-position fetch via `SocialService:fetchPositionById` | | `hooks/useTraderPosition.test.ts` | NEW — 5 unit tests (query key, disabled state, success, loading, error) | | `components/TraderPositionSkeleton.tsx` | NEW — full-screen skeleton | | `components/TraderPositionFallback.tsx` | NEW — error state with back-to-profile CTA | | `TraderPositionView.tsx` | Resolve position via row-tap → hook fallback; resolve trader profile via `useTraderProfile` fallback; render skeleton / fallback / content | | `TraderPositionView.testIds.ts` | Added SKELETON / FALLBACK / FALLBACK_PRIMARY_ACTION test IDs | | `TraderPositionView.test.tsx` | New mocks for the two new hooks; replaced two stale tests + added 3 new flow tests | | `core/NavigationService/types.ts` | Made `traderName`/`traderImageUrl` optional; added `positionId?: string` | | `locales/languages/en.json` | Added 4 fallback strings | No changes to `TraderProfileView` — row-tap fast path stays free of any extra fetch. ## **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: no-changelog ## **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] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** <!-- 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** > Adds new data-fetching paths and route contract changes for `TraderPositionView`, which could affect deep-link/edge-case navigation and initial loading behavior, but scope is limited to the Social Leaderboard feature. > > **Overview** > `TraderPositionView` can now be opened without an in-memory `position` snapshot by optionally accepting a `positionId` route param and fetching the canonical position via a new `useTraderPosition` hook; `traderName`/`traderImageUrl` are also now optional and are fetched via `useTraderProfile` when missing. > > The screen adds explicit **skeleton**, **fallback**, and **content** render states (with new `TraderPositionSkeleton`/`TraderPositionFallback` components and i18n strings), gates Quick Buy to only open once a position is resolved, and updates/extends tests and test IDs to cover the new flows. Dependency `@metamask/social-controllers` is bumped to `^2.2.0` to support the new fetch-by-id API. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7ba0bd0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: António Regadas <antonio.regadas@consensys.net>
1 parent cdbc929 commit 262d190

11 files changed

Lines changed: 562 additions & 78 deletions

File tree

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

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,27 @@ jest.mock('@metamask/controller-utils', () => ({
8080
handleFetch: jest.fn().mockResolvedValue({}),
8181
}));
8282

83+
// New hooks added for deep-link self-sufficiency. Existing tests pass `position`
84+
// via route params, so these are effectively no-ops in the existing flow.
85+
jest.mock('./hooks/useTraderPosition', () => ({
86+
useTraderPosition: jest.fn(() => ({
87+
position: undefined,
88+
isLoading: false,
89+
error: null,
90+
})),
91+
}));
92+
93+
jest.mock('../TraderProfileView/hooks/useTraderProfile', () => ({
94+
useTraderProfile: jest.fn(() => ({
95+
profile: null,
96+
isLoading: false,
97+
error: null,
98+
isFollowing: false,
99+
toggleFollow: jest.fn(),
100+
refresh: jest.fn(),
101+
})),
102+
}));
103+
83104
jest.mock('../../../hooks/useAnalytics/useAnalytics', () => {
84105
const { createMockUseAnalyticsHook } = jest.requireActual(
85106
'../../../../util/test/analyticsMock',
@@ -184,25 +205,70 @@ describe('TraderPositionView', () => {
184205
expect(mockGoBack).toHaveBeenCalledTimes(1);
185206
});
186207

187-
it('falls back to the route tokenSymbol when position is undefined', () => {
208+
it('renders the fallback when position is undefined and no positionId is provided', () => {
188209
mockRouteParams.position = undefined;
189210
mockRouteParams.tokenSymbol = 'DOGE';
190211

191212
renderWithProvider(<TraderPositionView />, { state: mockState });
192213

193-
expect(screen.getAllByText('DOGE').length).toBeGreaterThanOrEqual(1);
214+
expect(
215+
screen.getByTestId(TraderPositionViewSelectorsIDs.FALLBACK),
216+
).toBeOnTheScreen();
217+
// Price chart should not be rendered in the fallback state
218+
expect(screen.queryByTestId('price-chart-mock')).toBeNull();
194219
});
195220

196-
it('does not leave price chart in loading state when position is undefined', async () => {
221+
it('renders the skeleton while a positionId fetch is in flight', () => {
222+
const { useTraderPosition } = jest.requireMock('./hooks/useTraderPosition');
223+
(useTraderPosition as jest.Mock).mockReturnValue({
224+
position: undefined,
225+
isLoading: true,
226+
error: null,
227+
});
228+
197229
mockRouteParams.position = undefined;
198-
mockRouteParams.tokenSymbol = 'DOGE';
230+
(mockRouteParams as { positionId?: string }).positionId = 'position-uuid-1';
199231

200232
renderWithProvider(<TraderPositionView />, { state: mockState });
201233

202-
await waitFor(() => {
203-
const lastCall =
204-
mockPriceChart.mock.calls[mockPriceChart.mock.calls.length - 1]?.[0];
205-
expect(lastCall).toMatchObject({ isLoading: false });
234+
expect(
235+
screen.getByTestId(TraderPositionViewSelectorsIDs.SKELETON),
236+
).toBeOnTheScreen();
237+
238+
(useTraderPosition as jest.Mock).mockReturnValue({
239+
position: undefined,
240+
isLoading: false,
241+
error: null,
242+
});
243+
});
244+
245+
it('renders content when position is fetched via positionId', () => {
246+
const { useTraderPosition } = jest.requireMock('./hooks/useTraderPosition');
247+
(useTraderPosition as jest.Mock).mockReturnValue({
248+
position: makeDefaultPosition(),
249+
isLoading: false,
250+
error: null,
251+
});
252+
253+
mockRouteParams.position = undefined;
254+
(mockRouteParams as { positionId?: string }).positionId = 'position-uuid-1';
255+
256+
renderWithProvider(<TraderPositionView />, { state: mockState });
257+
258+
expect(
259+
screen.getByTestId(TraderPositionViewSelectorsIDs.CONTAINER),
260+
).toBeOnTheScreen();
261+
expect(
262+
screen.queryByTestId(TraderPositionViewSelectorsIDs.SKELETON),
263+
).toBeNull();
264+
expect(
265+
screen.queryByTestId(TraderPositionViewSelectorsIDs.FALLBACK),
266+
).toBeNull();
267+
268+
(useTraderPosition as jest.Mock).mockReturnValue({
269+
position: undefined,
270+
isLoading: false,
271+
error: null,
206272
});
207273
});
208274

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,7 @@ export const TraderPositionViewSelectorsIDs = {
22
CONTAINER: 'trader-position-view-container',
33
CLOSE_BUTTON: 'trader-position-view-close-button',
44
BUY_BUTTON: 'trader-position-view-buy-button',
5+
SKELETON: 'trader-position-skeleton',
6+
FALLBACK: 'trader-position-fallback',
7+
FALLBACK_PRIMARY_ACTION: 'trader-position-fallback-primary-action',
58
};

app/components/Views/SocialLeaderboard/TraderPositionView/TraderPositionView.tsx

Lines changed: 97 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,43 @@ import TraderPositionChartSection from './components/TraderPositionChartSection'
2323
import TraderTimePeriodSelector from './components/TraderTimePeriodSelector';
2424
import TraderPositionPnLCard from './components/TraderPositionPnLCard';
2525
import TraderTradesSection from './components/TraderTradesSection';
26+
import TraderPositionSkeleton from './components/TraderPositionSkeleton';
27+
import TraderPositionFallback from './components/TraderPositionFallback';
2628
import { useTraderPositionData } from './useTraderPositionData';
29+
import { useTraderPosition } from './hooks/useTraderPosition';
30+
import { useTraderProfile } from '../TraderProfileView/hooks/useTraderProfile';
2731

2832
const TraderPositionView = () => {
2933
const navigation = useNavigation<NavigationProp<RootStackParamList>>();
3034
const route = useRoute<RouteProp<RootStackParamList, 'TraderPositionView'>>();
3135
const tw = useTailwind();
3236

3337
const {
34-
traderName,
35-
traderImageUrl,
38+
traderId,
39+
traderName: traderNameParam,
40+
traderImageUrl: traderImageUrlParam,
3641
tokenSymbol,
3742
position: positionParam,
43+
positionId,
3844
} = route.params;
3945

4046
const [isQuickBuyVisible, setIsQuickBuyVisible] = useState(false);
4147

48+
// Position resolution: prefer the row-tap snapshot; fetch via UUID only when
49+
// it isn't there (deep link / out-of-app entry).
50+
const { position: fetchedPosition, isLoading: isPositionLoading } =
51+
useTraderPosition(positionParam ? undefined : positionId);
52+
const resolvedPosition = positionParam ?? fetchedPosition;
53+
54+
// Trader profile: fetch only if name/image weren't passed in nav params.
55+
const needsProfile = !traderNameParam || !traderImageUrlParam;
56+
const { profile: fetchedProfile, isLoading: isProfileLoading } =
57+
useTraderProfile(needsProfile ? traderId : '');
58+
const traderName = traderNameParam ?? fetchedProfile?.profile?.name ?? '';
59+
const traderImageUrl =
60+
traderImageUrlParam ?? fetchedProfile?.profile?.imageUrl ?? undefined;
61+
62+
const positionData = useTraderPositionData(resolvedPosition, tokenSymbol);
4263
const {
4364
symbol,
4465
marketCap,
@@ -55,15 +76,17 @@ const TraderPositionView = () => {
5576
activeTimePeriod,
5677
setActiveTimePeriod,
5778
timePeriods,
58-
} = useTraderPositionData(positionParam, tokenSymbol);
79+
} = positionData;
5980

6081
const handleClose = useCallback(() => {
6182
navigation.goBack();
6283
}, [navigation]);
6384

6485
const handleBuyPress = useCallback(() => {
65-
setIsQuickBuyVisible(true);
66-
}, []);
86+
if (resolvedPosition) {
87+
setIsQuickBuyVisible(true);
88+
}
89+
}, [resolvedPosition]);
6790

6891
const handleQuickBuyClose = useCallback(() => {
6992
setIsQuickBuyVisible(false);
@@ -73,6 +96,11 @@ const TraderPositionView = () => {
7396
// Future: update displayed price on scrub
7497
}, []);
7598

99+
const isInitialLoading =
100+
!resolvedPosition && (isPositionLoading || isProfileLoading);
101+
const hasFailed =
102+
!resolvedPosition && !isPositionLoading && !isProfileLoading;
103+
76104
return (
77105
<SafeAreaView
78106
style={tw.style('flex-1 bg-default')}
@@ -84,62 +112,70 @@ const TraderPositionView = () => {
84112
closeButtonTestID={TraderPositionViewSelectorsIDs.CLOSE_BUTTON}
85113
/>
86114

87-
<ScrollView
88-
showsVerticalScrollIndicator={false}
89-
contentContainerStyle={tw.style('pb-6')}
90-
>
91-
<TraderTokenInfoRow
92-
symbol={symbol}
93-
position={positionParam}
94-
marketCap={marketCap}
95-
pricePercentChange={pricePercentChange}
96-
activeTimePeriodLabel={activeTimePeriod}
97-
/>
98-
99-
<TraderPositionChartSection
100-
historicalPrices={historicalPrices}
101-
priceDiff={priceDiff}
102-
isPricesLoading={isPricesLoading}
103-
onChartIndexChange={handleChartIndexChange}
104-
/>
105-
106-
<TraderTimePeriodSelector
107-
timePeriods={timePeriods}
108-
activeTimePeriod={activeTimePeriod}
109-
onSelectPeriod={setActiveTimePeriod}
110-
/>
111-
112-
<TraderPositionPnLCard
113-
isClosed={isClosed}
114-
positionValue={positionValue}
115-
pnlValue={pnlValue}
116-
pnlPercent={pnlPercent}
117-
isPnlPositive={isPnlPositive}
118-
/>
119-
120-
<TraderTradesSection
121-
trades={trades}
122-
traderName={traderName}
123-
traderImageUrl={traderImageUrl}
124-
/>
125-
</ScrollView>
126-
127-
<Box twClassName="px-4 py-3">
128-
<Button
129-
variant={ButtonVariant.Secondary}
130-
isFullWidth
131-
onPress={handleBuyPress}
132-
testID={TraderPositionViewSelectorsIDs.BUY_BUTTON}
133-
>
134-
{strings('social_leaderboard.trader_position.buy')}
135-
</Button>
136-
</Box>
137-
138-
<QuickBuyBottomSheet
139-
isVisible={isQuickBuyVisible}
140-
position={positionParam ?? null}
141-
onClose={handleQuickBuyClose}
142-
/>
115+
{isInitialLoading ? (
116+
<TraderPositionSkeleton />
117+
) : hasFailed ? (
118+
<TraderPositionFallback traderId={traderId} traderName={traderName} />
119+
) : (
120+
<>
121+
<ScrollView
122+
showsVerticalScrollIndicator={false}
123+
contentContainerStyle={tw.style('pb-6')}
124+
>
125+
<TraderTokenInfoRow
126+
symbol={symbol}
127+
position={resolvedPosition}
128+
marketCap={marketCap}
129+
pricePercentChange={pricePercentChange}
130+
activeTimePeriodLabel={activeTimePeriod}
131+
/>
132+
133+
<TraderPositionChartSection
134+
historicalPrices={historicalPrices}
135+
priceDiff={priceDiff}
136+
isPricesLoading={isPricesLoading}
137+
onChartIndexChange={handleChartIndexChange}
138+
/>
139+
140+
<TraderTimePeriodSelector
141+
timePeriods={timePeriods}
142+
activeTimePeriod={activeTimePeriod}
143+
onSelectPeriod={setActiveTimePeriod}
144+
/>
145+
146+
<TraderPositionPnLCard
147+
isClosed={isClosed}
148+
positionValue={positionValue}
149+
pnlValue={pnlValue}
150+
pnlPercent={pnlPercent}
151+
isPnlPositive={isPnlPositive}
152+
/>
153+
154+
<TraderTradesSection
155+
trades={trades}
156+
traderName={traderName}
157+
traderImageUrl={traderImageUrl}
158+
/>
159+
</ScrollView>
160+
161+
<Box twClassName="px-4 py-3">
162+
<Button
163+
variant={ButtonVariant.Secondary}
164+
isFullWidth
165+
onPress={handleBuyPress}
166+
testID={TraderPositionViewSelectorsIDs.BUY_BUTTON}
167+
>
168+
{strings('social_leaderboard.trader_position.buy')}
169+
</Button>
170+
</Box>
171+
172+
<QuickBuyBottomSheet
173+
isVisible={isQuickBuyVisible}
174+
position={resolvedPosition ?? null}
175+
onClose={handleQuickBuyClose}
176+
/>
177+
</>
178+
)}
143179
</SafeAreaView>
144180
);
145181
};

0 commit comments

Comments
 (0)