From 887b9e7f92617fe1379f92f06fd682acf97efd35 Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Mon, 7 Sep 2026 11:59:28 +1200 Subject: [PATCH 1/4] feat(notifications): add toggleable inbox-only push notifications :sparkles: --- app/(main)/admin/_tabs/policy.tsx | 1 + components/push-notification-prompt.tsx | 7 ++- components/settings/notification-settings.tsx | 16 +++++ lib/__tests__/web-push.test.ts | 63 ++++++++++++++----- lib/web-push.ts | 49 ++++++++++++--- locales/ar/common.json | 4 +- locales/ca/common.json | 4 +- locales/cs/common.json | 4 +- locales/da/common.json | 4 +- locales/de/common.json | 4 +- locales/en/common.json | 4 +- locales/es/common.json | 4 +- locales/fa/common.json | 4 +- locales/fr/common.json | 4 +- locales/he/common.json | 4 +- locales/hu/common.json | 4 +- locales/it/common.json | 4 +- locales/ja/common.json | 4 +- locales/ko/common.json | 4 +- locales/lv/common.json | 4 +- locales/mn/common.json | 4 +- locales/nb/common.json | 4 +- locales/nl/common.json | 4 +- locales/pl/common.json | 4 +- locales/pt/common.json | 4 +- locales/ro/common.json | 4 +- locales/ru/common.json | 4 +- locales/sk/common.json | 4 +- locales/tr/common.json | 4 +- locales/uk/common.json | 4 +- locales/zh-TW/common.json | 4 +- locales/zh/common.json | 4 +- stores/__tests__/settings-store-sync.test.ts | 15 +++++ stores/settings-store.ts | 4 ++ 34 files changed, 211 insertions(+), 52 deletions(-) diff --git a/app/(main)/admin/_tabs/policy.tsx b/app/(main)/admin/_tabs/policy.tsx index e40904a5c..29c1a6a56 100644 --- a/app/(main)/admin/_tabs/policy.tsx +++ b/app/(main)/admin/_tabs/policy.tsx @@ -57,6 +57,7 @@ const RESTRICTABLE_SETTINGS = [ { key: 'sessionTimeout', label: 'Session Timeout', category: 'Privacy', type: 'number' }, { key: 'emailNotificationsEnabled', label: 'Email Notifications', category: 'Notifications', type: 'boolean' }, { key: 'calendarNotificationsEnabled', label: 'Calendar Notifications', category: 'Notifications', type: 'boolean' }, + { key: 'pushNotifyInboxOnly', label: 'Inbox-only Push Notifications', category: 'Notifications', type: 'boolean' }, { key: 'debugMode', label: 'Debug Mode', category: 'Advanced', type: 'boolean' }, ]; diff --git a/components/push-notification-prompt.tsx b/components/push-notification-prompt.tsx index dcae89ac9..d43492cdc 100644 --- a/components/push-notification-prompt.tsx +++ b/components/push-notification-prompt.tsx @@ -61,6 +61,9 @@ export function PushNotificationPrompt() { const emailNotificationsEnabled = useSettingsStore( (state) => state.emailNotificationsEnabled, ); + const pushNotifyInboxOnly = useSettingsStore( + (state) => state.pushNotifyInboxOnly, + ); const policyLoaded = usePolicyStore((state) => state.loaded); const policy = usePolicyStore((state) => state.policy); const userPushRelayUrl = useSettingsStore((state) => state.pushRelayUrl); @@ -123,6 +126,7 @@ export function PushNotificationPrompt() { client, relayBaseUrl, accountLabel: username ?? undefined, + inboxOnly: pushNotifyInboxOnly, }); }; resync(); @@ -131,7 +135,7 @@ export function PushNotificationPrompt() { const onVisible = () => { if (document.visibilityState === 'visible') resync(); }; document.addEventListener('visibilitychange', onVisible); return () => document.removeEventListener('visibilitychange', onVisible); - }, [accountId, client, isAuthenticated, isDemoMode, policyLoaded, relayBaseUrl, username]); + }, [accountId, client, isAuthenticated, isDemoMode, policyLoaded, pushNotifyInboxOnly, relayBaseUrl, username]); useEffect(() => { let cancelled = false; @@ -193,6 +197,7 @@ export function PushNotificationPrompt() { client, relayBaseUrl, accountLabel: username ?? undefined, + inboxOnly: pushNotifyInboxOnly, }); setShowPrompt(false); } catch (err) { diff --git a/components/settings/notification-settings.tsx b/components/settings/notification-settings.tsx index 75b05bd28..d55cb0676 100644 --- a/components/settings/notification-settings.tsx +++ b/components/settings/notification-settings.tsx @@ -47,6 +47,7 @@ export function NotificationSettings() { emailNotificationsEnabled, emailNotificationSound, notificationSoundChoice, + pushNotifyInboxOnly, calendarNotificationsEnabled, calendarNotificationSound, calendarInvitationParsingEnabled, @@ -130,6 +131,7 @@ export function NotificationSettings() { relayBaseUrl: activeRelayUrl, accountLabel: username ?? undefined, forceRecreate, + inboxOnly: pushNotifyInboxOnly, }); setPushStatus({ kind: 'enabled' }); } catch (err) { @@ -357,6 +359,20 @@ export function NotificationSettings() { disabled={!emailNotificationsEnabled} /> + + {!isSettingHidden('pushNotifyInboxOnly') && ( + + updateSetting('pushNotifyInboxOnly', checked)} + disabled={!emailNotificationsEnabled} + /> + + )} diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts index af35b9b0f..86e4d5fdb 100644 --- a/lib/__tests__/web-push.test.ts +++ b/lib/__tests__/web-push.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; import type { EmailPushConfig, PushSubscription as JmapPushSubscription } from '@/lib/jmap/types'; import { + buildEmailPushConfig, disableWebPush, enableWebPush, listPushDevices, @@ -171,7 +172,7 @@ describe('enableWebPush', () => { const client = makeClient([sub('push-old', THIS_DEVICE)]); installFetch({}); - const result = await enableWebPush({ client, relayBaseUrl: RELAY }); + const result = await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); expect(result.subscriptionId).toBe('push-old'); expect(client.createPushSubscription).not.toHaveBeenCalled(); @@ -184,7 +185,7 @@ describe('enableWebPush', () => { const client = makeClient([sub('push-old', THIS_DEVICE)]); installFetch({}); - const result = await enableWebPush({ client, relayBaseUrl: RELAY, forceRecreate: true }); + const result = await enableWebPush({ client, relayBaseUrl: RELAY, forceRecreate: true, inboxOnly: false }); expect(client.destroyed).toContain('push-old'); expect(client.createPushSubscription).toHaveBeenCalledTimes(1); @@ -198,7 +199,7 @@ describe('enableWebPush', () => { const client = makeClient([sub('push-other', OTHER_DEVICE)]); installFetch({ [OTHER_DEVICE]: 'unknown' }); - await enableWebPush({ client, relayBaseUrl: RELAY, forceRecreate: true }); + await enableWebPush({ client, relayBaseUrl: RELAY, forceRecreate: true, inboxOnly: false }); expect(client.destroyed).not.toContain('push-other'); }); @@ -214,7 +215,7 @@ describe('resyncWebPush', () => { const client = makeClient([sub('push-old', THIS_DEVICE)], { emailPushCapability: true }); installFetch({}); - expect(await resyncWebPush({ client, relayBaseUrl: RELAY })).toBe(true); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true); expect(client.createPushSubscription).not.toHaveBeenCalled(); expect(client.updatePushSubscription).toHaveBeenCalledWith( @@ -228,7 +229,7 @@ describe('resyncWebPush', () => { const client = makeClient([], { emailPushCapability: true }); installFetch({}); - expect(await resyncWebPush({ client, relayBaseUrl: RELAY })).toBe(false); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(false); expect(client.listPushSubscriptions).not.toHaveBeenCalled(); expect(client.createPushSubscription).not.toHaveBeenCalled(); @@ -241,13 +242,13 @@ describe('resyncWebPush', () => { installFetch({}); const now = vi.spyOn(Date, 'now').mockReturnValue(1_000_000); - expect(await resyncWebPush({ client, relayBaseUrl: RELAY })).toBe(true); - expect(await resyncWebPush({ client, relayBaseUrl: RELAY })).toBe(false); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(false); expect(client.listPushSubscriptions).toHaveBeenCalledTimes(1); // A tab left open past a day renews again, before Stalwart's 7-day expiry. now.mockReturnValue(1_000_000 + 25 * 60 * 60 * 1000); - expect(await resyncWebPush({ client, relayBaseUrl: RELAY })).toBe(true); + expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true); expect(client.listPushSubscriptions).toHaveBeenCalledTimes(2); now.mockRestore(); }); @@ -263,7 +264,7 @@ describe('resyncWebPush', () => { throw new Error('relay down'); })); - await expect(resyncWebPush({ client, relayBaseUrl: RELAY })).resolves.toBe(false); + await expect(resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).resolves.toBe(false); }); }); @@ -277,7 +278,7 @@ describe('enableWebPush delivery filter (emailPush)', () => { const client = makeClient([], { emailPushCapability: true }); installFetch({}); - await enableWebPush({ client, relayBaseUrl: RELAY }); + await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); expect(client.createPushSubscription).toHaveBeenCalledTimes(1); const params = (client.createPushSubscription as ReturnType).mock.calls[0][0]; @@ -290,7 +291,7 @@ describe('enableWebPush delivery filter (emailPush)', () => { const client = makeClient([]); installFetch({}); - await enableWebPush({ client, relayBaseUrl: RELAY }); + await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); const params = (client.createPushSubscription as ReturnType).mock.calls[0][0]; expect(params).not.toHaveProperty('emailPush'); @@ -303,7 +304,7 @@ describe('enableWebPush delivery filter (emailPush)', () => { const client = makeClient([sub('push-old', THIS_DEVICE)], { emailPushCapability: true }); installFetch({}); - const result = await enableWebPush({ client, relayBaseUrl: RELAY }); + const result = await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); expect(result.subscriptionId).toBe('push-old'); expect(client.createPushSubscription).not.toHaveBeenCalled(); @@ -332,7 +333,7 @@ describe('enableWebPush delivery filter (emailPush)', () => { ); installFetch({}); - await enableWebPush({ client, relayBaseUrl: RELAY }); + await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); expect(client.updatePushSubscription).toHaveBeenCalledWith( 'push-old', @@ -356,12 +357,46 @@ describe('enableWebPush delivery filter (emailPush)', () => { ); installFetch({}); - await enableWebPush({ client, relayBaseUrl: RELAY }); + await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); expect(client.updatePushSubscription).not.toHaveBeenCalled(); }); }); +// Inbox-only mode swaps the "not in Junk" mailbox condition for a positive +// "in Inbox" one, so mail a Sieve rule files into any other folder never +// reaches the push subscription. +describe('buildEmailPushConfig inbox-only mode', () => { + it('keeps current behaviour when inboxOnly is false', async () => { + const client = makeClient([], { emailPushCapability: true }); + expect(await buildEmailPushConfig(client, false)).toEqual(EXPECTED_EMAIL_PUSH); + }); + + it('requires the message to be in each account Inbox when inboxOnly is true', async () => { + const client = makeClient([], { emailPushCapability: true }); + + const config = await buildEmailPushConfig(client, true); + + expect(config[ACCOUNT_ID].filter).toEqual({ + operator: 'AND', + conditions: [{ notKeyword: '$junk' }, { inMailbox: 'mb-inbox' }], + }); + expect(config[SHARED_ACCOUNT_ID].filter).toEqual({ + operator: 'AND', + conditions: [{ notKeyword: '$junk' }, { inMailbox: 'mb-shared-inbox' }], + }); + }); + + it('throws when an account has no discoverable Inbox mailbox', async () => { + const client = makeClient([], { emailPushCapability: true }); + client.getAllMailboxes = vi.fn(async () => [ + { id: JUNK_ID, originalId: JUNK_ID, name: 'junk', role: 'junk', accountId: ACCOUNT_ID }, + ]) as unknown as IJMAPClient['getAllMailboxes']; + + await expect(buildEmailPushConfig(client, true)).rejects.toThrow(/No Inbox mailbox/); + }); +}); + describe('disableWebPush', () => { it('destroys every subscription this device owns, not just the recorded id', async () => { localStorage.setItem(DEVICE_KEY, THIS_DEVICE); diff --git a/lib/web-push.ts b/lib/web-push.ts index 0af89c82d..f193640dd 100644 --- a/lib/web-push.ts +++ b/lib/web-push.ts @@ -72,31 +72,53 @@ export function serverSupportsEmailPush(client: IJMAPClient): boolean { } /** - * The delivery filter we want on every account the subscription fans out to: - * skip anything the spam filter tagged `$junk` and anything that lives only in - * a Junk-role mailbox (Sieve `fileinto` doesn't set the keyword). The two are - * ANDed so a stale mailbox id - the user deleted and recreated Junk - degrades - * to keyword-only filtering rather than letting everything through. + * The delivery filter we want on every account the subscription fans out to. + * + * Default mode: skip anything the spam filter tagged `$junk` and anything + * that lives only in a Junk-role mailbox (Sieve `fileinto` doesn't set the + * keyword). The two are ANDed so a stale mailbox id - the user deleted and + * recreated Junk - degrades to keyword-only filtering rather than letting + * everything through. + * + * Inbox-only mode: same `$junk` keyword guard, but the mailbox condition + * requires the message to be IN the account's Inbox rather than merely NOT + * in Junk - mail a Sieve rule files into any other folder stays silent. */ export async function buildEmailPushConfig( client: IJMAPClient, + inboxOnly: boolean, ): Promise> { const primary = client.getAccountId(); const junkByAccount = new Map([[primary, []]]); + const inboxByAccount = new Map(); const mailboxes = await client.getAllMailboxes().catch(() => [] as Mailbox[]); for (const m of mailboxes) { const accountId = m.accountId || primary; - const junk = junkByAccount.get(accountId) ?? []; // Shared-account mailboxes carry a client-side ":" id; // the server only knows the original. - if (m.role === 'junk') junk.push(m.originalId ?? m.id); - junkByAccount.set(accountId, junk); + if (m.role === 'junk') { + const junk = junkByAccount.get(accountId) ?? []; + junk.push(m.originalId ?? m.id); + junkByAccount.set(accountId, junk); + } + if (m.role === 'inbox') inboxByAccount.set(accountId, m.originalId ?? m.id); } + if (!junkByAccount.has(primary)) junkByAccount.set(primary, []); const config: Record = {}; for (const [accountId, junkIds] of junkByAccount) { const conditions: Record[] = [{ notKeyword: '$junk' }]; - if (junkIds.length > 0) conditions.push({ inMailboxOtherThan: [...junkIds].sort() }); + if (inboxOnly) { + const inboxId = inboxByAccount.get(accountId); + if (!inboxId) { + throw new Error( + `No Inbox mailbox found for account ${accountId}; cannot build an inbox-only push filter`, + ); + } + conditions.push({ inMailbox: inboxId }); + } else if (junkIds.length > 0) { + conditions.push({ inMailboxOtherThan: [...junkIds].sort() }); + } config[accountId] = { // Always the operator form: that's how the server echoes it back, so a // stored config compares equal to a freshly built one. @@ -142,6 +164,10 @@ export interface EnableWebPushParams { // that outlives a permission change keeps pushing for mailboxes the user can // no longer read - recreating is the only client-side remedy (#841). forceRecreate?: boolean; + // Scope OS push notifications to Inbox-only mail. Mirrors settings-store's + // pushNotifyInboxOnly. Required, not optional: it changes the server-side + // delivery filter, so every caller must decide explicitly. + inboxOnly: boolean; } export interface EnableWebPushResult { @@ -451,7 +477,7 @@ export async function enableWebPush( // revoked shared-mailbox access (#841). const existingSubs = await params.client.listPushSubscriptions().catch(() => []); const emailPush = serverSupportsEmailPush(params.client) - ? await buildEmailPushConfig(params.client) + ? await buildEmailPushConfig(params.client, params.inboxOnly) : null; const subIdKey = subscriptionIdKey(accountId); const storedServerId = localStorage.getItem(subIdKey); @@ -663,6 +689,8 @@ export interface ResyncWebPushParams { client: IJMAPClient; relayBaseUrl?: string; accountLabel?: string; + // Mirrors settings-store's pushNotifyInboxOnly - see EnableWebPushParams. + inboxOnly: boolean; } /** @@ -689,6 +717,7 @@ export async function resyncWebPush(params: ResyncWebPushParams): Promise { }); }); + it('defaults pushNotifyInboxOnly to false and round-trips it through the sync payload', async () => { + const settings = useSettingsStore.getState(); + expect(settings.pushNotifyInboxOnly).toBe(false); + + settings.enableSync('a@example.com', 'https://mail-a.example.com'); + settings.updateSetting('pushNotifyInboxOnly', true); + + await useSettingsStore.getState().flushSync(); + + const request = apiFetch.mock.calls[0][1] as RequestInit; + expect(JSON.parse(request.body as string)).toMatchObject({ + settings: { pushNotifyInboxOnly: true }, + }); + }); + it('does not rewrite a pending account snapshot after another account becomes active', async () => { const settings = useSettingsStore.getState(); settings.enableSync('a@example.com', 'https://mail-a.example.com'); diff --git a/stores/settings-store.ts b/stores/settings-store.ts index 75d8372c6..668cd5dc9 100644 --- a/stores/settings-store.ts +++ b/stores/settings-store.ts @@ -396,6 +396,8 @@ interface SettingsState { emailNotificationsEnabled: boolean; emailNotificationSound: boolean; notificationSoundChoice: NotificationSoundChoice; + /** Web Push only fires for mail that lands in the Inbox; Sieve-filed mail stays silent. */ + pushNotifyInboxOnly: boolean; /** Chosen Web Push relay URL. Empty = the admin-configured default. */ pushRelayUrl: string; @@ -629,6 +631,7 @@ const DEFAULT_SETTINGS = { emailNotificationsEnabled: true, emailNotificationSound: true, notificationSoundChoice: 'default' as NotificationSoundChoice, + pushNotifyInboxOnly: false, pushRelayUrl: '', // Protocol Handlers @@ -828,6 +831,7 @@ export const useSettingsStore = create()( emailNotificationsEnabled: state.emailNotificationsEnabled, emailNotificationSound: state.emailNotificationSound, notificationSoundChoice: state.notificationSoundChoice, + pushNotifyInboxOnly: state.pushNotifyInboxOnly, pushRelayUrl: state.pushRelayUrl, protocolOpenMode: state.protocolOpenMode, calendarNotificationsEnabled: state.calendarNotificationsEnabled, From 405f6b78dbae052022524d920e6953510742ae3b Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Mon, 7 Sep 2026 12:36:44 +1200 Subject: [PATCH 2/4] fix(web-push): keep filter entry for junk-less accounts, surface fetch errors in inbox-only :bug: --- lib/__tests__/web-push.test.ts | 25 +++++++++++++++++++++++++ lib/web-push.ts | 18 ++++++++++++++---- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts index 86e4d5fdb..798a550a3 100644 --- a/lib/__tests__/web-push.test.ts +++ b/lib/__tests__/web-push.test.ts @@ -395,6 +395,31 @@ describe('buildEmailPushConfig inbox-only mode', () => { await expect(buildEmailPushConfig(client, true)).rejects.toThrow(/No Inbox mailbox/); }); + + it('rethrows a mailbox-fetch failure in inbox-only mode instead of reporting no Inbox', async () => { + const client = makeClient([], { emailPushCapability: true }); + client.getAllMailboxes = vi.fn(async () => { + throw new Error('JMAP down'); + }) as unknown as IJMAPClient['getAllMailboxes']; + + await expect(buildEmailPushConfig(client, true)).rejects.toThrow(/JMAP down/); + }); + + it('still emits a filter for an account that has an Inbox but no Junk mailbox', async () => { + const client = makeClient([], { emailPushCapability: true }); + client.getAllMailboxes = vi.fn(async () => [ + { id: 'mb-inbox', originalId: 'mb-inbox', name: 'inbox', role: 'inbox', accountId: ACCOUNT_ID }, + { id: `${SHARED_ACCOUNT_ID}:mb-si`, originalId: 'mb-si', name: 'inbox', role: 'inbox', accountId: SHARED_ACCOUNT_ID }, + ]) as unknown as IJMAPClient['getAllMailboxes']; + + const config = await buildEmailPushConfig(client, false); + + expect(Object.keys(config).sort()).toEqual([ACCOUNT_ID, SHARED_ACCOUNT_ID].sort()); + expect(config[SHARED_ACCOUNT_ID].filter).toEqual({ + operator: 'AND', + conditions: [{ notKeyword: '$junk' }], + }); + }); }); describe('disableWebPush', () => { diff --git a/lib/web-push.ts b/lib/web-push.ts index f193640dd..9c5b9f197 100644 --- a/lib/web-push.ts +++ b/lib/web-push.ts @@ -89,11 +89,21 @@ export async function buildEmailPushConfig( inboxOnly: boolean, ): Promise> { const primary = client.getAccountId(); - const junkByAccount = new Map([[primary, []]]); + // Every account that has any mailbox gets a filter entry - a secondary/shared + // account without a Junk-role mailbox still needs one so its subscription + // isn't left unfiltered. + const accountIds = new Set([primary]); + const junkByAccount = new Map(); const inboxByAccount = new Map(); - const mailboxes = await client.getAllMailboxes().catch(() => [] as Mailbox[]); + // In inbox-only mode a swallowed fetch failure would masquerade as "no Inbox + // mailbox" below, so let the real error surface; the default mode can still + // degrade to keyword-only filtering on a transient miss. + const mailboxes = inboxOnly + ? await client.getAllMailboxes() + : await client.getAllMailboxes().catch(() => [] as Mailbox[]); for (const m of mailboxes) { const accountId = m.accountId || primary; + accountIds.add(accountId); // Shared-account mailboxes carry a client-side ":" id; // the server only knows the original. if (m.role === 'junk') { @@ -103,10 +113,10 @@ export async function buildEmailPushConfig( } if (m.role === 'inbox') inboxByAccount.set(accountId, m.originalId ?? m.id); } - if (!junkByAccount.has(primary)) junkByAccount.set(primary, []); const config: Record = {}; - for (const [accountId, junkIds] of junkByAccount) { + for (const accountId of accountIds) { + const junkIds = junkByAccount.get(accountId) ?? []; const conditions: Record[] = [{ notKeyword: '$junk' }]; if (inboxOnly) { const inboxId = inboxByAccount.get(accountId); From c89fdcbf0c2ce1020c7b33daead40e18f6cbf0db Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Mon, 7 Sep 2026 15:21:35 +1200 Subject: [PATCH 3/4] feat(notifications): apply inbox-only push filter immediately on toggle :sparkles: Flipping the setting only wrote to the store; the server-side JMAP push filter stayed stale until the next app launch ran resyncWebPush. When push is already enabled on the device, re-run enableWebPush on change so the toggle takes effect without a restart. --- .../__tests__/notification-settings.test.tsx | 86 +++++++++++++++++++ components/settings/notification-settings.tsx | 39 ++++++++- 2 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 components/settings/__tests__/notification-settings.test.tsx diff --git a/components/settings/__tests__/notification-settings.test.tsx b/components/settings/__tests__/notification-settings.test.tsx new file mode 100644 index 000000000..3477523e0 --- /dev/null +++ b/components/settings/__tests__/notification-settings.test.tsx @@ -0,0 +1,86 @@ +import { render, waitFor } from '@testing-library/react'; +import { act } from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { NotificationSettings } from '../notification-settings'; +import { useSettingsStore } from '@/stores/settings-store'; +import { useAuthStore } from '@/stores/auth-store'; +import { usePolicyStore } from '@/stores/policy-store'; +import * as webPush from '@/lib/web-push'; + +// next-intl's useTranslations -> identity so labels are their keys. +vi.mock('next-intl', () => ({ + useTranslations: () => (key: string) => key, +})); + +// Keep the section wrapper out of the way. +vi.mock('../settings-section', async (importOriginal) => ({ + ...(await importOriginal()), + SettingsSection: ({ children }: { children: React.ReactNode }) =>
{children}
, +})); + +vi.mock('@/lib/web-push', () => ({ + WebPushUnsupportedError: class WebPushUnsupportedError extends Error {}, + isWebPushSupported: () => true, + isWebPushEnabled: vi.fn(async () => true), + enableWebPush: vi.fn(async () => ({ subscriptionId: 'sub-1' })), + disableWebPush: vi.fn(async () => undefined), + listPushDevices: vi.fn(async () => []), + revokePushDevice: vi.fn(async () => undefined), +})); + +const fakeClient = { getAccountId: () => 'acct-1' } as unknown as ReturnType< + typeof useAuthStore.getState +>['client']; + +describe('NotificationSettings - pushNotifyInboxOnly', () => { + beforeEach(() => { + vi.clearAllMocks(); + useSettingsStore.setState({ emailNotificationsEnabled: true, pushNotifyInboxOnly: false }); + useAuthStore.setState({ client: fakeClient, username: 'me@example.com' }); + usePolicyStore.setState({ + ...usePolicyStore.getState(), + policy: {}, + isSettingLocked: () => false, + isSettingHidden: () => false, + } as never); + }); + + it('does not re-sync push on mount', async () => { + render(); + // Let the "is push enabled?" effect settle. + await waitFor(() => expect(webPush.isWebPushEnabled).toHaveBeenCalled()); + expect(webPush.enableWebPush).not.toHaveBeenCalled(); + }); + + it('re-runs enableWebPush with the new flag when the toggle flips while push is on', async () => { + render(); + await waitFor(() => expect(webPush.isWebPushEnabled).toHaveBeenCalled()); + + act(() => { + useSettingsStore.getState().updateSetting('pushNotifyInboxOnly', true); + }); + + await waitFor(() => expect(webPush.enableWebPush).toHaveBeenCalledTimes(1)); + expect(webPush.enableWebPush).toHaveBeenCalledWith( + expect.objectContaining({ client: fakeClient, inboxOnly: true }), + ); + // forceRecreate stays off - a filter PATCH, not a destroy/recreate. + expect( + (webPush.enableWebPush as ReturnType).mock.calls[0][0], + ).not.toHaveProperty('forceRecreate', true); + }); + + it('does not re-sync when push is disabled on this device', async () => { + (webPush.isWebPushEnabled as ReturnType).mockResolvedValueOnce(false); + render(); + await waitFor(() => expect(webPush.isWebPushEnabled).toHaveBeenCalled()); + + act(() => { + useSettingsStore.getState().updateSetting('pushNotifyInboxOnly', true); + }); + + // Give any effect a chance to fire. + await Promise.resolve(); + expect(webPush.enableWebPush).not.toHaveBeenCalled(); + }); +}); diff --git a/components/settings/notification-settings.tsx b/components/settings/notification-settings.tsx index d55cb0676..9adaef737 100644 --- a/components/settings/notification-settings.tsx +++ b/components/settings/notification-settings.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { useTranslations } from 'next-intl'; import { useSettingsStore } from '@/stores/settings-store'; import { SettingsSection, SettingItem, ToggleSwitch, Select } from './settings-section'; @@ -148,6 +148,43 @@ export function NotificationSettings() { } }; + // Flipping "Inbox only" only writes the setting; the server-side JMAP push + // filter stays stale until the next app launch runs resyncWebPush. When push + // is already on for this device, re-run the enable flow now so the toggle + // takes effect immediately instead of "on next restart". The ref seeds to the + // mounted value so this never fires on mount or on an unrelated re-render. + const lastSyncedInboxOnly = useRef(pushNotifyInboxOnly); + useEffect(() => { + if (lastSyncedInboxOnly.current === pushNotifyInboxOnly) return; + lastSyncedInboxOnly.current = pushNotifyInboxOnly; + // Push off on this device: enabling it later builds the filter from the + // current setting, so there's nothing to re-sync now. + if (!client || pushStatus.kind !== 'enabled') return; + + let cancelled = false; + setPushStatus({ kind: 'busy' }); + void enableWebPush({ + client, + relayBaseUrl: activeRelayUrl, + accountLabel: username ?? undefined, + inboxOnly: pushNotifyInboxOnly, + }) + .then(() => { if (!cancelled) setPushStatus({ kind: 'enabled' }); }) + .catch((err) => { + if (cancelled) return; + if (err instanceof WebPushUnsupportedError) { + setPushStatus({ kind: 'unsupported' }); + return; + } + setPushStatus({ + kind: 'error', + message: err instanceof Error ? err.message : 'Failed to update push filter', + }); + }) + .finally(() => { if (!cancelled) void refreshDevices(); }); + return () => { cancelled = true; }; + }, [pushNotifyInboxOnly, client, pushStatus.kind, activeRelayUrl, username, refreshDevices]); + const handleDisablePush = async () => { if (!client) return; const confirmed = await confirmDialog({ From 4078a7494a8f3994bb937c5db446243c5b23c48a Mon Sep 17 00:00:00 2001 From: Aaron Guise Date: Mon, 7 Sep 2026 16:24:17 +1200 Subject: [PATCH 4/4] =?UTF-8?q?fix(notifications):=20don't=20let=20the=20i?= =?UTF-8?q?nbox-only=20re-sync=20hang=20on=20"Working=E2=80=A6"=20:bug:?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-sync effect depended on pushStatus.kind, so its own setPushStatus({ kind: 'busy' }) re-fired the effect, whose cleanup cancelled the in-flight enableWebPush - the push row then sat on "Working…" forever and never updated the filter. Depend only on the setting flip; read client/relay/status live from the recreated closure. --- .../__tests__/notification-settings.test.tsx | 23 +++++++++++++++++-- components/settings/notification-settings.tsx | 7 +++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/components/settings/__tests__/notification-settings.test.tsx b/components/settings/__tests__/notification-settings.test.tsx index 3477523e0..18ebcd5d7 100644 --- a/components/settings/__tests__/notification-settings.test.tsx +++ b/components/settings/__tests__/notification-settings.test.tsx @@ -53,8 +53,8 @@ describe('NotificationSettings - pushNotifyInboxOnly', () => { }); it('re-runs enableWebPush with the new flag when the toggle flips while push is on', async () => { - render(); - await waitFor(() => expect(webPush.isWebPushEnabled).toHaveBeenCalled()); + const { findByText } = render(); + await findByText('push.status_active'); act(() => { useSettingsStore.getState().updateSetting('pushNotifyInboxOnly', true); @@ -70,6 +70,25 @@ describe('NotificationSettings - pushNotifyInboxOnly', () => { ).not.toHaveProperty('forceRecreate', true); }); + it('does not get stuck on "busy" after the re-sync resolves', async () => { + const { findByText, queryByText } = render(); + await findByText('push.status_active'); + const devicesLoadsBefore = (webPush.listPushDevices as ReturnType).mock.calls.length; + + act(() => { + useSettingsStore.getState().updateSetting('pushNotifyInboxOnly', true); + }); + + // enableWebPush resolves -> status returns to active, device list refreshes. + await findByText('push.status_active'); + expect(queryByText('push.status_busy')).toBeNull(); + await waitFor(() => + expect( + (webPush.listPushDevices as ReturnType).mock.calls.length, + ).toBeGreaterThan(devicesLoadsBefore), + ); + }); + it('does not re-sync when push is disabled on this device', async () => { (webPush.isWebPushEnabled as ReturnType).mockResolvedValueOnce(false); render(); diff --git a/components/settings/notification-settings.tsx b/components/settings/notification-settings.tsx index 9adaef737..438b36b00 100644 --- a/components/settings/notification-settings.tsx +++ b/components/settings/notification-settings.tsx @@ -183,7 +183,12 @@ export function NotificationSettings() { }) .finally(() => { if (!cancelled) void refreshDevices(); }); return () => { cancelled = true; }; - }, [pushNotifyInboxOnly, client, pushStatus.kind, activeRelayUrl, username, refreshDevices]); + // Only the setting flip should trigger a re-sync. client / relay / username + // are read live from the recreated closure; depending on pushStatus.kind + // here would re-fire the effect on the busy->enabled transition it causes + // and cancel its own in-flight call (leaving the row stuck on "Working"). + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [pushNotifyInboxOnly]); const handleDisablePush = async () => { if (!client) return;