Skip to content

fix(money): always route add money to deposit screen and fix redirect_target (MUSD-959)#31590

Open
Kureev wants to merge 1 commit into
mainfrom
kureev/MUSD-959
Open

fix(money): always route add money to deposit screen and fix redirect_target (MUSD-959)#31590
Kureev wants to merge 1 commit into
mainfrom
kureev/MUSD-959

Conversation

@Kureev

@Kureev Kureev commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Description

MoneyBalanceCard's Add button (and the mUSD row Add button on Money home, which shares the same routing hook) used to preselect mUSD as the payment token and redirect to the Ramp Buy flow when the wallet held no mUSD.

Per MUSD-959, the Add press now always opens the MM Pay deposit screen with no explicit preselection. MM Pay already implements both acceptance criteria in useAutomaticTransactionPayToken: it auto-selects the highest-balance crypto token when the user has tokens, and auto-selects a fiat payment method when the user has none (autoSelectFiatPayment || tokens.length === 0). Duplicating a "has tokens" check on our side would only risk diverging from MM Pay's authoritative token filtering (disabled tokens, minimum balances, hardware-wallet rules).

useMoneyAccountAddRouting therefore collapses to a single routeAddMoney that calls initiateDeposit(); the now-dead moveMusd, depositFunds, convertCrypto, and hasMusdBalance members are removed (no remaining consumers).

The event properties part: both call sites fired trackButtonClicked with a predictive redirect_target (hasMusdBalance ? money_deposit : ramp_buy) computed before routing, which no longer matched where the user actually landed. Both now report the constant money_deposit.

Changelog

CHANGELOG entry: Fixed the wallet home balance card Add button to always open the deposit screen, preselecting fiat payment when the wallet holds no tokens

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MUSD-959

Manual testing steps

Feature: MoneyBalanceCard add money routing

  Scenario: user with crypto tokens presses Add on the balance card
    Given the wallet holds at least one crypto token

    When user presses Add on the wallet home balance card
    Then the MM Pay deposit screen opens with the highest-balance crypto token preselected as the payment token

  Scenario: user without tokens presses Add on the balance card
    Given the wallet holds no tokens

    When user presses Add on the wallet home balance card
    Then the MM Pay deposit screen opens with a fiat payment method preselected
    And the user is not redirected to the Ramp Buy flow

  Scenario: user presses Add on the mUSD row of Money home
    Given the user is on the Money home screen

    When user presses Add on the MetaMask USD row
    Then the MM Pay deposit screen opens
    And the Button Clicked event reports redirect_target money_deposit

Screenshots/Recordings

Before

After

Simulator.Screen.Recording.-.iPhone.17.-.2026-06-11.at.23.19.04.mov

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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

Medium Risk
Changes end-user add-money navigation (no more Ramp Buy for zero-mUSD wallets) and simplifies a shared routing hook used from wallet home and Money home; deposit preselection now relies entirely on MM Pay.

Overview
Add money from the wallet home Money balance card and the Money home mUSD row no longer branches on whether the wallet holds mUSD or sends users to Ramp Buy. Both paths now call routeAddMoney, which only runs initiateDeposit() with no preselected payment token so MM Pay handles token vs fiat selection.

useMoneyAccountAddRouting is reduced to that single behavior; hasMusdBalance, moveMusd, depositFunds, convertCrypto, and Ramp/mUSD conversion wiring are removed. trackButtonClicked redirect_target on those Add/Earn actions is always money_deposit instead of toggling ramp_buy vs deposit.

Tests drop balance-based routing cases, assert the new analytics constant, add Money home coverage for condensed info card navigation (How it works, mUSD asset, learn more URL), and shrink the hook unit tests to initiateDeposit success/failure handling.

Reviewed by Cursor Bugbot for commit 34dd202. Bugbot is set up for automated code reviews on this repo. Configure here.

@Kureev Kureev added team-earn pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. labels Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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.

@Kureev Kureev removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jun 11, 2026
@Kureev Kureev self-assigned this Jun 11, 2026
@Kureev Kureev marked this pull request as ready for review June 11, 2026 21:09
@Kureev Kureev requested a review from a team as a code owner June 11, 2026 21:09
@github-actions github-actions Bot added the risk:low AI analysis: low risk label Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeMoney, SmokeConfirmations
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR simplifies the useMoneyAccountAddRouting hook by removing the conditional routing logic that previously checked hasMusdBalance to decide between MONEY_DEPOSIT and RAMP_BUY flows. Now the hook always routes to MONEY_DEPOSIT via initiateDeposit(). This affects:

  1. SmokeMoney: The changes directly impact the MetaMask Card / Money feature's "Add Money" flow. Both MoneyHomeView and MoneyBalanceCard components are modified to always use SCREEN_NAMES.MONEY_DEPOSIT as the redirect target, removing the conditional branch to RAMP_BUY. This is a functional routing change that needs E2E validation.

  2. SmokeConfirmations: Per the SmokeMoney tag description, when selecting SmokeMoney for Card Add Funds or similar flows that execute deposits/swaps, SmokeConfirmations should also be selected since the deposit flow involves on-chain transactions.

The changes are scoped to the Money/Card feature area only. No changes to core infrastructure, controllers, Engine, navigation framework, or shared components that would affect other test areas. The risk is medium because this is a functional routing change (not just cosmetic) that could affect user flows if the deposit path behaves differently than the previous conditional routing.

Performance Test Selection:
The changes are limited to routing logic simplification within the Money/Card feature (removing conditional mUSD balance check for routing). This does not affect app launch, login, onboarding, asset loading, swap execution, or any other performance-sensitive flows measured by the available performance test tags. No performance tests are warranted.

View GitHub Actions results

@Kureev Kureev enabled auto-merge June 11, 2026 21:23
component_name: COMPONENT_NAMES.MONEY_MUSD_TOKEN_SECTION,
redirect_target: hasMusdBalance
? SCREEN_NAMES.MONEY_DEPOSIT
: SCREEN_NAMES.RAMP_BUY,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete this orphaned enum if it isn't used anymore.

MUSD_TOKEN_ADDRESS_BY_CHAIN,
MUSD_TOKEN_ASSET_ID_BY_CHAIN,
} from '../../Earn/constants/musd';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's delete this hook if all it's doing is wrapping a call to initiateDeposit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:low AI analysis: low risk size-M team-earn

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants