From 51b678dc1917b577ea5a83ffef354618ba93bc7b Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 28 Oct 2026 03:59:44 +0100 Subject: [PATCH] fix: let every admin switch on a page with a save bar wait for Save, plugins, themes and telemetry included --- .../__tests__/save-bar-switches.test.tsx | 84 ++++++++++++++++++ app/(main)/admin/_pages/extension-detail.tsx | 53 ++++++++---- app/(main)/admin/_pages/plugins.tsx | 85 ++++++++++--------- app/(main)/admin/_pages/telemetry.tsx | 75 ++++++++-------- app/(main)/admin/_pages/themes.tsx | 85 ++++++++++--------- 5 files changed, 253 insertions(+), 129 deletions(-) create mode 100644 app/(main)/admin/__tests__/save-bar-switches.test.tsx diff --git a/app/(main)/admin/__tests__/save-bar-switches.test.tsx b/app/(main)/admin/__tests__/save-bar-switches.test.tsx new file mode 100644 index 000000000..23a5d4ed2 --- /dev/null +++ b/app/(main)/admin/__tests__/save-bar-switches.test.tsx @@ -0,0 +1,84 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useAdminUnsavedStore } from '@/stores/admin-unsaved-store'; + +// On an admin page with a save bar, every control waits for Save: the +// per-plugin switch and the telemetry consent used to save at once, next to +// policy switches that waited. + +const calls: Array<{ url: string; method: string; body: Record }> = []; +const routes: Record = {}; +vi.mock('@/lib/browser-navigation', () => ({ + apiFetch: vi.fn(async (url: string, init?: RequestInit) => { + const method = init?.method ?? 'GET'; + calls.push({ url, method, body: JSON.parse(String(init?.body ?? '{}')) }); + const data = method === 'GET' ? routes[url.split('?')[0]] ?? {} : { ok: true }; + return new Response(JSON.stringify(data), { status: 200, headers: { 'content-type': 'application/json' } }); + }), + withBasePath: (p: string) => p, +})); +vi.mock('next/link', () => ({ default: ({ children }: { children: React.ReactNode }) => children })); +vi.mock('../_pages/plugin-approvals-panel', () => ({ PluginApprovalsPanel: () => null })); +vi.mock('../_pages/use-extension-icons', () => ({ useExtensionIcons: () => () => null })); + +import { PluginsPage } from '../_pages/plugins'; +import { TelemetryPage } from '../_pages/telemetry'; + +const writes = () => calls.filter((c) => c.method !== 'GET'); + +beforeEach(() => { + calls.length = 0; + routes['/api/admin/plugins'] = [ + { id: 'jitsi', name: 'Jitsi', version: '1.0.0', enabled: true, author: 'x', description: '', permissions: [] }, + ]; + routes['/api/admin/policy'] = { features: {}, themePolicy: {} }; + routes['/api/admin/telemetry'] = { + consent: 'off', consentSource: 'file', endpoint: 'https://t.example', defaultEndpoint: 'https://t.example', + consentedAt: null, lastSentAt: null, nextScheduledAt: null, payloadPreview: {}, accountCounts: { total: 0, active7d: 0 }, + }; +}); +afterEach(() => { + useAdminUnsavedStore.setState({ dirty: {}, pendingLeave: null, pendingScope: null }); +}); + +describe('admin plugins page', () => { + it('turns a plugin off only when Save is pressed', async () => { + render(); + const toggle = await screen.findByRole('switch', { name: 'Jitsi enabled' }); + fireEvent.click(toggle); + + expect(toggle.getAttribute('aria-checked')).toBe('false'); + expect(writes()).toEqual([]); + expect(useAdminUnsavedStore.getState().dirty).toEqual({ plugins: true }); + + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(writes()).toEqual([ + expect.objectContaining({ url: '/api/admin/plugins', method: 'PATCH', body: { id: 'jitsi', enabled: false } }), + ])); + await waitFor(() => expect(useAdminUnsavedStore.getState().dirty).toEqual({})); + }); + + it('puts the switch back on Discard without saving', async () => { + render(); + const toggle = await screen.findByRole('switch', { name: 'Jitsi enabled' }); + fireEvent.click(toggle); + fireEvent.click(screen.getByRole('button', { name: 'Discard' })); + expect(toggle.getAttribute('aria-checked')).toBe('true'); + expect(writes()).toEqual([]); + }); +}); + +describe('admin telemetry page', () => { + it('sends the consent only when Save is pressed', async () => { + render(); + const consent = await screen.findByRole('switch', { name: 'Send anonymous heartbeats' }); + fireEvent.click(consent); + expect(writes()).toEqual([]); + expect(screen.getByTestId('admin-save-bar')).toBeTruthy(); + + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(writes()).toEqual([ + expect.objectContaining({ url: '/api/admin/telemetry', body: { action: 'set-consent', consent: 'on' } }), + ])); + }); +}); diff --git a/app/(main)/admin/_pages/extension-detail.tsx b/app/(main)/admin/_pages/extension-detail.tsx index 361260db4..cb90eae4e 100644 --- a/app/(main)/admin/_pages/extension-detail.tsx +++ b/app/(main)/admin/_pages/extension-detail.tsx @@ -127,7 +127,12 @@ export function ExtensionDetailPage({ kind, id }: Props) { }, [schema, savedValues]); const dirty = Object.keys(savedValues).some((key) => (formValues[key] ?? '') !== savedValues[key]); - useAdminUnsavedChanges(`${kind}-config`, dirty); + // On/off and "force on" wait for Save like the settings below. + const [availabilityDraft, setAvailabilityDraft] = useState<{ enabled?: boolean; forceEnabled?: boolean }>({}); + const shownForced = availabilityDraft.forceEnabled ?? !!entry?.forceEnabled; + const shownEnabled = shownForced || (availabilityDraft.enabled ?? !!entry?.enabled); + const availabilityDirty = !!entry && (shownEnabled !== entry.enabled || shownForced !== !!entry.forceEnabled); + useAdminUnsavedChanges(`${kind}-config`, dirty || availabilityDirty); async function saveConfig() { if (!schema) return; @@ -205,16 +210,21 @@ export function ExtensionDetailPage({ kind, id }: Props) { } } - async function setEnabled(enabled: boolean) { - if (await patch({ enabled }, 'enabled')) setEntry((prev) => (prev ? { ...prev, enabled } : prev)); + async function saveAvailability(): Promise { + if (!entry) return true; + const body: Record = {}; + if (shownForced !== !!entry.forceEnabled) body.forceEnabled = shownForced; + if (shownEnabled !== entry.enabled) body.enabled = shownEnabled; + const ok = await patch(body, 'availability'); + if (ok) setEntry((prev) => (prev ? { ...prev, enabled: shownEnabled, forceEnabled: shownForced } : prev)); + setAvailabilityDraft({}); + return ok; } - async function setForced(forceEnabled: boolean) { - const body: Record = { forceEnabled }; - if (forceEnabled) body.enabled = true; - if (await patch(body, 'forced')) { - setEntry((prev) => (prev ? { ...prev, forceEnabled, ...(forceEnabled ? { enabled: true } : {}) } : prev)); - } + async function save() { + if (availabilityDirty && !(await saveAvailability())) return; + if (dirty) await saveConfig(); + else setMessage({ type: 'success', text: 'Saved' }); } async function rollback(version: string) { @@ -326,8 +336,13 @@ export function ExtensionDetailPage({ kind, id }: Props) { actions={( <> - @@ -244,8 +252,9 @@ export function PluginsPage() { )} control={( { setMessage(null); void setEnabled(plugin.id, checked); }} + checked={isOn(plugin)} + disabled={savingPolicy} + onChange={(checked) => { setMessage(null); setEnabledDraft((prev) => ({ ...prev, [plugin.id]: checked })); }} ariaLabel={`${plugin.name} enabled`} /> )} @@ -274,11 +283,11 @@ export function PluginsPage() { setPolicy(savedPolicy)} - saveLabel="Save policy" + onSave={() => void save()} + onDiscard={() => { setPolicy(savedPolicy); setEnabledDraft({}); }} + saveLabel="Save" note="Users see the change on their next login" /> diff --git a/app/(main)/admin/_pages/telemetry.tsx b/app/(main)/admin/_pages/telemetry.tsx index a745ac45c..23ceaf91d 100644 --- a/app/(main)/admin/_pages/telemetry.tsx +++ b/app/(main)/admin/_pages/telemetry.tsx @@ -46,9 +46,14 @@ export function TelemetryPage() { const [busy, setBusy] = useState(null); const [endpointDraft, setEndpointDraft] = useState(''); const [message, setMessage] = useFlash(); + // The heartbeat switch waits for Save like the endpoint (one page, one + // save bar). + const [consentDraft, setConsentDraft] = useState<'on' | 'off' | null>(null); const endpointDirty = status !== null && endpointDraft !== status.endpoint; - useAdminUnsavedChanges('telemetry', endpointDirty); + const consentDirty = status !== null && consentDraft !== null && consentDraft !== status.consent; + const dirty = endpointDirty || consentDirty; + useAdminUnsavedChanges('telemetry', dirty); async function refresh(): Promise { setLoading(true); @@ -66,38 +71,35 @@ export function TelemetryPage() { } useEffect(() => { void refresh(); }, []); - async function setConsent(consent: 'on' | 'off'): Promise { - setBusy('consent'); - setMessage(null); - try { - const r = await apiFetch('/api/admin/telemetry', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ action: 'set-consent', consent }), - }); - if (!r.ok) { - const j = (await r.json().catch(() => ({}))) as { error?: string }; - setMessage({ type: 'error', text: j.error ?? 'failed' }); - } - await refresh(); - } finally { setBusy(null); } + /** Sends one change; returns the error text, or null. */ + async function post(body: Record): Promise { + const r = await apiFetch('/api/admin/telemetry', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }); + if (r.ok) return null; + const j = (await r.json().catch(() => ({}))) as { error?: string }; + return j.error ?? 'failed'; } - async function saveEndpoint(): Promise { - setBusy('endpoint'); + async function save(): Promise { + setBusy('save'); setMessage(null); try { - const r = await apiFetch('/api/admin/telemetry', { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ action: 'set-endpoint', endpoint: endpointDraft }), - }); - if (!r.ok) { - const j = (await r.json().catch(() => ({}))) as { error?: string }; - setMessage({ type: 'error', text: j.error ?? 'failed' }); - } else { - setMessage({ type: 'success', text: 'Endpoint saved.' }); + const errors: string[] = []; + if (consentDirty && consentDraft) { + const error = await post({ action: 'set-consent', consent: consentDraft }); + if (error) errors.push(`Heartbeats: ${error}`); } + if (endpointDirty) { + const error = await post({ action: 'set-endpoint', endpoint: endpointDraft }); + if (error) errors.push(`Endpoint: ${error}`); + } + setConsentDraft(null); + setMessage(errors.length > 0 + ? { type: 'error', text: `Couldn't save: ${errors.join('; ')}` } + : { type: 'success', text: 'Telemetry settings saved.' }); await refresh(); } finally { setBusy(null); } } @@ -134,6 +136,7 @@ export function TelemetryPage() { const envOverridden = status.consentSource === 'env'; const isOn = status.consent === 'on'; + const shownOn = (consentDraft ?? status.consent) === 'on'; const consentDescription = [ status.consent === 'pending' && 'Initialising - no heartbeats sent yet.', @@ -167,9 +170,9 @@ export function TelemetryPage() { void setConsent(checked ? 'on' : 'off')} + checked={shownOn} + disabled={busy === 'save' || envOverridden} + onChange={(checked) => { setConsentDraft(checked ? 'on' : 'off'); setMessage(null); }} /> @@ -240,11 +243,11 @@ export function TelemetryPage() { void saveEndpoint()} - onDiscard={() => setEndpointDraft(status.endpoint)} - saveLabel="Save endpoint" + dirty={dirty} + saving={busy === 'save'} + onSave={() => void save()} + onDiscard={() => { setEndpointDraft(status.endpoint); setConsentDraft(null); }} + saveLabel="Save" /> ); diff --git a/app/(main)/admin/_pages/themes.tsx b/app/(main)/admin/_pages/themes.tsx index e50896f7a..c9e1db2ac 100644 --- a/app/(main)/admin/_pages/themes.tsx +++ b/app/(main)/admin/_pages/themes.tsx @@ -42,8 +42,9 @@ export function ThemesPage() { const iconFor = useExtensionIcons(); const [loading, setLoading] = useState(true); const [uploading, setUploading] = useState(false); - const [bulkBusy, setBulkBusy] = useState(false); const [message, setMessage] = useFlash(); + // On/off per theme waits for Save, like the policy switches below. + const [enabledDraft, setEnabledDraft] = useState>({}); const fileInputRef = useRef(null); const [policy, setPolicy] = useState(() => withThemeDefaults({ ...DEFAULT_POLICY })); const [savedPolicy, setSavedPolicy] = useState(() => withThemeDefaults({ ...DEFAULT_POLICY })); @@ -74,7 +75,10 @@ export function ThemesPage() { const policyDirty = JSON.stringify(policy.themePolicy) !== JSON.stringify(savedPolicy.themePolicy) || JSON.stringify(policy.features) !== JSON.stringify(savedPolicy.features); - useAdminUnsavedChanges('themes', policyDirty); + const isOn = (t: ExtensionEntry) => enabledDraft[t.id] ?? t.enabled; + const switched = themes.filter((t) => isOn(t) !== t.enabled); + const dirty = policyDirty || switched.length > 0; + useAdminUnsavedChanges('themes', dirty); function setFeature(key: 'themesEnabled' | 'userThemesEnabled', value: boolean) { setPolicy(prev => ({ ...prev, features: { ...prev.features, [key]: value } })); @@ -141,9 +145,8 @@ export function ThemesPage() { setMessage(null); } - async function savePolicy() { - setSavingPolicy(true); - setMessage(null); + /** Stores the policy; returns the error text, or null. */ + async function savePolicy(): Promise { try { const res = await apiFetch('/api/admin/policy', { method: 'PUT', @@ -152,18 +155,34 @@ export function ThemesPage() { }); if (res.ok) { setSavedPolicy(policy); - setMessage({ type: 'success', text: 'Theme policy saved. Users see the change on their next login.' }); - } else { - const data = await res.json().catch(() => ({})); - setMessage({ type: 'error', text: data.error || 'Could not save the theme policy' }); + return null; } + const data = await res.json().catch(() => ({})); + return data.error || 'Could not save the theme policy'; } catch { - setMessage({ type: 'error', text: 'Could not save the theme policy' }); - } finally { - setSavingPolicy(false); + return 'Could not save the theme policy'; } } + async function save() { + setSavingPolicy(true); + setMessage(null); + const errors: string[] = []; + for (const t of switched) { + const error = await setEnabled(t.id, isOn(t)); + if (error) errors.push(`${t.name}: ${error}`); + } + setEnabledDraft({}); + if (policyDirty) { + const error = await savePolicy(); + if (error) errors.push(error); + } + setSavingPolicy(false); + setMessage(errors.length > 0 + ? { type: 'error', text: `Couldn't save everything. ${errors.join('; ')}` } + : { type: 'success', text: 'Themes saved. Users see the change on their next login.' }); + } + async function handleUpload(e: React.ChangeEvent) { const file = e.target.files?.[0]; if (!file) return; @@ -189,7 +208,8 @@ export function ThemesPage() { } } - async function setEnabled(id: string, enabled: boolean): Promise { + /** Turns one theme on or off on the server; returns the error text, or null. */ + async function setEnabled(id: string, enabled: boolean): Promise { const res = await apiFetch('/api/admin/themes', { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, @@ -197,30 +217,18 @@ export function ThemesPage() { }); if (res.ok) { setThemes(prev => prev.map(t => (t.id === id ? { ...t, enabled } : t))); - return true; + return null; } const data = await res.json().catch(() => ({})); - setMessage({ type: 'error', text: data.error || 'Could not change the theme' }); - return false; + return data.error || 'Could not change the theme'; } - async function setAll(enabled: boolean) { - const targets = themes.filter(t => t.enabled !== enabled); - if (targets.length === 0) return; - setBulkBusy(true); + function setAll(enabled: boolean) { + setEnabledDraft(Object.fromEntries(themes.map((t) => [t.id, enabled]))); setMessage(null); - let failed = 0; - for (const t of targets) { - if (!(await setEnabled(t.id, enabled))) failed++; - } - await fetchThemes(); - setBulkBusy(false); - setMessage(failed === 0 - ? { type: 'success', text: `${targets.length} theme${targets.length === 1 ? '' : 's'} ${enabled ? 'enabled' : 'disabled'}` } - : { type: 'error', text: `${failed} theme${failed === 1 ? '' : 's'} could not be ${enabled ? 'enabled' : 'disabled'}` }); } - const enabledCount = themes.filter(t => t.enabled).length; + const enabledCount = themes.filter(isOn).length; const themesEnabled = policy.features.themesEnabled ?? true; const userThemesEnabled = policy.features.userThemesEnabled ?? true; const lockColorMode = policy.themePolicy?.lockColorMode === true; @@ -277,10 +285,10 @@ export function ThemesPage() { <> {themes.length > 1 && (
- -
@@ -310,8 +318,9 @@ export function ThemesPage() { )} control={( { setMessage(null); void setEnabled(theme.id, checked); }} + checked={isOn(theme)} + disabled={savingPolicy} + onChange={(checked) => { setMessage(null); setEnabledDraft((prev) => ({ ...prev, [theme.id]: checked })); }} ariaLabel={`${theme.name} enabled`} /> )} @@ -392,11 +401,11 @@ export function ThemesPage() { setPolicy(savedPolicy)} - saveLabel="Save policy" + onSave={() => void save()} + onDiscard={() => { setPolicy(savedPolicy); setEnabledDraft({}); }} + saveLabel="Save" note="Users see the change on their next login" />