Skip to content

fix(ramps): consolidate terminal order status checks behind controller export (TRAM-3703)#33393

Draft
amitabh94 wants to merge 1 commit into
mainfrom
cursor/consolidate-terminal-order-statuses-fd61
Draft

fix(ramps): consolidate terminal order status checks behind controller export (TRAM-3703)#33393
amitabh94 wants to merge 1 commit into
mainfrom
cursor/consolidate-terminal-order-statuses-fd61

Conversation

@amitabh94

@amitabh94 amitabh94 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Consolidates four duplicated ramp terminal-order-status sets behind a single source of truth from @metamask/ramps-controller.

Closes TRAM-3703.

Problem

Mobile maintained independent copies of the terminal order status set in:

  • OrderContent.tsx
  • BankDetails.tsx (missing IdExpired — live inconsistency)
  • useFiatOrderStatus.ts
  • ramps-controller/event-handlers/analytics.ts

Each copy had to be kept in lockstep manually and had already drifted.

Solution

  1. Controller export (interim patch): Adds TERMINAL_ORDER_STATUSES and isTerminalOrderStatus() to @metamask/ramps-controller@17.0.0 via a yarn patch until the upstream MetaMask/core release lands.
  2. Consumer cleanup: Replaces all four local Set declarations with isTerminalOrderStatus() from the controller package.
  3. BankDetails fix: IdExpired orders now redirect to order details like the other terminal statuses.

Risk

  • Low — behavior change is limited to BankDetails treating IdExpired as terminal (intentional bugfix aligning with controller + other consumers).
  • Patch can be dropped once @metamask/ramps-controller publishes the export natively.

Follow-up

Verification

yarn jest app/core/Engine/controllers/ramps-controller/event-handlers/analytics.test.ts
yarn jest app/components/Views/confirmations/hooks/activity/useFiatOrderStatus.test.ts
yarn jest app/components/UI/Ramp/Views/OrderDetails/OrderContent.test.tsx
yarn jest app/components/UI/Ramp/Views/NativeFlow/BankDetails.test.tsx -t "ID_EXPIRED"

All targeted tests pass.

Open in Web Open in Cursor 

…r export

TRAM-3703: Export TERMINAL_ORDER_STATUSES and isTerminalOrderStatus from
@metamask/ramps-controller via a yarn patch until the upstream core release
lands. Replace four hand-rolled terminal-status sets in OrderContent,
BankDetails, useFiatOrderStatus, and ramps analytics with the shared
predicate. BankDetails now treats IdExpired as terminal, matching the other
call sites.

Co-authored-by: Amitabh Aggarwal <amitabh94@users.noreply.github.com>
@metamask-ci metamask-ci Bot added team-money-movement issues related to Money Movement features INVALID-PR-TEMPLATE PR's body doesn't match template labels Jul 16, 2026
@metamask-ci

metamask-ci Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

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

Warnings — informational, address before merging:

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

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: high
  • AI Confidence: 100%
click to see 🤖 AI reasoning details

E2E Test Selection:
Hard rule (controller-version-update): @MetaMask controller package version updated in package.json: @metamask/network-enablement-controller@npm:^5.4.1, @metamask/ramps-controller@npm:^17.0.0, @metamask/ramps-controller. Running all tests.

Performance Test Selection:
No performance-sensitive code paths are changed. This PR is a refactoring that replaces locally-defined terminal status sets with a shared utility function. No rendering, data fetching, or app launch paths are affected.

View GitHub Actions results

@github-actions

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/Ramp/Views/NativeFlow/BankDetails.test.tsx 0/122 0/168 0/374

AI-detected flaky patterns

app/components/UI/Ramp/Views/NativeFlow/BankDetails.test.tsx

  • J1 — Missing act() around async state updates (critical)
    • This test triggers a UI state update (toggling bank info visibility) using fireEvent.press without wrapping the call in act(). Other button press tests in the same file correctly use 'await act(async () => { fireEvent.press(...) })'. Per the loaded skill, this matches the J1 anti-pattern for async state updates and can cause intermittent failures or React act() warnings under different test execution timings. The historical data showed no prior flakiness for this file but the pattern is present in the current code.
    • Suggested fix in app/components/UI/Ramp/Views/NativeFlow/BankDetails.test.tsx:
      -    expect(queryByText('Test Bank')).not.toBeOnTheScreen();
      -    fireEvent.press(getByText('deposit.bank_details.show_bank_info'));
      -    expect(getByText('Test Bank')).toBeOnTheScreen();
      +    expect(queryByText('Test Bank')).not.toBeOnTheScreen();
      +
      +    await act(async () => {
      +      fireEvent.press(getByText('deposit.bank_details.show_bank_info'));
      +    });
      +
      +    expect(getByText('Test Bank')).toBeOnTheScreen();

This check is informational only and does not block merging.

@sonarqubecloud

Copy link
Copy Markdown

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

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template size-M team-money-movement issues related to Money Movement features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants