Skip to content

Commit 28b7e89

Browse files
runway-github[bot]sophieqgu
runway-github[bot]
and
sophieqgu
authored
chore(runway): cherry-pick fix(Rewards): error when visiting rewards tab (#29875)
- fix(Rewards): error when visiting rewards tab (#29823) <!-- 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** https://consensyssoftware.atlassian.net/browse/RWDS-1267 The Rewards crash likely came from an older persisted Rewards Redux state that did not have newer array fields, especially campaigns. On affected devices, selectCampaigns could return undefined. It was account/device dependent because persisted state differs per user/install. Fix Applied: hardened both the restore path and selector path to fallback to initial state (empty array or object instead of undefined) <!-- 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? --> ## **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: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/RWDS-1267 ## **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. --> - [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. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] 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 - [x] 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`. --> - [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] > **Low Risk** > Low risk: changes are defensive fallbacks for missing/undefined persisted state and selector inputs, with added tests; behavior should only differ for legacy/partial states. > > **Overview** > Prevents Rewards tab crashes caused by older/partial persisted Redux/controller state by **defaulting missing fields to safe empty values**. > > Selectors and rehydrate logic now coalesce `undefined` arrays/objects (e.g., `campaigns`, season arrays, `benefits`, dismissed-toasts map, controller `accounts`/`subscriptions`) to empty defaults, and `RewardsController` default state was extracted into `defaultState.ts` and reused. Test coverage was expanded to lock in these upgrade/undefined-state behaviors. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d7b351d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [8208502](8208502) Co-authored-by: sophieqgu <37032128+sophieqgu@users.noreply.github.com>
1 parent a06a359 commit 28b7e89

9 files changed

Lines changed: 329 additions & 54 deletions

File tree

‎app/components/UI/Rewards/hooks/useOndoOutcomeToast.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,13 @@ function setupDefaults({
138138
subscriptionId = SUBSCRIPTION_ID,
139139
outcome = null,
140140
}: {
141-
campaigns?: ReturnType<typeof makeCompletedOndoCampaign>[];
141+
campaigns?: ReturnType<typeof makeCompletedOndoCampaign>[] | undefined;
142142
dismissed?: Record<string, boolean>;
143143
subscriptionId?: string | null;
144144
outcome?: OndoGmCampaignParticipantOutcomeDto | null;
145145
} = {}) {
146146
mockUseSelector.mockImplementation((selector) => {
147-
if (selector === selectCampaigns) return campaigns;
147+
if (selector === selectCampaigns) return campaigns ?? [];
148148
if (selector === selectDismissedCampaignOutcomeToasts) return dismissed;
149149
if (selector === selectRewardsSubscriptionId) return subscriptionId;
150150
return undefined;
@@ -179,6 +179,15 @@ describe('useOndoOutcomeToast', () => {
179179
);
180180
});
181181

182+
it('passes undefined to useOndoCampaignParticipantOutcome when campaigns are missing from persisted state', () => {
183+
setupDefaults({ campaigns: undefined });
184+
renderHook(() => useOndoOutcomeToast());
185+
expect(mockUseOndoCampaignParticipantOutcome).toHaveBeenCalledWith(
186+
undefined,
187+
);
188+
expect(mockShowToast).not.toHaveBeenCalled();
189+
});
190+
182191
it('passes completed ONDO campaign id to useOndoCampaignParticipantOutcome', () => {
183192
const campaign = makeCompletedOndoCampaign();
184193
setupDefaults({ campaigns: [campaign] });

‎app/core/Engine/controllers/rewards-controller/RewardsController.ts‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ import {
4848
BASE32_REGEX,
4949
CampaignType,
5050
} from './types';
51+
import {
52+
defaultRewardsControllerState,
53+
getRewardsControllerDefaultState,
54+
} from './defaultState';
5155
import type { RewardsControllerMessenger } from '../../messengers/rewards-controller-messenger';
5256
import {
5357
storeSubscriptionToken,
@@ -273,33 +277,7 @@ const metadata: StateMetadata<RewardsControllerState> = {
273277
},
274278
};
275279

276-
/**
277-
* Get the default state for the RewardsController
278-
*/
279-
export const getRewardsControllerDefaultState = (): RewardsControllerState => ({
280-
activeAccount: null,
281-
accounts: {},
282-
subscriptions: {},
283-
subscriptionBenefits: {},
284-
seasons: {},
285-
subscriptionReferralDetails: {},
286-
seasonStatuses: {},
287-
activeBoosts: {},
288-
unlockedRewards: {},
289-
pointsEvents: {},
290-
offDeviceSubscriptionAccounts: {},
291-
campaigns: {},
292-
campaignParticipantStatus: {},
293-
ondoCampaignLeaderboard: {},
294-
ondoCampaignLeaderboardPositions: {},
295-
ondoCampaignPortfolio: {},
296-
ondoCampaignActivity: {},
297-
ondoCampaignDeposits: {},
298-
pointsEstimateHistory: [],
299-
rewardsEnvUrl: null,
300-
});
301-
302-
export const defaultRewardsControllerState = getRewardsControllerDefaultState();
280+
export { defaultRewardsControllerState, getRewardsControllerDefaultState };
303281

304282
type CacheReader<T> = (
305283
key: string,

‎app/core/Engine/controllers/rewards-controller/defaultState.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import type { RewardsControllerState } from './types';
2+
3+
/**
4+
* Get the default state for the RewardsController.
5+
*/
6+
export const getRewardsControllerDefaultState = (): RewardsControllerState => ({
7+
activeAccount: null,
8+
accounts: {},
9+
subscriptions: {},
10+
subscriptionBenefits: {},
11+
seasons: {},
12+
subscriptionReferralDetails: {},
13+
seasonStatuses: {},
14+
activeBoosts: {},
15+
unlockedRewards: {},
16+
pointsEvents: {},
17+
offDeviceSubscriptionAccounts: {},
18+
campaigns: {},
19+
campaignParticipantStatus: {},
20+
ondoCampaignLeaderboard: {},
21+
ondoCampaignLeaderboardPositions: {},
22+
ondoCampaignPortfolio: {},
23+
ondoCampaignActivity: {},
24+
ondoCampaignDeposits: {},
25+
pointsEstimateHistory: [],
26+
rewardsEnvUrl: null,
27+
});
28+
29+
export const defaultRewardsControllerState = getRewardsControllerDefaultState();

‎app/reducers/rewards/index.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2305,6 +2305,27 @@ describe('rewardsReducer', () => {
23052305
);
23062306
});
23072307

2308+
it('should default persisted season arrays to empty arrays when absent', () => {
2309+
const persistedRewardsStateWithoutFields = {
2310+
...initialState,
2311+
seasonTiers: undefined,
2312+
seasonActivityTypes: undefined,
2313+
seasonWaysToEarn: undefined,
2314+
} as unknown as RewardsState;
2315+
const rehydrateAction = {
2316+
type: 'persist/REHYDRATE',
2317+
payload: {
2318+
rewards: persistedRewardsStateWithoutFields,
2319+
},
2320+
};
2321+
2322+
const state = rewardsReducer(initialState, rehydrateAction);
2323+
2324+
expect(state.seasonTiers).toEqual([]);
2325+
expect(state.seasonActivityTypes).toEqual([]);
2326+
expect(state.seasonWaysToEarn).toEqual([]);
2327+
});
2328+
23082329
it('should preserve all persisted UI state fields', () => {
23092330
// Arrange
23102331
const persistedRewardsState: RewardsState = {
@@ -2658,6 +2679,21 @@ describe('rewardsReducer', () => {
26582679

26592680
expect(state.ondoCampaignActivity).toEqual({});
26602681
});
2682+
2683+
it('should default campaigns to [] when absent from persisted state (upgrade path)', () => {
2684+
const persistedRewardsStateWithoutField = {
2685+
...initialState,
2686+
campaigns: undefined,
2687+
} as unknown as RewardsState;
2688+
const rehydrateAction = {
2689+
type: 'persist/REHYDRATE',
2690+
payload: { rewards: persistedRewardsStateWithoutField },
2691+
};
2692+
2693+
const state = rewardsReducer(initialState, rehydrateAction);
2694+
2695+
expect(state.campaigns).toEqual([]);
2696+
});
26612697
});
26622698

26632699
describe('unknown actions', () => {
@@ -4600,6 +4636,17 @@ describe('setBenefits', () => {
46004636
expect(state.benefits).toEqual(mockBenefitsPayload.benefits);
46014637
});
46024638

4639+
it('sets benefits to empty array when payload benefits are missing', () => {
4640+
const action = setBenefits({
4641+
...mockBenefitsPayload,
4642+
benefits: undefined,
4643+
} as unknown as typeof mockBenefitsPayload);
4644+
4645+
const state = rewardsReducer(initialState, action);
4646+
4647+
expect(state.benefits).toEqual([]);
4648+
});
4649+
46034650
it('replaces existing benefits with new payload benefits', () => {
46044651
const stateWithBenefits: RewardsState = {
46054652
...initialState,

‎app/reducers/rewards/index.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,7 @@ const rewardsSlice = createSlice({
659659
},
660660

661661
setBenefits: (state, action: PayloadAction<SubscriptionBenefitsState>) => {
662-
state.benefits = action.payload.benefits;
662+
state.benefits = action.payload.benefits ?? [];
663663
},
664664

665665
setBenefitsLoading: (state, action: PayloadAction<boolean>) => {
@@ -803,9 +803,10 @@ const rewardsSlice = createSlice({
803803
seasonName: action.payload.rewards.seasonName,
804804
seasonStartDate: action.payload.rewards.seasonStartDate,
805805
seasonEndDate: action.payload.rewards.seasonEndDate,
806-
seasonTiers: action.payload.rewards.seasonTiers,
807-
seasonActivityTypes: action.payload.rewards.seasonActivityTypes,
808-
seasonWaysToEarn: action.payload.rewards.seasonWaysToEarn,
806+
seasonTiers: action.payload.rewards.seasonTiers ?? [],
807+
seasonActivityTypes:
808+
action.payload.rewards.seasonActivityTypes ?? [],
809+
seasonWaysToEarn: action.payload.rewards.seasonWaysToEarn ?? [],
809810
referralCode: action.payload.rewards.referralCode,
810811
refereeCount: action.payload.rewards.refereeCount,
811812
currentTier: action.payload.rewards.currentTier,
@@ -818,7 +819,7 @@ const rewardsSlice = createSlice({
818819
activeBoosts: action.payload.rewards.activeBoosts,
819820
pointsEvents: action.payload.rewards.pointsEvents,
820821
unlockedRewards: action.payload.rewards.unlockedRewards,
821-
campaigns: action.payload.rewards.campaigns,
822+
campaigns: action.payload.rewards.campaigns ?? [],
822823
campaignParticipantStatuses:
823824
action.payload.rewards.campaignParticipantStatuses ?? {},
824825
ondoCampaignLeaderboardPositions:

‎app/reducers/rewards/selectors.test.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
selectBulkLinkFailedAccounts,
4747
selectBulkLinkWasInterrupted,
4848
selectBulkLinkAccountProgress,
49+
selectBenefits,
4950
selectCampaigns,
5051
selectCampaignsLoading,
5152
selectCampaignsError,
@@ -84,6 +85,7 @@ import {
8485
SeasonWayToEarnDto,
8586
PointsEventDto,
8687
OndoGmActivityEntryDto,
88+
SubscriptionBenefitDto,
8789
} from '../../core/Engine/controllers/rewards-controller/types';
8890
import { RootState } from '..';
8991
import { RewardsState, AccountOptInBannerInfoStatus } from '.';
@@ -549,6 +551,16 @@ describe('Rewards selectors', () => {
549551
expect(result.current).toEqual([]);
550552
});
551553

554+
it('returns empty array when season tiers are undefined', () => {
555+
const mockState = {
556+
rewards: { seasonTiers: undefined },
557+
} as unknown as RootState;
558+
mockedUseSelector.mockImplementation((selector) => selector(mockState));
559+
560+
const { result } = renderHook(() => useSelector(selectSeasonTiers));
561+
expect(result.current).toEqual([]);
562+
});
563+
552564
it('returns season tiers when set', () => {
553565
const mockTiers: SeasonTierDto[] = [
554566
{
@@ -604,6 +616,18 @@ describe('Rewards selectors', () => {
604616
expect(result.current).toEqual([]);
605617
});
606618

619+
it('returns empty array when season activity types are undefined', () => {
620+
const mockState = {
621+
rewards: { seasonActivityTypes: undefined },
622+
} as unknown as RootState;
623+
mockedUseSelector.mockImplementation((selector) => selector(mockState));
624+
625+
const { result } = renderHook(() =>
626+
useSelector(selectSeasonActivityTypes),
627+
);
628+
expect(result.current).toEqual([]);
629+
});
630+
607631
it('returns season activity types when set', () => {
608632
const mockActivityTypes: SeasonActivityTypeDto[] = [
609633
{
@@ -638,6 +662,16 @@ describe('Rewards selectors', () => {
638662
expect(result.current).toEqual([]);
639663
});
640664

665+
it('returns empty array when season ways to earn are undefined', () => {
666+
const mockState = {
667+
rewards: { seasonWaysToEarn: undefined },
668+
} as unknown as RootState;
669+
mockedUseSelector.mockImplementation((selector) => selector(mockState));
670+
671+
const { result } = renderHook(() => useSelector(selectSeasonWaysToEarn));
672+
expect(result.current).toEqual([]);
673+
});
674+
641675
it('returns season ways to earn when set', () => {
642676
const mockWaysToEarn: SeasonWayToEarnDto[] = [
643677
{
@@ -1032,6 +1066,18 @@ describe('Rewards selectors', () => {
10321066
expect(result.current).toHaveLength(0);
10331067
});
10341068

1069+
it('returns empty array when account banner config is undefined', () => {
1070+
const mockState = {
1071+
rewards: { hideCurrentAccountNotOptedInBanner: undefined },
1072+
} as unknown as RootState;
1073+
mockedUseSelector.mockImplementation((selector) => selector(mockState));
1074+
1075+
const { result } = renderHook(() =>
1076+
useSelector(selectHideCurrentAccountNotOptedInBannerArray),
1077+
);
1078+
expect(result.current).toEqual([]);
1079+
});
1080+
10351081
it('returns single account configuration when set', () => {
10361082
const mockAccountConfig: AccountOptInBannerInfoStatus = {
10371083
accountGroupId: 'keyring:wallet1/1',
@@ -3119,6 +3165,37 @@ describe('Rewards selectors', () => {
31193165
showUpcomingDate: false,
31203166
};
31213167

3168+
describe('selectBenefits', () => {
3169+
const mockBenefit: SubscriptionBenefitDto = {
3170+
id: 101,
3171+
longTitle: 'Premium Access',
3172+
shortDescription: 'Get premium perks',
3173+
longDescription: 'Unlock premium partner benefits.',
3174+
thumbnail: 'https://example.com/benefits/premium.png',
3175+
validFrom: '2026-01-01T00:00:00.000Z',
3176+
validTo: '2026-12-31T00:00:00.000Z',
3177+
actionDate: '2026-06-01T00:00:00.000Z',
3178+
url: 'https://example.com/claim',
3179+
chain: 'ethereum',
3180+
type: {
3181+
id: 1,
3182+
name: 'Partner',
3183+
},
3184+
};
3185+
3186+
it('returns empty array when benefits are undefined', () => {
3187+
const state = createMockRootState({
3188+
benefits: undefined as unknown as SubscriptionBenefitDto[],
3189+
});
3190+
expect(selectBenefits(state)).toEqual([]);
3191+
});
3192+
3193+
it('returns benefits when they exist', () => {
3194+
const state = createMockRootState({ benefits: [mockBenefit] });
3195+
expect(selectBenefits(state)).toEqual([mockBenefit]);
3196+
});
3197+
});
3198+
31223199
describe('selectCampaigns', () => {
31233200
it('returns empty array when campaigns is empty', () => {
31243201
const mockState = { rewards: { campaigns: [] } };
@@ -3128,6 +3205,16 @@ describe('Rewards selectors', () => {
31283205
expect(result.current).toEqual([]);
31293206
});
31303207

3208+
it('returns empty array when campaigns is undefined', () => {
3209+
const mockState = {
3210+
rewards: { campaigns: undefined },
3211+
} as unknown as RootState;
3212+
mockedUseSelector.mockImplementation((selector) => selector(mockState));
3213+
3214+
const { result } = renderHook(() => useSelector(selectCampaigns));
3215+
expect(result.current).toEqual([]);
3216+
});
3217+
31313218
it('returns campaigns array when campaigns exist', () => {
31323219
const mockState = { rewards: { campaigns: [mockCampaign] } };
31333220
mockedUseSelector.mockImplementation((selector) => selector(mockState));
@@ -3142,6 +3229,13 @@ describe('Rewards selectors', () => {
31423229
expect(selectCampaigns(state)).toEqual([]);
31433230
});
31443231

3232+
it('returns empty array when campaigns is undefined', () => {
3233+
const state = createMockRootState({
3234+
campaigns: undefined as unknown as CampaignDto[],
3235+
});
3236+
expect(selectCampaigns(state)).toEqual([]);
3237+
});
3238+
31453239
it('returns campaigns when they exist', () => {
31463240
const state = createMockRootState({ campaigns: [mockCampaign] });
31473241
expect(selectCampaigns(state)).toEqual([mockCampaign]);
@@ -3827,6 +3921,16 @@ describe('Rewards selectors', () => {
38273921
expect(selectDismissedCampaignOutcomeToasts(state)).toEqual({});
38283922
});
38293923

3924+
it('returns empty object when dismissed toasts are undefined', () => {
3925+
const state = createMockRootState({
3926+
dismissedCampaignOutcomeToasts: undefined as unknown as Record<
3927+
string,
3928+
boolean
3929+
>,
3930+
});
3931+
expect(selectDismissedCampaignOutcomeToasts(state)).toEqual({});
3932+
});
3933+
38303934
it('returns the dismissed toasts map', () => {
38313935
const dismissed = {
38323936
'campaign-1:sub-1:winner_verify': true,

0 commit comments

Comments
 (0)