Skip to content

Commit 232fb5a

Browse files
authored
fix: prevent Basic Functionality modal crash when route params are undefined (#32189)
## **Description** Turning off Basic Functionality from **Settings → Security & Privacy** opens `BasicFunctionalityModal` without route params. On confirm, `closeBottomSheet` read `route.params.caller` and threw when `params` was `undefined`. Only the Notifications flow passes a `caller` param; Security Settings does not. This change uses optional chaining so the notifications-specific branch runs only when `caller` is present, and adds a unit test for the undefined-params path. ## **Changelog** CHANGELOG entry: Fixed a crash when turning off Basic Functionality from Security & Privacy settings ## **Related issues** Fixes: crash when disabling Basic Functionality from Security Settings without modal route params ## **Manual testing steps** ```gherkin Feature: Basic Functionality modal Scenario: user turns off Basic Functionality from Security Settings Given the app is unlocked and Basic Functionality is enabled When user opens Settings → Security & Privacy → Basic Functionality And user confirms they understand and taps Turn off Then the modal dismisses without crashing And Basic Functionality is disabled Scenario: user opens Basic Functionality from Notifications with caller param Given the app is unlocked and Basic Functionality is disabled When user opens Notifications settings and is prompted to enable Basic Functionality And user completes the modal flow Then the modal dismisses without crashing And the notifications-specific follow-up still runs when applicable ``` ## **Screenshots/Recordings** N/A — crash fix with no visual UI change; verified via unit test and manual modal dismiss flow. ### **Before** https://github.com/user-attachments/assets/8e7b171d-2bcb-4bcb-9619-51c36416fb09 ### **After** https://github.com/user-attachments/assets/4518a50f-0c9b-4003-b78a-f17040ece4be ## **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. #### 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** - [ ] 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] > **Low Risk** > Single optional-chaining guard and a focused unit test; no auth, data, or broader navigation behavior changes beyond avoiding the crash. > > **Overview** > Fixes a crash when users turn off Basic Functionality from **Settings → Security & Privacy**, where the modal opens **without** navigation route params. > > `closeBottomSheet` now uses optional chaining on `route.params?.caller` so the notifications-only follow-up (`enableNotificationsFromModal`) runs only when a `caller` is present, instead of throwing on `undefined.params`. > > Adds a unit test that mocks `useRoute` with `params: undefined` and asserts **Turn off** still dispatches `toggleBasicFunctionality(false)`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 54f2b84. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent bd42563 commit 232fb5a

2 files changed

Lines changed: 14 additions & 1 deletion

File tree

app/components/UI/BasicFunctionality/BasicFunctionalityModal/BasicFunctionalityModal.test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,17 @@ describe('BasicFunctionalityModal', () => {
8686

8787
expect(toggleBasicFunctionality).toHaveBeenCalledWith(false);
8888
});
89+
90+
it('turns off basic functionality when route params are undefined', () => {
91+
mockRouteParams = undefined;
92+
93+
const { getByText } = renderWithProvider(<BasicFunctionalityModal />, {
94+
state: mockInitialState,
95+
});
96+
97+
fireEvent.press(getByText('I understand and want to continue'));
98+
fireEvent.press(getByText('Turn off'));
99+
100+
expect(toggleBasicFunctionality).toHaveBeenCalledWith(false);
101+
});
89102
});

app/components/UI/BasicFunctionality/BasicFunctionalityModal/BasicFunctionalityModal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ const BasicFunctionalityModal = () => {
9595
.build(),
9696
);
9797
});
98-
if (route.params.caller === Routes.SETTINGS.NOTIFICATIONS) {
98+
if (route.params?.caller === Routes.SETTINGS.NOTIFICATIONS) {
9999
await enableNotificationsFromModal();
100100
}
101101
};

0 commit comments

Comments
 (0)