From 9c84627ea8fd82308ef8760769dd3c5a37fde5ed Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 24 Sep 2025 11:36:37 +0100 Subject: [PATCH 1/8] feat: update notification account settings to use BIP-44 designs we still need to update tests and styles, which I'll get to later --- .../AccountCell/AccountCell.tsx | 44 ++++--- .../AccountListHeader/AccountListHeader.tsx | 30 ++--- .../AccountListHeader.types.ts | 3 + .../AccountsList.hooks.tsx | 115 ++++++++++++------ .../NotificationsSettings/AccountsList.tsx | 28 +++-- .../NotificationOptionToggle/index.tsx | 100 ++++++--------- .../NotificationOptionToggle/styles.ts | 2 +- .../Settings/NotificationsSettings/index.tsx | 26 ++-- 8 files changed, 194 insertions(+), 154 deletions(-) diff --git a/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx b/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx index 78d7b21c71e8..ec53205b5761 100644 --- a/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx +++ b/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx @@ -30,6 +30,7 @@ interface AccountCellProps { isSelected: boolean; hideMenu?: boolean; startAccessory?: React.ReactNode; + endContainer?: React.ReactNode; } const AccountCell = ({ @@ -38,6 +39,7 @@ const AccountCell = ({ isSelected, hideMenu = false, startAccessory, + endContainer, }: AccountCellProps) => { const { styles } = useStyles(styleSheet, { isSelected }); const { navigate } = useNavigation(); @@ -109,25 +111,29 @@ const AccountCell = ({ )} - - {displayBalance} - - {!hideMenu && ( - - - + {endContainer || ( + <> + + {displayBalance} + + {!hideMenu && ( + + + + )} + )} diff --git a/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.tsx b/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.tsx index b534ca399efe..c72ec89700e3 100644 --- a/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.tsx +++ b/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.tsx @@ -9,21 +9,23 @@ import Text, { import createStyles from '../MultichainAccountSelectorList.styles'; import { AccountListHeaderProps } from './AccountListHeader.types'; -const AccountListHeader = memo(({ title }: AccountListHeaderProps) => { - const { styles } = useStyles(createStyles, {}); +const AccountListHeader = memo( + ({ title, containerStyle }: AccountListHeaderProps) => { + const { styles } = useStyles(createStyles, {}); - return ( - - - {title} - - - ); -}); + return ( + + + {title} + + + ); + }, +); AccountListHeader.displayName = 'AccountListHeader'; diff --git a/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.types.ts b/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.types.ts index 8dc8ed707f51..35ef604221af 100644 --- a/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.types.ts +++ b/app/component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader/AccountListHeader.types.ts @@ -1,3 +1,6 @@ +import { ViewStyle } from 'react-native'; + export interface AccountListHeaderProps { title: string; + containerStyle?: ViewStyle; } diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx index e5994aa1b382..14792d2b5219 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx @@ -1,65 +1,104 @@ -import { useCallback, useMemo } from 'react'; +import { useCallback } from 'react'; import { useSelector } from 'react-redux'; -import { useAccounts } from '../../../hooks/useAccounts'; import { useFetchAccountNotifications } from '../../../../util/notifications/hooks/useSwitchNotifications'; import { getValidNotificationAccounts } from '../../../../selectors/notifications'; -import { - areAddressesEqual, - toFormattedAddress, -} from '../../../../util/address'; +import { toFormattedAddress } from '../../../../util/address'; import { selectAvatarAccountType } from '../../../../selectors/settings'; +import { selectAccountGroupsByWallet } from '../../../../selectors/multichainAccounts/accountTreeController'; +import { AccountWalletType } from '@metamask/account-api'; +import { selectInternalAccountsById } from '../../../../selectors/accountsController'; +import { isEvmAccountType } from '@metamask/keyring-api'; -export function useNotificationAccountListProps(addresses: string[]) { +export function useNotificationAccountListProps() { + const accountAddresses = useSelector(getValidNotificationAccounts); + const accountsMap = useSelector(selectInternalAccountsById); const { update, initialLoading, accountsBeingUpdated, data } = - useFetchAccountNotifications(addresses); + useFetchAccountNotifications(accountAddresses); - // Loading is determined on initial fetch or if any account is being updated - const updatingAccounts = accountsBeingUpdated.length > 0; - const isAnyAccountLoading = initialLoading || updatingAccounts; + const isAnyAccountLoading = initialLoading || accountsBeingUpdated.length > 0; const refetchAccountSettings = useCallback(async () => { - await update(addresses); - }, [addresses, update]); + await update(accountAddresses); + }, [accountAddresses, update]); + + // Helper to get addresses from account IDs + const getEvmAddressesFromAccountIds = useCallback( + (accountIds: string[]) => + accountIds + .filter( + (id) => + Boolean(accountsMap?.[id]?.address) && + isEvmAccountType(accountsMap[id].type), + ) + .map((id) => accountsMap[id].address), + [accountsMap], + ); + + // Helper to normalize address lookup in data + const isAddressEnabled = useCallback( + (address: string) => + data?.[toFormattedAddress(address)] ?? + data?.[address.toLowerCase()] ?? + false, + [data], + ); + + const isAccountLoading = useCallback( + (accountIds: string[]) => { + const addresses = getEvmAddressesFromAccountIds(accountIds); + return accountsBeingUpdated.some((updatingAddr) => + addresses.some( + (addr) => + toFormattedAddress(updatingAddr) === toFormattedAddress(addr), + ), + ); + }, + [accountsBeingUpdated, getEvmAddressesFromAccountIds], + ); - const isAccountLoading = (address: string) => - accountsBeingUpdated.some( - (addr) => toFormattedAddress(addr) === toFormattedAddress(address), - ); + const isAccountEnabled = useCallback( + (accountIds: string[]) => { + const addresses = getEvmAddressesFromAccountIds(accountIds); + return addresses.some(isAddressEnabled); + }, + [getEvmAddressesFromAccountIds, isAddressEnabled], + ); - const isAccountEnabled = (address: string) => - data?.[toFormattedAddress(address)] ?? - data?.[address.toLowerCase()] ?? // fallback to lowercase address lookup - false; + const getEvmAddress = useCallback( + (accountIds: string[]) => { + const addresses = getEvmAddressesFromAccountIds(accountIds); + return addresses.at(0); // get first evm address - keyring only contains 1 EVM address + }, + [getEvmAddressesFromAccountIds], + ); return { isAnyAccountLoading, refetchAccountSettings, isAccountLoading, isAccountEnabled, + getEvmAddress, }; } -export function useAccountProps() { - const accountAddresses = useSelector(getValidNotificationAccounts); - const { accounts: allAccounts } = useAccounts(); - const accountAvatarType = useSelector(selectAvatarAccountType); +export function useFirstHDWalletAccounts() { + const accountGroupsByWallet = useSelector(selectAccountGroupsByWallet); - const accounts = useMemo( - () => - accountAddresses - .map((addr) => { - const account = allAccounts.find((a) => - areAddressesEqual(a.address, addr), - ); - return account; - }) - .filter((val: T | undefined): val is T => Boolean(val)), - [accountAddresses, allAccounts], + // TODO - do we have a reliable way of receiving the first HD wallet? + // Notifications only support the first HD wallet (due to backend limitations) + // This limitation will most likely be removed in near future + const firstHDWalletGroup = accountGroupsByWallet.find( + (w) => w.wallet.type === AccountWalletType.Entropy, ); + return firstHDWalletGroup; +} + +export function useAccountProps() { + const firstHDWalletGroups = useFirstHDWalletAccounts(); + const accountAvatarType = useSelector(selectAvatarAccountType); return { - accounts, + firstHDWalletGroups, accountAvatarType, - accountAddresses, }; } diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx index 5255a9b271d2..ab58ecee9ad6 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx @@ -6,34 +6,40 @@ import { } from './AccountsList.hooks'; import NotificationOptionToggle from './NotificationOptionToggle'; import { NotificationSettingsViewSelectorsIDs } from '../../../../../e2e/selectors/Notifications/NotificationSettingsView.selectors'; -import { toFormattedAddress } from '../../../../util/address'; +import AccountListHeader from '../../../../component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader'; export const AccountsList = () => { - const { accounts, accountAddresses, accountAvatarType } = useAccountProps(); + const { accountAvatarType, firstHDWalletGroups } = useAccountProps(); const { isAnyAccountLoading, isAccountLoading, isAccountEnabled, refetchAccountSettings, - } = useNotificationAccountListProps(accountAddresses); + getEvmAddress, + } = useNotificationAccountListProps(); + + if (!firstHDWalletGroups) { + return null; + } return ( + `address-${item.address}`} + data={firstHDWalletGroups.data} + keyExtractor={(item) => `address-${item.id}`} renderItem={({ item }) => ( )} diff --git a/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/index.tsx b/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/index.tsx index 24893b6f9d47..c995e83da315 100644 --- a/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/index.tsx +++ b/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/index.tsx @@ -3,21 +3,11 @@ import { ActivityIndicator, Switch, View } from 'react-native'; import { useMetrics } from '../../../../../components/hooks/useMetrics'; import { MetaMetricsEvents } from '../../../../../core/Analytics/MetaMetrics.events'; import { createStyles } from './styles'; -import Text, { - TextColor, - TextVariant, -} from '../../../../../component-library/components/Texts/Text'; import { useTheme } from '../../../../../util/theme'; -import { - AvatarSize, - AvatarVariant, -} from '../../../../../component-library/components/Avatars/Avatar/Avatar.types'; -import Avatar, { - AvatarAccountType, -} from '../../../../../component-library/components/Avatars/Avatar'; -import { formatAddress } from '../../../../../util/address'; -import { IconName } from '../../../../../component-library/components/Icons/Icon'; +import { AvatarAccountType } from '../../../../../component-library/components/Avatars/Avatar'; import { useAccountNotificationsToggle } from '../../../../../util/notifications/hooks/useSwitchNotifications'; +import { AccountGroupObject } from '@metamask/account-tree-controller'; +import AccountCell from '../../../../../component-library/components-temp/MultichainAccounts/AccountCell'; const NOTIFICATION_OPTIONS_TOGGLE_SWITCH_TEST_ID = 'notification_options_toggle_switch'; @@ -29,9 +19,9 @@ export const NOTIFICATION_OPTIONS_TOGGLE_LOADING_TEST_ID = ( ) => `${testID}:notification_options_toggle--loading`; interface NotificationOptionsToggleProps { - address: string; - title: string; - icon?: AvatarAccountType | IconName; + item: AccountGroupObject; + evmAddress?: string; + icon: AvatarAccountType; testId?: string; disabledSwitch?: boolean; isLoading?: boolean; @@ -41,7 +31,7 @@ interface NotificationOptionsToggleProps { } export function useUpdateAccountSetting( - address: string, + address: string | undefined, refetchAccountSettings: () => Promise, ) { const { onToggle, error } = useAccountNotificationsToggle(); @@ -51,6 +41,9 @@ export function useUpdateAccountSetting( const toggleAccount = useCallback( async (state: boolean) => { + if (!address) { + return; + } setLoading(true); try { await onToggle([address], state); @@ -71,8 +64,8 @@ export function useUpdateAccountSetting( * This component assumes that the parent will manage the state of the toggle. This is because most of the state is global. */ const NotificationOptionToggle = ({ - address, - title, + item, + evmAddress, icon, isEnabled, disabledSwitch, @@ -86,7 +79,7 @@ const NotificationOptionToggle = ({ const { trackEvent, createEventBuilder } = useMetrics(); const { toggleAccount, loading: isUpdatingAccount } = useUpdateAccountSetting( - address, + evmAddress, refetchNotificationAccounts, ); @@ -106,48 +99,33 @@ const NotificationOptionToggle = ({ }, [isEnabled, toggleAccount, trackEvent, createEventBuilder]); return ( - - + + ) : ( + + ) + } /> - - - {title} - - {Boolean(address) && ( - - {formatAddress(address, 'short')} - - )} - - - {loading ? ( - - ) : ( - - )} - ); }; diff --git a/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts b/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts index 0b17df4ccb48..aa3e2a7a6a2a 100644 --- a/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts +++ b/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts @@ -16,7 +16,7 @@ export const createStyles = () => flex: 1, }, switchElement: { - marginLeft: 16, + // marginLeft: 16, }, switch: { alignSelf: 'flex-start', diff --git a/app/components/Views/Settings/NotificationsSettings/index.tsx b/app/components/Views/Settings/NotificationsSettings/index.tsx index f6b64ed393cb..8b5af87676d1 100644 --- a/app/components/Views/Settings/NotificationsSettings/index.tsx +++ b/app/components/Views/Settings/NotificationsSettings/index.tsx @@ -31,6 +31,7 @@ import styleSheet, { import { ResetNotificationsButton } from './ResetNotificationsButton'; import SessionHeader from './sectionHeader'; import { PushNotificationToggle } from './PushNotificationToggle'; +import { useFirstHDWalletAccounts } from './AccountsList.hooks'; const NotificationsSettings = ({ navigation, route }: Props) => { const theme = useTheme(); @@ -38,6 +39,7 @@ const NotificationsSettings = ({ navigation, route }: Props) => { const isMetamaskNotificationsEnabled = useSelector( selectIsMetamaskNotificationsEnabled, ); + const hasFirstHDWallet = Boolean(useFirstHDWalletAccounts()); const loadingText = useSwitchNotificationLoadingText(); @@ -83,16 +85,20 @@ const NotificationsSettings = ({ navigation, route }: Props) => { {/* Account Notification Toggles */} - - + {hasFirstHDWallet && ( + <> + + + + )} {/* Reset Notifications Button */} From 926eab185affd3a741bb87242ed4261e7a569823 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 24 Sep 2025 14:16:50 +0100 Subject: [PATCH 2/8] test: fix UTs --- .../AccountCell/AccountCell.tsx | 87 ++++---- .../AccountsList.hooks.test.tsx | 204 ++++++++++++++---- .../AccountsList.hooks.tsx | 3 +- .../AccountsList.test.tsx | 96 +++++++-- .../NotificationsSettings/AccountsList.tsx | 2 +- 5 files changed, 299 insertions(+), 93 deletions(-) diff --git a/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx b/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx index ec53205b5761..ed6e683d51fd 100644 --- a/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx +++ b/app/component-library/components-temp/MultichainAccounts/AccountCell/AccountCell.tsx @@ -33,14 +33,10 @@ interface AccountCellProps { endContainer?: React.ReactNode; } -const AccountCell = ({ - accountGroup, - avatarAccountType, - isSelected, - hideMenu = false, - startAccessory, - endContainer, -}: AccountCellProps) => { +const BalanceEndContainer = ( + props: Pick, +) => { + const { accountGroup, hideMenu, isSelected } = props; const { styles } = useStyles(styleSheet, { isSelected }); const { navigate } = useNavigation(); @@ -56,12 +52,6 @@ const AccountCell = ({ const totalBalance = groupBalance?.totalBalanceInUserCurrency; const userCurrency = groupBalance?.userCurrency; - const selectEvmAddress = useMemo( - () => selectIconSeedAddressByAccountGroupId(accountGroup.id), - [accountGroup.id], - ); - const evmAddress = useSelector(selectEvmAddress); - const displayBalance = useMemo(() => { if (totalBalance == null || !userCurrency) { return undefined; @@ -72,6 +62,48 @@ const AccountCell = ({ }); }, [totalBalance, userCurrency]); + return ( + <> + + {displayBalance} + + {!hideMenu && ( + + + + )} + + ); +}; + +const AccountCell = ({ + accountGroup, + avatarAccountType, + isSelected, + hideMenu = false, + startAccessory, + endContainer, +}: AccountCellProps) => { + const { styles } = useStyles(styleSheet, { isSelected }); + + const selectEvmAddress = useMemo( + () => selectIconSeedAddressByAccountGroupId(accountGroup.id), + [accountGroup.id], + ); + const evmAddress = useSelector(selectEvmAddress); + return ( {endContainer || ( - <> - - {displayBalance} - - {!hideMenu && ( - - - - )} - + )} diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.test.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.test.tsx index 6d20f5b7760a..f29be518155a 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.test.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.test.tsx @@ -5,42 +5,140 @@ import { AvatarAccountType } from '../../../../component-library/components/Avat import * as UseSwitchNotificationsModule from '../../../../util/notifications/hooks/useSwitchNotifications'; import { renderHookWithProvider } from '../../../../util/test/renderWithProvider'; // eslint-disable-next-line import/no-namespace -import { useAccounts } from '../../../hooks/useAccounts'; import { getValidNotificationAccounts } from '../../../../selectors/notifications'; import { useAccountProps, useNotificationAccountListProps, } from './AccountsList.hooks'; +import { selectInternalAccountsById } from '../../../../selectors/accountsController'; +import { InternalAccount } from '@metamask/keyring-internal-api'; +import { selectAccountGroupsByWallet } from '../../../../selectors/multichainAccounts/accountTreeController'; +import { AccountGroupType, AccountWalletType } from '@metamask/account-api'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type MockVar = any; - -jest.mock('../../../hooks/useAccounts', () => ({ - useAccounts: jest.fn(), -})); jest.mock('../../../../selectors/notifications', () => ({ getValidNotificationAccounts: jest.fn(), })); +jest.mock('../../../../selectors/accountsController', () => ({ + ...jest.requireActual('../../../../selectors/accountsController'), + selectInternalAccountsById: jest.fn(), +})); +jest.mock( + '../../../../selectors/multichainAccounts/accountTreeController', + () => ({ + ...jest.requireActual( + '../../../../selectors/multichainAccounts/accountTreeController', + ), + selectAccountGroupsByWallet: jest.fn(), + }), +); const arrangeMockUseAccounts = () => { - const createAccounts = (addresses: string[]) => - addresses.map((address) => ({ address })); - const mockUseAccounts = jest.mocked(useAccounts).mockReturnValue({ - accounts: createAccounts(['0x123', '0x456']), - } as MockVar); - const mockGetNotificationAccounts = jest - .mocked(getValidNotificationAccounts) - .mockReturnValue(['0x123', '0x456']); + const createMockAccountGroup = ( + idx: number, + accounts: [string, ...string[]], + ) => + ({ + accounts, + id: `entropy:111/${idx}`, + type: AccountGroupType.MultichainAccount, + metadata: { + entropy: { + groupIndex: idx, + }, + hidden: false, + name: `Account ${idx}`, + pinned: false, + }, + } as const); + + const group1 = createMockAccountGroup(0, [ + `MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a29`, + 'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGRY', + ]); + const group2 = createMockAccountGroup(1, [ + `MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8EC`, + 'MOCK-ID-FOR-Agsjd8HjGH5DxiXLMWc8fR4jjgHhvJG3TXcCpc1ieD9B', + ]); + const group3 = createMockAccountGroup(0, [ + `MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a20`, + 'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGR0', + ]); + const group4 = createMockAccountGroup(1, [ + `MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8E0`, + 'MOCK-ID-FOR-Agsjd8HjGH5DxiXLMWc8fR4jjgHhvJG3TXcCpc1ieD90', + ]); + + const mockSelectoWallets = jest + .mocked(selectAccountGroupsByWallet) + .mockReturnValue([ + { + title: 'Wallet 1', + wallet: { + id: 'entropy:wallet-1', + type: AccountWalletType.Entropy, + metadata: { + entropy: { + id: '', + }, + name: 'Wallet 1', + }, + status: 'ready', + groups: { + [group1.id]: group1, + [group2.id]: group2, + }, + }, + data: [group1, group2], + }, + { + title: 'Wallet 2', + wallet: { + id: 'entropy:wallet-2', + type: AccountWalletType.Entropy, + metadata: { + entropy: { + id: '', + }, + name: 'Wallet 2', + }, + status: 'ready', + groups: { + [group3.id]: group3, + [group4.id]: group4, + }, + }, + data: [group3, group4], + }, + ]); return { - createAccounts, - mockUseAccounts, - mockGetNotificationAccounts, + mockSelectoWallets, }; }; describe('useNotificationAccountListProps', () => { - const arrangeMocks = () => { + const arrangeMocks = (addresses: string[]) => { + const mockGetNotificationAccounts = jest + .mocked(getValidNotificationAccounts) + .mockReturnValue(addresses); + + const createInternalAccount = (address: string): InternalAccount => + ({ + id: `id-${address}`, + address, + type: 'eip155:eoa', + } as InternalAccount); + + const accountsMap: Record = {}; + addresses.forEach((addr) => { + const account = createInternalAccount(addr); + accountsMap[account.id] = account; + }); + + const mockSelectInternalAccountsById = jest + .mocked(selectInternalAccountsById) + .mockReturnValue(accountsMap); + const mockUpdate = jest.fn(); const createUseFetchAccountNotificationsReturn = () => ({ accountsBeingUpdated: [], @@ -54,6 +152,8 @@ describe('useNotificationAccountListProps', () => { .mockReturnValue(createUseFetchAccountNotificationsReturn()); return { + mockSelectInternalAccountsById, + mockGetNotificationAccounts, mockUpdate, createUseFetchAccountNotificationsReturn, mockUseFetchAccountNotifications, @@ -64,14 +164,17 @@ describe('useNotificationAccountListProps', () => { addresses: string[], mutateMocks?: (m: ReturnType) => void, ) => { - const mocks = arrangeMocks(); + const mocks = arrangeMocks(addresses); mutateMocks?.(mocks); const hook = renderHookWithProvider(() => - useNotificationAccountListProps(addresses), + useNotificationAccountListProps(), ); return { mocks, hook }; }; + + beforeEach(() => jest.clearAllMocks()); + it('returns correct loading state', async () => { const addresses = ['0x123', '0x456']; const { hook } = arrange(addresses); @@ -86,8 +189,8 @@ describe('useNotificationAccountListProps', () => { accountsBeingUpdated: ['0x123'], }); }); - expect(hook.result.current.isAccountLoading('0x123')).toBe(true); - expect(hook.result.current.isAccountLoading('0x456')).toBe(false); + expect(hook.result.current.isAccountLoading(['id-0x123'])).toBe(true); + expect(hook.result.current.isAccountLoading(['id-0x456'])).toBe(false); }); it('returns correct account enabled state', async () => { @@ -99,12 +202,16 @@ describe('useNotificationAccountListProps', () => { data: { '0x123': true, '0x456': false }, }); }); - expect(hook.result.current.isAccountEnabled('0x123')).toBe(true); - expect(hook.result.current.isAccountEnabled('0x456')).toBe(false); + expect(hook.result.current.isAccountEnabled(['id-0x123'])).toBe(true); + expect(hook.result.current.isAccountEnabled(['id-0x456'])).toBe(false); }); it('handles account loading with different address formats', async () => { - const addresses = ['0x123', '0x456']; + const addresses = [ + '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', + '0x123', + '0x456', + ]; const { hook } = arrange(addresses, (m) => { m.mockUseFetchAccountNotifications.mockReturnValue({ ...m.createUseFetchAccountNotificationsReturn(), @@ -114,15 +221,19 @@ describe('useNotificationAccountListProps', () => { }); // Should match even when checking with different case expect( - hook.result.current.isAccountLoading( - '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', - ), + hook.result.current.isAccountLoading([ + 'id-0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', + ]), ).toBe(true); - expect(hook.result.current.isAccountLoading('0x456')).toBe(false); + expect(hook.result.current.isAccountLoading(['id-0x456'])).toBe(false); }); it('returns account enabled state with formatted address keys', async () => { - const addresses = ['0x123', '0x456']; + const addresses = [ + '0xc4955c0d639d99699bfd7ec54d9fafee40e4d272', + '0x123', + '0x456', + ]; const { hook } = arrange(addresses, (m) => { m.mockUseFetchAccountNotifications.mockReturnValue({ ...m.createUseFetchAccountNotificationsReturn(), @@ -132,14 +243,18 @@ describe('useNotificationAccountListProps', () => { }); // Should find the account even with different case input expect( - hook.result.current.isAccountEnabled( - '0xc4955c0d639d99699bfd7ec54d9fafee40e4d272', - ), + hook.result.current.isAccountEnabled([ + 'id-0xc4955c0d639d99699bfd7ec54d9fafee40e4d272', + ]), ).toBe(true); }); it('falls back to lowercase address when formatted address not found', async () => { - const addresses = ['0x123', '0x456']; + const addresses = [ + '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', + '0x123', + '0x456', + ]; const { hook } = arrange(addresses, (m) => { m.mockUseFetchAccountNotifications.mockReturnValue({ ...m.createUseFetchAccountNotificationsReturn(), @@ -149,9 +264,9 @@ describe('useNotificationAccountListProps', () => { }); // Should find the account using lowercase fallback expect( - hook.result.current.isAccountEnabled( - '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', - ), + hook.result.current.isAccountEnabled([ + 'id-0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272', + ]), ).toBe(true); }); @@ -164,7 +279,7 @@ describe('useNotificationAccountListProps', () => { }); }); // Should return false when address not found - expect(hook.result.current.isAccountEnabled('0x999')).toBe(false); + expect(hook.result.current.isAccountEnabled(['id-0x999'])).toBe(false); }); it('refetches account settings', async () => { @@ -201,8 +316,15 @@ describe('useAccountProps', () => { state: mocks.mockStore(), }); - expect(result.current.accounts).toHaveLength(2); expect(result.current.accountAvatarType).toBe(AvatarAccountType.Maskicon); - expect(result.current.accountAddresses).toEqual(['0x123', '0x456']); + expect(result.current.firstHDWalletGroups).toStrictEqual( + expect.objectContaining({ + title: 'Wallet 1', + wallet: expect.objectContaining({ + id: 'entropy:wallet-1', + type: AccountWalletType.Entropy, + }), + }), + ); }); }); diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx index 14792d2b5219..b10936f70f62 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx @@ -67,7 +67,8 @@ export function useNotificationAccountListProps() { const getEvmAddress = useCallback( (accountIds: string[]) => { const addresses = getEvmAddressesFromAccountIds(accountIds); - return addresses.at(0); // get first evm address - keyring only contains 1 EVM address + const address = addresses.at(0); // get first evm address - keyring only contains 1 EVM address + return address && toFormattedAddress(address); }, [getEvmAddressesFromAccountIds], ); diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx index 142a89e0c022..30ab76594c7d 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { fireEvent, render, waitFor } from '@testing-library/react-native'; +import { fireEvent, waitFor } from '@testing-library/react-native'; import { AccountsList } from './AccountsList'; // eslint-disable-next-line import/no-namespace import * as AccountListHooksModule from './AccountsList.hooks'; @@ -12,16 +12,23 @@ import { } from './NotificationOptionToggle'; import { NotificationSettingsViewSelectorsIDs } from '../../../../../e2e/selectors/Notifications/NotificationSettingsView.selectors'; import { toFormattedAddress } from '../../../../util/address'; +import { AccountGroupType, AccountWalletType } from '@metamask/account-api'; +import renderWithProvider from '../../../../util/test/renderWithProvider'; +// eslint-disable-next-line import/no-namespace +import * as AccountSelectorsModule from '../../../../selectors/multichainAccounts/accounts'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type MockVar = any; -jest.mock( - '../../../../util/notifications/hooks/useSwitchNotifications', - () => ({ - useAccountNotificationsToggle: jest.fn(), - }), -); +jest.mock('@react-navigation/native', () => { + const actual = jest.requireActual('@react-navigation/native'); + return { + ...actual, + useNavigation: () => ({ + navigate: jest.fn(), + }), + }; +}); const ADDRESS_1 = '0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a29'.toLowerCase(); const ADDRESS_2 = '0x700CcD8172BC3807D893883a730A1E0E6630F8EC'.toLowerCase(); @@ -62,19 +69,71 @@ const ACCOUNT_2_TEST_ID = { }; describe('AccountList', () => { + const arrangeSelectors = () => { + jest + .spyOn(AccountSelectorsModule, 'selectIconSeedAddressByAccountGroupId') + .mockReturnValue((() => ADDRESS_1) as MockVar); + }; + const arrangeMocks = () => { + arrangeSelectors(); + const createMockAccounts = (addresses: string[]) => addresses.map((address, idx) => ({ address, name: `My Account ${idx}`, })); + const createMockAccountGroup = ( + idx: number, + accounts: [string, ...string[]], + ) => + ({ + accounts, + id: `entropy:111/${idx}`, + type: AccountGroupType.MultichainAccount, + metadata: { + entropy: { + groupIndex: idx, + }, + hidden: false, + name: `Account ${idx}`, + pinned: false, + }, + } as const); + + const group1 = createMockAccountGroup(0, [ + `MOCK-ID-FOR-${CHECKSUMMED_ADDRESS_1}`, + 'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGRY', + ]); + const group2 = createMockAccountGroup(1, [ + `MOCK-ID-FOR-${CHECKSUMMED_ADDRESS_2}`, + 'MOCK-ID-FOR-Agsjd8HjGH5DxiXLMWc8fR4jjgHhvJG3TXcCpc1ieD9B', + ]); + const mockUseAccountProps = jest .spyOn(AccountListHooksModule, 'useAccountProps') .mockReturnValue({ - accounts: createMockAccounts([ADDRESS_1, ADDRESS_2]) as MockVar, accountAvatarType: AvatarAccountType.JazzIcon, - accountAddresses: [ADDRESS_1, ADDRESS_2], + firstHDWalletGroups: { + title: 'Wallet 1', + wallet: { + id: 'entropy:wallet-1', + type: AccountWalletType.Entropy, + metadata: { + entropy: { + id: '', + }, + name: 'Wallet 1', + }, + status: 'ready', + groups: { + [group1.id]: group1, + [group2.id]: group2, + }, + }, + data: [group1, group2], + }, }); const mockRefetchAccountSettings = jest.fn(); @@ -83,10 +142,19 @@ describe('AccountList', () => { refetchAccountSettings: mockRefetchAccountSettings, isAccountLoading: jest .fn() - .mockImplementation((address) => address === ADDRESS_1), + .mockImplementation((accountIds: string[]) => + accountIds.includes(`MOCK-ID-FOR-${CHECKSUMMED_ADDRESS_1}`), + ), isAccountEnabled: jest .fn() - .mockImplementation((address) => address === ADDRESS_1), + .mockImplementation((accountIds: string[]) => + accountIds.includes(`MOCK-ID-FOR-${CHECKSUMMED_ADDRESS_1}`), + ), + getEvmAddress: jest + .fn() + .mockImplementation((accountIds: string) => + accountIds.at(0)?.replace('MOCK-ID-FOR-', ''), + ), }); const mockUseNotificationAccountListProps = jest .spyOn(AccountListHooksModule, 'useNotificationAccountListProps') @@ -114,7 +182,7 @@ describe('AccountList', () => { it('renders correctly', async () => { arrangeMocks(); - const { getByTestId, queryByTestId } = render(); + const { getByTestId, queryByTestId } = renderWithProvider(); // Assert - Items exist expect(getByTestId(ACCOUNT_1_TEST_ID.item)).toBeTruthy(); @@ -138,7 +206,7 @@ describe('AccountList', () => { isAccountLoading: () => false, }); - const { getByTestId } = render(); + const { getByTestId } = renderWithProvider(); // Assert switches are disabled since we are loading expect(getByTestId(ACCOUNT_1_TEST_ID.itemSwitch).props.disabled).toBe(true); @@ -153,7 +221,7 @@ describe('AccountList', () => { isAccountLoading: () => false, }); - const { getByTestId } = render(); + const { getByTestId } = renderWithProvider(); // Act const toggleSwitch = getByTestId(ACCOUNT_1_TEST_ID.itemSwitch); diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx index ab58ecee9ad6..c0447f83a13b 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx @@ -39,7 +39,7 @@ export const AccountsList = () => { isEnabled={isAccountEnabled(item.accounts)} refetchNotificationAccounts={refetchAccountSettings} testID={NotificationSettingsViewSelectorsIDs.ACCOUNT_NOTIFICATION_TOGGLE( - item.id, + getEvmAddress(item.accounts) ?? '', )} /> )} From b8f76b458aea1ba267240113eaff1be1396b5bd8 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 24 Sep 2025 15:20:50 +0100 Subject: [PATCH 3/8] style: improve notification setting styling --- .../NotificationsSettings/AccountsList.tsx | 11 ++++++++++- .../NotificationOptionToggle/styles.ts | 4 ---- .../NotificationsSettings.styles.ts | 19 +++++++++++++++---- .../Settings/NotificationsSettings/index.tsx | 17 +++++++++++++---- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx index c0447f83a13b..b3d6f0742790 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.tsx @@ -7,8 +7,14 @@ import { import NotificationOptionToggle from './NotificationOptionToggle'; import { NotificationSettingsViewSelectorsIDs } from '../../../../../e2e/selectors/Notifications/NotificationSettingsView.selectors'; import AccountListHeader from '../../../../component-library/components-temp/MultichainAccounts/MultichainAccountSelectorList/AccountListHeader'; +import { useTheme } from '../../../../util/theme'; +import { useStyles } from '../../../../component-library/hooks'; +import styleSheet from './NotificationsSettings.styles'; export const AccountsList = () => { + const theme = useTheme(); + const { styles } = useStyles(styleSheet, { theme }); + const { accountAvatarType, firstHDWalletGroups } = useAccountProps(); const { isAnyAccountLoading, @@ -24,7 +30,10 @@ export const AccountsList = () => { return ( - + `address-${item.id}`} diff --git a/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts b/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts index aa3e2a7a6a2a..3f43e2c79854 100644 --- a/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts +++ b/app/components/Views/Settings/NotificationsSettings/NotificationOptionToggle/styles.ts @@ -6,7 +6,6 @@ export const createStyles = () => container: { flexDirection: 'row', alignItems: 'center', - marginBottom: 24, }, titleContainer: { alignItems: 'flex-start', @@ -15,9 +14,6 @@ export const createStyles = () => title: { flex: 1, }, - switchElement: { - // marginLeft: 16, - }, switch: { alignSelf: 'flex-start', }, diff --git a/app/components/Views/Settings/NotificationsSettings/NotificationsSettings.styles.ts b/app/components/Views/Settings/NotificationsSettings/NotificationsSettings.styles.ts index 126e41134c1e..83cdffad19cb 100644 --- a/app/components/Views/Settings/NotificationsSettings/NotificationsSettings.styles.ts +++ b/app/components/Views/Settings/NotificationsSettings/NotificationsSettings.styles.ts @@ -4,12 +4,16 @@ import { Theme } from '../../../../util/theme/models'; const styleSheet = (params: { theme: Theme }) => StyleSheet.create({ - wrapper: { - backgroundColor: params.theme.colors.background.default, - flex: 1, + container: { padding: 24, paddingBottom: 48, }, + line: { + borderTopWidth: 1, + borderTopColor: params.theme.colors.border.muted, + marginVertical: 16, + marginHorizontal: -24, + }, heading: { marginTop: 16, }, @@ -17,7 +21,14 @@ const styleSheet = (params: { theme: Theme }) => marginTop: 16, }, setting: { - marginVertical: 16, + marginTop: 16, + }, + productAnnouncementContainer: { + marginTop: 16, + }, + accountHeader: { + marginTop: 16, + marginLeft: -16, }, clearHistoryConfirm: { marginTop: 18, diff --git a/app/components/Views/Settings/NotificationsSettings/index.tsx b/app/components/Views/Settings/NotificationsSettings/index.tsx index 8b5af87676d1..14b9158cc699 100644 --- a/app/components/Views/Settings/NotificationsSettings/index.tsx +++ b/app/components/Views/Settings/NotificationsSettings/index.tsx @@ -1,7 +1,7 @@ /* eslint-disable react/display-name */ import { NavigationProp, ParamListBase } from '@react-navigation/native'; import React, { useEffect } from 'react'; -import { ScrollView } from 'react-native'; +import { ScrollView, View } from 'react-native'; import { useSelector } from 'react-redux'; import { strings } from '../../../../../locales/i18n'; @@ -39,7 +39,10 @@ const NotificationsSettings = ({ navigation, route }: Props) => { const isMetamaskNotificationsEnabled = useSelector( selectIsMetamaskNotificationsEnabled, ); - const hasFirstHDWallet = Boolean(useFirstHDWalletAccounts()); + const firstHDWallet = useFirstHDWalletAccounts(); + const hasFirstHDWallet = Boolean( + firstHDWallet?.data && firstHDWallet?.data.length > 0, + ); const loadingText = useSwitchNotificationLoadingText(); @@ -62,7 +65,7 @@ const NotificationsSettings = ({ navigation, route }: Props) => { }, [colors, isFullScreenModal, navigation]); return ( - + {/* Main Toggle */} @@ -72,6 +75,8 @@ const NotificationsSettings = ({ navigation, route }: Props) => { {/* Push Notifications Toggle */} + + {/* Feature Announcement Toggle */} { )} styles={styles} /> - + + + + + {/* Account Notification Toggles */} {hasFirstHDWallet && ( From b12a6d38cb246f68b79bf01319f41291acb290cd Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Thu, 25 Sep 2025 13:45:20 +0100 Subject: [PATCH 4/8] test: update UTs --- .../__snapshots__/index.test.tsx.snap | 1 - .../MainNotificationToggle.hooks.test.tsx | 228 +++++++++--------- .../__snapshots__/index.test.tsx.snap | 4 +- 3 files changed, 119 insertions(+), 114 deletions(-) diff --git a/app/components/Views/Settings/NotificationsSettings/CustomNotificationsRow/__snapshots__/index.test.tsx.snap b/app/components/Views/Settings/NotificationsSettings/CustomNotificationsRow/__snapshots__/index.test.tsx.snap index f65c50657d0b..a790fcefe700 100644 --- a/app/components/Views/Settings/NotificationsSettings/CustomNotificationsRow/__snapshots__/index.test.tsx.snap +++ b/app/components/Views/Settings/NotificationsSettings/CustomNotificationsRow/__snapshots__/index.test.tsx.snap @@ -6,7 +6,6 @@ exports[`CustomNotificationsRow should render correctly 1`] = ` { "alignItems": "center", "flexDirection": "row", - "marginBottom": 24, } } testID="notifications-switch--container" diff --git a/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx b/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx index 2b93a721a6dc..beba83456214 100644 --- a/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx +++ b/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx @@ -1,141 +1,149 @@ -import { act } from '@testing-library/react-hooks'; -import { waitFor } from '@testing-library/react-native'; -import { AvatarAccountType } from '../../../../component-library/components/Avatars/Avatar'; -// eslint-disable-next-line import/no-namespace -import * as UseSwitchNotificationsModule from '../../../../util/notifications/hooks/useSwitchNotifications'; +import { useNavigation } from '@react-navigation/native'; +import { useMainNotificationToggle } from './MainNotificationToggle.hooks'; +import Routes from '../../../../constants/navigation/Routes'; +import { useNotificationsToggle } from '../../../../util/notifications/hooks/useSwitchNotifications'; +import { useMetrics } from '../../../hooks/useMetrics'; +import { MetricsEventBuilder } from '../../../../core/Analytics/MetricsEventBuilder'; import { renderHookWithProvider } from '../../../../util/test/renderWithProvider'; -import { useAccounts } from '../../../hooks/useAccounts'; -import { - useAccountProps, - useNotificationAccountListProps, -} from './AccountsList.hooks'; +import { EVENT_NAME } from '../../../../core/Analytics'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type MockVar = any; +// Mock dependencies +jest.mock('@react-navigation/native'); +jest.mock('../../../../util/notifications/hooks/useSwitchNotifications'); +jest.mock('../../../hooks/useMetrics'); + +const mockUseNavigation = jest.mocked(useNavigation); +const mockUseNotificationsToggle = jest.mocked(useNotificationsToggle); +const mockUseMetrics = jest.mocked(useMetrics); + +describe('useMainNotificationToggle', () => { + beforeEach(() => jest.clearAllMocks()); + + const arrangeState = () => { + const mockState = { + settings: { + basicFunctionalityEnabled: true, + }, + engine: { + backgroundState: { + UserStorageController: { + isBackupAndSyncEnabled: true, + }, + }, + }, + }; + return { mockState }; + }; -describe('useNotificationAccountListProps', () => { const arrangeMocks = () => { - const mockUpdate = jest.fn(); - const createMockUseFetchAccountNotificationsReturn = () => ({ - accountsBeingUpdated: [], - data: {}, + const navigate = jest.fn(); + const switchNotifications = jest.fn(); + const trackEvent = jest.fn(); + + mockUseNavigation.mockReturnValue({ + navigate, + } as never); + + mockUseNotificationsToggle.mockReturnValue({ + data: true, + loading: false, error: null, - initialLoading: false, - update: mockUpdate, + switchNotifications, }); - const mockUseFetchAccountNotifications = jest - .spyOn(UseSwitchNotificationsModule, 'useFetchAccountNotifications') - .mockReturnValue(createMockUseFetchAccountNotificationsReturn()); + + mockUseMetrics.mockReturnValue({ + trackEvent, + createEventBuilder: MetricsEventBuilder.createEventBuilder, + } as never); return { - mockUpdate, - mockUseFetchAccountNotifications, - createMockUseFetchAccountNotificationsReturn, + navigate, + switchNotifications, + trackEvent, + ...arrangeState(), }; }; - type Mocks = ReturnType; - const arrange = (addresses: string[], mutateMocks?: (m: Mocks) => void) => { - // Arrange + const arrangeTest = ( + overridesMocks?: (m: ReturnType) => void, + ) => { const mocks = arrangeMocks(); - mutateMocks?.(mocks); - const hook = renderHookWithProvider(() => - useNotificationAccountListProps(addresses), + overridesMocks?.(mocks); + + const { result } = renderHookWithProvider( + () => useMainNotificationToggle(), + { state: mocks.mockState }, ); - return { mocks, hook }; + return { + mocks, + result, + }; }; - it('returns correct loading state', async () => { - const addresses = ['0x123', '0x456']; - const { hook } = arrange(addresses); - expect(hook.result.current.isAnyAccountLoading).toBe(false); + beforeEach(() => { + jest.clearAllMocks(); }); - it('returns correct account loading state', async () => { - const addresses = ['0x123', '0x456']; - const { hook } = arrange(addresses, (m) => { - m.mockUseFetchAccountNotifications.mockReturnValue({ - ...m.createMockUseFetchAccountNotificationsReturn(), - accountsBeingUpdated: ['0x123'], - }); - }); - expect(hook.result.current.isAccountLoading('0x123')).toBe(true); - expect(hook.result.current.isAccountLoading('0x456')).toBe(false); + it('toggles notifications from false to true', () => { + const { mocks, result } = arrangeTest(); + + result.current.onToggle(); + + expect(mocks.switchNotifications).toHaveBeenCalledWith(false); + expect(mocks.trackEvent).toHaveBeenCalledWith( + expect.objectContaining({ + name: EVENT_NAME.NOTIFICATIONS_SETTINGS_UPDATED, + properties: expect.objectContaining({ + settings_type: 'notifications', + old_value: true, + new_value: false, + was_profile_syncing_on: true, + }), + }), + ); }); - it('returns correct account enabled state', async () => { - const addresses = ['0x123', '0x456']; - const { hook } = arrange(addresses, (m) => { - m.mockUseFetchAccountNotifications.mockReturnValue({ - ...m.createMockUseFetchAccountNotificationsReturn(), - data: { '0x123': true, '0x456': false }, + it('toggles from false to true', () => { + const { mocks, result } = arrangeTest((m) => { + mockUseNotificationsToggle.mockReturnValue({ + data: false, + loading: false, + error: null, + switchNotifications: m.switchNotifications, }); }); - expect(hook.result.current.isAccountEnabled('0x123')).toBe(true); - expect(hook.result.current.isAccountEnabled('0x456')).toBe(false); - }); - - it('refetches account settings', async () => { - const addresses = ['0x123', '0x456']; - const { mocks, hook } = await arrange(addresses); - - // Act - await act(async () => { - await hook.result.current.refetchAccountSettings(); - }); - await waitFor(() => { - expect(mocks.mockUpdate).toHaveBeenCalledWith(addresses); - }); + result.current.onToggle(); + + expect(mocks.switchNotifications).toHaveBeenCalledWith(true); + expect(mocks.trackEvent).toHaveBeenCalledWith( + expect.objectContaining({ + name: EVENT_NAME.NOTIFICATIONS_SETTINGS_UPDATED, + properties: expect.objectContaining({ + settings_type: 'notifications', + old_value: false, + new_value: true, + was_profile_syncing_on: true, + }), + }), + ); }); -}); - -jest.mock('../../../hooks/useAccounts', () => ({ - useAccounts: jest.fn(), -})); -const arrangeUseAccounts = () => { - const createMockAccounts = (addresses: string[]) => - addresses.map((address) => ({ address })); - - const mockUseAccounts = jest.mocked(useAccounts).mockReturnValue({ - accounts: createMockAccounts(['0x123', '0x456']), - } as MockVar); - - return { - createMockAccounts, - mockUseAccounts, - }; -}; - -describe('useAccountProps', () => { - const arrangeMocks = () => { - const mockStore = jest.fn().mockReturnValue({ - engine: { - backgroundState: { - NotificationServicesController: { - subscriptionAccountsSeen: ['0x123', '0x456'], - }, - }, - }, - settings: { avatarAccountType: AvatarAccountType.Maskicon }, + it('navigate to basic functionality screen when basicFunctionalityEnabled is false', () => { + const { mocks, result } = arrangeTest((m) => { + m.mockState.settings.basicFunctionalityEnabled = false; }); - return { - ...arrangeUseAccounts(), - mockStore, - }; - }; + result.current.onToggle(); - it('returns correct account props', () => { - const mocks = arrangeMocks(); - const { result } = renderHookWithProvider(() => useAccountProps(), { - state: mocks.mockStore(), + expect(mocks.navigate).toHaveBeenCalledWith(Routes.MODAL.ROOT_MODAL_FLOW, { + screen: Routes.SHEET.BASIC_FUNCTIONALITY, + params: { + caller: Routes.SETTINGS.NOTIFICATIONS, + }, }); - - expect(result.current.accounts).toHaveLength(2); - expect(result.current.accountAvatarType).toBe(AvatarAccountType.Maskicon); - expect(result.current.accountAddresses).toEqual(['0x123', '0x456']); + expect(mocks.switchNotifications).not.toHaveBeenCalled(); + expect(mocks.trackEvent).not.toHaveBeenCalled(); }); }); diff --git a/app/components/Views/Settings/NotificationsSettings/__snapshots__/index.test.tsx.snap b/app/components/Views/Settings/NotificationsSettings/__snapshots__/index.test.tsx.snap index a39ad8be56de..74b19e651066 100644 --- a/app/components/Views/Settings/NotificationsSettings/__snapshots__/index.test.tsx.snap +++ b/app/components/Views/Settings/NotificationsSettings/__snapshots__/index.test.tsx.snap @@ -4,8 +4,6 @@ exports[`NotificationsSettings render matches snapshot 1`] = ` From d641cb1f3c6b28dbc6f84cc1d33080c0157d7b9d Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 26 Sep 2025 12:51:55 +0100 Subject: [PATCH 5/8] test: update e2e notification tests --- .../Views/Settings/NotificationsSettings/index.tsx | 8 +++++++- e2e/pages/Notifications/NotificationSettingsView.ts | 6 ++++++ .../NotificationSettingsView.selectors.ts | 2 ++ .../notifications/notification-settings-flow.spec.ts | 11 +++++++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/components/Views/Settings/NotificationsSettings/index.tsx b/app/components/Views/Settings/NotificationsSettings/index.tsx index 14b9158cc699..d7aef52717dd 100644 --- a/app/components/Views/Settings/NotificationsSettings/index.tsx +++ b/app/components/Views/Settings/NotificationsSettings/index.tsx @@ -32,6 +32,7 @@ import { ResetNotificationsButton } from './ResetNotificationsButton'; import SessionHeader from './sectionHeader'; import { PushNotificationToggle } from './PushNotificationToggle'; import { useFirstHDWalletAccounts } from './AccountsList.hooks'; +import { NotificationSettingsViewSelectorsIDs } from '../../../../../e2e/selectors/Notifications/NotificationSettingsView.selectors'; const NotificationsSettings = ({ navigation, route }: Props) => { const theme = useTheme(); @@ -91,7 +92,12 @@ const NotificationsSettings = ({ navigation, route }: Props) => { - + {/* Account Notification Toggles */} {hasFirstHDWallet && ( diff --git a/e2e/pages/Notifications/NotificationSettingsView.ts b/e2e/pages/Notifications/NotificationSettingsView.ts index c24cdeb5f7f4..015e137a8e67 100644 --- a/e2e/pages/Notifications/NotificationSettingsView.ts +++ b/e2e/pages/Notifications/NotificationSettingsView.ts @@ -33,6 +33,12 @@ class NotificationsSettingsView { ); } + get featureAnnouncementSeparator() { + return Matchers.getElementByID( + NotificationSettingsViewSelectorsIDs.FEATURE_ANNOUNCEMENT_SEPARATOR, + ); + } + get accountActivitySection() { return Matchers.getElementByText( NotificationSettingsViewSelectorsText.ACCOUNT_ACTIVITY_SECTION, diff --git a/e2e/selectors/Notifications/NotificationSettingsView.selectors.ts b/e2e/selectors/Notifications/NotificationSettingsView.selectors.ts index 3c7ab968febd..7bf568b32af4 100644 --- a/e2e/selectors/Notifications/NotificationSettingsView.selectors.ts +++ b/e2e/selectors/Notifications/NotificationSettingsView.selectors.ts @@ -10,6 +10,8 @@ export const NotificationSettingsViewSelectorsIDs = { PUSH_NOTIFICATIONS_TOGGLE: 'notification-settings-push-notifications-toggle', FEATURE_ANNOUNCEMENTS_TOGGLE: 'notification-settings-feature-announcements-toggle', + FEATURE_ANNOUNCEMENT_SEPARATOR: + 'notification-settings-feature-announcements-separator', PERPS_NOTIFICATIONS_TOGGLE: 'notification-settings-perps-notifications-toggle', ACCOUNT_NOTIFICATION_TOGGLE: (address: string) => diff --git a/e2e/specs/notifications/notification-settings-flow.spec.ts b/e2e/specs/notifications/notification-settings-flow.spec.ts index 4fcee4a91a3a..60b5b847f943 100644 --- a/e2e/specs/notifications/notification-settings-flow.spec.ts +++ b/e2e/specs/notifications/notification-settings-flow.spec.ts @@ -8,6 +8,7 @@ import { loginToApp } from '../../viewHelper'; import TabBarComponent from '../../pages/wallet/TabBarComponent'; import SettingsView from '../../pages/Settings/SettingsView'; import NotificationSettingsView from '../../pages/Notifications/NotificationSettingsView'; +import { Gestures } from '../../framework'; describe(SmokeNetworkAbstractions('Notification Onboarding'), () => { beforeAll(async () => { @@ -54,6 +55,11 @@ describe(SmokeNetworkAbstractions('Notification Onboarding'), () => { 'on', ); + await Gestures.swipe( + NotificationSettingsView.featureAnnouncementSeparator, + 'up', + ); + // Test account notifications toggle functionality await NotificationSettingsView.tapAccountNotificationsToggleAndVerifyState( DEFAULT_FIXTURE_ACCOUNT_CHECKSUM, @@ -64,6 +70,11 @@ describe(SmokeNetworkAbstractions('Notification Onboarding'), () => { 'on', ); + await Gestures.swipe( + NotificationSettingsView.featureAnnouncementSeparator, + 'down', + ); + // Disable main toggle and verify all sub-settings are hidden await NotificationSettingsView.tapNotificationToggleAndVerifyState( 'off', From fbf39c0bd39cccf6763f007d55a8013445bc390f Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 7 Nov 2025 11:39:51 +0000 Subject: [PATCH 6/8] chore: fix lint issues from merge conflicts --- .../MainNotificationToggle.hooks.test.tsx | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx b/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx index beba83456214..4ede9585a809 100644 --- a/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx +++ b/app/components/Views/Settings/NotificationsSettings/MainNotificationToggle.hooks.test.tsx @@ -5,7 +5,6 @@ import { useNotificationsToggle } from '../../../../util/notifications/hooks/use import { useMetrics } from '../../../hooks/useMetrics'; import { MetricsEventBuilder } from '../../../../core/Analytics/MetricsEventBuilder'; import { renderHookWithProvider } from '../../../../util/test/renderWithProvider'; -import { EVENT_NAME } from '../../../../core/Analytics'; // Mock dependencies jest.mock('@react-navigation/native'); @@ -91,17 +90,6 @@ describe('useMainNotificationToggle', () => { result.current.onToggle(); expect(mocks.switchNotifications).toHaveBeenCalledWith(false); - expect(mocks.trackEvent).toHaveBeenCalledWith( - expect.objectContaining({ - name: EVENT_NAME.NOTIFICATIONS_SETTINGS_UPDATED, - properties: expect.objectContaining({ - settings_type: 'notifications', - old_value: true, - new_value: false, - was_profile_syncing_on: true, - }), - }), - ); }); it('toggles from false to true', () => { @@ -117,17 +105,6 @@ describe('useMainNotificationToggle', () => { result.current.onToggle(); expect(mocks.switchNotifications).toHaveBeenCalledWith(true); - expect(mocks.trackEvent).toHaveBeenCalledWith( - expect.objectContaining({ - name: EVENT_NAME.NOTIFICATIONS_SETTINGS_UPDATED, - properties: expect.objectContaining({ - settings_type: 'notifications', - old_value: false, - new_value: true, - was_profile_syncing_on: true, - }), - }), - ); }); it('navigate to basic functionality screen when basicFunctionalityEnabled is false', () => { From bba30192383eec45d4d39e7936008a409688423e Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 7 Nov 2025 11:46:07 +0000 Subject: [PATCH 7/8] chore: remove todo comment --- .../Settings/NotificationsSettings/AccountsList.hooks.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx index b10936f70f62..1241eb8541ab 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx @@ -84,10 +84,6 @@ export function useNotificationAccountListProps() { export function useFirstHDWalletAccounts() { const accountGroupsByWallet = useSelector(selectAccountGroupsByWallet); - - // TODO - do we have a reliable way of receiving the first HD wallet? - // Notifications only support the first HD wallet (due to backend limitations) - // This limitation will most likely be removed in near future const firstHDWalletGroup = accountGroupsByWallet.find( (w) => w.wallet.type === AccountWalletType.Entropy, ); From 0e45afd0b86adc15e97e7415c205828f78f37dd9 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Fri, 7 Nov 2025 12:51:49 +0000 Subject: [PATCH 8/8] chore: fix UTs after merging main, some places use other selectors, might as well just provide mock state to our tests --- .../NotificationsSettings/AccountsList.test.tsx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx b/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx index c8fbc4b0be6e..e0951f51e007 100644 --- a/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx +++ b/app/components/Views/Settings/NotificationsSettings/AccountsList.test.tsx @@ -16,6 +16,7 @@ import { AccountGroupType, AccountWalletType } from '@metamask/account-api'; import renderWithProvider from '../../../../util/test/renderWithProvider'; // eslint-disable-next-line import/no-namespace import * as AccountSelectorsModule from '../../../../selectors/multichainAccounts/accounts'; +import initialRootState from '../../../../util/test/initial-root-state'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type MockVar = any; @@ -182,7 +183,10 @@ describe('AccountList', () => { it('renders correctly', async () => { arrangeMocks(); - const { getByTestId, queryByTestId } = renderWithProvider(); + const { getByTestId, queryByTestId } = renderWithProvider( + , + { state: initialRootState }, + ); // Assert - Items exist expect(getByTestId(ACCOUNT_1_TEST_ID.item)).toBeTruthy(); @@ -206,7 +210,9 @@ describe('AccountList', () => { isAccountLoading: () => false, }); - const { getByTestId } = renderWithProvider(); + const { getByTestId } = renderWithProvider(, { + state: initialRootState, + }); // Assert switches are disabled since we are loading expect(getByTestId(ACCOUNT_1_TEST_ID.itemSwitch).props.disabled).toBe(true); @@ -221,7 +227,9 @@ describe('AccountList', () => { isAccountLoading: () => false, }); - const { getByTestId } = renderWithProvider(); + const { getByTestId } = renderWithProvider(, { + state: initialRootState, + }); // Act const toggleSwitch = getByTestId(ACCOUNT_1_TEST_ID.itemSwitch);