Skip to content

refactor: Move set existing user from login to vault recovery method#24252

Merged
Cal-L merged 2 commits intomainfrom
refactor/MCWP-240-move-setting-existing-user
Jan 6, 2026
Merged

refactor: Move set existing user from login to vault recovery method#24252
Cal-L merged 2 commits intomainfrom
refactor/MCWP-240-move-setting-existing-user

Conversation

@Cal-L
Copy link
Copy Markdown
Contributor

@Cal-L Cal-L commented Jan 5, 2026 •
edited by cursor bot
Loading

Description

This change moves setting existing user from the Login component to the initializeVaultFromBackup method in EngineService. It used to live in the Login component since we needed to ensure that existing user was set to true so that users would not re-encounter the vault recovery flow when backgrounding/re-opening post vault recovery. However, it does not need to belong in the component if vault recovery relies on initializeVaultFromBackup, which is where we've moved the set existing user call.

Changelog

CHANGELOG entry:

Related issues

Fixes: Part of https://consensyssoftware.atlassian.net/browse/MCWP-240

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

Existing user is set to true on vault recovery
https://github.com/user-attachments/assets/a1cb3040-4545-44f3-8472-74b280644e41

Pre-merge author checklist

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

Shifts responsibility for marking returning users to the engine layer during vault recovery.

  • Sets existingUser via Redux in EngineService.initializeVaultFromBackup after successful re-init; adds test asserting dispatch of setExistingUser(true)
  • Removes setExistingUser usage and isVaultRecovery logic from Login (no useDispatch; route params/type cleaned up)
  • Updates WalletRestored to navigate to Routes.ONBOARDING.LOGIN without { isVaultRecovery }; adjusts related tests
  • Cleans up Login tests by removing mocks and cases tied to vault recovery flag

Written by Cursor Bugbot for commit 97b1a1a. This will update automatically on new commits. Configure here.

@Cal-L Cal-L requested a review from a team as a code owner January 5, 2026 23:38
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 5, 2026

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.

@metamaskbot metamaskbot added the team-mobile-platform Mobile Platform team label Jan 5, 2026
@github-actions github-actions bot added the size-M label Jan 5, 2026
cursor[bot]
cursor bot reviewed Jan 5, 2026
View reviewed changes
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 6, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeCore, SmokeWalletPlatform, SmokeAccounts
  • Risk Level: medium
  • AI Confidence: 75%
click to see 🤖 AI reasoning details

This PR refactors the vault recovery flow by moving the setExistingUser(true) dispatch from the Login component to the EngineService's initializeVaultFromBackup() method. The change is a code reorganization rather than new functionality - the same action is dispatched, just at a different point in the flow (earlier, during vault initialization instead of after login).

Key changes:

  1. EngineService.ts (CRITICAL): Added setExistingUser(true) dispatch after successful vault unlock in initializeVaultFromBackup()
  2. Login/index.tsx: Removed isVaultRecovery parameter handling and the conditional dispatch
  3. WalletRestored.tsx: Removed isVaultRecovery navigation parameter
  4. Test files: Updated to reflect the new behavior location

The risk is medium because:

  • EngineService is a critical core component that affects wallet initialization
  • The vault recovery flow is important for users recovering their wallets
  • The existingUser flag affects app behavior on restart (prevents vault recovery screen from appearing again)
  • However, this is a refactoring that maintains the same functionality, just reorganized

Selected tags rationale:

  • SmokeCore: EngineService is core infrastructure; tests core wallet functionality and app state
  • SmokeWalletPlatform: Login and wallet restoration are core wallet platform features
  • SmokeAccounts: The existingUser flag relates to account/user state management

View GitHub Actions results

@Cal-L Cal-L added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jan 6, 2026
@github-project-automation github-project-automation bot moved this to Needs dev review in PR review queue Jan 6, 2026
@Cal-L Cal-L added No QA Needed Apply this label when your PR does not need any QA effort. no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. labels Jan 6, 2026
@Cal-L
Copy link
Copy Markdown
Contributor Author

Cal-L commented Jan 6, 2026

Quality Gate Failed Quality Gate failed

Failed conditions 50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Skipping sonarcloud since we removed code, not adding it

cursor[bot]
cursor bot reviewed Jan 6, 2026
View reviewed changes
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jan 6, 2026

Quality Gate Passed 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

@Cal-L Cal-L enabled auto-merge January 6, 2026 17:59
@Cal-L Cal-L added the no changelog required No changelog entry is required for this change label Jan 6, 2026
@Cal-L Cal-L added this pull request to the merge queue Jan 6, 2026
@github-project-automation github-project-automation bot moved this from Needs dev review to Review finalised - Ready to be merged in PR review queue Jan 6, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 6, 2026
@Cal-L Cal-L added this pull request to the merge queue Jan 6, 2026
Merged via the queue into main with commit 2af3c67 Jan 6, 2026
114 of 115 checks passed
@Cal-L Cal-L deleted the refactor/MCWP-240-move-setting-existing-user branch January 6, 2026 21:06
@github-project-automation github-project-automation bot moved this from Review finalised - Ready to be merged to Merged, Closed or Archived in PR review queue Jan 6, 2026
@github-actions github-actions bot locked and limited conversation to collaborators Jan 6, 2026
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Jan 6, 2026
@metamaskbot metamaskbot added the release-7.62.0 Issue or pull request that will be included in release 7.62.0 label Jan 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@vinnyhoward vinnyhoward vinnyhoward approved these changes

@tommasini tommasini tommasini approved these changes

Assignees

No one assigned

Labels

no changelog required No changelog entry is required for this change No QA Needed Apply this label when your PR does not need any QA effort. no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed release-7.62.0 Issue or pull request that will be included in release 7.62.0 size-M skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-mobile-platform Mobile Platform team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Cal-L @vinnyhoward @tommasini @metamaskbot