From 2acef40c35dac0d8f13dab9a8c3e1b17b31fd8f6 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 10:25:45 +0200 Subject: [PATCH] fix(admin): keep a later notification toggle when an earlier one fails on the phone When a toggle on the phone matrix failed after a later toggle had already rendered, its catch put back the preferences from its own click and so also undid the later cell. The live preferences are now mirrored in a ref and a failure reverts only its own cell, as the desktop panel already does. The test hangs the first save, lets the second one through and then fails the first. --- .../screens/admin/MAdminNotifyMatrix.tsx | 29 ++++++++---- .../mobile/admin/MAdminNotifyMatrix.test.tsx | 47 ++++++++++++++++++- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/client/src/mobile/screens/admin/MAdminNotifyMatrix.tsx b/client/src/mobile/screens/admin/MAdminNotifyMatrix.tsx index b17b3615c..b9bd382c1 100644 --- a/client/src/mobile/screens/admin/MAdminNotifyMatrix.tsx +++ b/client/src/mobile/screens/admin/MAdminNotifyMatrix.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useEffect, useRef, useState } from 'react' import { adminApi } from '../../../api/client' import type { TranslationFn } from '../../../types' import type { useToast } from '../../../components/shared/Toast' @@ -24,9 +24,20 @@ const BUILTIN_CHANNELS = ['inapp', 'email', 'webhook', 'ntfy'] as const // AdminNotificationsPanel. Loads its own data and auto-saves each toggle. export default function MAdminNotifyMatrix({ t, toast }: { t: TranslationFn; toast: ReturnType }) { const [matrix, setMatrix] = useState(null) + // Toggles fire faster than React re-renders, so the live preferences are mirrored in a + // ref. Reading state out of the render closure would let a second toggle undo the first. + const prefsRef = useRef(null) + + const writePrefs = (prefs: MatrixData['preferences']) => { + prefsRef.current = prefs + setMatrix((m) => (m ? { ...m, preferences: prefs } : m)) + } useEffect(() => { - adminApi.getNotificationPreferences().then((data: MatrixData) => setMatrix(data)).catch(() => {}) + adminApi.getNotificationPreferences().then((data: MatrixData) => { + prefsRef.current = data.preferences + setMatrix(data) + }).catch(() => {}) }, []) if (!matrix) { @@ -53,16 +64,16 @@ export default function MAdminNotifyMatrix({ t, toast }: { t: TranslationFn; toa ) const toggle = async (eventType: string, channel: string) => { - const current = matrix.preferences[eventType]?.[channel] ?? true - const updated = { - ...matrix.preferences, - [eventType]: { ...matrix.preferences[eventType], [channel]: !current }, - } - setMatrix((m) => (m ? { ...m, preferences: updated } : m)) + const before = prefsRef.current ?? matrix.preferences + const current = before[eventType]?.[channel] ?? true + const updated = { ...before, [eventType]: { ...before[eventType], [channel]: !current } } + writePrefs(updated) try { await adminApi.updateNotificationPreferences(updated) } catch { - setMatrix((m) => (m ? { ...m, preferences: matrix.preferences } : m)) + // Revert this cell only: a toggle that already went through keeps its value. + const latest = prefsRef.current ?? updated + writePrefs({ ...latest, [eventType]: { ...latest[eventType], [channel]: current } }) toast.error(t('common.error')) } } diff --git a/client/tests/unit/mobile/admin/MAdminNotifyMatrix.test.tsx b/client/tests/unit/mobile/admin/MAdminNotifyMatrix.test.tsx index ecc93d99f..ab10c5a5e 100644 --- a/client/tests/unit/mobile/admin/MAdminNotifyMatrix.test.tsx +++ b/client/tests/unit/mobile/admin/MAdminNotifyMatrix.test.tsx @@ -1,8 +1,8 @@ -// FE-MOB-AMATRIX-001 to FE-MOB-AMATRIX-011 +// FE-MOB-AMATRIX-001 to FE-MOB-AMATRIX-012 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { http, HttpResponse } from 'msw'; import userEvent from '@testing-library/user-event'; -import { render, screen, waitFor } from '../../../helpers/render'; +import { act, render, screen, waitFor } from '../../../helpers/render'; import { server } from '../../../helpers/msw/server'; import { resetAllStores } from '../../../helpers/store'; import { useTranslation } from '../../../../src/i18n'; @@ -201,4 +201,47 @@ describe('MAdminNotifyMatrix', () => { expect(screen.queryByRole('switch')).not.toBeInTheDocument(); expect(screen.queryByText('In-App')).not.toBeInTheDocument(); }); + it('FE-MOB-AMATRIX-012: a failing earlier save reverts only its own cell, not a later one', async () => { + matrixRespondsWith(MATRIX); + const bodies: Record[] = []; + let failFirst: () => void = () => {}; + server.use( + http.put('/api/admin/notification-preferences', async ({ request }) => { + bodies.push((await request.json()) as Record); + if (bodies.length === 1) { + await new Promise((resolve) => { + failFirst = resolve; + }); + return HttpResponse.json({}, { status: 500 }); + } + return HttpResponse.json({ success: true }); + }) + ); + const toast = buildToast(); + const user = userEvent.setup(); + render(); + + const inappToggle = await screen.findByRole('switch', { name: 'version_available inapp' }); + const emailToggle = screen.getByRole('switch', { name: 'version_available email' }); + + // The first save hangs while the screen renders the optimistic value. + await user.click(inappToggle); // on -> off, will fail + await waitFor(() => expect(bodies).toHaveLength(1)); + expect(inappToggle).toHaveAttribute('aria-checked', 'false'); + + // A later tap on another cell goes through. + await user.click(emailToggle); // off -> on, succeeds + await waitFor(() => expect(bodies).toHaveLength(2)); + expect(bodies[1]).toEqual({ version_available: { inapp: false, email: true } }); + expect(emailToggle).toHaveAttribute('aria-checked', 'true'); + + // Now the first save fails: only its own cell goes back. + await act(async () => { + failFirst(); + }); + + await waitFor(() => expect(toast.error).toHaveBeenCalledWith('Error')); + expect(inappToggle).toHaveAttribute('aria-checked', 'true'); + expect(emailToggle).toHaveAttribute('aria-checked', 'true'); + }); });