diff --git a/frontend/src/components/settings/DataRetentionSection.tsx b/frontend/src/components/settings/DataRetentionSection.tsx index dc02d485..ad6a1346 100644 --- a/frontend/src/components/settings/DataRetentionSection.tsx +++ b/frontend/src/components/settings/DataRetentionSection.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useEffect, useMemo } from 'react'; +import { useState, useEffect } from 'react'; import { Input } from '@/components/ui/input'; import { Skeleton } from '@/components/ui/skeleton'; import { RefreshCw } from 'lucide-react'; @@ -15,6 +15,7 @@ import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; +import { useSettingsDirty } from './useSettingsDirty'; interface DataRetentionSectionProps { onDirtyChange?: (dirty: boolean) => void; @@ -44,23 +45,10 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp const { isPaid } = useLicense(); const { activeNode } = useNodes(); const readOnly = !isAdmin; - const [settings, setSettings] = useState({ ...DEFAULT_DATA_RETENTION }); - const serverSettingsRef = useRef({ ...DEFAULT_DATA_RETENTION }); + const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_DATA_RETENTION }); const [isLoading, setIsLoading] = useState(false); const [isSaving, setIsSaving] = useState(false); - const dirtyCount = useMemo(() => { - const baseline = serverSettingsRef.current; - let n = 0; - if (settings.metrics_retention_hours !== baseline.metrics_retention_hours) n++; - if (settings.log_retention_days !== baseline.log_retention_days) n++; - if (settings.audit_retention_days !== baseline.audit_retention_days) n++; - if (settings.scan_history_per_image_limit !== baseline.scan_history_per_image_limit) n++; - return n; - }, [settings]); - - const hasChanges = dirtyCount > 0; - useEffect(() => { onDirtyChange?.(hasChanges); }, [hasChanges, onDirtyChange]); @@ -89,8 +77,7 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp audit_retention_days: nodeData.audit_retention_days ?? DEFAULT_SETTINGS.audit_retention_days, scan_history_per_image_limit: nodeData.scan_history_per_image_limit ?? DEFAULT_SETTINGS.scan_history_per_image_limit, }; - setSettings(safe); - serverSettingsRef.current = { ...safe }; + reset(safe); } catch (e) { console.error('Failed to fetch data retention settings', e); } finally { @@ -106,17 +93,18 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp }; const saveSettings = async () => { + const submitted = { ...settings }; const payload: DataRetentionFields = { - metrics_retention_hours: settings.metrics_retention_hours, - log_retention_days: settings.log_retention_days, - scan_history_per_image_limit: settings.scan_history_per_image_limit, + metrics_retention_hours: submitted.metrics_retention_hours, + log_retention_days: submitted.log_retention_days, + scan_history_per_image_limit: submitted.scan_history_per_image_limit, }; // audit_retention_days is a paid-only key the backend rejects from a // Community operator. The field renders only when isPaid, so include it // in the save only then; otherwise a Community save would 403 on a key // the operator cannot edit and never sees. if (isPaid) { - payload.audit_retention_days = settings.audit_retention_days; + payload.audit_retention_days = submitted.audit_retention_days; } setIsSaving(true); try { @@ -129,7 +117,7 @@ export function DataRetentionSection({ onDirtyChange }: DataRetentionSectionProp toast.error(err?.error || err?.message || 'Failed to save settings.'); return; } - serverSettingsRef.current = { ...settings }; + markSaved(submitted); toast.success('Data retention saved.'); window.dispatchEvent(new CustomEvent(SENCHO_SETTINGS_CHANGED, { detail: { changedKeys: Object.keys(payload) }, diff --git a/frontend/src/components/settings/DeveloperSection.tsx b/frontend/src/components/settings/DeveloperSection.tsx index 09fe37e3..235a9f26 100644 --- a/frontend/src/components/settings/DeveloperSection.tsx +++ b/frontend/src/components/settings/DeveloperSection.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useEffect } from 'react'; +import { useState, useEffect } from 'react'; import { TogglePill } from '@/components/ui/toggle-pill'; import { Skeleton } from '@/components/ui/skeleton'; import { useAuth } from '@/context/AuthContext'; @@ -14,6 +14,7 @@ import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; +import { useSettingsDirty } from './useSettingsDirty'; interface DeveloperSectionProps { onDirtyChange?: (dirty: boolean) => void; @@ -37,13 +38,10 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { const { isAdmin } = useAuth(); const { activeNode } = useNodes(); const readOnly = !isAdmin; - const [settings, setSettings] = useState({ ...DEFAULT_DEVELOPER }); - const serverSettingsRef = useRef({ ...DEFAULT_DEVELOPER }); + const { settings, setSettings, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_DEVELOPER }); const [isLoading, setIsLoading] = useState(false); const [isSaving, setIsSaving] = useState(false); - const hasChanges = settings.developer_mode !== serverSettingsRef.current.developer_mode; - useEffect(() => { onDirtyChange?.(hasChanges); }, [hasChanges, onDirtyChange]); @@ -69,8 +67,7 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { const safe: DeveloperFields = { developer_mode: (nodeData.developer_mode as '0' | '1') ?? DEFAULT_SETTINGS.developer_mode, }; - setSettings(safe); - serverSettingsRef.current = { ...safe }; + reset(safe); } catch (e) { console.error('Failed to fetch developer settings', e); } finally { @@ -86,8 +83,9 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { }; const saveSettings = async () => { + const submitted = { ...settings }; const payload = { - developer_mode: settings.developer_mode, + developer_mode: submitted.developer_mode, }; setIsSaving(true); try { @@ -100,7 +98,7 @@ export function DeveloperSection({ onDirtyChange }: DeveloperSectionProps) { toast.error(err?.error || err?.message || 'Failed to save settings.'); return; } - serverSettingsRef.current = { ...settings }; + markSaved(submitted); toast.success('Developer settings saved.'); window.dispatchEvent(new CustomEvent(SENCHO_SETTINGS_CHANGED, { detail: { changedKeys: Object.keys(payload) }, diff --git a/frontend/src/components/settings/DockerStorageSection.tsx b/frontend/src/components/settings/DockerStorageSection.tsx index 36937bdf..0a5547b5 100644 --- a/frontend/src/components/settings/DockerStorageSection.tsx +++ b/frontend/src/components/settings/DockerStorageSection.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useEffect, useMemo } from 'react'; +import { useState, useEffect } from 'react'; import { Skeleton } from '@/components/ui/skeleton'; import { RefreshCw } from 'lucide-react'; import { apiFetch } from '@/lib/api'; @@ -11,6 +11,7 @@ import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; +import { useSettingsDirty } from './useSettingsDirty'; import { TogglePill } from '@/components/ui/toggle-pill'; import { NumberChip } from './SystemControls'; @@ -40,22 +41,10 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp const { activeNode } = useNodes(); const { isAdmin } = useAuth(); const readOnly = !isAdmin; - const [settings, setSettings] = useState({ ...DEFAULT_DOCKER_STORAGE }); - const serverSettingsRef = useRef({ ...DEFAULT_DOCKER_STORAGE }); + const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_DOCKER_STORAGE }); const [isLoading, setIsLoading] = useState(false); const [isSaving, setIsSaving] = useState(false); - const dirtyCount = useMemo(() => { - const baseline = serverSettingsRef.current; - let n = 0; - if (settings.docker_janitor_gb !== baseline.docker_janitor_gb) n++; - if (settings.prune_on_update !== baseline.prune_on_update) n++; - if (settings.reclaim_hero !== baseline.reclaim_hero) n++; - return n; - }, [settings]); - - const hasChanges = dirtyCount > 0; - useEffect(() => { onDirtyChange?.(hasChanges); }, [hasChanges, onDirtyChange]); @@ -83,8 +72,7 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp prune_on_update: (nodeData.prune_on_update as '0' | '1') ?? DEFAULT_SETTINGS.prune_on_update, reclaim_hero: (nodeData.reclaim_hero as '0' | '1') ?? DEFAULT_SETTINGS.reclaim_hero, }; - setSettings(safe); - serverSettingsRef.current = { ...safe }; + reset(safe); } catch (e) { console.error('Failed to fetch Docker & storage settings', e); } finally { @@ -100,18 +88,19 @@ export function DockerStorageSection({ onDirtyChange }: DockerStorageSectionProp }; const saveSettings = async () => { + const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', - body: JSON.stringify(settings), + body: JSON.stringify(submitted), }); if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); return; } - serverSettingsRef.current = { ...settings }; + markSaved(submitted); toast.success('Docker & storage settings saved.'); } catch (e: unknown) { toast.error((e as Error)?.message || 'Something went wrong.'); diff --git a/frontend/src/components/settings/FleetMeshSection.tsx b/frontend/src/components/settings/FleetMeshSection.tsx index 6beb3c1a..bb6d4739 100644 --- a/frontend/src/components/settings/FleetMeshSection.tsx +++ b/frontend/src/components/settings/FleetMeshSection.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useEffect, useMemo } from 'react'; +import { useState, useEffect } from 'react'; import { Skeleton } from '@/components/ui/skeleton'; import { RefreshCw } from 'lucide-react'; import { apiFetch } from '@/lib/api'; @@ -11,6 +11,7 @@ import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; +import { useSettingsDirty } from './useSettingsDirty'; import { TogglePill } from '@/components/ui/toggle-pill'; interface FleetMeshSectionProps { @@ -36,21 +37,10 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { const { activeNode } = useNodes(); const { isAdmin } = useAuth(); const readOnly = !isAdmin; - const [settings, setSettings] = useState({ ...DEFAULT_FLEET_MESH }); - const serverSettingsRef = useRef({ ...DEFAULT_FLEET_MESH }); + const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_FLEET_MESH }); const [isLoading, setIsLoading] = useState(false); const [isSaving, setIsSaving] = useState(false); - const dirtyCount = useMemo(() => { - const baseline = serverSettingsRef.current; - let n = 0; - if (settings.mesh_auto_recreate !== baseline.mesh_auto_recreate) n++; - if (settings.snapshot_documentation !== baseline.snapshot_documentation) n++; - return n; - }, [settings]); - - const hasChanges = dirtyCount > 0; - useEffect(() => { onDirtyChange?.(hasChanges); }, [hasChanges, onDirtyChange]); @@ -77,8 +67,7 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { mesh_auto_recreate: (nodeData.mesh_auto_recreate as '0' | '1') ?? DEFAULT_SETTINGS.mesh_auto_recreate, snapshot_documentation: (nodeData.snapshot_documentation as '0' | '1') ?? DEFAULT_SETTINGS.snapshot_documentation, }; - setSettings(safe); - serverSettingsRef.current = { ...safe }; + reset(safe); } catch (e) { console.error('Failed to fetch fleet mesh settings', e); } finally { @@ -94,18 +83,19 @@ export function FleetMeshSection({ onDirtyChange }: FleetMeshSectionProps) { }; const saveSettings = async () => { + const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', - body: JSON.stringify(settings), + body: JSON.stringify(submitted), }); if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); return; } - serverSettingsRef.current = { ...settings }; + markSaved(submitted); toast.success('Fleet settings saved.'); } catch (e: unknown) { toast.error((e as Error)?.message || 'Something went wrong.'); diff --git a/frontend/src/components/settings/HostAlertsSection.tsx b/frontend/src/components/settings/HostAlertsSection.tsx index ce8dde23..ec9e8fc9 100644 --- a/frontend/src/components/settings/HostAlertsSection.tsx +++ b/frontend/src/components/settings/HostAlertsSection.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useEffect, useMemo } from 'react'; +import { useState, useEffect } from 'react'; import { Skeleton } from '@/components/ui/skeleton'; import { RefreshCw } from 'lucide-react'; import { apiFetch } from '@/lib/api'; @@ -11,6 +11,7 @@ import { SettingsSection } from './SettingsSection'; import { SettingsField } from './SettingsField'; import { SettingsActions, SettingsPrimaryButton } from './SettingsActions'; import { useMastheadStats } from './MastheadStatsContext'; +import { useSettingsDirty } from './useSettingsDirty'; import { TogglePill } from '@/components/ui/toggle-pill'; import { NumberChip } from './SystemControls'; @@ -45,26 +46,10 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { const { activeNode } = useNodes(); const { isAdmin } = useAuth(); const readOnly = !isAdmin; - const [settings, setSettings] = useState({ ...DEFAULT_HOST_ALERTS }); - const serverSettingsRef = useRef({ ...DEFAULT_HOST_ALERTS }); + const { settings, setSettings, dirtyCount, hasChanges, reset, markSaved } = useSettingsDirty({ ...DEFAULT_HOST_ALERTS }); const [isLoading, setIsLoading] = useState(false); const [isSaving, setIsSaving] = useState(false); - const dirtyCount = useMemo(() => { - const baseline = serverSettingsRef.current; - let n = 0; - if (settings.host_cpu_limit !== baseline.host_cpu_limit) n++; - if (settings.host_ram_limit !== baseline.host_ram_limit) n++; - if (settings.host_disk_limit !== baseline.host_disk_limit) n++; - if (settings.host_alert_suppression_mins !== baseline.host_alert_suppression_mins) n++; - if (settings.global_crash !== baseline.global_crash) n++; - if (settings.health_gate_enabled !== baseline.health_gate_enabled) n++; - if (settings.health_gate_window_seconds !== baseline.health_gate_window_seconds) n++; - return n; - }, [settings]); - - const hasChanges = dirtyCount > 0; - useEffect(() => { onDirtyChange?.(hasChanges); }, [hasChanges, onDirtyChange]); @@ -96,8 +81,7 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { health_gate_enabled: (nodeData.health_gate_enabled as '0' | '1') ?? DEFAULT_SETTINGS.health_gate_enabled, health_gate_window_seconds: nodeData.health_gate_window_seconds ?? DEFAULT_SETTINGS.health_gate_window_seconds, }; - setSettings(safe); - serverSettingsRef.current = { ...safe }; + reset(safe); } catch (e) { console.error('Failed to fetch host alert settings', e); } finally { @@ -113,18 +97,19 @@ export function HostAlertsSection({ onDirtyChange }: HostAlertsSectionProps) { }; const saveSettings = async () => { + const submitted = { ...settings }; setIsSaving(true); try { const res = await apiFetch('/settings', { method: 'PATCH', - body: JSON.stringify(settings), + body: JSON.stringify(submitted), }); if (!res.ok) { const err = await res.json().catch(() => ({})); toast.error(err?.error || err?.message || 'Failed to save settings.'); return; } - serverSettingsRef.current = { ...settings }; + markSaved(submitted); toast.success('Host alerts saved.'); } catch (e: unknown) { toast.error((e as Error)?.message || 'Something went wrong.'); diff --git a/frontend/src/components/settings/__tests__/SettingsDirtyReconcile.test.tsx b/frontend/src/components/settings/__tests__/SettingsDirtyReconcile.test.tsx new file mode 100644 index 00000000..b6211b81 --- /dev/null +++ b/frontend/src/components/settings/__tests__/SettingsDirtyReconcile.test.tsx @@ -0,0 +1,235 @@ +/** + * After a successful save the section must reconcile its dirty state immediately, + * with no remount: the parent dirty flag (onDirtyChange) clears, the masthead + * EDITED stat flips from pending to saved, and the Save button disables. A failed + * save must keep the section dirty and retryable, and an edit made while a save is + * in flight must survive (the submitted snapshot, not the live state, becomes the + * new baseline). + * + * The onDirtyChange boolean asserted here is exactly the value SettingsPage stores + * in its dirtyFlags map and the sidebar renders the unsaved dot from, so a clean + * transition here proves the parent indicators clear. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor, act, cleanup } from '@testing-library/react'; +import type { MastheadMetadataItem } from '@/components/ui/PageMasthead'; + +const { masthead } = vi.hoisted(() => ({ + masthead: { last: null as MastheadMetadataItem[] | null }, +})); + +vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { error: vi.fn(), success: vi.fn(), warning: vi.fn(), info: vi.fn(), loading: vi.fn(), dismiss: vi.fn() }, +})); +vi.mock('@/context/AuthContext', () => ({ useAuth: () => ({ isAdmin: true }) })); +vi.mock('@/context/NodeContext', () => ({ useNodes: () => ({ activeNode: { id: 'local' } }) })); +vi.mock('@/context/LicenseContext', () => ({ useLicense: vi.fn(() => ({ isPaid: true })) })); +vi.mock('../MastheadStatsContext', () => ({ + useMastheadStats: (stats: MastheadMetadataItem[] | null) => { masthead.last = stats; }, +})); + +import { apiFetch } from '@/lib/api'; +import { useLicense } from '@/context/LicenseContext'; +import { HostAlertsSection } from '../HostAlertsSection'; +import { DockerStorageSection } from '../DockerStorageSection'; +import { FleetMeshSection } from '../FleetMeshSection'; +import { DataRetentionSection } from '../DataRetentionSection'; +import { DeveloperSection } from '../DeveloperSection'; + +const mockedFetch = apiFetch as unknown as ReturnType; +const mockedLicense = useLicense as unknown as ReturnType; + +const FULL_SETTINGS: Record = { + host_cpu_limit: '90', + host_ram_limit: '90', + host_disk_limit: '90', + host_alert_suppression_mins: '60', + global_crash: '1', + docker_janitor_gb: '5', + prune_on_update: '1', + reclaim_hero: '1', + mesh_auto_recreate: '0', + metrics_retention_hours: '24', + log_retention_days: '30', + audit_retention_days: '90', + scan_history_per_image_limit: '50', + developer_mode: '0', +}; + +/** GET /settings resolves load data; PATCH resolves ok unless overridden per-test. */ +function wireDefaultFetch() { + mockedFetch.mockImplementation((_url: string, opts?: { method?: string }) => { + if (opts?.method === 'PATCH') return Promise.resolve({ ok: true, json: async () => ({}) }); + return Promise.resolve({ ok: true, json: async () => ({ ...FULL_SETTINGS }) }); + }); +} + +function lastDirty(spy: ReturnType): boolean | undefined { + const calls = spy.mock.calls; + return calls.length ? (calls[calls.length - 1][0] as boolean) : undefined; +} + +beforeEach(() => { + mockedFetch.mockReset(); + mockedLicense.mockReturnValue({ isPaid: true }); + masthead.last = null; + wireDefaultFetch(); +}); + +describe('settings dirty reconcile on save', () => { + it('clears the masthead, parent dirty flag, and Save button after a successful save (no remount)', async () => { + const onDirty = vi.fn(); + render(); + const save = await screen.findByRole('button', { name: /save alerts/i }); + + // Edit: section becomes dirty. + fireEvent.click(screen.getAllByRole('switch')[0]); // global_crash + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + expect(masthead.last?.[0]).toMatchObject({ label: 'EDITED', value: '1 pending', tone: 'warn' }); + expect(save).not.toBeDisabled(); + + // Save succeeds: everything reconciles without a remount. + fireEvent.click(save); + await waitFor(() => expect(lastDirty(onDirty)).toBe(false)); + expect(masthead.last?.[0]).toMatchObject({ label: 'EDITED', value: 'saved', tone: 'value' }); + expect(save).toBeDisabled(); + // A remount would re-run the load effect; the section loads exactly once. + const getCalls = mockedFetch.mock.calls.filter(c => c[1]?.method !== 'PATCH'); + expect(getCalls).toHaveLength(1); + }); + + it('keeps the section dirty and retryable when the save fails', async () => { + mockedFetch.mockImplementation((_url: string, opts?: { method?: string }) => { + if (opts?.method === 'PATCH') return Promise.resolve({ ok: false, json: async () => ({ error: 'nope' }) }); + return Promise.resolve({ ok: true, json: async () => ({ ...FULL_SETTINGS }) }); + }); + const onDirty = vi.fn(); + render(); + const save = await screen.findByRole('button', { name: /save alerts/i }); + + fireEvent.click(screen.getAllByRole('switch')[0]); + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + + fireEvent.click(save); + await waitFor(() => expect(mockedFetch.mock.calls.some(c => c[1]?.method === 'PATCH')).toBe(true)); + // Still dirty, still enabled, masthead still pending: the operator can retry. + // Wait for the save to settle so the assertion does not race the isSaving reset. + await waitFor(() => expect(save).not.toBeDisabled()); + expect(lastDirty(onDirty)).toBe(true); + expect(masthead.last?.[0]).toMatchObject({ label: 'EDITED', value: '1 pending', tone: 'warn' }); + }); + + it('preserves an edit made while the PATCH is in flight', async () => { + let resolvePatch: ((v: { ok: boolean; json: () => Promise }) => void) | undefined; + mockedFetch.mockImplementation((_url: string, opts?: { method?: string }) => { + if (opts?.method === 'PATCH') { + return new Promise<{ ok: boolean; json: () => Promise }>(res => { resolvePatch = res; }); + } + return Promise.resolve({ ok: true, json: async () => ({ ...FULL_SETTINGS }) }); + }); + const onDirty = vi.fn(); + render(); + const save = await screen.findByRole('button', { name: /save alerts/i }); + + // Change field A and submit (PATCH now pending). + fireEvent.click(screen.getAllByRole('switch')[0]); // global_crash + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + fireEvent.click(save); + await waitFor(() => expect(mockedFetch.mock.calls.some(c => c[1]?.method === 'PATCH')).toBe(true)); + + // Change field B while the save is still in flight (fieldset stays editable). + const healthGate = screen.getAllByRole('switch')[1]; // health_gate_enabled + const healthBefore = healthGate.getAttribute('aria-checked'); + fireEvent.click(healthGate); + expect(healthGate.getAttribute('aria-checked')).not.toBe(healthBefore); + + // Resolve the save: only the submitted snapshot becomes the baseline. + await act(async () => { + resolvePatch?.({ ok: true, json: async () => ({}) }); + }); + + // The later edit survives and keeps the section dirty/retryable. + expect(healthGate.getAttribute('aria-checked')).not.toBe(healthBefore); + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + expect(save).not.toBeDisabled(); + }); +}); + +describe('every migrated section clears its dirty flag on save', () => { + interface Case { + name: string; + render: (onDirty: (d: boolean) => void) => void; + saveName: RegExp; + edit: () => void; + } + + const cases: Case[] = [ + { + name: 'HostAlertsSection', + render: onDirty => render(), + saveName: /save alerts/i, + edit: () => fireEvent.click(screen.getAllByRole('switch')[0]), + }, + { + name: 'DockerStorageSection', + render: onDirty => render(), + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getAllByRole('switch')[0]), + }, + { + name: 'FleetMeshSection', + render: onDirty => render(), + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getAllByRole('switch')[0]), + }, + { + name: 'DataRetentionSection', + render: onDirty => render(), + saveName: /save settings/i, + edit: () => fireEvent.change(screen.getAllByRole('spinbutton')[0], { target: { value: '48' } }), + }, + { + name: 'DeveloperSection', + render: onDirty => render(), + saveName: /save settings/i, + edit: () => fireEvent.click(screen.getByRole('switch')), + }, + ]; + + for (const c of cases) { + it(`${c.name} reports clean after a successful save`, async () => { + const onDirty = vi.fn(); + c.render(onDirty); + const save = await screen.findByRole('button', { name: c.saveName }); + c.edit(); + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + fireEvent.click(save); + await waitFor(() => expect(lastDirty(onDirty)).toBe(false)); + cleanup(); + }); + } + + it('DataRetentionSection reconciles for a Community operator even though the paid key is omitted from the payload', async () => { + // markSaved adopts the full submitted snapshot (incl. audit_retention_days), + // not the trimmed Community payload, so the section re-baselines and clears. + mockedLicense.mockReturnValue({ isPaid: false }); + const onDirty = vi.fn(); + render(); + const save = await screen.findByRole('button', { name: /save settings/i }); + fireEvent.change(screen.getAllByRole('spinbutton')[0], { target: { value: '48' } }); + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + fireEvent.click(save); + await waitFor(() => expect(lastDirty(onDirty)).toBe(false)); + expect(save).toBeDisabled(); + }); + + it('DeveloperSection masthead stays a DEV MODE stat, not an EDITED pending count', async () => { + const onDirty = vi.fn(); + render(); + await screen.findByRole('button', { name: /save settings/i }); + fireEvent.click(screen.getByRole('switch')); // developer_mode 0 -> 1 + await waitFor(() => expect(lastDirty(onDirty)).toBe(true)); + expect(masthead.last?.[0]).toMatchObject({ label: 'DEV MODE', value: 'on' }); + }); +}); diff --git a/frontend/src/components/settings/__tests__/useSettingsDirty.test.ts b/frontend/src/components/settings/__tests__/useSettingsDirty.test.ts new file mode 100644 index 00000000..feca4b22 --- /dev/null +++ b/frontend/src/components/settings/__tests__/useSettingsDirty.test.ts @@ -0,0 +1,62 @@ +/** + * Covers dirty counting, the load (reset) vs save-success (markSaved) split, and + * the in-flight case: markSaved moves only the baseline, so an edit made while a + * PATCH is in flight is preserved. + */ +import { describe, it, expect } from 'vitest'; +import { act, renderHook } from '@testing-library/react'; +import { useSettingsDirty } from '../useSettingsDirty'; + +type Fields = Record<'a' | 'b', string>; + +const INITIAL: Fields = { a: '1', b: '1' }; + +describe('useSettingsDirty', () => { + it('starts clean', () => { + const { result } = renderHook(() => useSettingsDirty(INITIAL)); + expect(result.current.dirtyCount).toBe(0); + expect(result.current.hasChanges).toBe(false); + }); + + it('counts changed fields against the baseline', () => { + const { result } = renderHook(() => useSettingsDirty(INITIAL)); + act(() => result.current.setSettings(prev => ({ ...prev, a: '2' }))); + expect(result.current.dirtyCount).toBe(1); + expect(result.current.hasChanges).toBe(true); + act(() => result.current.setSettings(prev => ({ ...prev, b: '2' }))); + expect(result.current.dirtyCount).toBe(2); + }); + + it('reset adopts both current values and baseline, and edits count against the new baseline', () => { + const { result } = renderHook(() => useSettingsDirty(INITIAL)); + act(() => result.current.reset({ a: '9', b: '9' })); + expect(result.current.settings).toEqual({ a: '9', b: '9' }); + expect(result.current.hasChanges).toBe(false); + // An edit after reset is measured against the freshly adopted baseline. + act(() => result.current.setSettings(prev => ({ ...prev, a: '8' }))); + expect(result.current.dirtyCount).toBe(1); + }); + + it('markSaved clears dirty without touching current settings', () => { + const { result } = renderHook(() => useSettingsDirty(INITIAL)); + act(() => result.current.setSettings(prev => ({ ...prev, a: '2' }))); + const submitted = result.current.settings; + act(() => result.current.markSaved(submitted)); + expect(result.current.settings).toEqual({ a: '2', b: '1' }); + expect(result.current.hasChanges).toBe(false); + }); + + it('keeps an edit made after the submitted snapshot dirty (in-flight save)', () => { + const { result } = renderHook(() => useSettingsDirty(INITIAL)); + // User changes a, "submits", then changes b before the save resolves. + act(() => result.current.setSettings(prev => ({ ...prev, a: '2' }))); + const submitted = result.current.settings; // { a: '2', b: '1' } + act(() => result.current.setSettings(prev => ({ ...prev, b: '2' }))); + // Save resolves: baseline adopts the submitted snapshot only. + act(() => result.current.markSaved(submitted)); + // b was changed after submitting, so the section stays dirty on b. + expect(result.current.settings).toEqual({ a: '2', b: '2' }); + expect(result.current.dirtyCount).toBe(1); + expect(result.current.hasChanges).toBe(true); + }); +}); diff --git a/frontend/src/components/settings/useSettingsDirty.ts b/frontend/src/components/settings/useSettingsDirty.ts new file mode 100644 index 00000000..211828ed --- /dev/null +++ b/frontend/src/components/settings/useSettingsDirty.ts @@ -0,0 +1,53 @@ +import { useCallback, useMemo, useState } from 'react'; +import type { Dispatch, SetStateAction } from 'react'; + +/** Settings field values are flat primitives, compared shallowly with `!==`. */ +type SettingsValue = string | number | boolean | null | undefined; +type SettingsFields = Record; + +/** + * Tracks dirty state for a flat settings-fields object against a saved baseline. + * + * The baseline lives in state (not a ref) so adopting a new baseline re-renders + * and re-runs the dirty calculation. Load and save-success are deliberately + * separate operations: a section's fields stay editable while a save is in + * flight, so the save path must move only the baseline and leave the current + * values alone, or an edit made between clicking Save and the PATCH resolving + * would be discarded. + */ +export interface SettingsDirty { + settings: T; + setSettings: Dispatch>; + dirtyCount: number; + hasChanges: boolean; + /** Load / node-switch: adopt `next` as both the current values and the saved baseline. */ + reset: (next: T) => void; + /** Save success: adopt the submitted snapshot as the saved baseline only, + * preserving edits made while the PATCH was in flight. Pass a snapshot + * captured before the request, not live settings read after the await. */ + markSaved: (submitted: T) => void; +} + +export function useSettingsDirty(initial: T): SettingsDirty { + const [settings, setSettings] = useState(initial); + const [baseline, setBaseline] = useState(initial); + + const dirtyCount = useMemo(() => { + let n = 0; + for (const key of Object.keys(settings) as (keyof T)[]) { + if (settings[key] !== baseline[key]) n += 1; + } + return n; + }, [settings, baseline]); + + const reset = useCallback((next: T) => { + setSettings({ ...next }); + setBaseline({ ...next }); + }, []); + + const markSaved = useCallback((submitted: T) => { + setBaseline({ ...submitted }); + }, []); + + return { settings, setSettings, dirtyCount, hasChanges: dirtyCount > 0, reset, markSaved }; +}