Skip to content

Commit feb0582

Browse files
authored
feat: remove wallet activity in-app and push toggles + source preferences from Trigger API (MetaMask#35474)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> <!-- mms-check directive vocabulary — read by .github/scripts/shared/pr-template-checks.ts at module load to build the validation plan. Directives are invisible in rendered markdown and must NOT be removed or edited without updating the validator registry. type=text Section must contain non-placeholder prose. type=changelog Section must have a valid CHANGELOG entry: line. type=issue-link Section must have a Fixes:/Closes:/Refs: line with a value. type=manual-testing Section must have real testing steps or an explicit N/A. type=screenshot Section must have evidence (image/URL) or an explicit N/A. type=checklist Section must have all checkboxes consciously checked. required=true|false Whether a missing/invalid section runs the validator at all. blocking=true|false Whether a failure of this check fails the CI workflow. Default: false — failures are shown as warnings in the sticky comment but do not block the PR. Sections without a directive are checked for structural presence only. --> ## **Description** <!-- mms-check: type=text required=true --> <!-- 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? --> **Reason for the change.** Since `@metamask/notification-services-controller` v24, the wallet-activity address list has been read from Authenticated User Storage, which is keyed by canonical profile ID. Profile pairing shares that ID across every SRP belonging to the same user, so the stored list pooled the addresses of unrelated SRPs and users could receive wallet-activity notifications for addresses they do not own. [MetaMask/core#9985](MetaMask/core#9985) restores the pre-v24 model: the keyring is the source of truth for *which* addresses exist, and the Trigger API for *which* of them are enabled. This PR consumes that controller change on mobile and adapts the wallet-activity settings UI to it. **What changes.** - Bumps `@metamask/notification-services-controller` to the preview build of the core fix (`26.0.1-preview-1a9713847`; to be replaced by the stable release once the core PR is merged and released). - Wallet activity settings section no longer shows the section-level Push and In-app channel toggles — per-account subscriptions in the Trigger API are now the whole settings surface. The Wallet activity row in Settings → Notifications correspondingly no longer shows the "Push, In app" subtext; the row keeps the same height as before with its title vertically centered (min-height derived from the `sBodyMD`/`sBodySM` typography tokens, no hardcoded value). - Select all / Deselect all no longer writes channel flags to AUS; it only flips the per-account Trigger API subscriptions. - A failed account-settings read is now surfaced as an inline error with switches disabled, instead of silently rendering every account as off — the controller now rejects on an unreadable Trigger API config rather than reporting all accounts disabled. - Account toggles no longer refetch the AUS preferences blob (account writes no longer touch AUS). - Appium mock trigger server now mocks the reintroduced `POST /api/v2/notifications` upsert endpoint (statefully, so reads reflect writes) and seeds the fixture account as subscribed, which the keyring-driven fetch path requires for wallet notifications to appear in smoke tests. No client-side migration is needed: the controller change heals affected users on its own — the in-app list is correct immediately and push links are replaced on the first daily re-subscribe after update. ## **Changelog** <!-- mms-check: type=changelog required=true blocking=true --> <!-- 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: Fixed wallet activity notifications being scoped to shared profile storage instead of the wallet's own accounts, and simplified the Wallet activity settings screen to per-account switches ## **Related issues** <!-- mms-check: type=issue-link required=true --> Refs: [MetaMask/core#9985](MetaMask/core#9985) JIRA: https://consensyssoftware.atlassian.net/browse/GE-460 ## **Manual testing steps** <!-- mms-check: type=manual-testing required=true --> ```gherkin Feature: Wallet activity notification settings Scenario: user views the notifications settings list Given the user has notifications enabled When user opens Settings > Notifications Then the Wallet activity row shows no "Push, In app" subtext And the Wallet activity row has the same height as the other section rows And every other section row still shows its channel subtext ("Push, In app" or "Off") Scenario: user manages per-account wallet activity notifications Given the user has notifications enabled When user opens Settings > Notifications > Wallet activity Then no Push or In-app channel toggles are shown And the user sees their accounts with individual switches reflecting their subscription state When user turns off an account's switch Then the switch stays off after closing and reopening the app And no wallet activity notifications arrive for that account Scenario: user toggles all accounts at once Given the user is on the Wallet activity settings section with at least one account enabled When user taps "Deselect all" Then every account switch turns off And the selection persists after relaunching the app Scenario: user sees only their own wallet activity Given the user has notifications enabled When user opens the notifications list Then only notifications for accounts in the user's wallet are shown ``` ## **Screenshots/Recordings** <!-- mms-check: type=screenshot required=true --> <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="371" height="786" alt="before-nav" src="https://github.com/user-attachments/assets/6416acfa-a8b9-4f85-aaa2-d9abc269c69e" /> <img width="384" height="775" alt="before-settings" src="https://github.com/user-attachments/assets/767580d5-3c2f-4ba9-a596-ddf1a002ca2e" /> ### **After** <img width="383" height="787" alt="after-nav" src="https://github.com/user-attachments/assets/8f3e6c44-ed9b-4fdf-97f4-6059db3991c0" /> <img width="377" height="786" alt="after-settings" src="https://github.com/user-attachments/assets/17b02517-5938-462f-bc9e-15a1b2eee6c5" /> ## **Pre-merge author checklist** <!-- mms-check: type=checklist required=true --> <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [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. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [x] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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. <!-- Generated with the help of the pr-description AI skill --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes wallet-activity notification source of truth and settings UX; incorrect Trigger API or error handling could mis-scope subscriptions or block account toggles, but scope is limited to notifications settings and test mocks. > > **Overview** > Aligns mobile wallet-activity notification settings with **@metamask/notification-services-controller** `^27.0.1`: per-account enablement is read and written via the **Trigger API**, not Authenticated User Storage channel flags. > > **Wallet activity UI** no longer shows Push/In-app toggles on the section screen or a channel summary on the main Notifications list row (layout keeps row height via `notificationRow` / optional `status`). **Select all / Deselect all** only calls `toggleAllAccounts` and analytics; it does **not** `updatePreferencesSection` for push/in-app. **`refetchAccountSettings`** only refetches account settings from the Trigger API (preferences blob refetch removed). > > **Failed reads** from the Trigger API expose `accountSettingsError`, show the new `accounts_load_error` copy, and disable switches when there is no cached data; if earlier settings exist, switches stay enabled and the error is hidden. > > Smoke **Appium** mocks add the Trigger API **upsert** route and seed the default fixture account as subscribed so keyring-driven fetches work in E2E. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 45a458b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c1baf11 commit feb0582

14 files changed

Lines changed: 274 additions & 229 deletions

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

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,7 @@ jest.mock(
3636
}),
3737
);
3838

39-
const mockRefetchPreferences = jest.fn();
40-
jest.mock('./hooks/useNotificationStoragePreferences', () => ({
41-
useNotificationStoragePreferences: () => ({
42-
refetch: mockRefetchPreferences,
43-
}),
44-
}));
45-
39+
beforeEach(() => jest.clearAllMocks());
4640
// Un-spy the notification hook modules between tests (clearAllMocks does not).
4741
afterEach(() => jest.restoreAllMocks());
4842

@@ -246,8 +240,6 @@ describe('useNotificationAccountListProps', () => {
246240
return { mocks, hook };
247241
};
248242

249-
beforeEach(() => jest.clearAllMocks());
250-
251243
it('returns correct loading state', async () => {
252244
const addresses = ['0x123', '0x456'];
253245
const { hook } = arrange(addresses);
@@ -369,14 +361,31 @@ describe('useNotificationAccountListProps', () => {
369361
});
370362
});
371363

372-
it('refetches the preferences blob alongside account settings', async () => {
373-
const { hook } = arrange(['0x123', '0x456']);
364+
it('surfaces a failed read with no settings to show', async () => {
365+
const { hook } = arrange(['0x123', '0x456'], (m) => {
366+
m.mockUseFetchAccountNotifications.mockReturnValue({
367+
...m.createUseFetchAccountNotificationsReturn(),
368+
error: 'Failed to get account settings',
369+
});
370+
});
374371

375-
await act(async () => hook.result.current.refetchAccountSettings());
372+
expect(hook.result.current.accountSettingsError).toBe(
373+
'Failed to get account settings',
374+
);
375+
expect(hook.result.current.shouldDisableSwitches).toBe(true);
376+
});
376377

377-
await waitFor(() => {
378-
expect(mockRefetchPreferences).toHaveBeenCalledTimes(1);
378+
it('keeps switches interactive when a failed read has earlier settings to show', async () => {
379+
const { hook } = arrange(['0x123', '0x456'], (m) => {
380+
m.mockUseFetchAccountNotifications.mockReturnValue({
381+
...m.createUseFetchAccountNotificationsReturn(),
382+
data: { '0x123': true },
383+
error: 'Failed to get account settings',
384+
});
379385
});
386+
387+
expect(hook.result.current.accountSettingsError).toBeNull();
388+
expect(hook.result.current.shouldDisableSwitches).toBe(false);
380389
});
381390
});
382391

@@ -498,8 +507,6 @@ describe('useWalletActivityAccountSelection', () => {
498507
};
499508
};
500509

501-
beforeEach(() => jest.clearAllMocks());
502-
503510
it('deselects all visible EVM accounts when any account is enabled', async () => {
504511
const mocks = arrangeMocks({
505512
[FORMATTED_EVM_ADDRESSES[0]]: true,

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,27 @@ import { selectAvatarAccountType } from '../../../../selectors/settings';
1010
import { selectAccountGroupsByWallet } from '../../../../selectors/multichainAccounts/accountTreeController';
1111
import { selectInternalAccountsById } from '../../../../selectors/accountsController';
1212
import { isEvmAccountType } from '@metamask/keyring-api';
13-
import { useNotificationStoragePreferences } from './hooks/useNotificationStoragePreferences';
1413

1514
export function useNotificationAccountListProps() {
1615
const accountAddresses = useSelector(getValidNotificationAccounts);
1716
const accountsMap = useSelector(selectInternalAccountsById);
18-
const { update, initialLoading, accountsBeingUpdated, data } =
17+
const { update, initialLoading, accountsBeingUpdated, data, error } =
1918
useFetchAccountNotifications(accountAddresses);
20-
const { refetch: refetchPreferences } = useNotificationStoragePreferences();
2119

22-
// Only disable switches during initial data loading, not when individual accounts are updating
23-
const shouldDisableSwitches = initialLoading;
20+
// Account settings live in the Trigger API, which reports an unreadable
21+
// config as a failure rather than "every account disabled". Surface that
22+
// failure instead of showing switches in the wrong position — but only when
23+
// there is no earlier successful read to fall back on.
24+
const hasAccountSettings = Object.keys(data).length > 0;
25+
const accountSettingsError = hasAccountSettings ? null : error;
26+
27+
// Only disable switches during initial data loading or when a failed read
28+
// left us with no settings to show, not when individual accounts are updating
29+
const shouldDisableSwitches = initialLoading || Boolean(accountSettingsError);
2430

25-
// Account toggles rewrite `walletActivity.accounts` behind the cached
26-
// preferences, so refresh both — otherwise the next section write PUTs a
27-
// stale accounts array.
2831
const refetchAccountSettings = useCallback(async () => {
29-
await Promise.all([update(accountAddresses), refetchPreferences()]);
30-
}, [accountAddresses, update, refetchPreferences]);
32+
await update(accountAddresses);
33+
}, [accountAddresses, update]);
3134

3235
// Helper to get addresses from account IDs
3336
const getEvmAddressesFromAccountIds = useCallback(
@@ -85,6 +88,7 @@ export function useNotificationAccountListProps() {
8588
return {
8689
shouldDisableSwitches,
8790
isAnyAccountUpdating,
91+
accountSettingsError,
8892
refetchAccountSettings,
8993
isAccountLoading,
9094
isAccountEnabled,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ interface RenderAccountsListMocks {
8686
}
8787

8888
describe('AccountList', () => {
89+
beforeEach(() => jest.clearAllMocks());
90+
8991
const arrangeSelectors = () => {
9092
jest
9193
.spyOn(AccountSelectorsModule, 'selectIconSeedAddressByAccountGroupId')
@@ -192,6 +194,7 @@ describe('AccountList', () => {
192194
const notificationAccountListProps: NotificationAccountListProps = {
193195
shouldDisableSwitches: false,
194196
isAnyAccountUpdating: false,
197+
accountSettingsError: null,
195198
refetchAccountSettings: mockRefetchAccountSettings,
196199
isAccountLoading: jest
197200
.fn()

app/components/Views/Settings/NotificationsSettings/NotificationSettingsSection.test.tsx

Lines changed: 17 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -97,22 +97,6 @@ jest.mock('./AccountsList.hooks', () => ({
9797
}),
9898
}));
9999

100-
interface WalletActivitySection {
101-
pushNotificationsEnabled: boolean;
102-
inAppNotificationsEnabled: boolean;
103-
accounts: { address: string; enabled: boolean }[];
104-
}
105-
106-
// Runs the updater passed to updatePreferencesSection to get the persisted section.
107-
const applyWalletActivityUpdate = (
108-
walletActivity: WalletActivitySection = mockPreferences.walletActivity,
109-
) => {
110-
const [, sectionUpdate] = mockUpdatePreferencesSection.mock.calls[0];
111-
return (sectionUpdate as (section: WalletActivitySection) => unknown)(
112-
walletActivity,
113-
);
114-
};
115-
116100
const marketingDisclaimer =
117101
'By turning this on, you agree to receive product news and marketing updates from MetaMask.';
118102

@@ -176,7 +160,7 @@ describe('NotificationSettingsSection', () => {
176160
expect(screen.getByText(marketingDisclaimer)).toBeOnTheScreen();
177161
});
178162

179-
it('disables both channels and tracks an ALL update when deselecting all accounts', async () => {
163+
it('tracks an ALL update without touching stored preferences when deselecting all accounts', async () => {
180164
renderSection({
181165
type: 'walletActivity',
182166
title: 'Wallet activity',
@@ -206,19 +190,11 @@ describe('NotificationSettingsSection', () => {
206190
.build(),
207191
);
208192
});
209-
expect(mockUpdatePreferencesSection).toHaveBeenCalledTimes(1);
210-
expect(mockUpdatePreferencesSection).toHaveBeenCalledWith(
211-
'walletActivity',
212-
expect.any(Function),
213-
);
214-
expect(applyWalletActivityUpdate()).toStrictEqual({
215-
...mockPreferences.walletActivity,
216-
pushNotificationsEnabled: false,
217-
inAppNotificationsEnabled: false,
218-
});
193+
// Account subscriptions live in the Trigger API; no preferences blob write.
194+
expect(mockUpdatePreferencesSection).not.toHaveBeenCalled();
219195
});
220196

221-
it('enables both channels and tracks an ALL update when selecting all accounts', async () => {
197+
it('tracks an ALL update without touching stored preferences when selecting all accounts', async () => {
222198
mockHasEnabledAccount = false;
223199

224200
renderSection({
@@ -250,52 +226,10 @@ describe('NotificationSettingsSection', () => {
250226
.build(),
251227
);
252228
});
253-
expect(mockUpdatePreferencesSection).toHaveBeenCalledTimes(1);
254-
expect(mockUpdatePreferencesSection).toHaveBeenCalledWith(
255-
'walletActivity',
256-
expect.any(Function),
257-
);
258-
expect(applyWalletActivityUpdate()).toStrictEqual({
259-
...mockPreferences.walletActivity,
260-
pushNotificationsEnabled: true,
261-
inAppNotificationsEnabled: true,
262-
});
229+
expect(mockUpdatePreferencesSection).not.toHaveBeenCalled();
263230
});
264231

265-
it('keeps the accounts written by the controller when flipping channels for all accounts', async () => {
266-
renderSection({
267-
type: 'walletActivity',
268-
title: 'Wallet Activity',
269-
description: 'Buy, sells, transfers, swaps and rewards',
270-
});
271-
272-
await act(async () => {
273-
fireEvent.press(
274-
screen.getByTestId(
275-
NotificationSettingsViewSelectorsIDs.ACCOUNT_NOTIFICATIONS_SELECT_ALL,
276-
),
277-
);
278-
});
279-
280-
await waitFor(() => {
281-
expect(mockUpdatePreferencesSection).toHaveBeenCalledTimes(1);
282-
});
283-
284-
// Updater must apply to the refreshed preferences, not this render's copy.
285-
const refreshedWalletActivity = {
286-
pushNotificationsEnabled: true,
287-
inAppNotificationsEnabled: true,
288-
accounts: [{ address: '0x1', enabled: false }],
289-
};
290-
291-
expect(applyWalletActivityUpdate(refreshedWalletActivity)).toStrictEqual({
292-
pushNotificationsEnabled: false,
293-
inAppNotificationsEnabled: false,
294-
accounts: [{ address: '0x1', enabled: false }],
295-
});
296-
});
297-
298-
it('disables the accounts section when both wallet activity channels are off', () => {
232+
it('keeps the wallet activity accounts interactive when both stored channel flags are off', () => {
299233
mockPreferences.walletActivity.pushNotificationsEnabled = false;
300234
mockPreferences.walletActivity.inAppNotificationsEnabled = false;
301235

@@ -312,27 +246,8 @@ describe('NotificationSettingsSection', () => {
312246
.UNSAFE_getAllByType(Text)
313247
.find((node) => node.props.children === 'Deselect all');
314248

315-
expect(selectAll).toBeDisabled();
316-
expect(deselectAllLabel?.props.color).toBe(TextColor.TextMuted);
317-
});
318-
319-
it('keeps the accounts section interactive when at least one channel is on', () => {
320-
mockPreferences.walletActivity.pushNotificationsEnabled = true;
321-
mockPreferences.walletActivity.inAppNotificationsEnabled = false;
322-
323-
renderSection({
324-
type: 'walletActivity',
325-
title: 'Wallet Activity',
326-
description: 'Buy, sells, transfers, swaps and rewards',
327-
});
328-
329-
const selectAll = screen.getByTestId(
330-
NotificationSettingsViewSelectorsIDs.ACCOUNT_NOTIFICATIONS_SELECT_ALL,
331-
);
332-
const deselectAllLabel = screen
333-
.UNSAFE_getAllByType(Text)
334-
.find((node) => node.props.children === 'Deselect all');
335-
249+
// Wallet activity has no channel toggles; stale stored flags must not
250+
// grey out the only remaining settings surface.
336251
expect(selectAll).not.toBeDisabled();
337252
expect(deselectAllLabel?.props.color).toBe(TextColor.PrimaryDefault);
338253
});
@@ -479,9 +394,9 @@ describe('NotificationSettingsSection', () => {
479394
}),
480395
);
481396
renderSection({
482-
type: 'walletActivity',
483-
title: 'Wallet activity',
484-
description: 'Buy, sells, transfers, swaps',
397+
type: 'perps',
398+
title: 'Trading Activity',
399+
description: 'Perps position changes',
485400
});
486401

487402
const pushToggle = screen.getByTestId(
@@ -499,7 +414,7 @@ describe('NotificationSettingsSection', () => {
499414
expect(inAppToggle.props.disabled).not.toBe(true);
500415
});
501416
expect(mockUpdateSectionChannel).toHaveBeenCalledWith(
502-
'walletActivity',
417+
'perps',
503418
'pushNotificationsEnabled',
504419
false,
505420
);
@@ -519,9 +434,9 @@ describe('NotificationSettingsSection', () => {
519434
)
520435
.mockResolvedValueOnce(undefined);
521436
renderSection({
522-
type: 'walletActivity',
523-
title: 'Wallet activity',
524-
description: 'Buy, sells, transfers, and swaps',
437+
type: 'perps',
438+
title: 'Trading Activity',
439+
description: 'Perps position changes',
525440
});
526441

527442
await act(async () => {
@@ -569,13 +484,13 @@ describe('NotificationSettingsSection', () => {
569484
});
570485
expect(mockUpdateSectionChannel).toHaveBeenNthCalledWith(
571486
1,
572-
'walletActivity',
487+
'perps',
573488
'pushNotificationsEnabled',
574489
false,
575490
);
576491
expect(mockUpdateSectionChannel).toHaveBeenNthCalledWith(
577492
2,
578-
'walletActivity',
493+
'perps',
579494
'pushNotificationsEnabled',
580495
true,
581496
);

0 commit comments

Comments
 (0)