From d745beb5f1f2a5c661b653e24404db09493bf084 Mon Sep 17 00:00:00 2001 From: httpjunkie Date: Fri, 17 Jan 2025 10:51:20 -0500 Subject: [PATCH] fix: refactor `useSmartTransactionsEnabled` hook to use selectors instead. - Create new selectors file - Update the SmartTransactionsMigrationBanner component to use these selectors directly - Delete the hook file since its functionality would now be handled by selectors --- .../SmartTransactionsMigrationBanner.test.tsx | 83 +++++++++--- .../SmartTransactionsMigrationBanner.tsx | 29 ++++- .../useSmartTransactionsEnabled.test.ts | 118 ------------------ .../useSmartTransactionsEnabled.ts | 66 ---------- app/selectors/preferencesController.ts | 12 ++ 5 files changed, 101 insertions(+), 207 deletions(-) delete mode 100644 app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.test.ts delete mode 100644 app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.ts diff --git a/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.test.tsx b/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.test.tsx index c5eeb1b4bb5..f07482a0e42 100644 --- a/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.test.tsx +++ b/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.test.tsx @@ -1,53 +1,96 @@ import React from 'react'; import { render, fireEvent } from '@testing-library/react-native'; +import { Provider } from 'react-redux'; +import configureMockStore from 'redux-mock-store'; import SmartTransactionsMigrationBanner from './SmartTransactionsMigrationBanner'; -import useSmartTransactionsEnabled from '../../../../hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled'; +import Engine from '../../../../../core/Engine'; + +jest.mock('../../../../../core/Engine', () => ({ + context: { + PreferencesController: { + setFeatureFlag: jest.fn(), + }, + }, +})); -jest.mock('../../../../hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled'); jest.mock('../../../../../../locales/i18n', () => ({ strings: jest.fn((key) => key), })); describe('SmartTransactionsMigrationBanner', () => { - const mockDismissBanner = jest.fn(); + const mockStore = configureMockStore(); + const mockSetFeatureFlag = jest.mocked(Engine.context.PreferencesController.setFeatureFlag); + + const createMockState = (override = {}) => ({ + engine: { + backgroundState: { + PreferencesController: { + smartTransactionsOptInStatus: true, + smartTransactionsMigrationApplied: true, + featureFlags: { + smartTransactionsBannerDismissed: false, + }, + ...override, + }, + }, + }, + }); beforeEach(() => { - (useSmartTransactionsEnabled as jest.Mock).mockReturnValue({ - shouldShowBanner: true, - dismissBanner: mockDismissBanner, - }); - mockDismissBanner.mockClear(); + jest.clearAllMocks(); }); - it('renders nothing when shouldShowBanner is false', () => { - (useSmartTransactionsEnabled as jest.Mock).mockReturnValue({ - shouldShowBanner: false, - dismissBanner: mockDismissBanner, - }); + it('renders nothing when banner should be hidden', () => { + const store = mockStore(createMockState({ + featureFlags: { smartTransactionsBannerDismissed: true }, + })); - const { queryByTestId } = render(); + const { queryByTestId } = render( + + + + ); expect(queryByTestId('smart-transactions-enabled-banner')).toBeNull(); }); - it('renders banner when shouldShowBanner is true', () => { - const { getByTestId, getByText } = render(); + it('renders banner when conditions are met', () => { + const store = mockStore(createMockState()); + + const { getByTestId, getByText } = render( + + + + ); expect(getByTestId('smart-transactions-enabled-banner')).toBeDefined(); expect(getByText('smart_transactions_enabled.title')).toBeDefined(); expect(getByText('smart_transactions_enabled.link')).toBeDefined(); }); - it('calls dismissBanner when close button is pressed', () => { - const { getByTestId } = render(); + it('calls setFeatureFlag when close button is pressed', () => { + const store = mockStore(createMockState()); + + const { getByTestId } = render( + + + + ); fireEvent.press(getByTestId('banner-close-button-icon')); - expect(mockDismissBanner).toHaveBeenCalled(); + expect(mockSetFeatureFlag).toHaveBeenCalledWith( + 'smartTransactionsBannerDismissed', + true, + ); }); it('accepts and applies custom styles', () => { + const store = mockStore(createMockState()); const customStyle = { marginTop: 20 }; + const { getByTestId } = render( - , + + + ); const banner = getByTestId('smart-transactions-enabled-banner'); diff --git a/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.tsx b/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.tsx index 0fd4fbf1db6..e780a94e490 100644 --- a/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.tsx +++ b/app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.tsx @@ -1,4 +1,5 @@ -import React from 'react'; +import React, { useCallback, useMemo } from 'react'; +import { useSelector } from 'react-redux'; import { Linking } from 'react-native'; import BannerAlert from '../../../../../component-library/components/Banners/Banner/variants/BannerAlert/BannerAlert'; import { BannerAlertSeverity } from '../../../../../component-library/components/Banners/Banner/variants/BannerAlert/BannerAlert.types'; @@ -8,7 +9,13 @@ import Text from '../../../../../component-library/components/Texts/Text'; import { useStyles } from '../../../../../component-library/hooks/useStyles'; import styleSheet from './SmartTransactionsMigrationBanner.styles'; import { SmartTransactionsMigrationBannerProps } from './SmartTransactionsMigrationBanner.types'; -import useSmartTransactionsEnabled from '../../../../hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled'; +import Engine from '../../../../../core/Engine'; +import Logger from '../../../../../util/Logger'; +import { + selectSmartTransactionsMigrationApplied, + selectSmartTransactionsBannerDismissed, + selectSmartTransactionsOptInStatus +} from '../../../../../selectors/preferencesController'; const SMART_TRANSACTIONS_LEARN_MORE = AppConstants.URLS.SMART_TXS; @@ -16,7 +23,23 @@ const SmartTransactionsMigrationBanner = ({ style, }: SmartTransactionsMigrationBannerProps) => { const { styles } = useStyles(styleSheet, { style }); - const { shouldShowBanner, dismissBanner } = useSmartTransactionsEnabled(); + const isEnabled = useSelector(selectSmartTransactionsOptInStatus); + const isMigrationApplied = useSelector(selectSmartTransactionsMigrationApplied); + const isBannerDismissed = useSelector(selectSmartTransactionsBannerDismissed); + + const shouldShowBanner = useMemo( + () => isEnabled && isMigrationApplied && !isBannerDismissed, + [isEnabled, isMigrationApplied, isBannerDismissed] + ); + + const dismissBanner = useCallback(async () => { + try { + const { PreferencesController } = Engine.context; + PreferencesController.setFeatureFlag('smartTransactionsBannerDismissed', true); + } catch (error) { + Logger.error(error as Error, 'Failed to dismiss banner:'); + } + }, []); if (!shouldShowBanner) { return null; diff --git a/app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.test.ts b/app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.test.ts deleted file mode 100644 index 7c01b37a5f2..00000000000 --- a/app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.test.ts +++ /dev/null @@ -1,118 +0,0 @@ -import { act, renderHook } from '@testing-library/react-hooks'; -import { useSelector } from 'react-redux'; -import useSmartTransactionsEnabled, { type RootState } from './useSmartTransactionsEnabled'; -import Engine from '../../../core/Engine'; - -jest.mock('react-redux', () => ({ - useSelector: jest.fn((selector: (state: RootState) => unknown) => - selector({ - engine: { - backgroundState: { - PreferencesController: { - smartTransactionsOptInStatus: false, - smartTransactionsMigrationApplied: false, - featureFlags: {}, - }, - }, - }, - } as RootState), - ), -})); - -jest.mock('../../../core/Engine', () => ({ - context: { - PreferencesController: { - setFeatureFlag: jest.fn(), - }, - }, - default: { - context: { - PreferencesController: { - setFeatureFlag: jest.fn(), - }, - }, - }, -})); - -describe('useSmartTransactionsEnabled', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('returns false for all flags when preferences are undefined', () => { - const { result } = renderHook(() => useSmartTransactionsEnabled()); - - expect(result.current.isEnabled).toBe(false); - expect(result.current.isMigrationApplied).toBe(false); - expect(result.current.isBannerDismissed).toBe(false); - expect(result.current.shouldShowBanner).toBe(false); - }); - - it('returns correct values when preferences exist', () => { - (useSelector as jest.Mock).mockImplementation((selector: (state: RootState) => unknown) => - selector({ - engine: { - backgroundState: { - PreferencesController: { - smartTransactionsOptInStatus: true, - smartTransactionsMigrationApplied: true, - featureFlags: { - smartTransactionsBannerDismissed: false, - }, - }, - }, - }, - } as RootState), - ); - - const { result } = renderHook(() => useSmartTransactionsEnabled()); - - expect(result.current.isEnabled).toBe(true); - expect(result.current.isMigrationApplied).toBe(true); - expect(result.current.isBannerDismissed).toBe(false); - expect(result.current.shouldShowBanner).toBe(true); - }); - - it('shouldShowBanner returns false when banner is dismissed', () => { - (useSelector as jest.Mock).mockImplementation((selector: (state: RootState) => unknown) => - selector({ - engine: { - backgroundState: { - PreferencesController: { - smartTransactionsOptInStatus: true, - smartTransactionsMigrationApplied: true, - featureFlags: { - smartTransactionsBannerDismissed: true, - }, - }, - }, - }, - } as RootState), - ); - - const { result } = renderHook(() => useSmartTransactionsEnabled()); - expect(result.current.shouldShowBanner).toBe(false); - }); - - it('dismissBanner updates preferences through setFeatureFlag', async () => { - const { result } = renderHook(() => useSmartTransactionsEnabled()); - - // Verify mock is clean at start - expect(jest.mocked(Engine.context.PreferencesController.setFeatureFlag)).not.toHaveBeenCalled(); - - await act(async () => { - await result.current.dismissBanner(); - }); - - // Verify mock was called exactly once - expect(jest.mocked(Engine.context.PreferencesController.setFeatureFlag)).toHaveBeenCalledTimes(1); - - // Verify correct arguments - expect( - jest.mocked(Engine.context.PreferencesController.setFeatureFlag) - ).toHaveBeenCalledWith( - 'smartTransactionsBannerDismissed', - true, - ); - }); -}); diff --git a/app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.ts b/app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.ts deleted file mode 100644 index ea11dd46faf..00000000000 --- a/app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { useCallback, useEffect, useMemo } from 'react'; -import { useSelector } from 'react-redux'; -import Engine from '../../../core/Engine'; -import Logger from '../../../util/Logger'; - -// non-optional types -interface PreferencesState { - smartTransactionsOptInStatus: boolean; - smartTransactionsMigrationApplied: boolean; - featureFlags: { - smartTransactionsBannerDismissed: boolean; - [key: string]: boolean | undefined; - }; -} - -export interface RootState { - engine: { - backgroundState: { - PreferencesController: PreferencesState; - }; - }; -} - -const useSmartTransactionsEnabled = () => { - const isEnabled = useSelector( - (state: RootState) => - state.engine.backgroundState.PreferencesController - .smartTransactionsOptInStatus ?? false, - ); - - const isMigrationApplied = useSelector( - (state: RootState) => - state.engine.backgroundState.PreferencesController - .smartTransactionsMigrationApplied ?? false, - ); - - const isBannerDismissed = useSelector( - (state: RootState) => - state.engine.backgroundState.PreferencesController.featureFlags?.smartTransactionsBannerDismissed - ?? false, - ); - - const dismissBanner = useCallback(async () => { - try { - const { PreferencesController } = Engine.context; - PreferencesController.setFeatureFlag('smartTransactionsBannerDismissed', true); - } catch (error) { - Logger.error(error as Error, 'Failed to dismiss banner:'); - } - }, []); - - const shouldShowBanner = useMemo( - () => isEnabled && isMigrationApplied && !isBannerDismissed, - [isEnabled, isMigrationApplied, isBannerDismissed] - ); - - return { - isEnabled, - isMigrationApplied, - isBannerDismissed, - shouldShowBanner, - dismissBanner, - }; -}; - -export default useSmartTransactionsEnabled; diff --git a/app/selectors/preferencesController.ts b/app/selectors/preferencesController.ts index 66558c9b4fa..174509f5a00 100644 --- a/app/selectors/preferencesController.ts +++ b/app/selectors/preferencesController.ts @@ -146,3 +146,15 @@ export const selectPrivacyMode = createSelector( (preferencesControllerState: PreferencesState) => preferencesControllerState.privacyMode, ); + +export const selectSmartTransactionsMigrationApplied = createSelector( + selectPreferencesControllerState, + (preferencesControllerState: PreferencesState) => + preferencesControllerState.smartTransactionsMigrationApplied ?? false, +); + +export const selectSmartTransactionsBannerDismissed = createSelector( + selectPreferencesControllerState, + (preferencesControllerState: PreferencesState) => + preferencesControllerState.featureFlags?.smartTransactionsBannerDismissed ?? false, +);