Skip to content

Commit f53f8d6

Browse files
VGR-GITVGR-GIT
authored
chore: rewards add account require explicit internal account (MetaMask#23044)
## **Description** - Small change in the `AddRewardsAccount`; this component shouldn't deduce an account scope but always work with the one passed in as a prop. - Instead of using `getFirstSubscriptionId`, we are switching to `getCandidateSubscriptionId` just like in the perps implementation & in extension swap flow. All flows were tested locally and still work. ## **Changelog** CHANGELOG entry: null <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Require an explicit InternalAccount for AddRewardsAccount, update rewards hooks to use getCandidateSubscriptionId and expose rewardsAccountScope, and adjust UI/tests to conditionally render rewards UI based on opt-in and scope. > > - **Rewards integration**: > - **Hooks** (`useRewards`, `usePredictRewards`): > - Replace `RewardsController:getFirstSubscriptionId` with `getCandidateSubscriptionId`. > - Return `rewardsAccountScope` and refine `shouldShowRewardsRow` (requires opted-in or available account scope). > - **UI**: > - `AddRewardsAccount`: remove account auto-derivation; now requires explicit `account` prop and hides when absent/successful. > - `Bridge/QuoteDetailsCard`: when rewards row is shown, display `RewardsAnimations` if `accountOptedIn`, otherwise render `AddRewardsAccount` with `rewardsAccountScope`; add error tooltip handling. > - `Predict/PredictFeeSummary`: show rewards row only if `accountOptedIn` or `rewardsAccountScope`; pass `rewardsAccountScope` to `AddRewardsAccount`. > - `PredictBuyPreview`: plumbs `rewardsAccountScope` into `PredictFeeSummary`. > - **Tests**: > - Update tests to mock `useRewards`, expect `rewardsAccountScope`, new rendering conditions, and `getCandidateSubscriptionId` calls. > - Remove Engine-based rewards mocks; add cases for loading/error/zero-points and price impact warning flag. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 2400cb8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 50dbb45 commit f53f8d6

11 files changed

Lines changed: 339 additions & 443 deletions

File tree

‎app/components/UI/Bridge/components/QuoteDetailsCard/QuoteDetailsCard.test.tsx‎

Lines changed: 173 additions & 209 deletions
Large diffs are not rendered by default.

‎app/components/UI/Bridge/components/QuoteDetailsCard/QuoteDetailsCard.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ const QuoteDetailsCard: React.FC = () => {
7171
shouldShowRewardsRow,
7272
hasError: hasRewardsError,
7373
accountOptedIn,
74+
rewardsAccountScope,
7475
} = useRewards({
7576
activeQuote,
7677
isQuoteLoading,
@@ -323,8 +324,13 @@ const QuoteDetailsCard: React.FC = () => {
323324
: RewardAnimationState.Idle
324325
}
325326
/>
327+
) : rewardsAccountScope ? (
328+
<AddRewardsAccount
329+
testID="bridge-add-rewards-account"
330+
account={rewardsAccountScope}
331+
/>
326332
) : (
327-
<AddRewardsAccount testID="bridge-add-rewards-account" />
333+
<></>
328334
)}
329335
</Box>
330336
),

‎app/components/UI/Bridge/hooks/useRewards/useRewards.test.ts‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ const mockActiveQuote = {
171171

172172
describe('useRewards', () => {
173173
const mockCall = Engine.controllerMessenger.call as jest.Mock;
174+
const mockSubscribe = Engine.controllerMessenger.subscribe as jest.Mock;
175+
const mockUnsubscribe = Engine.controllerMessenger.unsubscribe as jest.Mock;
174176

175177
const defaultSourceToken = {
176178
address: '0x0000000000000000000000000000000000000000' as Hex,
@@ -190,6 +192,9 @@ describe('useRewards', () => {
190192

191193
beforeEach(() => {
192194
jest.clearAllMocks();
195+
// Reset subscription handler storage
196+
mockSubscribe.mockClear();
197+
mockUnsubscribe.mockClear();
193198
});
194199

195200
describe('when rewards feature is disabled', () => {
@@ -225,6 +230,7 @@ describe('useRewards', () => {
225230
estimatedPoints: null,
226231
hasError: false,
227232
accountOptedIn: null,
233+
rewardsAccountScope: expect.any(Object),
228234
});
229235
});
230236

@@ -240,7 +246,7 @@ describe('useRewards', () => {
240246
if (method === 'RewardsController:isRewardsFeatureEnabled') {
241247
return Promise.resolve(true);
242248
}
243-
if (method === 'RewardsController:getFirstSubscriptionId') {
249+
if (method === 'RewardsController:getCandidateSubscriptionId') {
244250
return Promise.resolve('subscription-id-1');
245251
}
246252
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -276,11 +282,12 @@ describe('useRewards', () => {
276282
estimatedPoints: null,
277283
hasError: false,
278284
accountOptedIn: false,
285+
rewardsAccountScope: expect.any(Object),
279286
});
280287
});
281288

282289
expect(mockCall).toHaveBeenCalledWith(
283-
'RewardsController:getFirstSubscriptionId',
290+
'RewardsController:getCandidateSubscriptionId',
284291
);
285292
expect(mockCall).toHaveBeenCalledWith(
286293
'RewardsController:getHasAccountOptedIn',
@@ -297,7 +304,7 @@ describe('useRewards', () => {
297304
if (method === 'RewardsController:isRewardsFeatureEnabled') {
298305
return Promise.resolve(true);
299306
}
300-
if (method === 'RewardsController:getFirstSubscriptionId') {
307+
if (method === 'RewardsController:getCandidateSubscriptionId') {
301308
return Promise.resolve('subscription-id-1');
302309
}
303310
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -333,11 +340,12 @@ describe('useRewards', () => {
333340
estimatedPoints: null,
334341
hasError: false,
335342
accountOptedIn: false,
343+
rewardsAccountScope: expect.any(Object),
336344
});
337345
});
338346

339347
expect(mockCall).toHaveBeenCalledWith(
340-
'RewardsController:getFirstSubscriptionId',
348+
'RewardsController:getCandidateSubscriptionId',
341349
);
342350
expect(mockCall).toHaveBeenCalledWith(
343351
'RewardsController:getHasAccountOptedIn',
@@ -356,7 +364,7 @@ describe('useRewards', () => {
356364
if (method === 'RewardsController:isRewardsFeatureEnabled') {
357365
return Promise.resolve(true);
358366
}
359-
if (method === 'RewardsController:getFirstSubscriptionId') {
367+
if (method === 'RewardsController:getCandidateSubscriptionId') {
360368
return Promise.resolve('subscription-id-1');
361369
}
362370
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -392,6 +400,7 @@ describe('useRewards', () => {
392400
estimatedPoints: 100,
393401
hasError: false,
394402
accountOptedIn: true,
403+
rewardsAccountScope: expect.any(Object),
395404
});
396405
});
397406

@@ -426,7 +435,7 @@ describe('useRewards', () => {
426435
if (method === 'RewardsController:isRewardsFeatureEnabled') {
427436
return Promise.resolve(true);
428437
}
429-
if (method === 'RewardsController:getFirstSubscriptionId') {
438+
if (method === 'RewardsController:getCandidateSubscriptionId') {
430439
return Promise.resolve('subscription-id-1');
431440
}
432441
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -499,6 +508,7 @@ describe('useRewards', () => {
499508
estimatedPoints: null,
500509
hasError: false,
501510
accountOptedIn: null,
511+
rewardsAccountScope: expect.any(Object),
502512
});
503513

504514
// Should not call Engine methods
@@ -529,6 +539,7 @@ describe('useRewards', () => {
529539
estimatedPoints: null,
530540
hasError: false,
531541
accountOptedIn: null,
542+
rewardsAccountScope: null,
532543
});
533544
});
534545

@@ -556,6 +567,7 @@ describe('useRewards', () => {
556567
estimatedPoints: null,
557568
hasError: false,
558569
accountOptedIn: null,
570+
rewardsAccountScope: expect.any(Object),
559571
});
560572
});
561573

@@ -583,6 +595,7 @@ describe('useRewards', () => {
583595
estimatedPoints: null,
584596
hasError: false,
585597
accountOptedIn: null,
598+
rewardsAccountScope: expect.any(Object),
586599
});
587600
});
588601
});
@@ -593,7 +606,7 @@ describe('useRewards', () => {
593606
if (method === 'RewardsController:isRewardsFeatureEnabled') {
594607
return Promise.resolve(true);
595608
}
596-
if (method === 'RewardsController:getFirstSubscriptionId') {
609+
if (method === 'RewardsController:getCandidateSubscriptionId') {
597610
return Promise.resolve(null);
598611
}
599612
return Promise.resolve(null);
@@ -623,14 +636,15 @@ describe('useRewards', () => {
623636
estimatedPoints: null,
624637
hasError: false,
625638
accountOptedIn: null,
639+
rewardsAccountScope: expect.any(Object),
626640
});
627641
});
628642

629643
expect(mockCall).toHaveBeenCalledWith(
630644
'RewardsController:isRewardsFeatureEnabled',
631645
);
632646
expect(mockCall).toHaveBeenCalledWith(
633-
'RewardsController:getFirstSubscriptionId',
647+
'RewardsController:getCandidateSubscriptionId',
634648
);
635649
expect(mockCall).not.toHaveBeenCalledWith(
636650
'RewardsController:getHasAccountOptedIn',
@@ -645,7 +659,7 @@ describe('useRewards', () => {
645659
if (method === 'RewardsController:isRewardsFeatureEnabled') {
646660
return Promise.resolve(true);
647661
}
648-
if (method === 'RewardsController:getFirstSubscriptionId') {
662+
if (method === 'RewardsController:getCandidateSubscriptionId') {
649663
return Promise.resolve('subscription-id-1');
650664
}
651665
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -681,6 +695,7 @@ describe('useRewards', () => {
681695
estimatedPoints: null,
682696
hasError: true,
683697
accountOptedIn: true,
698+
rewardsAccountScope: expect.any(Object),
684699
});
685700
});
686701
});
@@ -717,6 +732,7 @@ describe('useRewards', () => {
717732
estimatedPoints: null,
718733
hasError: true,
719734
accountOptedIn: null,
735+
rewardsAccountScope: expect.any(Object),
720736
});
721737
});
722738
});
@@ -726,7 +742,7 @@ describe('useRewards', () => {
726742
if (method === 'RewardsController:isRewardsFeatureEnabled') {
727743
return Promise.resolve(true);
728744
}
729-
if (method === 'RewardsController:getFirstSubscriptionId') {
745+
if (method === 'RewardsController:getCandidateSubscriptionId') {
730746
return Promise.resolve('subscription-id-1');
731747
}
732748
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -759,6 +775,7 @@ describe('useRewards', () => {
759775
estimatedPoints: null,
760776
hasError: true,
761777
accountOptedIn: null,
778+
rewardsAccountScope: expect.any(Object),
762779
});
763780
});
764781
});
@@ -769,7 +786,7 @@ describe('useRewards', () => {
769786
if (method === 'RewardsController:isRewardsFeatureEnabled') {
770787
return Promise.resolve(true);
771788
}
772-
if (method === 'RewardsController:getFirstSubscriptionId') {
789+
if (method === 'RewardsController:getCandidateSubscriptionId') {
773790
return Promise.resolve('subscription-id-1');
774791
}
775792
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -808,7 +825,7 @@ describe('useRewards', () => {
808825
if (method === 'RewardsController:isRewardsFeatureEnabled') {
809826
return Promise.resolve(true);
810827
}
811-
if (method === 'RewardsController:getFirstSubscriptionId') {
828+
if (method === 'RewardsController:getCandidateSubscriptionId') {
812829
return Promise.resolve('subscription-id-1');
813830
}
814831
if (method === 'RewardsController:getHasAccountOptedIn') {
@@ -840,6 +857,7 @@ describe('useRewards', () => {
840857
estimatedPoints: 100,
841858
hasError: false,
842859
accountOptedIn: true,
860+
rewardsAccountScope: expect.any(Object),
843861
});
844862
});
845863
});

‎app/components/UI/Bridge/hooks/useRewards/useRewards.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
import { useBridgeQuoteData } from '../useBridgeQuoteData';
2424
import Logger from '../../../../../util/Logger';
2525
import usePrevious from '../../../../hooks/usePrevious';
26+
import { InternalAccount } from '@metamask/keyring-internal-api';
2627

2728
/**
2829
*
@@ -69,6 +70,7 @@ interface UseRewardsResult {
6970
estimatedPoints: number | null;
7071
hasError: boolean;
7172
accountOptedIn: boolean | null;
73+
rewardsAccountScope: InternalAccount | null;
7274
}
7375

7476
/**
@@ -152,11 +154,11 @@ export const useRewards = ({
152154
}
153155

154156
// Check if there's a subscription first
155-
const firstSubscriptionId = await Engine.controllerMessenger.call(
156-
'RewardsController:getFirstSubscriptionId',
157+
const candidateSubscriptionId = await Engine.controllerMessenger.call(
158+
'RewardsController:getCandidateSubscriptionId',
157159
);
158160

159-
if (!firstSubscriptionId) {
161+
if (!candidateSubscriptionId) {
160162
setEstimatedPoints(null);
161163
setShouldShowRewardsRow(false);
162164
setAccountOptedIn(null);
@@ -312,10 +314,12 @@ export const useRewards = ({
312314
}, [estimatePoints]);
313315

314316
return {
315-
shouldShowRewardsRow,
317+
shouldShowRewardsRow:
318+
shouldShowRewardsRow && (accountOptedIn || Boolean(selectedAccount)),
316319
isLoading: isLoading || isQuoteLoading,
317320
estimatedPoints,
318321
hasError,
319322
accountOptedIn,
323+
rewardsAccountScope: selectedAccount ?? null,
320324
};
321325
};

‎app/components/UI/Predict/components/PredictFeeSummary/PredictFeeSummary.test.tsx‎

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React from 'react';
22
import { render, fireEvent } from '@testing-library/react-native';
33
import PredictFeeSummary from './PredictFeeSummary';
4+
import { InternalAccount } from '@metamask/keyring-internal-api';
45

56
jest.mock('../../utils/format', () => ({
67
formatPrice: jest.fn((value, options) =>
@@ -271,11 +272,21 @@ describe('PredictFeeSummary', () => {
271272
expect(getByText('0 points')).toBeOnTheScreen();
272273
});
273274

274-
it('displays AddRewardsAccount when accountOptedIn is false', () => {
275+
it('displays AddRewardsAccount when rewardsAccountScope is provided and accountOptedIn is false', () => {
276+
const mockRewardsAccountScope = {
277+
id: 'account-1',
278+
address: '0x1234567890123456789012345678901234567890',
279+
name: 'Test Account',
280+
type: 'eip155:eoa',
281+
scopes: [],
282+
metadata: {},
283+
};
275284
const props = {
276285
...defaultProps,
277286
shouldShowRewardsRow: true,
278287
accountOptedIn: false,
288+
rewardsAccountScope:
289+
mockRewardsAccountScope as unknown as InternalAccount,
279290
estimatedPoints: 100,
280291
};
281292

@@ -287,11 +298,21 @@ describe('PredictFeeSummary', () => {
287298
expect(queryByTestId('rewards-animation')).toBeNull();
288299
});
289300

290-
it('displays AddRewardsAccount when accountOptedIn is null', () => {
301+
it('displays AddRewardsAccount when rewardsAccountScope is provided and accountOptedIn is null', () => {
302+
const mockRewardsAccountScope = {
303+
id: 'account-1',
304+
address: '0x1234567890123456789012345678901234567890',
305+
name: 'Test Account',
306+
type: 'eip155:eoa',
307+
scopes: [],
308+
metadata: {},
309+
};
291310
const props = {
292311
...defaultProps,
293312
shouldShowRewardsRow: true,
294313
accountOptedIn: null,
314+
rewardsAccountScope:
315+
mockRewardsAccountScope as unknown as InternalAccount,
295316
estimatedPoints: 100,
296317
};
297318

@@ -303,6 +324,50 @@ describe('PredictFeeSummary', () => {
303324
expect(queryByTestId('rewards-animation')).toBeNull();
304325
});
305326

327+
it('does not display rewards row when both accountOptedIn and rewardsAccountScope are null/false', () => {
328+
const props = {
329+
...defaultProps,
330+
shouldShowRewardsRow: true,
331+
accountOptedIn: false,
332+
rewardsAccountScope: null,
333+
estimatedPoints: 100,
334+
};
335+
336+
const { queryByText, queryByTestId } = render(
337+
<PredictFeeSummary {...props} />,
338+
);
339+
340+
expect(queryByText('Est. points')).toBeNull();
341+
expect(queryByTestId('rewards-animation')).toBeNull();
342+
expect(queryByTestId('add-rewards-account')).toBeNull();
343+
});
344+
345+
it('displays RewardsAnimations when accountOptedIn is true even if rewardsAccountScope is provided', () => {
346+
const mockRewardsAccountScope = {
347+
id: 'account-1',
348+
address: '0x1234567890123456789012345678901234567890',
349+
name: 'Test Account',
350+
type: 'eip155:eoa',
351+
scopes: [],
352+
metadata: {},
353+
};
354+
const props = {
355+
...defaultProps,
356+
shouldShowRewardsRow: true,
357+
accountOptedIn: true,
358+
rewardsAccountScope:
359+
mockRewardsAccountScope as unknown as InternalAccount,
360+
estimatedPoints: 50,
361+
};
362+
363+
const { getByTestId, queryByTestId } = render(
364+
<PredictFeeSummary {...props} />,
365+
);
366+
367+
expect(getByTestId('rewards-animation')).toBeOnTheScreen();
368+
expect(queryByTestId('add-rewards-account')).toBeNull();
369+
});
370+
306371
it('displays loading state when isLoadingRewards is true', () => {
307372
const props = {
308373
...defaultProps,

0 commit comments

Comments
 (0)