From aa759ba54abd187df78f7e18ee49e7e70a2cdcc9 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 17:25:20 +0200 Subject: [PATCH] fix(settings): phone notification toggles save only the switch that was tapped On the phone, every notification toggle sent the whole preference matrix as it looked when the switch was tapped. With two toggles in flight the later request could overwrite the earlier one on the server, and when the first request failed the screen rolled that switch back while the second request had already saved its value anyway. The cause was the sendWholeMatrix option of useNotificationSettings, which only the phone turned on. The server merges whatever cells it receives, so the desktop already sent just the toggled cell. The option is gone and both views send only the cell that changed. Regression test FE-MOB-SETNOTIF-035 (MSettingsNotifications.test.tsx) was red on the old code: "expected [ { trip_invite: { inapp: false, webhook: false } }, { trip_invite: { inapp: false, webhook: true } } ] to deeply equal [ { trip_invite: { inapp: false } }, { trip_invite: { webhook: true } } ]". Changed assertions: - useNotificationSettings.test.ts FE-COMP-NOTIFSETTINGS-004: the phone toggle body was { trip_invite: { inapp: false, email: false } }, now { trip_invite: { inapp: false } } (the hook is built with the phone's remaining option, skipMaskedToken). - MSettingsNotifications.test.tsx FE-MOB-SETNOTIF-026: the PUT body was { trip_invite: { inapp: false, webhook: false } }, now { trip_invite: { inapp: false } }. - MSettingsNotifications.test.tsx FE-MOB-SETNOTIF-032: the mock told the in-app write apart by body.trip_invite.webhook === false, which only the whole matrix carried; it now checks for the inapp key. --- .../Settings/useNotificationSettings.test.ts | 6 +-- .../Settings/useNotificationSettings.ts | 44 ++++++------------- .../settings/MSettingsNotifications.tsx | 2 +- .../settings/MSettingsNotifications.test.tsx | 34 ++++++++++++-- 4 files changed, 48 insertions(+), 38 deletions(-) diff --git a/client/src/components/Settings/useNotificationSettings.test.ts b/client/src/components/Settings/useNotificationSettings.test.ts index aa6a85440..e4d5bca62 100644 --- a/client/src/components/Settings/useNotificationSettings.test.ts +++ b/client/src/components/Settings/useNotificationSettings.test.ts @@ -77,12 +77,12 @@ describe('useNotificationSettings', () => { expect(result.current.saving).toBe(false); }); - it('FE-COMP-NOTIFSETTINGS-004: the phone toggle sends the whole matrix', async () => { + it('FE-COMP-NOTIFSETTINGS-004: the phone toggle sends only the toggled cell too', async () => { stubLoad(); const update = vi.spyOn(notificationsApi, 'updatePreferences').mockResolvedValue({}); - const { result } = await loaded({ sendWholeMatrix: true }); + const { result } = await loaded({ skipMaskedToken: true }); await act(() => result.current.toggle('trip_invite', 'inapp')); - expect(update).toHaveBeenCalledWith({ trip_invite: { inapp: false, email: false } }); + expect(update).toHaveBeenCalledWith({ trip_invite: { inapp: false } }); expect(result.current.matrix?.preferences.trip_invite).toEqual({ inapp: false, email: false }); }); diff --git a/client/src/components/Settings/useNotificationSettings.ts b/client/src/components/Settings/useNotificationSettings.ts index 59127519b..62b811851 100644 --- a/client/src/components/Settings/useNotificationSettings.ts +++ b/client/src/components/Settings/useNotificationSettings.ts @@ -31,11 +31,6 @@ export interface PreferencesMatrix { export const MASKED = '••••••••'; export interface NotificationSettingsOptions { - /** - * The phone sends the whole preference matrix on a toggle; the desktop sends only - * the toggled cell, so a second toggle in flight cannot be overwritten by it. - */ - sendWholeMatrix?: boolean; /** The phone never sends an ntfy token that reads exactly like the mask. */ skipMaskedToken?: boolean; } @@ -45,10 +40,7 @@ export interface NotificationSettingsOptions { * phone section render their own markup over this): the event/channel matrix, the * webhook and ntfy credentials, and the test sends for every channel. */ -export function useNotificationSettings({ - sendWholeMatrix = false, - skipMaskedToken = false, -}: NotificationSettingsOptions = {}) { +export function useNotificationSettings({ skipMaskedToken = false }: NotificationSettingsOptions = {}) { const { t } = useTranslation(); const toast = useToast(); const [matrix, setMatrix] = useState(null); @@ -128,28 +120,18 @@ export function useNotificationSettings({ const toggle = async (eventType: string, channel: string) => { if (!matrix || isLockedCell(matrix.locked, eventType, channel)) return; const current = matrix.preferences[eventType]?.[channel] ?? true; - let payload: Record>; - if (sendWholeMatrix) { - const updated = { - ...matrix.preferences, - [eventType]: { ...matrix.preferences[eventType], [channel]: !current }, - }; - setMatrix((m) => (m ? { ...m, preferences: updated } : m)); - payload = updated; - } else { - setMatrix((m) => - m - ? { - ...m, - preferences: { ...m.preferences, [eventType]: { ...m.preferences[eventType], [channel]: !current } }, - } - : m - ); - // Only the toggled cell goes out. The server merges what it gets, and sending the - // whole matrix would carry this render's value for every other cell: a second - // toggle made while this request is in flight would be overwritten by it. - payload = { [eventType]: { [channel]: !current } }; - } + setMatrix((m) => + m + ? { + ...m, + preferences: { ...m.preferences, [eventType]: { ...m.preferences[eventType], [channel]: !current } }, + } + : m + ); + // Only the toggled cell goes out. The server merges what it gets, and sending the + // whole matrix would carry this render's value for every other cell: a second + // toggle made while this request is in flight would be overwritten by it. + const payload = { [eventType]: { [channel]: !current } }; setSaving(true); try { await notificationsApi.updatePreferences(payload); diff --git a/client/src/mobile/screens/settings/MSettingsNotifications.tsx b/client/src/mobile/screens/settings/MSettingsNotifications.tsx index b2e8fa1e6..8d171ae8a 100644 --- a/client/src/mobile/screens/settings/MSettingsNotifications.tsx +++ b/client/src/mobile/screens/settings/MSettingsNotifications.tsx @@ -21,7 +21,7 @@ export default function MSettingsNotifications() { webhookUrl, setWebhookUrl, webhookIsSet, webhookSaving, webhookTesting, saveWebhookUrl, testWebhookUrl, ntfyTopic, setNtfyTopic, ntfyServer, setNtfyServer, ntfyToken, setNtfyToken, ntfyTokenIsSet, ntfySaving, ntfyTesting, saveNtfySettings, clearNtfyToken, testNtfySettings, - } = useNotificationSettings({ sendWholeMatrix: true, skipMaskedToken: true }) + } = useNotificationSettings({ skipMaskedToken: true }) return ( diff --git a/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx b/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx index 7cb5205d2..4299149cb 100644 --- a/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx +++ b/client/tests/unit/mobile/settings/MSettingsNotifications.test.tsx @@ -442,7 +442,7 @@ describe('MSettingsNotifications', () => { }); describe('event matrix', () => { - it('FE-MOB-SETNOTIF-026: tapping a chip flips the preference and PUTs the whole matrix', async () => { + it('FE-MOB-SETNOTIF-026: tapping a chip flips the preference and PUTs only that cell', async () => { const user = userEvent.setup(); let body: Record> | null = null; server.use( @@ -457,7 +457,7 @@ describe('MSettingsNotifications', () => { await user.click(chip); await waitFor(() => expect(body).not.toBeNull()); - expect(body).toEqual({ trip_invite: { inapp: false, webhook: false } }); + expect(body).toEqual({ trip_invite: { inapp: false } }); }); it('FE-MOB-SETNOTIF-027: a channel with no stored preference defaults to on', async () => { @@ -511,7 +511,7 @@ describe('MSettingsNotifications', () => { http.put('/api/notifications/preferences', async ({ request }) => { const body = (await request.json()) as Record>; // The first write is the in-app flip; it hangs until the test rejects it. - if (body.trip_invite.webhook === false) { + if ('inapp' in body.trip_invite) { return new Promise(resolve => { rejectInapp = () => resolve(HttpResponse.json({ error: 'nope' }, { status: 500 }) as unknown as Response); }); @@ -541,6 +541,34 @@ describe('MSettingsNotifications', () => { expect(screen.getByRole('button', { name: 'Webhook' }).className).toBe(activeClass); }); + it('FE-MOB-SETNOTIF-035: a second toggle while the first is in flight sends only its own cell', async () => { + const user = userEvent.setup(); + const bodies: Record>[] = []; + let rejectFirst!: () => void; + server.use( + http.put('/api/notifications/preferences', async ({ request }) => { + bodies.push((await request.json()) as Record>); + if (bodies.length === 1) { + return new Promise(resolve => { + rejectFirst = () => resolve(HttpResponse.json({ error: 'nope' }, { status: 500 }) as unknown as Response); + }); + } + return HttpResponse.json({ success: true }); + }), + ); + render(); + + await user.click(await screen.findByRole('button', { name: 'In-App' })); + await waitFor(() => expect(bodies).toHaveLength(1)); + await user.click(screen.getByRole('button', { name: 'Webhook' })); + await waitFor(() => expect(bodies).toHaveLength(2)); + rejectFirst(); + + // The failed in-app flip is rolled back on screen, so the server must not get it + // through the webhook write either. + expect(bodies).toEqual([{ trip_invite: { inapp: false } }, { trip_invite: { webhook: true } }]); + }); + it('FE-MOB-SETNOTIF-029: the saving hint shows while the update is in flight', async () => { const user = userEvent.setup(); let release!: () => void;