From 116d2a2ed05bb580003bbf3cd3f3aa5c06595f5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Tani=C3=A7a?= Date: Wed, 25 Feb 2026 18:01:35 -0700 Subject: [PATCH 1/3] refactor(predict): migrate usePredictActivity to React Query (#26598) ## **Description** Migrated `usePredictActivity` from manual loading/refresh state to a React Query-based implementation and aligned transactions view + tests with the query-result contract. This removes legacy hook options/refresh plumbing and keeps activity data fresh by invalidating the new activity query after relevant Predict mutations. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Predict activity history uses React Query Scenario: user views and refreshes transaction history Given user is on the Predict Transactions tab with an account that has activity When user opens the Transactions tab Then activity rows are loaded from the query and rendered When user pulls to refresh Then the view calls query refetch and keeps rendering using query loading states ``` ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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** > Moderate risk due to a behavioral refactor of activity fetching/refresh semantics (React Query caching, address-scoped keys, and removal of focus-based refresh), which could affect when/which activity data is shown. > > **Overview** > Migrates `usePredictActivity` from manual state management + navigation focus refresh to a `@tanstack/react-query` `useQuery` implementation keyed by selected EVM `address`, with centralized query config in new `predictQueries.activity`. > > Updates `PredictTransactionsView` (and its tests) to consume the query-result contract (`data`, `isRefetching`, `refetch`) and adds activity query invalidation after order placement and when Predict transactions are confirmed, keeping the activity list in sync. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b2f1553fa0af0e68b2d02ee3d8111ea4bb6047d7. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .../Predict/hooks/usePredictActivity.test.ts | 168 ++++++++---------- .../UI/Predict/hooks/usePredictActivity.ts | 110 ++++-------- .../UI/Predict/hooks/usePredictPlaceOrder.ts | 4 + .../hooks/usePredictToastRegistrations.tsx | 4 + app/components/UI/Predict/queries/activity.ts | 16 ++ app/components/UI/Predict/queries/index.ts | 5 + .../PredictTransactionsView.test.tsx | 48 +++-- .../PredictTransactionsView.tsx | 14 +- 8 files changed, 167 insertions(+), 202 deletions(-) create mode 100644 app/components/UI/Predict/queries/activity.ts diff --git a/app/components/UI/Predict/hooks/usePredictActivity.test.ts b/app/components/UI/Predict/hooks/usePredictActivity.test.ts index efdbb1d2a35..efd8b32a9f0 100644 --- a/app/components/UI/Predict/hooks/usePredictActivity.test.ts +++ b/app/components/UI/Predict/hooks/usePredictActivity.test.ts @@ -1,134 +1,114 @@ -import { renderHook, act } from '@testing-library/react-hooks'; +import React from 'react'; +import { renderHook, waitFor, act } from '@testing-library/react-native'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { usePredictActivity } from './usePredictActivity'; -import Engine from '../../../../core/Engine'; -// Mock Engine +const MOCK_ADDRESS = '0x1234567890123456789012345678901234567890'; + +const mockGetActivity = jest.fn(); jest.mock('../../../../core/Engine', () => ({ context: { PredictController: { - getActivity: jest.fn(), + getActivity: (...args: unknown[]) => mockGetActivity(...args), }, }, })); -// Mock navigation focus effect without auto-invocation; provide manual trigger -jest.mock('@react-navigation/native', () => { - let focusCb: (() => void) | null = null; - return { - useFocusEffect: (cb: () => void) => { - focusCb = cb; - }, - __esModule: true, - __mock: { - invokeFocusEffect: () => { - focusCb?.(); - }, - }, - }; -}); +jest.mock('../utils/accounts', () => ({ + getEvmAccountFromSelectedAccountGroup: jest.fn(() => ({ + address: MOCK_ADDRESS, + })), +})); -describe('usePredictActivity', () => { - const mockGetActivity = jest.fn(); +const mockEnsurePolygonNetworkExists = jest.fn, []>(); +jest.mock('./usePredictNetworkManagement', () => ({ + usePredictNetworkManagement: () => ({ + ensurePolygonNetworkExists: mockEnsurePolygonNetworkExists, + }), +})); - beforeEach(() => { - jest.clearAllMocks(); - (Engine.context.PredictController.getActivity as jest.Mock) = - mockGetActivity; +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: Infinity } }, }); - afterEach(() => { - jest.clearAllMocks(); - }); + const Wrapper = ({ children }: { children: React.ReactNode }) => + React.createElement(QueryClientProvider, { client: queryClient }, children); - it('initializes and auto-loads activity on mount', async () => { - const data = [{ id: '1' }]; - mockGetActivity.mockResolvedValueOnce(data); + return { Wrapper }; +}; - const { result, waitForNextUpdate } = renderHook(() => - usePredictActivity(), - ); +describe('usePredictActivity', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockGetActivity.mockResolvedValue([]); + mockEnsurePolygonNetworkExists.mockResolvedValue(undefined); + }); - expect(result.current.isLoading).toBe(true); - expect(result.current.activity).toEqual([]); - expect(result.current.error).toBe(null); + it('fetches activity automatically on mount', async () => { + const { Wrapper } = createWrapper(); + const activity = [{ id: '1' }]; + mockGetActivity.mockResolvedValueOnce(activity); - await waitForNextUpdate(); + const { result } = renderHook(() => usePredictActivity(), { + wrapper: Wrapper, + }); - expect(result.current.isLoading).toBe(false); - expect(result.current.activity).toEqual(data); - expect(result.current.error).toBe(null); - expect(mockGetActivity).toHaveBeenCalledWith({ - providerId: undefined, + await waitFor(() => { + expect(result.current.isLoading).toBe(false); }); + + expect(result.current.data).toEqual(activity); + expect(result.current.error).toBeNull(); + expect(mockGetActivity).toHaveBeenCalledWith({ address: MOCK_ADDRESS }); }); - it('loads activity when hook is mounted', async () => { - mockGetActivity.mockResolvedValueOnce([]); + it('exposes error when activity fetch fails', async () => { + const { Wrapper } = createWrapper(); + mockGetActivity.mockRejectedValueOnce(new Error('Boom')); - const { waitForNextUpdate } = renderHook(() => usePredictActivity()); + const { result } = renderHook(() => usePredictActivity(), { + wrapper: Wrapper, + }); - await waitForNextUpdate(); + await waitFor(() => { + expect(result.current.error).toBeInstanceOf(Error); + }); - expect(mockGetActivity).toHaveBeenCalledWith({}); + expect(result.current.error?.message).toBe('Boom'); }); - it('can refresh with isRefresh=true and sets isRefreshing flag', async () => { + it('uses refetch for refresh behavior', async () => { + const { Wrapper } = createWrapper(); mockGetActivity.mockResolvedValueOnce([{ id: '1' }]); - const { result, waitForNextUpdate } = renderHook(() => - usePredictActivity(), - ); - await waitForNextUpdate(); + const { result } = renderHook(() => usePredictActivity(), { + wrapper: Wrapper, + }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); mockGetActivity.mockResolvedValueOnce([{ id: '2' }]); await act(async () => { - await result.current.loadActivity({ isRefresh: true }); + await result.current.refetch(); }); - expect(result.current.isRefreshing).toBe(false); - expect(result.current.activity).toEqual([{ id: '2' }]); - }); - - it('handles errors and sets error message', async () => { - mockGetActivity.mockRejectedValueOnce(new Error('Boom')); - - const { result, waitForNextUpdate } = renderHook(() => - usePredictActivity(), - ); - - await waitForNextUpdate(); - - expect(result.current.isLoading).toBe(false); - expect(result.current.error).toBe('Boom'); - expect(result.current.activity).toEqual([]); - }); - - it('supports disabling auto-load via loadOnMount=false', () => { - const { result } = renderHook(() => - usePredictActivity({ loadOnMount: false }), - ); - - expect(result.current.isLoading).toBe(true); - expect(result.current.activity).toEqual([]); - expect(mockGetActivity).not.toHaveBeenCalled(); + expect(mockGetActivity).toHaveBeenCalledTimes(2); + expect(result.current.isRefetching).toBe(false); }); - it('triggers refresh on focus when refreshOnFocus=true', async () => { - mockGetActivity.mockResolvedValueOnce([]); - - const { waitForNextUpdate } = renderHook(() => - usePredictActivity({ refreshOnFocus: true }), - ); - - await waitForNextUpdate(); + it('ensures polygon network before running query', async () => { + const { Wrapper } = createWrapper(); + const { result } = renderHook(() => usePredictActivity(), { + wrapper: Wrapper, + }); - mockGetActivity.mockResolvedValueOnce([]); - const { __mock } = jest.requireMock('@react-navigation/native'); - await act(async () => { - __mock.invokeFocusEffect(); - await Promise.resolve(); + await waitFor(() => { + expect(result.current.isLoading).toBe(false); }); - expect(mockGetActivity).toHaveBeenCalledTimes(2); + expect(mockEnsurePolygonNetworkExists).toHaveBeenCalledTimes(1); }); }); diff --git a/app/components/UI/Predict/hooks/usePredictActivity.ts b/app/components/UI/Predict/hooks/usePredictActivity.ts index b144aded8f2..66819feb2b7 100644 --- a/app/components/UI/Predict/hooks/usePredictActivity.ts +++ b/app/components/UI/Predict/hooks/usePredictActivity.ts @@ -1,89 +1,43 @@ -import { useFocusEffect } from '@react-navigation/native'; -import { useCallback, useEffect, useState } from 'react'; -import Engine from '../../../../core/Engine'; +import { useEffect } from 'react'; +import { useQuery, type UseQueryResult } from '@tanstack/react-query'; import Logger from '../../../../util/Logger'; import { PREDICT_CONSTANTS } from '../constants/errors'; import type { PredictActivity } from '../types'; +import { usePredictNetworkManagement } from './usePredictNetworkManagement'; +import { getEvmAccountFromSelectedAccountGroup } from '../utils/accounts'; +import { predictQueries } from '../queries'; import { ensureError } from '../utils/predictErrorHandler'; -interface UsePredictActivityOptions { - loadOnMount?: boolean; - refreshOnFocus?: boolean; -} - -interface UsePredictActivityReturn { - activity: PredictActivity[]; - isLoading: boolean; - isRefreshing: boolean; - error: string | null; - loadActivity: (options?: { isRefresh?: boolean }) => Promise; -} - -export function usePredictActivity( - options: UsePredictActivityOptions = {}, -): UsePredictActivityReturn { - const { loadOnMount = true, refreshOnFocus = true } = options; +export function usePredictActivity(): UseQueryResult { + const { ensurePolygonNetworkExists } = usePredictNetworkManagement(); - const [activity, setActivity] = useState([]); - const [isLoading, setIsLoading] = useState(true); - const [isRefreshing, setIsRefreshing] = useState(false); - const [error, setError] = useState(null); - - const loadActivity = useCallback( - async (loadOptions?: { isRefresh?: boolean }) => { - const { isRefresh = false } = loadOptions || {}; - try { - if (isRefresh) { - setIsRefreshing(true); - } else { - setIsLoading(true); - } - setError(null); - - const controller = Engine.context.PredictController; - const data = await controller.getActivity({}); - setActivity(data ?? []); - } catch (err) { - const message = - err instanceof Error ? err.message : 'Failed to load activity'; - setError(message); - - // Capture exception with activity loading context (no user address) - Logger.error(ensureError(err), { - tags: { - feature: PREDICT_CONSTANTS.FEATURE_NAME, - component: 'usePredictActivity', - }, - context: { - name: 'usePredictActivity', - data: { - method: 'loadActivity', - action: 'activity_load', - operation: 'data_fetching', - }, - }, - }); - } finally { - setIsLoading(false); - setIsRefreshing(false); - } - }, - [], - ); + const evmAccount = getEvmAccountFromSelectedAccountGroup(); + const address = evmAccount?.address ?? '0x0'; useEffect(() => { - if (loadOnMount) { - loadActivity(); - } - }, [loadOnMount, loadActivity]); + ensurePolygonNetworkExists().catch(() => undefined); + }, [ensurePolygonNetworkExists]); - useFocusEffect( - useCallback(() => { - if (refreshOnFocus) { - loadActivity({ isRefresh: true }); - } - }, [refreshOnFocus, loadActivity]), - ); + const queryResult = useQuery(predictQueries.activity.options({ address })); - return { activity, isLoading, isRefreshing, error, loadActivity }; + useEffect(() => { + if (!queryResult.error) return; + + Logger.error(ensureError(queryResult.error), { + tags: { + feature: PREDICT_CONSTANTS.FEATURE_NAME, + component: 'usePredictActivity', + }, + context: { + name: 'usePredictActivity', + data: { + method: 'queryFn', + action: 'activity_load', + operation: 'data_fetching', + }, + }, + }); + }, [queryResult.error]); + + return queryResult; } diff --git a/app/components/UI/Predict/hooks/usePredictPlaceOrder.ts b/app/components/UI/Predict/hooks/usePredictPlaceOrder.ts index b340887b190..d6b9c75ad53 100644 --- a/app/components/UI/Predict/hooks/usePredictPlaceOrder.ts +++ b/app/components/UI/Predict/hooks/usePredictPlaceOrder.ts @@ -190,6 +190,10 @@ export function usePredictPlaceOrder( queryKey: predictQueries.positions.keys.all(), }); + queryClient.invalidateQueries({ + queryKey: predictQueries.activity.keys.all(), + }); + if (side === Side.BUY) { showOrderPlacedToast(); } else { diff --git a/app/components/UI/Predict/hooks/usePredictToastRegistrations.tsx b/app/components/UI/Predict/hooks/usePredictToastRegistrations.tsx index cb27c952b48..af1e687ffbe 100644 --- a/app/components/UI/Predict/hooks/usePredictToastRegistrations.tsx +++ b/app/components/UI/Predict/hooks/usePredictToastRegistrations.tsx @@ -153,6 +153,10 @@ export const usePredictToastRegistrations = (): ToastRegistration[] => { queryClient.invalidateQueries({ queryKey: predictQueries.balance.keys.all(), }); + + queryClient.invalidateQueries({ + queryKey: predictQueries.activity.keys.all(), + }); } if (type === 'deposit') { diff --git a/app/components/UI/Predict/queries/activity.ts b/app/components/UI/Predict/queries/activity.ts new file mode 100644 index 00000000000..6f4b305bda7 --- /dev/null +++ b/app/components/UI/Predict/queries/activity.ts @@ -0,0 +1,16 @@ +import { queryOptions } from '@tanstack/react-query'; +import Engine from '../../../../core/Engine'; +import type { PredictActivity } from '../types'; + +export const predictActivityKeys = { + all: () => ['predict', 'activity'] as const, + byAddress: (address: string) => + [...predictActivityKeys.all(), address] as const, +}; + +export const predictActivityOptions = ({ address }: { address: string }) => + queryOptions({ + queryKey: predictActivityKeys.byAddress(address), + queryFn: async (): Promise => + Engine.context.PredictController.getActivity({ address }), + }); diff --git a/app/components/UI/Predict/queries/index.ts b/app/components/UI/Predict/queries/index.ts index 29bb7a35454..fc150fc152b 100644 --- a/app/components/UI/Predict/queries/index.ts +++ b/app/components/UI/Predict/queries/index.ts @@ -1,7 +1,12 @@ +import { predictActivityKeys, predictActivityOptions } from './activity'; import { predictBalanceKeys, predictBalanceOptions } from './balance'; import { predictPositionsKeys, predictPositionsOptions } from './positions'; export const predictQueries = { + activity: { + keys: predictActivityKeys, + options: predictActivityOptions, + }, balance: { keys: predictBalanceKeys, options: predictBalanceOptions, diff --git a/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.test.tsx b/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.test.tsx index 7c7a4d63e96..e405c9e8dac 100644 --- a/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.test.tsx +++ b/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.test.tsx @@ -69,10 +69,10 @@ jest.mock('../../components/PredictActivity/PredictActivity', () => { // Mock usePredictActivity hook - external data dependency jest.mock('../../hooks/usePredictActivity', () => ({ usePredictActivity: jest.fn(() => ({ - activity: [], + data: [], isLoading: false, - isRefreshing: false, - loadActivity: jest.fn(), + isRefetching: false, + refetch: jest.fn(), })), })); @@ -85,29 +85,25 @@ describe('PredictTransactionsView', () => { jest.clearAllMocks(); }); - afterEach(() => { - jest.clearAllMocks(); - }); - const createUsePredictActivityValue = ( overrides: Partial<{ - activity: unknown[]; + data: unknown[]; isLoading: boolean; - isRefreshing: boolean; - loadActivity: jest.Mock; + isRefetching: boolean; + refetch: jest.Mock; }> = {}, ) => ({ - activity: [], + data: [], isLoading: false, - isRefreshing: false, - loadActivity: jest.fn(), + isRefetching: false, + refetch: jest.fn(), ...overrides, }); it('displays loading indicator when activity data loads', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ - activity: [], + data: [], isLoading: true, }), ); @@ -120,7 +116,7 @@ describe('PredictTransactionsView', () => { it('displays empty state message when activity list is empty', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ - activity: [], + data: [], isLoading: false, }), ); @@ -135,7 +131,7 @@ describe('PredictTransactionsView', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ isLoading: false, - activity: [ + data: [ { id: 'a1', title: 'Market A', @@ -175,7 +171,7 @@ describe('PredictTransactionsView', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ isLoading: false, - activity: [ + data: [ { id: 'a1', title: 'Market A', @@ -215,7 +211,7 @@ describe('PredictTransactionsView', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ isLoading: false, - activity: [ + data: [ { id: 'b2', title: 'Market B', @@ -254,7 +250,7 @@ describe('PredictTransactionsView', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ isLoading: false, - activity: [ + data: [ { id: 'c3', title: 'Market C', @@ -289,7 +285,7 @@ describe('PredictTransactionsView', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ isLoading: false, - activity: [ + data: [ { id: 'd4', title: 'Market D', @@ -324,7 +320,7 @@ describe('PredictTransactionsView', () => { (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ isLoading: true, - activity: [ + data: [ { id: 'refreshing-item', title: 'Market Refresh', @@ -349,13 +345,13 @@ describe('PredictTransactionsView', () => { }); it('passes refreshing state and triggers refresh handler on pull to refresh', async () => { - const mockLoadActivity = jest.fn().mockResolvedValue(undefined); + const mockRefetch = jest.fn().mockResolvedValue(undefined); const mockTimestamp = Math.floor(Date.now() / 1000); (usePredictActivity as jest.Mock).mockReturnValueOnce( createUsePredictActivityValue({ - isRefreshing: true, - loadActivity: mockLoadActivity, - activity: [ + isRefetching: true, + refetch: mockRefetch, + data: [ { id: 'refreshable', title: 'Market Refreshable', @@ -380,6 +376,6 @@ describe('PredictTransactionsView', () => { await sectionList.props.onRefresh(); }); - expect(mockLoadActivity).toHaveBeenCalledWith({ isRefresh: true }); + expect(mockRefetch).toHaveBeenCalledTimes(1); }); }); diff --git a/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.tsx b/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.tsx index 75a4d6e4213..81cb3d6928d 100644 --- a/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.tsx +++ b/app/components/UI/Predict/views/PredictTransactionsView/PredictTransactionsView.tsx @@ -62,8 +62,12 @@ const PredictTransactionsView: React.FC = ({ isVisible, }) => { const tw = useTailwind(); - const { activity, isLoading, isRefreshing, loadActivity } = - usePredictActivity({}); + const { + data: activity = [], + isLoading, + isRefetching, + refetch, + } = usePredictActivity(); // Track screen load performance (activity data loaded) usePredictMeasurement({ @@ -270,8 +274,10 @@ const PredictTransactionsView: React.FC = ({ showsVerticalScrollIndicator={false} style={tw.style('flex-1')} stickySectionHeadersEnabled - refreshing={isRefreshing} - onRefresh={() => loadActivity({ isRefresh: true })} + refreshing={isRefetching} + onRefresh={() => { + void refetch(); + }} maxToRenderPerBatch={20} initialNumToRender={20} windowSize={12} From 7a470286b7efc5e4173aacd97a5f4ba9879f6e3f Mon Sep 17 00:00:00 2001 From: CW Date: Wed, 25 Feb 2026 17:50:06 -0800 Subject: [PATCH 2/3] test: use unique BrowserStack Local identifier per job to prevent mm-connect tunnel routing conflicts (#26519) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** All MM-Connect performance tests on CI were consistently failing with "not able to connect to bs-local.com:8090", and after the initial fix a second issue surfaced causing `BROWSERSTACK_LOCAL_CONNECTION_FAILED` for all appwright tests. ### Root cause 1: localIdentifier tunnel group conflict Every job in a workflow run — `imported-wallet` (Android), `imported-wallet` (iOS), and `mm-connect` (Android) — calls the reusable `performance-test-runner.yml` and starts a BrowserStack Local tunnel using `local-identifier: ${{ github.run_id }}`. Because all concurrent jobs share the same `run_id`, BrowserStack treats their tunnel binaries as a **tunnel group** and may route any session's request to any binary in the group. The appwright patch hardcodes `local: true` for all sessions, so imported-wallet sessions also participate in the group. When a mm-connect session navigates to `http://bs-local.com:8090`, BrowserStack can route the request to an imported-wallet runner — where no dapp server is listening on port 8090 — causing a consistent connection failure. **Fix:** Append `build_type` and `matrix.device.name` to the `local-identifier` so each job registers an isolated tunnel that only its own session uses. ### Root cause 2: unsafe characters in localIdentifier breaking BrowserStack Local binary Device names (e.g. `"Samsung Galaxy S23 Ultra"`) contain spaces. When passed directly as `--local-identifier` to the BrowserStack Local binary, the spaces caused argument misparsing — the tunnel registered with a truncated identifier. The session then sent the full string as `localIdentifier`, found no matching tunnel, and threw `BROWSERSTACK_LOCAL_CONNECTION_FAILED` for every test. **Fix:** Add a dedicated "Compute BrowserStack Local Identifier" step that sanitizes both `inputs.build_type` and `matrix.device.name` using `tr -c 'a-zA-Z0-9-' '_'`, replacing every unsafe character (spaces, `/`, `\`, `@`, etc.) with an underscore. Uses `printf` instead of `echo` to avoid a trailing newline becoming a trailing underscore. Both values are validated to be non-empty and fail fast with a clear error if not, preventing malformed identifiers with trailing hyphens. The sanitized value is stored as a step output and referenced consistently in both `setup-local` and `BROWSERSTACK_LOCAL_IDENTIFIER`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MMQA-1488 ## **Manual testing steps** ```gherkin Feature: MM-Connect performance tests on CI Scenario: mm-connect tests run without tunnel routing conflicts Given the performance E2E workflow is triggered And imported-wallet and mm-connect jobs run concurrently When BrowserStack Local is started with a unique sanitized identifier per job Then each BrowserStack session routes bs-local.com traffic to its own runner And mm-connect tests can reach the dapp server on port 8090 And all three mm-connect specs pass on BrowserStack And no BROWSERSTACK_LOCAL_CONNECTION_FAILED errors appear for any test ``` ## **Screenshots/Recordings** ### **Before** - MM-Connect specs: "not able to connect to bs-local.com:8090" - All appwright tests after initial fix: `BROWSERSTACK_LOCAL_CONNECTION_FAILED` ### **After** [BrowserStack Build Results](https://app-automate.browserstack.com/dashboard/v2/public-build/Tjh6Y3N1b3pwYXNxSmtvQjAwNkV0TFA4ekNVOFZxdXBQMmNka041OC9XbDVHc1JHVVJabzczb254Qmc3SThYVjVybWlhYW5qbXA1RTZOZHk2TjM2Y0E5PS0tV0VIMEh3U2tUbnUrZGw5WmppWCt0UT09--74149fd798c3d43122ccbfd107c699ad988d74e1) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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] > **Low Risk** > Workflow-only change limited to BrowserStack Local tunnel configuration; main risk is mis-sanitization or identifier formatting causing tunnels not to match sessions. > > **Overview** > Prevents BrowserStack Local tunnel collisions in the reusable performance test workflow by generating a **unique per-job** `local-identifier` from `github.run_id`, `inputs.build_type`, and `matrix.device.name`. > > Adds a dedicated step to **sanitize** identifier components (replace non-alphanumeric/hyphen chars with `_`) and fail fast if inputs are empty, then reuses the computed value for both `setup-local` and `BROWSERSTACK_LOCAL_IDENTIFIER` during test execution. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5d9ba634cbad2de1f233bd54378bcec8c4337ea2. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --------- Co-authored-by: Claude Sonnet 4.6 --- .github/workflows/performance-test-runner.yml | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/workflows/performance-test-runner.yml b/.github/workflows/performance-test-runner.yml index 8a598d2b116..763ab5ccdc2 100644 --- a/.github/workflows/performance-test-runner.yml +++ b/.github/workflows/performance-test-runner.yml @@ -98,12 +98,28 @@ jobs: username: ${{ secrets.BROWSERSTACK_USERNAME }} access-key: ${{ secrets.BROWSERSTACK_ACCESS_KEY }} project-name: ${{ github.repository }} - + + - name: Compute BrowserStack Local Identifier + id: bs-local-id + run: | + # Strip any character that is not alphanumeric or a hyphen (covers spaces, /, \, @, etc.) + BUILD_TYPE_SAFE=$(printf '%s' "${{ inputs.build_type }}" | tr -c 'a-zA-Z0-9-' '_') + DEVICE_SAFE=$(printf '%s' "${{ matrix.device.name }}" | tr -c 'a-zA-Z0-9-' '_') + if [ -z "$BUILD_TYPE_SAFE" ]; then + echo "❌ Error: inputs.build_type is empty — cannot build a valid BrowserStack Local identifier" + exit 1 + fi + if [ -z "$DEVICE_SAFE" ]; then + echo "❌ Error: matrix.device.name is empty — cannot build a valid BrowserStack Local identifier" + exit 1 + fi + echo "value=${{ github.run_id }}-${BUILD_TYPE_SAFE}-${DEVICE_SAFE}" >> "$GITHUB_OUTPUT" + - name: Setup BrowserStack Local uses: browserstack/github-actions/setup-local@4478e16186f38e5be07721931642e65a028713c3 with: local-testing: start - local-identifier: ${{ github.run_id }} + local-identifier: ${{ steps.bs-local-id.outputs.value }} # Use same args for all build types. Do not use --include-hosts for mm-connect: # bs-local.com requests must be forwarded to localhost; --include-hosts can block them. local-args: '--force-local --verbose' @@ -147,7 +163,7 @@ jobs: - name: Run Tests env: BROWSERSTACK_LOCAL: 'true' - BROWSERSTACK_LOCAL_IDENTIFIER: ${{ github.run_id }} + BROWSERSTACK_LOCAL_IDENTIFIER: ${{ steps.bs-local-id.outputs.value }} BROWSERSTACK_BUILD_NAME: ${{ inputs.browserstack_build_name }} working-directory: '.' run: | From 6c5372cef7c4d062dc5cf6be501dcd8f0776850e Mon Sep 17 00:00:00 2001 From: asalsys Date: Thu, 26 Feb 2026 00:14:11 -0500 Subject: [PATCH 3/3] refactor(web3auth): remove unnecessary useNavigation generic types (#26571) ## **Description** **Reason for the change:** This is a preparatory change for upgrading the React Navigation library to v6. With global default types now applied to the navigation API, we no longer need to apply generic types to the `useNavigation` hook in most cases. **Improvement/Solution:** - Removed generic types from `useNavigation` hook invocations (now uses default global navigation types) - Applied `NavigationProp` where deeply nested navigation is required - Part of a series of PRs splitting the cleanup by codebase ownership **Note on exceptions:** Generic types may still be needed when navigating to deeper nested stacks. For those instances, we apply a recursive navigation type pattern. **References:** - Parent issue: #23763 - React Navigation nesting docs: https://reactnavigation.org/docs/nesting-navigators/ ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: #23763 (partial) ## **Manual testing steps** ```gherkin Feature: Navigation functionality after useNavigation cleanup Scenario: Web3auth/Onboarding navigation works correctly Given the app is running And the user is going through onboarding or social login flows When user navigates between onboarding screens Then the navigation should complete successfully And no TypeScript errors should occur ``` **Additional verification:** - Run `yarn lint:tsc` to verify no TypeScript errors are introduced - Run unit tests for affected components ## **Screenshots/Recordings** N/A - No visual changes (internal refactoring/type cleanup) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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. Made with [Cursor](https://cursor.com) --- > [!NOTE] > **Medium Risk** > Touches navigation behavior across onboarding/social login and vault recovery flows by switching from `replace` to dispatching `StackActions.replace`, which can subtly change how navigation state updates. Risk is mitigated by expanded/updated unit tests covering the new action dispatching behavior. > > **Overview** > Refactors several onboarding/social-login and vault-recovery screens to stop using explicit `useNavigation` generic typings (and removes related `@react-navigation/stack` type imports) in preparation for React Navigation v6. > > Standardizes route replacement to use `navigation.dispatch(StackActions.replace(...))` instead of `navigation.replace(...)` in `Onboarding`, `SocialLoginIosUser`, and the vault recovery flow (`RestoreWallet`, `WalletResetNeeded`, `WalletRestored`), including the offline social-login error-sheet path. > > Updates and adds unit tests to match the new dispatch-based replace behavior (mocking `dispatch` and asserting `REPLACE` actions), and refactors `SocialLoginErrorSheet` tests for more robust rendering and async assertions. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 69204022748b4f67e7c2f60604e033faabd07bff. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). Co-authored-by: Cursor --- .../Views/OAuthRehydration/index.tsx | 10 +- .../Views/Onboarding/index.test.tsx | 10 +- app/components/Views/Onboarding/index.tsx | 43 ++-- .../RestoreWallet/RestoreWallet.test.tsx | 179 ++++++++++++++++ .../Views/RestoreWallet/RestoreWallet.tsx | 17 +- .../RestoreWallet/WalletResetNeeded.test.tsx | 131 ++++++++++++ .../Views/RestoreWallet/WalletResetNeeded.tsx | 17 +- .../RestoreWallet/WalletRestored.test.tsx | 21 +- .../Views/RestoreWallet/WalletRestored.tsx | 9 +- .../Views/SocialLoginIosUser/index.test.tsx | 5 + .../Views/SocialLoginIosUser/index.tsx | 33 +-- .../SocialLoginErrorSheet.test.tsx | 195 +++++++++--------- .../SocialLoginErrorSheet.tsx | 5 +- 13 files changed, 506 insertions(+), 169 deletions(-) create mode 100644 app/components/Views/RestoreWallet/RestoreWallet.test.tsx create mode 100644 app/components/Views/RestoreWallet/WalletResetNeeded.test.tsx diff --git a/app/components/Views/OAuthRehydration/index.tsx b/app/components/Views/OAuthRehydration/index.tsx index 1e8d0fb6155..2122ae33760 100644 --- a/app/components/Views/OAuthRehydration/index.tsx +++ b/app/components/Views/OAuthRehydration/index.tsx @@ -67,16 +67,10 @@ import { import { useNetInfo } from '@react-native-community/netinfo'; import { SuccessErrorSheetParams } from '../SuccessErrorSheet/interface'; import { usePromptSeedlessRelogin } from '../../hooks/SeedlessHooks'; -import { - ParamListBase, - RouteProp, - useNavigation, - useRoute, -} from '@react-navigation/native'; +import { RouteProp, useNavigation, useRoute } from '@react-navigation/native'; import { useStyles } from '../../../component-library/hooks/useStyles'; import stylesheet from './styles'; import ReduxService from '../../../core/redux'; -import { StackNavigationProp } from '@react-navigation/stack'; import OAuthService from '../../../core/OAuthService/OAuthService'; import trackOnboarding from '../../../util/metrics/TrackOnboarding/trackOnboarding'; import { @@ -142,7 +136,7 @@ const OAuthRehydration: React.FC = ({ [loading, isDeletingInProgress], ); - const navigation = useNavigation>(); + const navigation = useNavigation(); const { styles, theme: { themeAppearance }, diff --git a/app/components/Views/Onboarding/index.test.tsx b/app/components/Views/Onboarding/index.test.tsx index 54fbe81ba86..b1764e1a194 100644 --- a/app/components/Views/Onboarding/index.test.tsx +++ b/app/components/Views/Onboarding/index.test.tsx @@ -258,6 +258,11 @@ const mockNav = { replace: mockReplace, reset: jest.fn(), setOptions: jest.fn(), + dispatch: jest.fn((action) => { + if (action.type === 'REPLACE') { + mockReplace(action.payload.name, action.payload.params); + } + }), }; jest.mock('@react-navigation/stack', () => ({ createStackNavigator: () => ({ @@ -662,7 +667,10 @@ describe('Onboarding', () => { }); expect(Authentication.resetVault).toHaveBeenCalled(); - expect(mockReplace).toHaveBeenCalledWith(Routes.ONBOARDING.HOME_NAV); + expect(mockReplace).toHaveBeenCalledWith( + Routes.ONBOARDING.HOME_NAV, + undefined, + ); }); it('navigates to LOGIN when unlock is pressed and password is set', async () => { diff --git a/app/components/Views/Onboarding/index.tsx b/app/components/Views/Onboarding/index.tsx index 6b11406a890..927be85ae91 100644 --- a/app/components/Views/Onboarding/index.tsx +++ b/app/components/Views/Onboarding/index.tsx @@ -56,9 +56,8 @@ import { useNavigation, useRoute, RouteProp, - ParamListBase, + StackActions, } from '@react-navigation/native'; -import { StackNavigationProp } from '@react-navigation/stack'; import { TraceName, TraceOperation, @@ -123,7 +122,7 @@ interface OnboardingRouteParams { } const Onboarding = () => { - const navigation = useNavigation>(); + const navigation = useNavigation(); const route = useRoute>(); const dispatch = useDispatch(); @@ -298,7 +297,7 @@ const Onboarding = () => { const onLogin = useCallback(async (): Promise => { if (!passwordSet) { await Authentication.resetVault(); - navigation.replace(Routes.ONBOARDING.HOME_NAV); + navigation.dispatch(StackActions.replace(Routes.ONBOARDING.HOME_NAV)); } else { await Authentication.lockApp({ navigateToLogin: true }); } @@ -636,22 +635,26 @@ const Onboarding = () => { try { const netState = await netInfoFetch(); if (!netState.isConnected || netState.isInternetReachable === false) { - navigation.replace(Routes.MODAL.ROOT_MODAL_FLOW, { - screen: Routes.SHEET.SUCCESS_ERROR_SHEET, - params: { - title: strings(`error_sheet.no_internet_connection_title`), - description: strings( - `error_sheet.no_internet_connection_description`, - ), - descriptionAlign: 'left', - buttonLabel: strings(`error_sheet.no_internet_connection_button`), - primaryButtonLabel: strings( - `error_sheet.no_internet_connection_button`, - ), - closeOnPrimaryButtonPress: true, - type: 'error', - }, - }); + navigation.dispatch( + StackActions.replace(Routes.MODAL.ROOT_MODAL_FLOW, { + screen: Routes.SHEET.SUCCESS_ERROR_SHEET, + params: { + title: strings(`error_sheet.no_internet_connection_title`), + description: strings( + `error_sheet.no_internet_connection_description`, + ), + descriptionAlign: 'left', + buttonLabel: strings( + `error_sheet.no_internet_connection_button`, + ), + primaryButtonLabel: strings( + `error_sheet.no_internet_connection_button`, + ), + closeOnPrimaryButtonPress: true, + type: 'error', + }, + }), + ); return; } } catch (error) { diff --git a/app/components/Views/RestoreWallet/RestoreWallet.test.tsx b/app/components/Views/RestoreWallet/RestoreWallet.test.tsx new file mode 100644 index 00000000000..3cc8931a8f5 --- /dev/null +++ b/app/components/Views/RestoreWallet/RestoreWallet.test.tsx @@ -0,0 +1,179 @@ +import React from 'react'; +import { Image, ActivityIndicator } from 'react-native'; +import { fireEvent, waitFor } from '@testing-library/react-native'; +import RestoreWallet from './RestoreWallet'; +import Routes from '../../../constants/navigation/Routes'; +import renderWithProvider from '../../../util/test/renderWithProvider'; +import { MetaMetricsEvents } from '../../../core/Analytics'; +import EngineService from '../../../core/EngineService'; +import { strings } from '../../../../locales/i18n'; + +const mockReplace = jest.fn(); +const mockDispatch = jest.fn((action) => { + if (action.type === 'REPLACE') { + mockReplace(action.payload.name, action.payload.params); + } +}); + +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + dispatch: mockDispatch, + }), + }; +}); + +jest.mock('../../../util/navigation/navUtils', () => ({ + createNavigationDetails: jest.fn( + (routeName: string, nestedRouteName?: string) => + (params?: Record) => [ + nestedRouteName ?? routeName, + params, + ], + ), + useParams: jest.fn(() => ({ + previousScreen: 'Login', + })), +})); + +const mockTrackEvent = jest.fn(); +const mockCreateEventBuilder = jest.fn(() => ({ + addProperties: jest.fn().mockReturnThis(), + build: jest.fn().mockReturnValue({ category: 'test' }), +})); + +jest.mock('../../../components/hooks/useMetrics', () => ({ + useMetrics: () => ({ + trackEvent: mockTrackEvent, + createEventBuilder: mockCreateEventBuilder, + }), +})); + +jest.mock('../../../util/metrics', () => jest.fn(() => ({ device: 'test' }))); + +jest.mock('../../../core/EngineService', () => ({ + initializeVaultFromBackup: jest.fn(), +})); + +describe('RestoreWallet', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('rendering', () => { + it('renders title text', () => { + const { getByText } = renderWithProvider(); + + expect( + getByText(strings('restore_wallet.restore_needed_title')), + ).toBeOnTheScreen(); + }); + + it('renders description text', () => { + const { getByText } = renderWithProvider(); + + expect( + getByText(strings('restore_wallet.restore_needed_description')), + ).toBeOnTheScreen(); + }); + + it('renders Restore button', () => { + const { getByText } = renderWithProvider(); + + expect( + getByText(strings('restore_wallet.restore_needed_action')), + ).toBeOnTheScreen(); + }); + + it('renders device image', () => { + const { UNSAFE_getByType } = renderWithProvider(); + + const imageElement = UNSAFE_getByType(Image); + expect(imageElement).toBeTruthy(); + }); + }); + + describe('analytics tracking', () => { + it('tracks screen viewed event on mount', () => { + renderWithProvider(); + + expect(mockCreateEventBuilder).toHaveBeenCalledWith( + MetaMetricsEvents.VAULT_CORRUPTION_RESTORE_WALLET_SCREEN_VIEWED, + ); + expect(mockTrackEvent).toHaveBeenCalled(); + }); + }); + + describe('handleOnNext', () => { + it('navigates to WalletRestored when restore succeeds', async () => { + (EngineService.initializeVaultFromBackup as jest.Mock).mockResolvedValue({ + success: true, + }); + const { getByText } = renderWithProvider(); + + fireEvent.press( + getByText(strings('restore_wallet.restore_needed_action')), + ); + + await waitFor(() => { + expect(mockCreateEventBuilder).toHaveBeenCalledWith( + MetaMetricsEvents.VAULT_CORRUPTION_RESTORE_WALLET_BUTTON_PRESSED, + ); + expect(EngineService.initializeVaultFromBackup).toHaveBeenCalled(); + expect(mockDispatch).toHaveBeenCalled(); + expect(mockReplace).toHaveBeenCalledWith( + Routes.VAULT_RECOVERY.WALLET_RESTORED, + undefined, + ); + }); + }); + + it('navigates to WalletResetNeeded when restore fails', async () => { + (EngineService.initializeVaultFromBackup as jest.Mock).mockResolvedValue({ + success: false, + }); + const { getByText } = renderWithProvider(); + + fireEvent.press( + getByText(strings('restore_wallet.restore_needed_action')), + ); + + await waitFor(() => { + expect(EngineService.initializeVaultFromBackup).toHaveBeenCalled(); + expect(mockDispatch).toHaveBeenCalled(); + expect(mockReplace).toHaveBeenCalledWith( + Routes.VAULT_RECOVERY.WALLET_RESET_NEEDED, + undefined, + ); + }); + }); + + it('shows loading indicator while restoring', async () => { + let resolveRestore: (value: { success: boolean }) => void; + const restorePromise = new Promise<{ success: boolean }>((resolve) => { + resolveRestore = resolve; + }); + (EngineService.initializeVaultFromBackup as jest.Mock).mockReturnValue( + restorePromise, + ); + const { getByText, UNSAFE_getByType } = renderWithProvider( + , + ); + + fireEvent.press( + getByText(strings('restore_wallet.restore_needed_action')), + ); + + expect(UNSAFE_getByType(ActivityIndicator)).toBeTruthy(); + + // @ts-expect-error resolveRestore is assigned in Promise constructor + resolveRestore({ success: true }); + + await waitFor(() => { + expect(mockDispatch).toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/app/components/Views/RestoreWallet/RestoreWallet.tsx b/app/components/Views/RestoreWallet/RestoreWallet.tsx index b0f6e3fb3b6..c3af43d18fe 100644 --- a/app/components/Views/RestoreWallet/RestoreWallet.tsx +++ b/app/components/Views/RestoreWallet/RestoreWallet.tsx @@ -14,14 +14,13 @@ import { import Routes from '../../../constants/navigation/Routes'; import EngineService from '../../../core/EngineService'; import { SafeAreaView } from 'react-native-safe-area-context'; -import { useNavigation } from '@react-navigation/native'; +import { useNavigation, StackActions } from '@react-navigation/native'; import { useAppThemeFromContext } from '../../../util/theme'; import { createWalletResetNeededNavDetails } from './WalletResetNeeded'; import { createWalletRestoredNavDetails } from './WalletRestored'; import { MetaMetricsEvents } from '../../../core/Analytics'; import generateDeviceAnalyticsMetaData from '../../../util/metrics'; -import { StackNavigationProp } from '@react-navigation/stack'; import { useMetrics } from '../../../components/hooks/useMetrics'; /* eslint-disable import/no-commonjs, @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports */ @@ -50,9 +49,7 @@ const RestoreWallet = () => { const [loading, setLoading] = useState(false); - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const { replace } = useNavigation>(); + const navigation = useNavigation(); const deviceMetaData = useMemo(() => generateDeviceAnalyticsMetaData(), []); const { previousScreen } = useParams(); @@ -79,13 +76,17 @@ const RestoreWallet = () => { ); const restoreResult = await EngineService.initializeVaultFromBackup(); if (restoreResult.success) { - replace(...createWalletRestoredNavDetails()); + navigation.dispatch( + StackActions.replace(...createWalletRestoredNavDetails()), + ); setLoading(false); } else { - replace(...createWalletResetNeededNavDetails()); + navigation.dispatch( + StackActions.replace(...createWalletResetNeededNavDetails()), + ); setLoading(false); } - }, [deviceMetaData, replace, trackEvent, createEventBuilder]); + }, [deviceMetaData, navigation, trackEvent, createEventBuilder]); return ( diff --git a/app/components/Views/RestoreWallet/WalletResetNeeded.test.tsx b/app/components/Views/RestoreWallet/WalletResetNeeded.test.tsx new file mode 100644 index 00000000000..a3b26d244d8 --- /dev/null +++ b/app/components/Views/RestoreWallet/WalletResetNeeded.test.tsx @@ -0,0 +1,131 @@ +import React from 'react'; +import { fireEvent } from '@testing-library/react-native'; +import WalletResetNeeded from './WalletResetNeeded'; +import Icon from '../../../component-library/components/Icons/Icon'; +import Routes from '../../../constants/navigation/Routes'; +import renderWithProvider from '../../../util/test/renderWithProvider'; +import { MetaMetricsEvents } from '../../../core/Analytics'; + +const mockNavigate = jest.fn(); +const mockReplace = jest.fn(); +const mockDispatch = jest.fn((action) => { + if (action.type === 'REPLACE') { + mockReplace(action.payload.name, action.payload.params); + } +}); + +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigation: () => ({ + navigate: mockNavigate, + dispatch: mockDispatch, + }), + }; +}); + +const mockTrackEvent = jest.fn(); +const mockCreateEventBuilder = jest.fn(() => ({ + addProperties: jest.fn().mockReturnThis(), + build: jest.fn().mockReturnValue({ category: 'test' }), +})); + +jest.mock('../../../components/hooks/useMetrics', () => ({ + useMetrics: () => ({ + trackEvent: mockTrackEvent, + createEventBuilder: mockCreateEventBuilder, + }), +})); + +jest.mock('../../../util/metrics', () => jest.fn(() => ({ device: 'test' }))); + +describe('WalletResetNeeded', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('rendering', () => { + it('renders title text', () => { + const { getByText } = renderWithProvider(); + + expect(getByText('New wallet needed')).toBeTruthy(); + }); + + it('renders description text', () => { + const { getByText } = renderWithProvider(); + + expect( + getByText( + "Something's wrong with your wallet, and you'll need to create a new one. Because your accounts are on the blockchain, they're still safe. Only the preferences, saved networks, account names, and related data saved on your device are gone.", + ), + ).toBeTruthy(); + }); + + it('renders Try again button', () => { + const { getByText } = renderWithProvider(); + + expect(getByText('Try recovering wallet')).toBeTruthy(); + }); + + it('renders Create new wallet button', () => { + const { getByText } = renderWithProvider(); + + expect(getByText('Create a new wallet')).toBeTruthy(); + }); + + it('renders danger icon', () => { + const { UNSAFE_getByType } = renderWithProvider(); + + const iconElement = UNSAFE_getByType(Icon); + expect(iconElement).toBeTruthy(); + }); + }); + + describe('analytics tracking', () => { + it('tracks screen viewed event on mount', () => { + renderWithProvider(); + + expect(mockCreateEventBuilder).toHaveBeenCalledWith( + MetaMetricsEvents.VAULT_CORRUPTION_WALLET_RESET_NEEDED_SCREEN_VIEWED, + ); + expect(mockTrackEvent).toHaveBeenCalled(); + }); + }); + + describe('handleCreateNewWallet', () => { + it('tracks button pressed event and navigates to delete wallet modal', () => { + const { getByText } = renderWithProvider(); + + fireEvent.press(getByText('Create a new wallet')); + + expect(mockCreateEventBuilder).toHaveBeenCalledWith( + MetaMetricsEvents.VAULT_CORRUPTION_WALLET_RESET_NEEDED_CREATE_NEW_WALLET_BUTTON_PRESSED, + ); + expect(mockTrackEvent).toHaveBeenCalled(); + expect(mockNavigate).toHaveBeenCalledWith(Routes.MODAL.ROOT_MODAL_FLOW, { + screen: Routes.MODAL.DELETE_WALLET, + }); + }); + }); + + describe('handleTryAgain', () => { + it('tracks button pressed event and navigates to restore wallet using dispatch', () => { + const { getByText } = renderWithProvider(); + + fireEvent.press(getByText('Try recovering wallet')); + + expect(mockCreateEventBuilder).toHaveBeenCalledWith( + MetaMetricsEvents.VAULT_CORRUPTION_WALLET_RESET_NEEDED_TRY_AGAIN_BUTTON_PRESSED, + ); + expect(mockTrackEvent).toHaveBeenCalled(); + expect(mockDispatch).toHaveBeenCalled(); + expect(mockReplace).toHaveBeenCalledWith( + Routes.VAULT_RECOVERY.RESTORE_WALLET, + expect.objectContaining({ + previousScreen: Routes.VAULT_RECOVERY.WALLET_RESET_NEEDED, + }), + ); + }); + }); +}); diff --git a/app/components/Views/RestoreWallet/WalletResetNeeded.tsx b/app/components/Views/RestoreWallet/WalletResetNeeded.tsx index 57745747e46..bddf4ba914b 100644 --- a/app/components/Views/RestoreWallet/WalletResetNeeded.tsx +++ b/app/components/Views/RestoreWallet/WalletResetNeeded.tsx @@ -14,8 +14,7 @@ import Icon, { IconName, IconSize, } from '../../../component-library/components/Icons/Icon'; -import { useNavigation } from '@react-navigation/native'; -import { StackNavigationProp } from '@react-navigation/stack'; +import { useNavigation, StackActions } from '@react-navigation/native'; import { createRestoreWalletNavDetails } from './RestoreWallet'; import { MetaMetricsEvents } from '../../../core/Analytics'; import generateDeviceAnalyticsMetaData from '../../../util/metrics'; @@ -30,9 +29,7 @@ const WalletResetNeeded = () => { const { trackEvent, createEventBuilder } = useMetrics(); const styles = createStyles(colors); - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const navigation = useNavigation>(); + const navigation = useNavigation(); const deviceMetaData = useMemo(() => generateDeviceAnalyticsMetaData(), []); @@ -67,10 +64,12 @@ const WalletResetNeeded = () => { .addProperties({ ...deviceMetaData }) .build(), ); - navigation.replace( - ...createRestoreWalletNavDetails({ - previousScreen: Routes.VAULT_RECOVERY.WALLET_RESET_NEEDED, - }), + navigation.dispatch( + StackActions.replace( + ...createRestoreWalletNavDetails({ + previousScreen: Routes.VAULT_RECOVERY.WALLET_RESET_NEEDED, + }), + ), ); }, [deviceMetaData, navigation, trackEvent, createEventBuilder]); diff --git a/app/components/Views/RestoreWallet/WalletRestored.test.tsx b/app/components/Views/RestoreWallet/WalletRestored.test.tsx index 1188a6c789f..8f1890a75d2 100644 --- a/app/components/Views/RestoreWallet/WalletRestored.test.tsx +++ b/app/components/Views/RestoreWallet/WalletRestored.test.tsx @@ -3,7 +3,7 @@ import { fireEvent, waitFor, act } from '@testing-library/react-native'; import { Linking } from 'react-native'; import FilesystemStorage from 'redux-persist-filesystem-storage'; import WalletRestored from './WalletRestored'; -import { useNavigation } from '@react-navigation/native'; +import { useNavigation, StackActions } from '@react-navigation/native'; import { useMetrics } from '../../../components/hooks/useMetrics'; import generateDeviceAnalyticsMetaData from '../../../util/metrics'; import Routes from '../../../constants/navigation/Routes'; @@ -16,6 +16,9 @@ import { MIGRATION_ERROR_HAPPENED } from '../../../constants/storage'; jest.mock('@react-navigation/native', () => ({ ...jest.requireActual('@react-navigation/native'), useNavigation: jest.fn(), + StackActions: { + replace: jest.fn(), + }, })); jest.mock('redux-persist-filesystem-storage', () => ({ removeItem: jest.fn(() => Promise.resolve()), @@ -53,12 +56,16 @@ describe('WalletRestored', () => { replace: jest.fn(), navigate: jest.fn(), goBack: jest.fn(), + dispatch: jest.fn(), }; + const mockReplaceAction = { type: 'REPLACE', payload: { name: 'Login' } }; + const mockTrackEvent = jest.fn(); beforeEach(() => { jest.clearAllMocks(); + (StackActions.replace as jest.Mock).mockReturnValue(mockReplaceAction); // Create a fresh mock chain each time const mockBuild = jest.fn().mockReturnValue({ name: 'test-event' }); @@ -110,6 +117,8 @@ describe('WalletRestored', () => { it('navigates to LOGIN with vault recovery flag when continue is pressed', async () => { // Arrange + const mockFilesystemRemoveItem = FilesystemStorage.removeItem as jest.Mock; + mockFilesystemRemoveItem.mockResolvedValue(undefined); const { getByText } = renderWithProvider(); const continueButton = getByText('Continue to wallet'); @@ -118,11 +127,12 @@ describe('WalletRestored', () => { fireEvent.press(continueButton); }); - // Assert + // Assert - wait for async finishWalletRestore to complete and dispatch navigation await waitFor(() => { - expect(mockNavigation.replace).toHaveBeenCalledWith( + expect(StackActions.replace).toHaveBeenCalledWith( Routes.ONBOARDING.LOGIN, ); + expect(mockNavigation.dispatch).toHaveBeenCalledWith(mockReplaceAction); }); }); @@ -159,7 +169,7 @@ describe('WalletRestored', () => { renderWithProvider(); // Assert - expect(mockNavigation.replace).not.toHaveBeenCalled(); + expect(mockNavigation.dispatch).not.toHaveBeenCalled(); }); it('clears migration error flag when continue is pressed', async () => { @@ -220,9 +230,10 @@ describe('WalletRestored', () => { // Assert - Navigation proceeds to allow user access, recovery will retry on next launch await waitFor(() => { - expect(mockNavigation.replace).toHaveBeenCalledWith( + expect(StackActions.replace).toHaveBeenCalledWith( Routes.ONBOARDING.LOGIN, ); + expect(mockNavigation.dispatch).toHaveBeenCalledWith(mockReplaceAction); }); }); }); diff --git a/app/components/Views/RestoreWallet/WalletRestored.tsx b/app/components/Views/RestoreWallet/WalletRestored.tsx index f1caaaed4a6..f58a46fea58 100644 --- a/app/components/Views/RestoreWallet/WalletRestored.tsx +++ b/app/components/Views/RestoreWallet/WalletRestored.tsx @@ -18,12 +18,11 @@ import StyledButton from '../../UI/StyledButton'; import { createNavigationDetails } from '../../../util/navigation/navUtils'; import Routes from '../../../constants/navigation/Routes'; import { SafeAreaView } from 'react-native-safe-area-context'; -import { useNavigation } from '@react-navigation/native'; +import { useNavigation, StackActions } from '@react-navigation/native'; import { useAppThemeFromContext } from '../../../util/theme'; import { MetaMetricsEvents } from '../../../core/Analytics'; import generateDeviceAnalyticsMetaData from '../../../util/metrics'; import { SRP_GUIDE_URL } from '../../../constants/urls'; -import { StackNavigationProp } from '@react-navigation/stack'; import { useMetrics } from '../../../components/hooks/useMetrics'; import Logger from '../../../util/Logger'; @@ -36,9 +35,7 @@ const WalletRestored = () => { const { colors } = useAppThemeFromContext(); const { trackEvent, createEventBuilder } = useMetrics(); const styles = createStyles(colors); - // TODO: Replace "any" with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const navigation = useNavigation>(); + const navigation = useNavigation(); const deviceMetaData = useMemo(() => generateDeviceAnalyticsMetaData(), []); @@ -59,7 +56,7 @@ const WalletRestored = () => { Logger.error(error as Error, 'Failed to clear migration error flag'); } - navigation.replace(Routes.ONBOARDING.LOGIN); + navigation.dispatch(StackActions.replace(Routes.ONBOARDING.LOGIN)); }, [navigation]); const onPressBackupSRP = useCallback(async (): Promise => { diff --git a/app/components/Views/SocialLoginIosUser/index.test.tsx b/app/components/Views/SocialLoginIosUser/index.test.tsx index fc5647da656..798efac4145 100644 --- a/app/components/Views/SocialLoginIosUser/index.test.tsx +++ b/app/components/Views/SocialLoginIosUser/index.test.tsx @@ -20,6 +20,11 @@ const mockReplace = jest.fn(); const mockNavigation = { replace: mockReplace, + dispatch: jest.fn((action) => { + if (action.type === 'REPLACE') { + mockReplace(action.payload.name, action.payload.params); + } + }), }; jest.mock('@react-navigation/native', () => ({ diff --git a/app/components/Views/SocialLoginIosUser/index.tsx b/app/components/Views/SocialLoginIosUser/index.tsx index 2cf52c940b4..2d5791dca20 100644 --- a/app/components/Views/SocialLoginIosUser/index.tsx +++ b/app/components/Views/SocialLoginIosUser/index.tsx @@ -4,9 +4,8 @@ import { SafeAreaView } from 'react-native-safe-area-context'; import { useNavigation, useRoute, - ParamListBase, + StackActions, } from '@react-navigation/native'; -import { StackNavigationProp } from '@react-navigation/stack'; import LottieView, { AnimationObject } from 'lottie-react-native'; import Text, { TextVariant, @@ -30,7 +29,7 @@ interface SocialLoginIosUserProps { } const SocialLoginIosUser: React.FC = ({ type }) => { - const navigation = useNavigation>(); + const navigation = useNavigation(); const route = useRoute(); const { accountName, oauthLoginSuccess, onboardingTraceCtx, provider } = @@ -42,21 +41,25 @@ const SocialLoginIosUser: React.FC = ({ type }) => { }) || {}; const handleSetMetaMaskPin = () => { - navigation.replace(Routes.ONBOARDING.CHOOSE_PASSWORD, { - [PREVIOUS_SCREEN]: ONBOARDING, - oauthLoginSuccess, - onboardingTraceCtx, - accountName, - provider, - }); + navigation.dispatch( + StackActions.replace(Routes.ONBOARDING.CHOOSE_PASSWORD, { + [PREVIOUS_SCREEN]: ONBOARDING, + oauthLoginSuccess, + onboardingTraceCtx, + accountName, + provider, + }), + ); }; const handleSecureWallet = () => { - navigation.replace('Rehydrate', { - [PREVIOUS_SCREEN]: ONBOARDING, - oauthLoginSuccess: true, - onboardingTraceCtx, - }); + navigation.dispatch( + StackActions.replace('Rehydrate', { + [PREVIOUS_SCREEN]: ONBOARDING, + oauthLoginSuccess: true, + onboardingTraceCtx, + }), + ); }; const isUserTypeNew = type === 'new'; diff --git a/app/components/Views/WalletCreationError/SocialLoginErrorSheet.test.tsx b/app/components/Views/WalletCreationError/SocialLoginErrorSheet.test.tsx index ff36a43a329..d83ec802013 100644 --- a/app/components/Views/WalletCreationError/SocialLoginErrorSheet.test.tsx +++ b/app/components/Views/WalletCreationError/SocialLoginErrorSheet.test.tsx @@ -1,39 +1,45 @@ -import React from 'react'; -import { fireEvent } from '@testing-library/react-native'; -import { Linking, Image, SafeAreaView } from 'react-native'; +import React, { ComponentType } from 'react'; +import { Image, Linking } from 'react-native'; +import { fireEvent, waitFor } from '@testing-library/react-native'; import SocialLoginErrorSheet from './SocialLoginErrorSheet'; -import Routes from '../../../constants/navigation/Routes'; -import AppConstants from '../../../core/AppConstants'; import renderWithProvider from '../../../util/test/renderWithProvider'; +import { backgroundState } from '../../../util/test/initial-root-state'; +import { Authentication } from '../../../core'; +import AppConstants from '../../../core/AppConstants'; +import Routes from '../../../constants/navigation/Routes'; -const mockReset = jest.fn(); +// Type helper for UNSAFE_getAllByType with mocked string components +const asComponentType = (name: string) => name as unknown as ComponentType; -jest.mock('@react-navigation/native', () => { - const actualNav = jest.requireActual('@react-navigation/native'); - return { - ...actualNav, - useNavigation: () => ({ - reset: mockReset, - }), - }; -}); +const mockReset = jest.fn(); -jest.mock('react-native/Libraries/Linking/Linking', () => ({ - openURL: jest.fn(), - addEventListener: jest.fn(() => ({ remove: jest.fn() })), - getInitialURL: jest.fn(() => Promise.resolve(null)), +jest.mock('@react-navigation/native', () => ({ + ...jest.requireActual('@react-navigation/native'), + useNavigation: () => ({ + reset: mockReset, + }), })); jest.mock('../../../core', () => ({ Authentication: { - deleteWallet: jest.fn().mockResolvedValue(undefined), + deleteWallet: jest.fn(), }, })); -import { Authentication } from '../../../core'; +jest.mock('react-native/Libraries/Linking/Linking', () => ({ + openURL: jest.fn(), + addEventListener: jest.fn(() => ({ remove: jest.fn() })), + getInitialURL: jest.fn(() => Promise.resolve(null)), +})); describe('SocialLoginErrorSheet', () => { - const mockError = new Error('Test social login error'); + const initialState = { + engine: { + backgroundState: { + ...backgroundState, + }, + }, + }; beforeEach(() => { jest.clearAllMocks(); @@ -43,98 +49,99 @@ describe('SocialLoginErrorSheet', () => { jest.resetAllMocks(); }); - describe('rendering', () => { - it('renders title text', () => { - const { getByText } = renderWithProvider( - , - ); - - expect(getByText('Something went wrong')).toBeTruthy(); + it('renders error title', () => { + // Arrange & Act + const { getByText } = renderWithProvider(, { + state: initialState, }); - it('renders description text', () => { - const { getByText } = renderWithProvider( - , - ); - - expect( - getByText( - /An error occurred while creating your wallet\. Try again and if the issue persists, contact/, - ), - ).toBeTruthy(); - }); - - it('renders Try again button', () => { - const { getByText } = renderWithProvider( - , - ); - - expect(getByText('Try again')).toBeTruthy(); - }); - - it('renders MetaMask Support link', () => { - const { getByText } = renderWithProvider( - , - ); - - expect(getByText('MetaMask Support')).toBeTruthy(); - }); - - it('renders without error prop', () => { - const { getByText } = renderWithProvider(); + // Assert + expect(getByText('Something went wrong')).toBeOnTheScreen(); + }); - expect(getByText('Something went wrong')).toBeTruthy(); - expect(getByText('Try again')).toBeTruthy(); + it('renders try again button', () => { + // Arrange & Act + const { getByText } = renderWithProvider(, { + state: initialState, }); - it('renders Fox logo image', () => { - const { UNSAFE_getByType } = renderWithProvider( - , - ); + // Assert + expect(getByText('Try again')).toBeOnTheScreen(); + }); - const image = UNSAFE_getByType(Image); - expect(image).toBeTruthy(); + it('renders MetaMask Support link', () => { + // Arrange & Act + const { getByText } = renderWithProvider(, { + state: initialState, }); - it('renders SafeAreaView container', () => { - const { UNSAFE_getByType } = renderWithProvider( - , - ); - - const container = UNSAFE_getByType(SafeAreaView); - expect(container).toBeTruthy(); - }); + // Assert + expect(getByText('MetaMask Support')).toBeOnTheScreen(); }); - describe('handleTryAgain', () => { - it('deletes wallet and navigates to onboarding root when Try again is pressed', async () => { - const { getByText } = renderWithProvider( - , - ); - - fireEvent.press(getByText('Try again')); + it('deletes wallet and resets navigation when try again is pressed', async () => { + // Arrange + (Authentication.deleteWallet as jest.Mock).mockResolvedValue(undefined); + const { getByText } = renderWithProvider(, { + state: initialState, + }); + const tryAgainButton = getByText('Try again'); - // Wait for async deleteWallet to complete - await Promise.resolve(); + // Act + fireEvent.press(tryAgainButton); + // Assert + await waitFor(() => { expect(Authentication.deleteWallet).toHaveBeenCalled(); + }); + await waitFor(() => { expect(mockReset).toHaveBeenCalledWith({ routes: [{ name: Routes.ONBOARDING.ROOT_NAV }], }); }); }); - describe('handleContactSupport', () => { - it('opens support URL when MetaMask Support is pressed', () => { - const { getByText } = renderWithProvider( - , - ); + it('opens support URL when MetaMask Support is pressed', () => { + // Arrange + const { getByText } = renderWithProvider(, { + state: initialState, + }); + const supportLink = getByText('MetaMask Support'); + + // Act + fireEvent.press(supportLink); - fireEvent.press(getByText('MetaMask Support')); + // Assert + expect(Linking.openURL).toHaveBeenCalledWith( + AppConstants.REVIEW_PROMPT.SUPPORT, + ); + }); - expect(Linking.openURL).toHaveBeenCalledWith( - AppConstants.REVIEW_PROMPT.SUPPORT, - ); - }); + it('renders fox logo image', () => { + // Arrange & Act + const { UNSAFE_getAllByType } = renderWithProvider( + , + { + state: initialState, + }, + ); + + // Assert + const images = UNSAFE_getAllByType(Image); + expect(images.length).toBeGreaterThan(0); + }); + + it('renders danger icon', () => { + // Arrange & Act + const { UNSAFE_getAllByType } = renderWithProvider( + , + { + state: initialState, + }, + ); + + // Assert + const icons = UNSAFE_getAllByType(asComponentType('SvgMock')); + expect(icons.length).toBeGreaterThan(0); }); }); diff --git a/app/components/Views/WalletCreationError/SocialLoginErrorSheet.tsx b/app/components/Views/WalletCreationError/SocialLoginErrorSheet.tsx index 7b712628613..06bef2bb7bb 100644 --- a/app/components/Views/WalletCreationError/SocialLoginErrorSheet.tsx +++ b/app/components/Views/WalletCreationError/SocialLoginErrorSheet.tsx @@ -1,7 +1,6 @@ import React, { useCallback } from 'react'; import { View, Image, Linking, SafeAreaView } from 'react-native'; -import { useNavigation, ParamListBase } from '@react-navigation/native'; -import { StackNavigationProp } from '@react-navigation/stack'; +import { useNavigation } from '@react-navigation/native'; import Text, { TextVariant, @@ -34,7 +33,7 @@ interface SocialLoginErrorSheetProps { // eslint-disable-next-line @typescript-eslint/no-unused-vars const SocialLoginErrorSheet = ({ error }: SocialLoginErrorSheetProps) => { - const navigation = useNavigation>(); + const navigation = useNavigation(); const { styles } = useStyles(styleSheet, {}); const handleTryAgain = useCallback(async () => {