Skip to content

Commit 9ad84cb

Browse files
authored
feat: Extend notification account toggles to all wallet keyrings (#27254)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Update **Notifications settings** so the **Account activity** section lists notification-eligible accounts from all wallet keyrings, not just the first HD wallet. This is tied to the relevant PRs in MM-core: - MetaMask/core#8108 - MetaMask/core#8449 Keep the existing per-account toggle behavior unchanged, so imported accounts use the same enable/disable notification flow as HD accounts. Add focused test coverage for multi-wallet rendering and update the settings snapshot accordingly. <img width="328" height="722" alt="image" src="https://github.com/user-attachments/assets/b2c296a2-2cb7-4087-b33f-84741a90c3fb" /> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Extend notification account toggles to all wallet keyrings ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes the account-listing/filtering logic in notification settings and upgrades the notification services controller, which could affect which accounts appear and push-link behavior if filtering/messenger wiring is incorrect. > > **Overview** > Extends **Notification Settings → Account activity** to list notification-eligible (EVM) accounts across *all* wallets/keyrings, instead of only the first HD/entropy wallet. > > Introduces `useNotificationWalletAccountGroups()` to filter wallet/account groups down to those containing EVM accounts, updates `AccountsList` to render per-wallet sections via `SectionList` (skipping non-EVM groups), and adjusts the settings screen to show/hide the section based on these filtered groups. Updates and expands unit/UI tests to cover multi-wallet rendering and empty/no-EVM scenarios, and refreshes smoke-test notification mocks. > > Also updates the notifications controller messenger delegation to include push link add/delete actions, and bumps `@metamask/notification-services-controller` to `^23.1.0`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9844b6e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 85dc04e commit 9ad84cb

11 files changed

Lines changed: 322 additions & 87 deletions

File tree

app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.test.tsx

Lines changed: 117 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ import { getValidNotificationAccounts } from '../../../../selectors/notification
99
import {
1010
useAccountProps,
1111
useNotificationAccountListProps,
12+
useNotificationWalletAccountGroups,
1213
} from './AccountsList.hooks';
1314
import { selectInternalAccountsById } from '../../../../selectors/accountsController';
1415
import { InternalAccount } from '@metamask/keyring-internal-api';
1516
import { selectAccountGroupsByWallet } from '../../../../selectors/multichainAccounts/accountTreeController';
1617
import { AccountGroupType, AccountWalletType } from '@metamask/account-api';
18+
import { KeyringTypes } from '@metamask/keyring-controller';
1719

1820
jest.mock('../../../../selectors/notifications', () => ({
1921
getValidNotificationAccounts: jest.fn(),
@@ -32,8 +34,39 @@ jest.mock(
3234
}),
3335
);
3436

37+
const MOCK_KEYRING_TYPE = 'HD Key Tree' as KeyringTypes;
38+
39+
const createNotificationAccountsMap = () =>
40+
({
41+
'MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a29': {
42+
id: 'MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a29',
43+
address: '0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a29',
44+
type: 'eip155:eoa',
45+
} as InternalAccount,
46+
'MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8EC': {
47+
id: 'MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8EC',
48+
address: '0x700CcD8172BC3807D893883a730A1E0E6630F8EC',
49+
type: 'eip155:eoa',
50+
} as InternalAccount,
51+
'MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a20': {
52+
id: 'MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a20',
53+
address: '0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a20',
54+
type: 'eip155:eoa',
55+
} as InternalAccount,
56+
'MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8E0': {
57+
id: 'MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8E0',
58+
address: '0x700CcD8172BC3807D893883a730A1E0E6630F8E0',
59+
type: 'eip155:eoa',
60+
} as InternalAccount,
61+
'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGRY': {
62+
id: 'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGRY',
63+
address: '63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGRY',
64+
type: 'solana:data-account',
65+
} as InternalAccount,
66+
}) satisfies Record<string, InternalAccount>;
67+
3568
const arrangeMockUseAccounts = () => {
36-
const createMockAccountGroup = (
69+
const createMockMultichainAccountGroup = (
3770
idx: number,
3871
accounts: [string, ...string[]],
3972
) =>
@@ -52,22 +85,40 @@ const arrangeMockUseAccounts = () => {
5285
},
5386
}) as const;
5487

55-
const group1 = createMockAccountGroup(0, [
88+
const createMockSingleAccountGroup = <
89+
T extends `keyring:${string}/${string}`,
90+
>(
91+
id: T,
92+
account: string,
93+
) =>
94+
({
95+
accounts: [account] as [string],
96+
id,
97+
type: AccountGroupType.SingleAccount,
98+
metadata: {
99+
hidden: false,
100+
lastSelected: 0,
101+
name: id,
102+
pinned: false,
103+
},
104+
}) as const;
105+
106+
const group1 = createMockMultichainAccountGroup(0, [
56107
`MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a29`,
57108
'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGRY',
58109
]);
59-
const group2 = createMockAccountGroup(1, [
110+
const group2 = createMockMultichainAccountGroup(1, [
60111
`MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8EC`,
61112
'MOCK-ID-FOR-Agsjd8HjGH5DxiXLMWc8fR4jjgHhvJG3TXcCpc1ieD9B',
62113
]);
63-
const group3 = createMockAccountGroup(0, [
114+
const group3 = createMockSingleAccountGroup(
115+
'keyring:wallet-2/0',
64116
`MOCK-ID-FOR-0xb2B92547A92C1aC55EAe3F6632Fa1aF87dc05a20`,
65-
'MOCK-ID-FOR-63jw5Q7pJXeHgHSvfTmKytUQ19hQgiAJQ5LZykmSMGR0',
66-
]);
67-
const group4 = createMockAccountGroup(1, [
117+
);
118+
const group4 = createMockSingleAccountGroup(
119+
'keyring:wallet-2/1',
68120
`MOCK-ID-FOR-0x700CcD8172BC3807D893883a730A1E0E6630F8E0`,
69-
'MOCK-ID-FOR-Agsjd8HjGH5DxiXLMWc8fR4jjgHhvJG3TXcCpc1ieD90',
70-
]);
121+
);
71122

72123
const mockSelectoWallets = jest
73124
.mocked(selectAccountGroupsByWallet)
@@ -94,13 +145,13 @@ const arrangeMockUseAccounts = () => {
94145
{
95146
title: 'Wallet 2',
96147
wallet: {
97-
id: 'entropy:wallet-2',
98-
type: AccountWalletType.Entropy,
148+
id: 'keyring:wallet-2',
149+
type: AccountWalletType.Keyring,
99150
metadata: {
100-
entropy: {
101-
id: '',
102-
},
103151
name: 'Wallet 2',
152+
keyring: {
153+
type: MOCK_KEYRING_TYPE,
154+
},
104155
},
105156
status: 'ready',
106157
groups: {
@@ -297,13 +348,54 @@ describe('useNotificationAccountListProps', () => {
297348
});
298349
});
299350

351+
describe('useNotificationWalletAccountGroups', () => {
352+
const arrangeMocks = () => {
353+
const mocks = arrangeMockUseAccounts();
354+
jest
355+
.mocked(selectInternalAccountsById)
356+
.mockReturnValue(createNotificationAccountsMap());
357+
358+
return mocks;
359+
};
360+
361+
it('returns wallet groups for all wallets with EVM accounts', () => {
362+
arrangeMocks();
363+
const { result } = renderHookWithProvider(() =>
364+
useNotificationWalletAccountGroups(),
365+
);
366+
367+
expect(result.current).toHaveLength(2);
368+
expect(result.current[0]).toStrictEqual(
369+
expect.objectContaining({
370+
title: 'Wallet 1',
371+
wallet: expect.objectContaining({
372+
id: 'entropy:wallet-1',
373+
type: AccountWalletType.Entropy,
374+
}),
375+
}),
376+
);
377+
expect(result.current[1]).toStrictEqual(
378+
expect.objectContaining({
379+
title: 'Wallet 2',
380+
wallet: expect.objectContaining({
381+
id: 'keyring:wallet-2',
382+
type: AccountWalletType.Keyring,
383+
}),
384+
}),
385+
);
386+
});
387+
});
388+
300389
describe('useAccountProps', () => {
301390
const arrangeMocks = () => {
302391
const mockStore = jest.fn().mockReturnValue({
303392
settings: {
304393
avatarAccountType: AvatarAccountType.Maskicon,
305394
},
306395
});
396+
jest
397+
.mocked(selectInternalAccountsById)
398+
.mockReturnValue(createNotificationAccountsMap());
307399

308400
return {
309401
...arrangeMockUseAccounts(),
@@ -318,7 +410,8 @@ describe('useAccountProps', () => {
318410
});
319411

320412
expect(result.current.accountAvatarType).toBe(AvatarAccountType.Maskicon);
321-
expect(result.current.firstHDWalletGroups).toStrictEqual(
413+
expect(result.current.accountWalletGroups).toHaveLength(2);
414+
expect(result.current.accountWalletGroups[0]).toStrictEqual(
322415
expect.objectContaining({
323416
title: 'Wallet 1',
324417
wallet: expect.objectContaining({
@@ -327,5 +420,14 @@ describe('useAccountProps', () => {
327420
}),
328421
}),
329422
);
423+
expect(result.current.accountWalletGroups[1]).toStrictEqual(
424+
expect.objectContaining({
425+
title: 'Wallet 2',
426+
wallet: expect.objectContaining({
427+
id: 'keyring:wallet-2',
428+
type: AccountWalletType.Keyring,
429+
}),
430+
}),
431+
);
330432
});
331433
});

app/components/Views/Settings/NotificationsSettings/AccountsList.hooks.tsx

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
import { useCallback } from 'react';
1+
import { useCallback, useMemo } from 'react';
22
import { useSelector } from 'react-redux';
33
import { useFetchAccountNotifications } from '../../../../util/notifications/hooks/useSwitchNotifications';
44
import { getValidNotificationAccounts } from '../../../../selectors/notifications';
55
import { toFormattedAddress } from '../../../../util/address';
66
import { selectAvatarAccountType } from '../../../../selectors/settings';
77
import { selectAccountGroupsByWallet } from '../../../../selectors/multichainAccounts/accountTreeController';
8-
import { AccountWalletType } from '@metamask/account-api';
98
import { selectInternalAccountsById } from '../../../../selectors/accountsController';
109
import { isEvmAccountType } from '@metamask/keyring-api';
1110

@@ -83,20 +82,41 @@ export function useNotificationAccountListProps() {
8382
};
8483
}
8584

86-
export function useFirstHDWalletAccounts() {
85+
export function useNotificationWalletAccountGroups() {
8786
const accountGroupsByWallet = useSelector(selectAccountGroupsByWallet);
88-
const firstHDWalletGroup = accountGroupsByWallet.find(
89-
(w) => w.wallet.type === AccountWalletType.Entropy,
87+
const accountsMap = useSelector(selectInternalAccountsById);
88+
89+
const isEvmAccountId = useCallback(
90+
(accountId: string) =>
91+
Boolean(accountsMap?.[accountId]?.address) &&
92+
isEvmAccountType(accountsMap[accountId].type),
93+
[accountsMap],
94+
);
95+
96+
const hasNotificationEligibleAccount = useCallback(
97+
(accountGroup: { accounts: string[] }) =>
98+
accountGroup.accounts.some(isEvmAccountId),
99+
[isEvmAccountId],
100+
);
101+
102+
return useMemo(
103+
() =>
104+
accountGroupsByWallet
105+
.map((walletGroup) => ({
106+
...walletGroup,
107+
data: walletGroup.data.filter(hasNotificationEligibleAccount),
108+
}))
109+
.filter((walletGroup) => walletGroup.data.length > 0),
110+
[accountGroupsByWallet, hasNotificationEligibleAccount],
90111
);
91-
return firstHDWalletGroup;
92112
}
93113

94114
export function useAccountProps() {
95-
const firstHDWalletGroups = useFirstHDWalletAccounts();
115+
const accountWalletGroups = useNotificationWalletAccountGroups();
96116
const accountAvatarType = useSelector(selectAvatarAccountType);
97117

98118
return {
99-
firstHDWalletGroups,
119+
accountWalletGroups,
100120
accountAvatarType,
101121
};
102122
}

0 commit comments

Comments
 (0)