Skip to content

Commit d54ea81

Browse files
VGR-GITVGR-GIT
authored
feat: debounce points estimation & alt fox icon if points is 0 (#37575)
## **Description** Previous PR related to rewards point estimation for swaps/bridge was not debouncing the estimation API call, which due to react dependency for active quote, resulted in sometimes doing X API calls instead of 1 for the last active quote. This PR fixes that by adding debouncing logic to `useRewards`. It also changes the fox icon color to alternative variant if points retrieved is 0. (just like in mobile) ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/RWDS-613 ## **Manual testing steps** 1. Have an account that is opted into rewards 2. Go to swaps page and open network tab 3. There should only be 1 call made to the estimation endpoint for rewards ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > Debounces rewards points estimation in useRewards, adds alt icon when points = 0, improves auth/opt-in flow and handles SeasonNotFound errors, with comprehensive tests. > > - **Hooks (useRewards)**: > - Add 750ms debounced `estimateRewardsPoints` flow using `lodash.debounce` to prevent duplicate calls. > - Refine opt-in checks, loading/error states, and USD fee price calculation; guard early-return conditions. > - Extensive tests for debounce behavior, error handling, loading, and selector stubs; use fake timers. > - **Controller (RewardsController)**: > - Prefetch bulk `getOptInStatus` during auth trigger; iterate silent auth by sorted accounts. > - Tighten `shouldSkipSilentAuth` to require both `subscriptionId` and a session token. > - On `SeasonNotFoundError`, clear `rewardsSeasons` only; preserve accounts/subscriptions. > - Tests for silent auth without token and season-not-found cache clearing. > - **Data Service (RewardsDataService)**: > - Introduce `SeasonNotFoundError`; map 404 "Season not found" responses in `getSeasonStatus`. > - Tests for season-not-found and message-based detection. > - **UI**: > - `MultichainBridgeQuoteCard`: pass `useAlternativeIconColor` to `RewardsBadge` when estimated points is 0. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 7915f79. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent d788715 commit d54ea81

File tree

7 files changed

+653
-143
lines changed

7 files changed

+653
-143
lines changed

‎app/scripts/controllers/rewards/rewards-controller.test.ts‎

Lines changed: 163 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
InvalidTimestampError,
5454
AccountAlreadyRegisteredError,
5555
AuthorizationFailedError,
56+
SeasonNotFoundError,
5657
} from './rewards-data-service';
5758
import {
5859
RewardsDataServiceEstimatePointsAction,
@@ -2154,6 +2155,9 @@ describe('Additional RewardsController edge cases', () => {
21542155
lastPerpsDiscountRateFetched: null,
21552156
},
21562157
},
2158+
rewardsSubscriptionTokens: {
2159+
[MOCK_SUBSCRIPTION_ID]: MOCK_SESSION_TOKEN,
2160+
},
21572161
};
21582162

21592163
await withController(
@@ -2170,6 +2174,57 @@ describe('Additional RewardsController edge cases', () => {
21702174
);
21712175
});
21722176

2177+
it('should perform silent auth when account is opted-in but no subscription token available', async () => {
2178+
const state: Partial<RewardsControllerState> = {
2179+
rewardsAccounts: {
2180+
[MOCK_CAIP_ACCOUNT]: {
2181+
account: MOCK_CAIP_ACCOUNT,
2182+
hasOptedIn: true,
2183+
subscriptionId: MOCK_SUBSCRIPTION_ID,
2184+
perpsFeeDiscount: null,
2185+
lastPerpsDiscountRateFetched: null,
2186+
},
2187+
},
2188+
};
2189+
2190+
await withController(
2191+
{ state, isDisabled: false },
2192+
async ({ controller, mockMessengerCall }) => {
2193+
mockMessengerCall.mockImplementation((actionType) => {
2194+
if (actionType === 'KeyringController:signPersonalMessage') {
2195+
return Promise.resolve('0xmocksignature');
2196+
}
2197+
if (actionType === 'RewardsDataService:login') {
2198+
return Promise.resolve(MOCK_LOGIN_RESPONSE);
2199+
}
2200+
if (actionType === 'RewardsDataService:getOptInStatus') {
2201+
return Promise.resolve({
2202+
ois: [true],
2203+
sids: [MOCK_SUBSCRIPTION_ID],
2204+
});
2205+
}
2206+
return undefined;
2207+
});
2208+
2209+
const result = await controller.performSilentAuth(
2210+
MOCK_INTERNAL_ACCOUNT,
2211+
true,
2212+
true,
2213+
);
2214+
2215+
expect(result).toBe(MOCK_SUBSCRIPTION_ID);
2216+
expect(controller.state.rewardsActiveAccount).toMatchObject({
2217+
account: MOCK_CAIP_ACCOUNT,
2218+
hasOptedIn: true,
2219+
subscriptionId: MOCK_SUBSCRIPTION_ID,
2220+
});
2221+
expect(
2222+
controller.state.rewardsSubscriptionTokens[MOCK_SUBSCRIPTION_ID],
2223+
).toBe(MOCK_SESSION_TOKEN);
2224+
},
2225+
);
2226+
});
2227+
21732228
it('should create new account state when skipping but no account state exists', async () => {
21742229
await withController({ isDisabled: false }, async ({ controller }) => {
21752230
const hardwareAccount: InternalAccount = {
@@ -2405,6 +2460,104 @@ describe('Additional RewardsController edge cases', () => {
24052460
},
24062461
);
24072462
});
2463+
2464+
it('should handle SeasonNotFoundError and clear seasons', async () => {
2465+
const state: Partial<RewardsControllerState> = {
2466+
rewardsSeasons: {
2467+
[MOCK_SEASON_ID]: {
2468+
id: MOCK_SEASON_ID,
2469+
name: 'Season 1',
2470+
startDate: new Date('2024-01-01').getTime(),
2471+
endDate: new Date('2024-12-31').getTime(),
2472+
tiers: MOCK_SEASON_TIERS,
2473+
},
2474+
},
2475+
rewardsActiveAccount: {
2476+
account: MOCK_CAIP_ACCOUNT,
2477+
hasOptedIn: true,
2478+
subscriptionId: MOCK_SUBSCRIPTION_ID,
2479+
perpsFeeDiscount: null,
2480+
lastPerpsDiscountRateFetched: null,
2481+
},
2482+
rewardsSubscriptionTokens: {
2483+
[MOCK_SUBSCRIPTION_ID]: MOCK_SESSION_TOKEN,
2484+
},
2485+
rewardsAccounts: {
2486+
[MOCK_CAIP_ACCOUNT]: {
2487+
account: MOCK_CAIP_ACCOUNT,
2488+
hasOptedIn: true,
2489+
subscriptionId: MOCK_SUBSCRIPTION_ID,
2490+
perpsFeeDiscount: null,
2491+
lastPerpsDiscountRateFetched: null,
2492+
},
2493+
},
2494+
rewardsSubscriptions: {
2495+
[MOCK_SUBSCRIPTION_ID]: MOCK_SUBSCRIPTION,
2496+
},
2497+
rewardsSeasonStatuses: {
2498+
[`${MOCK_SEASON_ID}:${MOCK_SUBSCRIPTION_ID}`]: {
2499+
season: {
2500+
id: MOCK_SEASON_ID,
2501+
name: 'Season 1',
2502+
startDate: new Date('2024-01-01').getTime(),
2503+
endDate: new Date('2024-12-31').getTime(),
2504+
tiers: MOCK_SEASON_TIERS,
2505+
},
2506+
balance: { total: 100 },
2507+
tier: {
2508+
currentTier: MOCK_SEASON_TIERS[0],
2509+
nextTier: MOCK_SEASON_TIERS[1],
2510+
nextTierPointsNeeded: 50,
2511+
},
2512+
// Set lastFetched to an old timestamp to make cache stale
2513+
// Cache TTL is 1 minute, so use 2 minutes ago to ensure cache miss
2514+
lastFetched: Date.now() - 2 * 60 * 1000,
2515+
},
2516+
},
2517+
};
2518+
2519+
await withController(
2520+
{ state, isDisabled: false },
2521+
async ({ controller, mockMessengerCall }) => {
2522+
mockMessengerCall.mockImplementation((actionType) => {
2523+
if (actionType === 'RewardsDataService:getSeasonStatus') {
2524+
throw new SeasonNotFoundError('Season not found');
2525+
}
2526+
return undefined;
2527+
});
2528+
2529+
await expect(
2530+
controller.getSeasonStatus(MOCK_SUBSCRIPTION_ID, MOCK_SEASON_ID),
2531+
).rejects.toThrow(SeasonNotFoundError);
2532+
2533+
// Verify that rewardsSeasons was cleared
2534+
expect(controller.state.rewardsSeasons).toEqual({});
2535+
2536+
// Verify that accounts and subscriptions were NOT invalidated
2537+
expect(controller.state.rewardsAccounts).toEqual({
2538+
[MOCK_CAIP_ACCOUNT]: {
2539+
account: MOCK_CAIP_ACCOUNT,
2540+
hasOptedIn: true,
2541+
subscriptionId: MOCK_SUBSCRIPTION_ID,
2542+
perpsFeeDiscount: null,
2543+
lastPerpsDiscountRateFetched: null,
2544+
},
2545+
});
2546+
expect(controller.state.rewardsSubscriptions).toEqual({
2547+
[MOCK_SUBSCRIPTION_ID]: MOCK_SUBSCRIPTION,
2548+
});
2549+
expect(controller.state.rewardsSubscriptionTokens).toEqual({
2550+
[MOCK_SUBSCRIPTION_ID]: MOCK_SESSION_TOKEN,
2551+
});
2552+
2553+
// Verify that rewardsActiveAccount was NOT invalidated
2554+
expect(controller.state.rewardsActiveAccount?.hasOptedIn).toBe(true);
2555+
expect(controller.state.rewardsActiveAccount?.subscriptionId).toBe(
2556+
MOCK_SUBSCRIPTION_ID,
2557+
);
2558+
},
2559+
);
2560+
});
24082561
});
24092562

24102563
describe('optIn - edge cases', () => {
@@ -2955,8 +3108,8 @@ describe('Additional RewardsController edge cases', () => {
29553108
rewardsAccounts: {
29563109
[MOCK_CAIP_ACCOUNT]: {
29573110
account: MOCK_CAIP_ACCOUNT,
2958-
hasOptedIn: true,
2959-
subscriptionId: MOCK_SUBSCRIPTION_ID,
3111+
hasOptedIn: false,
3112+
subscriptionId: null,
29603113
perpsFeeDiscount: null,
29613114
lastPerpsDiscountRateFetched: null,
29623115
},
@@ -2976,6 +3129,12 @@ describe('Additional RewardsController edge cases', () => {
29763129
if (actionType === 'KeyringController:signPersonalMessage') {
29773130
return Promise.reject(new Error('All accounts failed'));
29783131
}
3132+
if (actionType === 'RewardsDataService:getOptInStatus') {
3133+
return Promise.resolve({
3134+
ois: [false, false],
3135+
sids: [null, null],
3136+
});
3137+
}
29793138
return undefined;
29803139
});
29813140

@@ -2984,8 +3143,8 @@ describe('Additional RewardsController edge cases', () => {
29843143
// Should set active account to first account since it has account state
29853144
expect(controller.state.rewardsActiveAccount).toMatchObject({
29863145
account: MOCK_CAIP_ACCOUNT,
2987-
hasOptedIn: true,
2988-
subscriptionId: MOCK_SUBSCRIPTION_ID,
3146+
hasOptedIn: undefined, // as we had an error when trying to signPersonalMessage
3147+
subscriptionId: null,
29893148
});
29903149
},
29913150
);

‎app/scripts/controllers/rewards/rewards-controller.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
AccountAlreadyRegisteredError,
3838
AuthorizationFailedError,
3939
InvalidTimestampError,
40+
SeasonNotFoundError,
4041
} from './rewards-data-service';
4142
import { signSolanaRewardsMessage } from './utils/solana-snap';
4243
import { sortAccounts } from './utils/sortAccounts';
@@ -545,6 +546,15 @@ export class RewardsController extends BaseController<
545546
} else {
546547
const sortedAccounts = sortAccounts(accounts);
547548

549+
try {
550+
// Prefer to get opt in status in bulk for sorted accounts.
551+
await this.getOptInStatus({
552+
addresses: sortedAccounts.map((account) => account.address),
553+
});
554+
} catch {
555+
// Failed to get opt in status in bulk for sorted accounts, let silent auth do it individually
556+
}
557+
548558
// Try silent auth on each account until one succeeds
549559
let successAccount: InternalAccount | null = null;
550560
for (const account of sortedAccounts) {
@@ -609,7 +619,12 @@ export class RewardsController extends BaseController<
609619
);
610620
}
611621

612-
return true;
622+
return (
623+
Boolean(accountState.subscriptionId) &&
624+
Boolean(
625+
this.#getSubscriptionToken(accountState.subscriptionId as string),
626+
)
627+
);
613628
}
614629

615630
return false;
@@ -1319,6 +1334,11 @@ export class RewardsController extends BaseController<
13191334
this.invalidateAccountsAndSubscriptions();
13201335
throw error;
13211336
}
1337+
} else if (error instanceof SeasonNotFoundError) {
1338+
this.update((state: RewardsControllerState) => {
1339+
state.rewardsSeasons = {};
1340+
});
1341+
throw error;
13221342
}
13231343
log.error(
13241344
'RewardsController: Failed to get season status:',

‎app/scripts/controllers/rewards/rewards-data-service.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
InvalidTimestampError,
1818
AuthorizationFailedError,
1919
AccountAlreadyRegisteredError,
20+
SeasonNotFoundError,
2021
} from './rewards-data-service';
2122
import type {
2223
LoginResponseDto,
@@ -861,6 +862,46 @@ describe('RewardsDataService', () => {
861862
).rejects.toBeInstanceOf(AuthorizationFailedError);
862863
});
863864

865+
it('should throw SeasonNotFoundError when season is not found', async () => {
866+
const mockResponse = {
867+
ok: false,
868+
status: 404,
869+
json: jest.fn().mockResolvedValue({
870+
message: 'Season not found',
871+
}),
872+
} as unknown as Response;
873+
mockFetch.mockResolvedValue(mockResponse);
874+
875+
let caughtError: unknown;
876+
try {
877+
await service.getSeasonStatus(mockSeasonId, mockSubscriptionId);
878+
} catch (error) {
879+
caughtError = error;
880+
}
881+
882+
expect(caughtError).toBeInstanceOf(SeasonNotFoundError);
883+
const seasonNotFoundError = caughtError as SeasonNotFoundError;
884+
expect(seasonNotFoundError.name).toBe('SeasonNotFoundError');
885+
expect(seasonNotFoundError.message).toBe(
886+
'Season not found. Please try again with a different season.',
887+
);
888+
});
889+
890+
it('should detect season not found when message contains the phrase', async () => {
891+
const mockResponse = {
892+
ok: false,
893+
status: 404,
894+
json: jest.fn().mockResolvedValue({
895+
message: 'The requested Season not found in the system',
896+
}),
897+
} as unknown as Response;
898+
mockFetch.mockResolvedValue(mockResponse);
899+
900+
await expect(
901+
service.getSeasonStatus(mockSeasonId, mockSubscriptionId),
902+
).rejects.toBeInstanceOf(SeasonNotFoundError);
903+
});
904+
864905
it('should throw error when fetch fails', async () => {
865906
const fetchError = new Error('Network error');
866907
mockFetch.mockRejectedValue(fetchError);

‎app/scripts/controllers/rewards/rewards-data-service.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ export class AuthorizationFailedError extends Error {
4646
}
4747
}
4848

49+
/**
50+
* Custom error for season not found
51+
*/
52+
export class SeasonNotFoundError extends Error {
53+
constructor(message: string) {
54+
super(message);
55+
this.name = 'SeasonNotFoundError';
56+
}
57+
}
58+
4959
/**
5060
* Custom error for account already registered (409 conflict)
5161
*/
@@ -397,6 +407,12 @@ export class RewardsDataService {
397407
);
398408
}
399409

410+
if (errorData?.message?.includes('Season not found')) {
411+
throw new SeasonNotFoundError(
412+
'Season not found. Please try again with a different season.',
413+
);
414+
}
415+
400416
throw new Error(`Get season state failed: ${response.status}`);
401417
}
402418

0 commit comments

Comments
 (0)