Skip to content

fix: musd-1128 prevent apy from disappearing#33193

Open
Jwhiles wants to merge 1 commit into
mainfrom
MUSD0-1128
Open

fix: musd-1128 prevent apy from disappearing#33193
Jwhiles wants to merge 1 commit into
mainfrom
MUSD0-1128

Conversation

@Jwhiles

@Jwhiles Jwhiles commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Description

This is a fix to prevent the APY row from disappearing after users sat on the money home page for a while.

The root cause of that bug is that the @metamask/react-data-query library removes data from its cache even when UI components still observe that data. Because the money home re-renders every 30s when balances are fetched, when this data is removed the APY would disappear. We poll for APY every 5 minutes, so it would eventually re-appear - but there was a gap where no data exists.

I think the long term fix is to prevent the core library from removing observed data from the cache - but in the interest of fixing this user facing bug, we are now storing the most recently observed APY value in component state and using that if the value disappears.

Changelog

CHANGELOG entry: fix disappearing money account APY row

Related issues

Fixes: MUSD-1128

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

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 how APY is derived and when loading skeletons show across Money surfaces; incorrect carry or loading logic could misstate APY briefly, but scope is localized with new tests.

Overview
Fixes the Money home APY row vanishing after users stay on the screen while the vault APY query cache is cleared under active observers (react-data-query / BaseDataService behavior).

useMoneyAccountBalance now keeps the last successful live vault APY in a ref and uses it when query data drops to undefined, while still honoring override/fallback rules. It exposes isApyLoading (loading only when no APY is available from any source) so skeletons do not flicker during transient reloads.

Money home, balance card, and balance projection switch from vaultApyQuery.isLoading to isApyLoading. Hook tests cover last-known carry, override precedence, and loading semantics.

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

@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.

@metamask-ci

metamask-ci Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Manual testing steps still contain template content (the Gherkin example title or a [...] placeholder). Replace with real steps, or write N/A — <reason>.
  • Screenshots/Recordings section is empty. Add an image/video for user-facing changes, logs/console output for non-user-facing changes, or write N/A if no evidence is applicable.
  • Pre-merge author checklist has unchecked items (e.g. "I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards."). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/components/UI/Money/Views/MoneyHomeView/MoneyHomeView.test.tsx 0/50 0/178 0/375

AI-detected flaky patterns

app/components/UI/Money/Views/MoneyHomeView/MoneyHomeView.test.tsx

  • J10 — jest.spyOn without restoreAllMocks() afterward (medium)
    • Multiple tests in this file (e.g. those testing openURL calls for learn more, mUSD row, condensed cards) create spies with jest.spyOn(Linking, 'openURL') but there is no afterEach(jest.restoreAllMocks()). The beforeEach only does clearAllMocks() which does not restore the original implementation. This can cause mock leakage across tests, especially since some tests use requireMock for other modules and order is not guaranteed, leading to intermittent failures (matches J10 exactly). History showed 0 failures but pattern is present.
    • Suggested fix in app/components/UI/Money/Views/MoneyHomeView/MoneyHomeView.test.tsx:
      -  let defaultMoneyAccountBalance: ReturnType<typeof useMoneyAccountBalance>;
      -  beforeEach(() => {
      -    jest.clearAllMocks();
      -    global.alert = jest.fn();
      -    jest.mocked(selectPrivacyMode).mockReturnValue(false);
      -    mockUseMoneyAccountApiActivity.mockReturnValue(apiActivityResult());
      -    // ...
      -  });
      +  let defaultMoneyAccountBalance: ReturnType<typeof useMoneyAccountBalance>;
      +  beforeEach(() => {
      +    jest.clearAllMocks();
      +    global.alert = jest.fn();
      +    jest.mocked(selectPrivacyMode).mockReturnValue(false);
      +    mockUseMoneyAccountApiActivity.mockReturnValue(apiActivityResult());
      +    // ...
      +  });
      +
      +  afterEach(() => {
      +    jest.restoreAllMocks();
      +  });

This check is informational only and does not block merging.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
All 8 changed files are within the app/components/UI/Money/ directory, exclusively affecting the MetaMask Card/Money feature:

  1. useMoneyAccountBalance.ts: Core hook change - adds isApyLoading (a smarter loading flag that prevents UI flicker) and a lastKnownServiceApyRef to carry the last known APY value when BaseDataService clears cached data every 5 minutes. This is a behavioral fix for the APY loading state.

  2. MoneyHomeView.tsx: Replaces vaultApyQuery.isLoading with isApyLoading in two places (MoneyEarnings and BalanceProjection loading states).

  3. BalanceProjection.tsx: Replaces vaultApyQuery.isLoading with isApyLoading for skeleton rendering.

  4. MoneyBalanceCard.tsx: Replaces vaultApyQuery.isLoading with isApyLoading for skeleton rendering.

The remaining 4 files are test files (.test.tsx/.test.ts) for the above components.

The changes are contained within the Money/Card feature area and fix a UI flickering issue with APY loading states. SmokeMoney covers the MetaMask Card home screen, balance display, and related flows - exactly what's being modified here.

No other feature areas are impacted (no navigation changes, no shared components, no Engine/controller changes, no confirmation flows). The test files are unit tests, not E2E smoke tests, so they don't affect tag selection directly.

Performance Test Selection:
The changes fix a UI flickering issue with APY loading states in the Money/Card feature. While this touches balance display, the changes are about preventing unnecessary re-renders of skeleton states (not adding new data fetching or rendering complexity). There are no performance test tags specifically for the Money/Card balance card or APY display. The @PerformanceAssetLoading tag covers token list and portfolio value calculation, but the Money card APY loading fix is a targeted UI state improvement that doesn't meaningfully impact measured performance scenarios.

View GitHub Actions results

@Jwhiles Jwhiles marked this pull request as ready for review July 13, 2026 12:39
@Jwhiles Jwhiles requested a review from a team as a code owner July 13, 2026 12:39
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jul 13, 2026
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants