diff --git a/components/__tests__/push-notification-prompt.test.tsx b/components/__tests__/push-notification-prompt.test.tsx index d09e7d83b..fd71589b1 100644 --- a/components/__tests__/push-notification-prompt.test.tsx +++ b/components/__tests__/push-notification-prompt.test.tsx @@ -17,7 +17,12 @@ const mocks = vi.hoisted(() => ({ username: "alice@example.com", isAuthenticated: true, isDemoMode: false, + connectedAccountsRevision: 0, + // Login id -> client, as the auth store keeps it for the account switcher. + connected: new Map string }>(), + getAllConnectedClients: () => new Map(mocks.authState.connected), }, + accounts: [] as { id: string; username: string }[], settingsState: { emailNotificationsEnabled: true, pushRelayUrl: "" }, policyState: { loaded: true, @@ -34,8 +39,15 @@ vi.mock("next/navigation", () => ({ })); vi.mock("@/stores/auth-store", () => ({ - useAuthStore: (selector: (state: typeof mocks.authState) => unknown) => - selector(mocks.authState), + useAuthStore: Object.assign( + (selector: (state: typeof mocks.authState) => unknown) => + selector(mocks.authState), + { getState: () => mocks.authState }, + ), +})); + +vi.mock("@/stores/account-store", () => ({ + useAccountStore: { getState: () => ({ accounts: mocks.accounts }) }, })); vi.mock("@/stores/settings-store", () => ({ @@ -82,6 +94,9 @@ describe("PushNotificationPrompt", () => { mocks.supported = true; mocks.enabled = false; mocks.authState.client = { getAccountId: () => "account-1" }; + mocks.authState.connectedAccountsRevision = 0; + mocks.authState.connected = new Map([["alice@mail.example.com", mocks.authState.client]]); + mocks.accounts = [{ id: "alice@mail.example.com", username: "alice@example.com" }]; mocks.authState.username = "alice@example.com"; mocks.authState.isAuthenticated = true; mocks.authState.isDemoMode = false; @@ -188,6 +203,63 @@ describe("PushNotificationPrompt", () => { expect(mocks.enableWebPush).not.toHaveBeenCalled(); }); + it("renews push for every connected login, not only the active one", async () => { + // Push for bob was enabled from the all-accounts panel; he never becomes + // the active account, so only this background pass keeps his + // registration from hitting the server's 7-day expiry. + const bob = { getAccountId: () => "account-2" }; + mocks.authState.connected.set("bob@mail.example.com", bob); + mocks.accounts.push({ id: "bob@mail.example.com", username: "bob@example.com" }); + + render(); + await advancePromptDelay(); + + expect(mocks.resyncWebPush).toHaveBeenCalledTimes(2); + expect(mocks.resyncWebPush.mock.calls[0][0]).toMatchObject({ + client: mocks.authState.client, + accountLabel: "alice@example.com", + }); + expect(mocks.resyncWebPush.mock.calls[1][0]).toMatchObject({ + client: bob, + accountLabel: "bob@example.com", + relayBaseUrl: "https://notifications.relay.bulwarkmail.org", + }); + }); + + it("renews the other logins again when the tab returns to the foreground", async () => { + const bob = { getAccountId: () => "account-2" }; + mocks.authState.connected.set("bob@mail.example.com", bob); + render(); + await advancePromptDelay(); + mocks.resyncWebPush.mockClear(); + + await act(async () => { + document.dispatchEvent(new Event("visibilitychange")); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mocks.resyncWebPush.mock.calls.map(([p]) => p.client)).toEqual([ + mocks.authState.client, + bob, + ]); + }); + + it("reaches logins that finish connecting after the first pass", async () => { + const { rerender } = render(); + await advancePromptDelay(); + expect(mocks.resyncWebPush).toHaveBeenCalledTimes(1); + + // The background restore connects bob and bumps the revision. + const bob = { getAccountId: () => "account-2" }; + mocks.authState.connected.set("bob@mail.example.com", bob); + mocks.authState.connectedAccountsRevision = 1; + rerender(); + await advancePromptDelay(); + + expect(mocks.resyncWebPush.mock.calls.map(([p]) => p.client)).toContain(bob); + }); + it("does not touch push in demo mode", async () => { mocks.authState.isDemoMode = true; render(); diff --git a/components/push-notification-prompt.tsx b/components/push-notification-prompt.tsx index d43492cdc..fae24ee61 100644 --- a/components/push-notification-prompt.tsx +++ b/components/push-notification-prompt.tsx @@ -4,6 +4,7 @@ import { useCallback, useEffect, useLayoutEffect, useState } from "react"; import { usePathname } from "next/navigation"; import { useTranslations } from "next-intl"; import { Bell, Loader2, X } from "@/components/icons"; +import { useAccountStore } from "@/stores/account-store"; import { useAuthStore } from "@/stores/auth-store"; import { usePolicyStore } from "@/stores/policy-store"; import { useSettingsStore } from "@/stores/settings-store"; @@ -26,6 +27,37 @@ function dismissedKey(accountId: string): string { return `${DISMISSED_KEY_PREFIX}${accountId}`; } +interface ResyncTarget { + client: NonNullable["client"]>; + accountLabel?: string; +} + +/** + * Every login whose push registration this tab should keep alive: the active + * one first, then each other connected login. Push can be enabled for a login + * that is not on screen (the all-accounts panel in settings), and Stalwart + * expires a PushSubscription after 7 days, so renewing only the active login + * would let the others lapse. The other clients are the ones the auth store + * already keeps for the account switcher, so this opens no new connection; + * resyncWebPush itself skips logins without push in a local check. + */ +function resyncTargets( + activeClient: ResyncTarget["client"], + activeLabel: string | undefined, +): ResyncTarget[] { + const targets: ResyncTarget[] = [{ client: activeClient, accountLabel: activeLabel }]; + const auth = useAuthStore.getState(); + const others = auth.getAllConnectedClients?.() ?? new Map(); + if (others.size === 0) return targets; + const accounts = useAccountStore.getState().accounts; + for (const [loginId, other] of others) { + if (!other || other === activeClient) continue; + const account = accounts.find((a) => a.id === loginId); + targets.push({ client: other, accountLabel: account?.username }); + } + return targets; +} + function isExcludedPath(pathname: string): boolean { return /(^|\/)settings(?:\/|$)/.test(pathname) || /(^|\/)setup(?:\/|$)/.test(pathname) @@ -58,6 +90,11 @@ export function PushNotificationPrompt() { const username = useAuthStore((state) => state.username); const isAuthenticated = useAuthStore((state) => state.isAuthenticated); const isDemoMode = useAuthStore((state) => state.isDemoMode); + // Bumped when the background logins finish connecting: the renewal below + // re-runs then so it reaches them too. + const connectedAccountsRevision = useAuthStore( + (state) => state.connectedAccountsRevision, + ); const emailNotificationsEnabled = useSettingsStore( (state) => state.emailNotificationsEnabled, ); @@ -121,21 +158,30 @@ export function PushNotificationPrompt() { // live registration stale. useEffect(() => { if (!policyLoaded || !isAuthenticated || !client || !accountId || isDemoMode) return; - const resync = () => { - void resyncWebPush({ - client, - relayBaseUrl, - accountLabel: username ?? undefined, - inboxOnly: pushNotifyInboxOnly, - }); + let cancelled = false; + const resync = async () => { + // One login at a time: they share the browser's single + // PushSubscription, and a parallel re-create would race on it. + for (const target of resyncTargets(client, username ?? undefined)) { + if (cancelled) return; + await resyncWebPush({ + client: target.client, + relayBaseUrl, + accountLabel: target.accountLabel, + inboxOnly: pushNotifyInboxOnly, + }); + } }; - resync(); + void resync(); // A long-open tab comes back to the foreground: renew the subscription // before the server's 7-day expiry (resyncWebPush runs at most daily). - const onVisible = () => { if (document.visibilityState === 'visible') resync(); }; + const onVisible = () => { if (document.visibilityState === 'visible') void resync(); }; document.addEventListener('visibilitychange', onVisible); - return () => document.removeEventListener('visibilitychange', onVisible); - }, [accountId, client, isAuthenticated, isDemoMode, policyLoaded, pushNotifyInboxOnly, relayBaseUrl, username]); + return () => { + cancelled = true; + document.removeEventListener('visibilitychange', onVisible); + }; + }, [accountId, client, connectedAccountsRevision, isAuthenticated, isDemoMode, policyLoaded, pushNotifyInboxOnly, relayBaseUrl, username]); useEffect(() => { let cancelled = false; diff --git a/components/settings/__tests__/multi-account-push.test.tsx b/components/settings/__tests__/multi-account-push.test.tsx index 1c3f01eaf..202078635 100644 --- a/components/settings/__tests__/multi-account-push.test.tsx +++ b/components/settings/__tests__/multi-account-push.test.tsx @@ -1,4 +1,4 @@ -import { render, waitFor } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import { describe, it, expect, vi } from 'vitest'; import { MultiAccountPushSettings } from '../multi-account-push'; import { useAccountStore } from '@/stores/account-store'; @@ -6,7 +6,8 @@ import { useAuthStore } from '@/stores/auth-store'; import * as webPush from '@/lib/web-push'; vi.mock('next-intl', () => ({ - useTranslations: () => (key: string) => key, + useTranslations: () => (key: string, values?: Record) => + values ? `${key} ${JSON.stringify(values)}` : key, })); vi.mock('../settings-section', async (importOriginal) => ({ @@ -44,4 +45,38 @@ describe('MultiAccountPushSettings', () => { expect(webPush.isWebPushEnabled).toHaveBeenCalledWith('jmap-a'); expect(webPush.isWebPushEnabled).not.toHaveBeenCalledWith('a@mail.example.com'); }); + + it('names each account the bulk enable left off, with the reason', async () => { + const clients: Record string }> = { + 'a@mail.example.com': { getAccountId: () => 'jmap-a' }, + 'b@mail.example.com': { getAccountId: () => 'jmap-b' }, + 'c@mail.example.com': { getAccountId: () => 'jmap-c' }, + }; + useAccountStore.setState({ + accounts: [ + { id: 'a@mail.example.com', username: 'a', email: 'a@example.com', displayName: 'Alice', isConnected: true }, + { id: 'b@mail.example.com', username: 'b', email: 'b@example.com', displayName: 'Bob', isConnected: true }, + { id: 'c@mail.example.com', username: 'c', email: 'c@example.com', displayName: 'Carol', isConnected: true }, + ] as never, + }); + useAuthStore.setState({ getClientForAccount: ((id: string) => clients[id]) as never }); + vi.mocked(webPush.enableWebPushForAccounts).mockResolvedValueOnce({ + enabled: ['a@mail.example.com'], + failed: [ + { accountId: 'b@mail.example.com', error: new Error('relay unreachable') }, + { accountId: 'c@mail.example.com', error: 'timeout' }, + ], + }); + + render(); + await waitFor(() => expect(screen.getByText('select_all')).toBeInTheDocument()); + fireEvent.click(screen.getByLabelText('select_all')); + fireEvent.click(screen.getByText('enable_selected')); + + const alert = await screen.findByRole('alert'); + expect(screen.getByText(/result_partial/).textContent).toContain('"enabled":1,"failed":2'); + expect(alert.textContent).toContain('Bob (b@example.com): relay unreachable'); + expect(alert.textContent).toContain('Carol (c@example.com): timeout'); + expect(alert.textContent).not.toContain('Alice'); + }); }); diff --git a/components/settings/multi-account-push.tsx b/components/settings/multi-account-push.tsx index edf4711a1..9df3bfa4b 100644 --- a/components/settings/multi-account-push.tsx +++ b/components/settings/multi-account-push.tsx @@ -10,6 +10,18 @@ import { useAuthStore } from '@/stores/auth-store'; import { useSettingsStore } from '@/stores/settings-store'; import { disableWebPush, enableWebPushForAccounts, isWebPushEnabled, isWebPushSupported } from '@/lib/web-push'; +interface FailedAccount { + id: string; + name: string; + email: string; + reason: string; +} + +function failureReason(error: unknown): string { + if (error instanceof Error && error.message) return error.message; + return String(error); +} + /** * Turn push on or off for several accounts at once. * @@ -27,6 +39,9 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri const [chosen, setChosen] = useState>(new Set()); const [busy, setBusy] = useState(false); const [notice, setNotice] = useState(''); + // Which accounts the last bulk enable left off, and why: a bare count + // leaves the user guessing which login to retry. + const [failures, setFailures] = useState([]); const connected = accounts.filter(a => a.isConnected && useAuthStore.getState().getClientForAccount(a.id)); @@ -56,7 +71,7 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri const auth = useAuthStore.getState(); const targets = connected.filter(a => chosen.has(a.id)); if (!targets.length) return; - setBusy(true); setNotice(''); + setBusy(true); setNotice(''); setFailures([]); try { if (action === 'enable') { const outcome = await enableWebPushForAccounts( @@ -66,6 +81,15 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri setNotice(outcome.failed.length ? t('result_partial', { enabled: outcome.enabled.length, failed: outcome.failed.length }) : t('result_enabled', { count: outcome.enabled.length })); + setFailures(outcome.failed.map(({ accountId, error }) => { + const account = targets.find(a => a.id === accountId); + return { + id: accountId, + name: account ? (account.displayName || account.label || account.username) : accountId, + email: account ? (account.email || account.username) : '', + reason: failureReason(error), + }; + })); } else { // Teardown swallows its own failures, so every account ends up local-off. for (const account of targets) { @@ -112,6 +136,13 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri {notice &&

{notice}

} + {failures.length > 0 &&
    + {failures.map(f =>
  • + {f.name} + {f.email && f.email !== f.name && ({f.email})} + : {f.reason} +
    • )} +
} ; }