fix: Navigating twice to the login screen on onboarding on login

[tommasini]

Description

Navigating twice to login fix

Changelog

CHANGELOG entry:

Related issues

Fixes:

Manual testing steps

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

Before

After

Pre-merge author checklist

• I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). 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.

---

Note

On onboarding unlock, pass navigateToLogin to Authentication.lockApp and remove explicit navigation; update tests to reflect new behavior.

• Onboarding flow:
  • Update onLogin in app/components/Views/Onboarding/index.tsx to call Authentication.lockApp({ navigateToLogin: true }) when passwordSet is true, removing navigation.replace(Routes.ONBOARDING.LOGIN).
• Tests:
  • Adjust app/components/Views/Onboarding/index.test.tsx to stop asserting navigation.replace to Routes.ONBOARDING.LOGIN, verifying only Authentication.lockApp is called.

^{Written by Cursor Bugbot for commit 412ba83. This will update automatically on new commits. Configure here.}

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[cursor]

Bug: Test weakened by removing navigation assertion (Bugbot Rules)

The test "navigates to LOGIN when unlock is pressed and password is set" now only verifies that Authentication.lockApp was called, but doesn't verify the critical { navigateToLogin: true } parameter was passed. Since the production code changed from calling navigation.replace separately to passing this parameter to lockApp, the test assertion should be updated to expect(Authentication.lockApp).toHaveBeenCalledWith({ navigateToLogin: true }) to maintain equivalent coverage of the navigation behavior.

 

[ieow]

lgtm

[Cal-L]

Nice catch, LGTM

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.99%. Comparing base (7e3bea7) to head (cb4f9b7).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #23974      +/-   ##
==========================================
+ Coverage   78.97%   78.99%   +0.01%     
==========================================
  Files        4050     4052       +2     
  Lines      106496   106578      +82     
  Branches    21539    21557      +18     
==========================================
+ Hits        84109    84186      +77     
- Misses      16519    16526       +7     
+ Partials     5868     5866       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🔍 Smart E2E Test Selection

• Selected E2E tags: SmokeCore
• Risk Level: low
• AI Confidence: 85%

click to see 🤖 AI reasoning details

The change is a minor refactoring in the Onboarding component's onLogin callback. Previously, the code called Authentication.lockApp() followed by navigation.replace(Routes.ONBOARDING.LOGIN) separately. Now it consolidates this into a single call: Authentication.lockApp({ navigateToLogin: true }).

The lockApp method in Authentication.ts already has the navigateToLogin parameter (defaulting to true) that handles navigation to the login screen internally via NavigationService.navigation?.reset(). This is functionally equivalent behavior - the user still gets locked out and navigated to the login screen.

The test file was updated accordingly to remove the assertion checking for mockReplace being called with the login route, since navigation is now handled internally by lockApp.

This is a low-risk refactoring change that:

1. Does not change user-facing behavior
2. Consolidates navigation logic into the Authentication service
3. Has corresponding unit test updates

SmokeCore is selected because it covers core wallet functionality, framework, app state, and navigation - all of which are relevant to this authentication/onboarding flow change.

View GitHub Actions results

[cursor]

Bug: Test no longer verifies navigation behavior (Bugbot Rules)

The test named "navigates to LOGIN when unlock is pressed and password is set" removed the assertion that verified navigation occurs (mockReplace.toHaveBeenCalledWith(Routes.ONBOARDING.LOGIN)) but doesn't verify that Authentication.lockApp is called with the correct { navigateToLogin: true } parameter. The test now only checks that lockApp was called, not that it was called with parameters that would cause navigation. Per the testing guidelines, the test name describes behavior that is no longer being validated - this violates the coverage requirement that tests verify their stated behavior.

 

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud