Skip to content

Commit c6e4302

Browse files
wachuneiwachunei
authored
refactor: replace custom prediction hooks with team React Query hooks (MetaMask#27148)
## **Description** Replaces the custom controller-level implementations in `usePredictPositionsForHomepage` and `usePredictMarketsForHomepage` with thin wrappers around the Predict team's existing React Query-backed hooks (`usePredictPositions`, `usePredictMarketData`). This removes ~300 lines of manual caching (Map + TTL), staleness guards (`requestIdRef`), unmount protection (`isMountedRef`), and request deduplication that React Query handles automatically. Key changes: - `usePredictPositionsForHomepage` now delegates to `usePredictPositions`, accepts an options object `{ maxPositions?, claimable? }`, and returns `totalClaimableValue` - `usePredictMarketsForHomepage` now delegates to `usePredictMarketData` - `PredictionsSection` no longer manually computes `totalClaimable` or calls `refreshClaimable` after claiming (React Query handles cache invalidation) - All tests updated to mock team hooks instead of `Engine.context.PredictController` <details> <summary>Implementation Plan</summary> # Refactor Predictions Homepage Hooks to Use Team Hooks ## Problem `usePredictPositionsForHomepage` directly calls `Engine.context.PredictController.getPositions()` and implements its own module-level caching (Map + TTL), staleness guards (`requestIdRef`), and unmount protection (`isMountedRef`). The Predict team already has `usePredictPositions` which wraps the same controller call with React Query, providing automatic caching (5s stale time), deduplication, error handling, and optimistic polling -- all for free. The same applies to `usePredictMarketsForHomepage` which directly calls `PredictController.getMarkets()` with its own cache, while the team has `usePredictMarketData` with pagination support. ## Current vs Team Hook Comparison ### Positions | Aspect | `usePredictPositionsForHomepage` (current) | `usePredictPositions` (team) | |--------|-------------------------------------------|------------------------------| | Data source | `Engine.context.PredictController.getPositions()` | Same, via React Query `queryFn` | | Caching | Manual Map + 60s TTL | React Query, 5s stale time | | Staleness guard | `requestIdRef` counter | React Query built-in | | Unmount safety | `isMountedRef` | React Query built-in | | Filtering | `maxPositions` slice, `claimable` param | `claimable` select filter, `marketId` filter | | Account tracking | `selectSelectedInternalAccountFormattedAddress` | `getEvmAccountFromSelectedAccountGroup()` | | Return shape | `{ positions, isLoading, error, refresh }` | React Query result (`{ data, isLoading, error, refetch }`) | ### Markets `usePredictMarketsForHomepage` directly calls `PredictController.getMarkets()` with manual caching, while `usePredictMarketData` provides the same with pagination and error handling. ## Changes ### 1. `usePredictPositionsForHomepage` -- delegate to `usePredictPositions` - Change signature to options object `({ maxPositions?, claimable? })` - Remove manual state, caching, refs, and `useEffect` - Add `totalClaimableValue` to return type ### 2. `usePredictMarketsForHomepage` -- delegate to `usePredictMarketData` - Remove module-level cache, manual state, refs - Map `marketData` to `markets`, `isFetching` to `isLoading` ### 3. `PredictionsSection` -- update call sites - Use options object for claimable call - Remove local `totalClaimable` reduce - Remove `refreshClaimable()` from `handleClaim` (React Query handles it) - Remove `refreshClaimable` from `refresh` callback </details> ## **Changelog** CHANGELOG entry: null ## **Related issues** Refs: Feedback from Predict team (Luis) about using their existing hooks ## **Manual testing steps** ```gherkin Feature: Predictions section uses team hooks Scenario: Positions load correctly Given user has prediction positions When user views the homepage Predictions section Then positions are displayed correctly Scenario: Trending markets load correctly Given user has no prediction positions When user views the homepage Predictions section Then trending market cards are displayed in the carousel Scenario: Claim button works Given user has claimable positions When user taps the Claim button Then the claim is processed and positions update automatically ``` ## **Screenshots/Recordings** N/A -- internal refactor with no UI changes. ## **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** > Refactors data-fetching for the homepage Predictions section to rely on shared React Query hooks, which may subtly change caching/refresh behavior and when claimable amounts update. UI logic is mostly unchanged but depends on new hook return shapes (`refetch`, `totalClaimableValue`). > > **Overview** > **Refactors the homepage Predictions section to use the Predict team’s React Query hooks instead of controller calls + manual caching.** `usePredictMarketsForHomepage` now wraps `usePredictMarketData` and `usePredictPositionsForHomepage` wraps `usePredictPositions`, replacing the old `refresh` APIs with `refetch` and adding `totalClaimableValue` for claimable positions. > > `PredictionsSection` is updated to use the new hook signatures, show the claim button based on `totalClaimableValue`, and simplify pull-to-refresh to always `refetch` positions + markets (no separate claimable refresh; claim no longer triggers a manual refresh). > > Tests are updated accordingly, including mocking `usePredictPositions`/`usePredictMarketData` in `Homepage.test.tsx` and adjusting section/hook tests to the new `refetch`/options-object APIs. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fa612f2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0dfec6d commit c6e4302

7 files changed

Lines changed: 285 additions & 851 deletions

‎app/components/Views/Homepage/Homepage.test.tsx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,27 @@ jest.mock('../../UI/Predict/selectors/featureFlags', () => ({
106106
selectPredictEnabledFlag: jest.fn(() => true),
107107
}));
108108

109+
jest.mock('../../UI/Predict/hooks/usePredictPositions', () => ({
110+
usePredictPositions: () => ({
111+
data: [],
112+
isLoading: false,
113+
error: null,
114+
refetch: jest.fn().mockResolvedValue(undefined),
115+
}),
116+
}));
117+
118+
jest.mock('../../UI/Predict/hooks/usePredictMarketData', () => ({
119+
usePredictMarketData: () => ({
120+
marketData: [],
121+
isFetching: false,
122+
isFetchingMore: false,
123+
error: null,
124+
hasMore: false,
125+
refetch: jest.fn().mockResolvedValue(undefined),
126+
fetchMore: jest.fn().mockResolvedValue(undefined),
127+
}),
128+
}));
129+
109130
jest.mock(
110131
'../../../selectors/featureFlagController/assetsDefiPositions',
111132
() => ({

‎app/components/Views/Homepage/Sections/Predictions/PredictionsSection.test.tsx‎

Lines changed: 56 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import Routes from '../../../../../constants/navigation/Routes';
66

77
const mockNavigate = jest.fn();
88
const mockClaim = jest.fn();
9-
const mockRefreshClaimable = jest.fn();
109

1110
jest.mock('@react-navigation/native', () => {
1211
const actualNav = jest.requireActual('@react-navigation/native');
@@ -51,13 +50,13 @@ jest.mock('./hooks', () => ({
5150
markets: [],
5251
isLoading: false,
5352
error: null,
54-
refresh: jest.fn(),
53+
refetch: jest.fn(),
5554
})),
5655
usePredictPositionsForHomepage: jest.fn(() => ({
5756
positions: [],
5857
isLoading: false,
5958
error: null,
60-
refresh: jest.fn(),
59+
refetch: jest.fn(),
6160
})),
6261
}));
6362

@@ -140,7 +139,6 @@ describe('PredictionsSection', () => {
140139
beforeEach(() => {
141140
jest.clearAllMocks();
142141
mockClaim.mockResolvedValue(undefined);
143-
mockRefreshClaimable.mockResolvedValue(undefined);
144142

145143
// Reset mock return value to default (true) to ensure test isolation
146144
jest
@@ -165,16 +163,16 @@ describe('PredictionsSection', () => {
165163
],
166164
isLoading: false,
167165
error: null,
168-
refresh: jest.fn(),
166+
refetch: jest.fn(),
169167
});
170168

171-
// Differentiate active vs claimable positions calls by second arg
172169
mockUsePredictPositionsForHomepage.mockImplementation(
173-
(_maxPositions?: number, claimable = false) => ({
170+
(_options: { maxPositions?: number; claimable?: boolean } = {}) => ({
174171
positions: [],
175172
isLoading: false,
176173
error: null,
177-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
174+
totalClaimableValue: 0,
175+
refetch: jest.fn(),
178176
}),
179177
);
180178
});
@@ -214,11 +212,14 @@ describe('PredictionsSection', () => {
214212
describe('when user has positions', () => {
215213
beforeEach(() => {
216214
mockUsePredictPositionsForHomepage.mockImplementation(
217-
(_maxPositions?: number, claimable = false) => ({
215+
({
216+
claimable = false,
217+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
218218
positions: claimable ? [] : mockActivePositions,
219219
isLoading: false,
220220
error: null,
221-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
221+
totalClaimableValue: 0,
222+
refetch: jest.fn(),
222223
}),
223224
);
224225
});
@@ -236,11 +237,14 @@ describe('PredictionsSection', () => {
236237

237238
it('shows position skeletons when loading positions', () => {
238239
mockUsePredictPositionsForHomepage.mockImplementation(
239-
(_maxPositions?: number, claimable = false) => ({
240+
({
241+
claimable = false,
242+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
240243
positions: [],
241244
isLoading: !claimable, // only active positions loading
242245
error: null,
243-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
246+
totalClaimableValue: 0,
247+
refetch: jest.fn(),
244248
}),
245249
);
246250

@@ -280,7 +284,7 @@ describe('PredictionsSection', () => {
280284
markets: mockMarkets,
281285
isLoading: false,
282286
error: null,
283-
refresh: jest.fn(),
287+
refetch: jest.fn(),
284288
});
285289

286290
renderWithProvider(
@@ -297,7 +301,7 @@ describe('PredictionsSection', () => {
297301
markets: [],
298302
isLoading: true,
299303
error: null,
300-
refresh: jest.fn(),
304+
refetch: jest.fn(),
301305
});
302306

303307
renderWithProvider(
@@ -313,7 +317,7 @@ describe('PredictionsSection', () => {
313317
markets: [],
314318
isLoading: false,
315319
error: null,
316-
refresh: jest.fn(),
320+
refetch: jest.fn(),
317321
});
318322

319323
const { toJSON } = renderWithProvider(
@@ -330,7 +334,7 @@ describe('PredictionsSection', () => {
330334
markets: [],
331335
isLoading: false,
332336
error: 'Network error',
333-
refresh: jest.fn(),
337+
refetch: jest.fn(),
334338
});
335339

336340
renderWithProvider(
@@ -346,7 +350,7 @@ describe('PredictionsSection', () => {
346350
markets: [],
347351
isLoading: true,
348352
error: null,
349-
refresh: jest.fn(),
353+
refetch: jest.fn(),
350354
});
351355

352356
renderWithProvider(
@@ -363,11 +367,14 @@ describe('PredictionsSection', () => {
363367
beforeEach(() => {
364368
// Show positions so the positions branch renders
365369
mockUsePredictPositionsForHomepage.mockImplementation(
366-
(_maxPositions?: number, claimable = false) => ({
370+
({
371+
claimable = false,
372+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
367373
positions: claimable ? [] : mockActivePositions,
368374
isLoading: false,
369375
error: null,
370-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
376+
totalClaimableValue: 0,
377+
refetch: jest.fn(),
371378
}),
372379
);
373380
});
@@ -381,13 +388,15 @@ describe('PredictionsSection', () => {
381388
});
382389

383390
it('shows claim button with total amount when claimable positions exist', async () => {
384-
// totalClaimable = 75 + 125 = 200
385391
mockUsePredictPositionsForHomepage.mockImplementation(
386-
(_maxPositions?: number, claimable = false) => ({
392+
({
393+
claimable = false,
394+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
387395
positions: claimable ? mockClaimablePositions : mockActivePositions,
388396
isLoading: false,
389397
error: null,
390-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
398+
totalClaimableValue: claimable ? 200 : 0,
399+
refetch: jest.fn(),
391400
}),
392401
);
393402

@@ -402,11 +411,14 @@ describe('PredictionsSection', () => {
402411

403412
it('does not show claim button while claimable positions are loading', () => {
404413
mockUsePredictPositionsForHomepage.mockImplementation(
405-
(_maxPositions?: number, claimable = false) => ({
414+
({
415+
claimable = false,
416+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
406417
positions: [],
407418
isLoading: claimable, // claimable fetch still loading
408419
error: null,
409-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
420+
totalClaimableValue: 0,
421+
refetch: jest.fn(),
410422
}),
411423
);
412424

@@ -419,11 +431,14 @@ describe('PredictionsSection', () => {
419431

420432
it('does not show claim button while active positions are loading', () => {
421433
mockUsePredictPositionsForHomepage.mockImplementation(
422-
(_maxPositions?: number, claimable = false) => ({
434+
({
435+
claimable = false,
436+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
423437
positions: [],
424438
isLoading: !claimable, // active fetch still loading
425439
error: null,
426-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
440+
totalClaimableValue: 0,
441+
refetch: jest.fn(),
427442
}),
428443
);
429444

@@ -434,13 +449,16 @@ describe('PredictionsSection', () => {
434449
expect(screen.queryByText(/Claim \$/)).not.toBeOnTheScreen();
435450
});
436451

437-
it('calls claim and then refreshes claimable positions on press', async () => {
452+
it('calls claim on press without manual refresh', async () => {
438453
mockUsePredictPositionsForHomepage.mockImplementation(
439-
(_maxPositions?: number, claimable = false) => ({
454+
({
455+
claimable = false,
456+
}: { maxPositions?: number; claimable?: boolean } = {}) => ({
440457
positions: claimable ? mockClaimablePositions : mockActivePositions,
441458
isLoading: false,
442459
error: null,
443-
refresh: claimable ? mockRefreshClaimable : jest.fn(),
460+
totalClaimableValue: claimable ? 200 : 0,
461+
refetch: jest.fn(),
444462
}),
445463
);
446464

@@ -456,31 +474,29 @@ describe('PredictionsSection', () => {
456474

457475
await waitFor(() => {
458476
expect(mockClaim).toHaveBeenCalledTimes(1);
459-
expect(mockRefreshClaimable).toHaveBeenCalledTimes(1);
460477
});
461478
});
462479
});
463480

464481
describe('refresh functionality', () => {
465-
it('refreshes markets and claimable when user has no positions', async () => {
466-
const mockRefreshActivePositions = jest.fn().mockResolvedValue(undefined);
467-
const mockRefreshMarkets = jest.fn().mockResolvedValue(undefined);
482+
it('refreshes both positions and markets on pull-to-refresh', async () => {
483+
const mockRefetchPositions = jest.fn().mockResolvedValue(undefined);
484+
const mockRefetchMarkets = jest.fn().mockResolvedValue(undefined);
468485

469486
mockUsePredictPositionsForHomepage.mockImplementation(
470-
(_maxPositions?: number, claimable = false) => ({
487+
(_options: { maxPositions?: number; claimable?: boolean } = {}) => ({
471488
positions: [],
472489
isLoading: false,
473490
error: null,
474-
refresh: claimable
475-
? mockRefreshClaimable
476-
: mockRefreshActivePositions,
491+
totalClaimableValue: 0,
492+
refetch: mockRefetchPositions,
477493
}),
478494
);
479495
mockUsePredictMarketsForHomepage.mockReturnValue({
480496
markets: [],
481497
isLoading: false,
482498
error: null,
483-
refresh: mockRefreshMarkets,
499+
refetch: mockRefetchMarkets,
484500
});
485501

486502
const ref = React.createRef<{ refresh: () => Promise<void> }>();
@@ -494,46 +510,8 @@ describe('PredictionsSection', () => {
494510

495511
await ref.current?.refresh();
496512

497-
expect(mockRefreshActivePositions).not.toHaveBeenCalled();
498-
expect(mockRefreshMarkets).toHaveBeenCalled();
499-
expect(mockRefreshClaimable).toHaveBeenCalled();
500-
});
501-
502-
it('refreshes positions, markets, and claimable when user has positions', async () => {
503-
const mockRefreshActivePositions = jest.fn().mockResolvedValue(undefined);
504-
const mockRefreshMarkets = jest.fn().mockResolvedValue(undefined);
505-
506-
mockUsePredictPositionsForHomepage.mockImplementation(
507-
(_maxPositions?: number, claimable = false) => ({
508-
positions: claimable ? [] : mockActivePositions,
509-
isLoading: false,
510-
error: null,
511-
refresh: claimable
512-
? mockRefreshClaimable
513-
: mockRefreshActivePositions,
514-
}),
515-
);
516-
mockUsePredictMarketsForHomepage.mockReturnValue({
517-
markets: [],
518-
isLoading: false,
519-
error: null,
520-
refresh: mockRefreshMarkets,
521-
});
522-
523-
const ref = React.createRef<{ refresh: () => Promise<void> }>();
524-
renderWithProvider(
525-
<PredictionsSection
526-
sectionIndex={0}
527-
totalSectionsLoaded={1}
528-
ref={ref}
529-
/>,
530-
);
531-
532-
await ref.current?.refresh();
533-
534-
expect(mockRefreshActivePositions).toHaveBeenCalled();
535-
expect(mockRefreshMarkets).toHaveBeenCalled();
536-
expect(mockRefreshClaimable).toHaveBeenCalled();
513+
expect(mockRefetchPositions).toHaveBeenCalled();
514+
expect(mockRefetchMarkets).toHaveBeenCalled();
537515
});
538516
});
539517
});

0 commit comments

Comments
 (0)