Skip to content

Conversation

@weitingsun
Copy link
Contributor

@weitingsun weitingsun commented Nov 13, 2025

Description

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

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

Adds an OTA update check at app startup (feature-flagged) and displays a loading screen while checking, with supporting selector and tests.

  • App Startup & Navigation:
    • Split App into AppContent and wrapper App that uses useOTAUpdates to gate rendering; shows FoxLoader while isCheckingUpdates is true (app/components/Nav/App/App.tsx).
  • Hooks:
    • New useOTAUpdates hook using expo-updates to check/fetch/reload OTA updates based on selectOTAUpdatesEnabled (app/components/hooks/useOTAUpdates.ts).
  • Feature Flags & Selectors:
    • New selector selectOTAUpdatesEnabled with version-gated validation (app/selectors/featureFlagController/otaUpdatesEnabled).
    • Added description for otaUpdatesEnabled in app/util/feature-flags/index.ts.
  • Tests:
    • Added comprehensive tests for useOTAUpdates behaviors (app/components/hooks/useOTAUpdates.test.ts).
    • Updated App.test.tsx to mock useOTAUpdates and assert FoxLoader renders during update check.

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

@weitingsun weitingsun marked this pull request as ready for review November 13, 2025 19:40
@github-actions
Copy link
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.

@metamaskbot metamaskbot added the team-mobile-platform Mobile Platform team label Nov 13, 2025
};

checkForUpdates();
}, [otaUpdatesEnabled]);
Copy link

Choose a reason for hiding this comment

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

Bug: UI State Mismatch During Update Check

When otaUpdatesEnabled changes from false to true, the update check runs but isCheckingUpdates remains false from the previous effect run. This prevents the loading UI from displaying during the actual update check, creating an inconsistent state where updates are being checked but the user sees no loading indicator.

Fix in Cursor Fix in Web

@github-actions
Copy link
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

After thorough investigation, these changes introduce a new OTA (Over-The-Air) updates feature that is:

  1. Feature-Flag Controlled: The OTA updates functionality is controlled by a remote feature flag (otaUpdatesEnabled) with version gating capability. This means the feature can be enabled/disabled remotely without requiring app updates.

  2. Scope of Changes:

    • Created new selector: app/selectors/featureFlagController/otaUpdatesEnabled/ - handles reading the feature flag from RemoteFeatureFlagController
    • Created new hook: app/components/hooks/useOTAUpdates.ts - manages OTA update checking logic when app starts
    • Modified app/components/Nav/App/App.tsx - added call to useOTAUpdates hook and displays FoxLoader while checking for updates
    • Updated app/util/feature-flags/index.ts - added description for the new feature flag
    • All changes include comprehensive unit tests (*.test.ts files)
  3. User-Facing Impact: The changes add a loading screen (FoxLoader) during app startup if:

    • The feature flag is enabled remotely
    • An OTA update is available and being fetched
    • The app is NOT in development mode
  4. Risk Assessment - LOW because:

    • Feature is disabled by default (feature flag not enabled yet)
    • Only runs on production builds (skips DEV mode)
    • Has proper error handling - if OTA check fails, app continues normally
    • All new code is well-tested with unit tests
    • Does not modify core wallet functionality, controllers, or Engine
    • Does not affect existing user flows (only adds a potential loading screen on startup)
    • Infrastructure change that's non-breaking and gracefully degrades
  5. E2E Test Impact:

    • No existing E2E tests are affected since this is a new feature
    • Feature is behind a flag, so E2E tests would need the flag enabled to test it
    • Current E2E tests don't appear to interact with OTA update mechanisms
    • The loading screen would be transparent to tests when feature is disabled
  6. Why No Tags Selected:

    • This is internal infrastructure for app updates, not a user-facing feature test area
    • Feature is disabled by default and controlled remotely
    • Changes are well-isolated and thoroughly unit tested
    • No modifications to critical paths (controllers, Engine, core functionality)
    • When enabled, the feature would affect app startup universally, not specific flows
    • E2E tests run in development/test environments where this feature is skipped

The comprehensive unit test coverage (300+ lines for the hook, 106 lines for the selector, extensive App.tsx tests) provides confidence that the feature works as intended when enabled.

View GitHub Actions results

await waitFor(() => {
expect(mockCheckForUpdateAsync).toHaveBeenCalledTimes(1);
});
});
Copy link

Choose a reason for hiding this comment

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

Bug: Enforce AAA Pattern Blank Line Separation (Bugbot Rules)

Test violates mandatory AAA pattern by missing blank line between Act and Assert sections. Line 96 contains the Act step (renderHook), and line 97 immediately starts the Assert step (await waitFor) without a separating blank line. The unit testing guidelines require blank line separation between Arrange, Act, and Assert sections in every test.

Fix in Cursor Fix in Web

@sonarqubecloud
Copy link

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

Labels

size-L team-mobile-platform Mobile Platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants