From 3950047975954257c5416bcdb15cf4d1d839c67d Mon Sep 17 00:00:00 2001 From: Pliable Pixels Date: Sat, 1 Aug 2026 13:59:30 -0400 Subject: [PATCH] fix: forget monitors ZoneMinder no longer has Monitor ids are persisted in four places: the per-profile hidden list, each montage group's hidden list and working layout, and dashboard widget settings. Deleting a monitor in ZoneMinder removes it from the API but from none of those, so it lingered as a ghost. A hidden monitor that no longer existed was the worst case. It stayed in the hidden count while being absent from the list that would let you un-hide it, so the count could never be brought back to zero. A deleted monitor also kept its place in a dashboard widget's display order, listed as "Monitor 12". AppLayout now mounts a hook that drops stored ids the server does not report. It reads the same monitors-including-excluded query the hidden monitors setting already uses, so it costs no extra fetch. Two details are load-bearing. The list has to be the one that includes excluded monitors: the ordinary monitors query has already dropped the hidden ones, and reconciling the hidden list against it would delete every entry. And nothing is pruned unless the query succeeded and returned at least one monitor, because an empty or failed response looks exactly like "every monitor was deleted". Named montage saved layouts are left alone. They are arrangements the user saved and may reload later, and an entry for a missing monitor renders nothing. refs #323 refs #324 Co-Authored-By: Claude Opus 5 --- app/src/components/layout/AppLayout.tsx | 4 + .../useReconcileDeletedMonitors.test.tsx | 114 ++++++++++++++++++ app/src/hooks/useReconcileDeletedMonitors.ts | 77 ++++++++++++ .../__tests__/prune-deleted-monitors.test.ts | 108 +++++++++++++++++ app/src/lib/monitor/prune-deleted-monitors.ts | 104 ++++++++++++++++ .../05-component-architecture.rst | 30 +++++ 6 files changed, 437 insertions(+) create mode 100644 app/src/hooks/__tests__/useReconcileDeletedMonitors.test.tsx create mode 100644 app/src/hooks/useReconcileDeletedMonitors.ts create mode 100644 app/src/lib/monitor/__tests__/prune-deleted-monitors.test.ts create mode 100644 app/src/lib/monitor/prune-deleted-monitors.ts diff --git a/app/src/components/layout/AppLayout.tsx b/app/src/components/layout/AppLayout.tsx index ea645832..e50e6848 100644 --- a/app/src/components/layout/AppLayout.tsx +++ b/app/src/components/layout/AppLayout.tsx @@ -38,6 +38,7 @@ import { KioskOverlay } from '../kiosk/KioskOverlay'; import { SidebarContent } from './SidebarContent'; import { DeveloperNoticeBanner } from './DeveloperNoticeBanner'; import { OfflineBanner } from './OfflineBanner'; +import { useReconcileDeletedMonitors } from '../../hooks/useReconcileDeletedMonitors'; import { CertTrustBanner } from '../CertTrustBanner'; import { DeleteBatchBar } from '../events/DeleteBatchBar'; import { AssistantWidget } from '../assistant/AssistantWidget'; @@ -105,6 +106,9 @@ export default function AppLayout() { // Apply global insomnia setting useInsomnia({ enabled: settings.insomnia }); + // Forget monitors ZoneMinder no longer has (refs #323, #324) + useReconcileDeletedMonitors(); + const { isLocked, previousInsomniaState } = useKioskStore( useShallow((state) => ({ isLocked: state.isLocked, diff --git a/app/src/hooks/__tests__/useReconcileDeletedMonitors.test.tsx b/app/src/hooks/__tests__/useReconcileDeletedMonitors.test.tsx new file mode 100644 index 00000000..4a184528 --- /dev/null +++ b/app/src/hooks/__tests__/useReconcileDeletedMonitors.test.tsx @@ -0,0 +1,114 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { ReactNode } from 'react'; +import { useReconcileDeletedMonitors } from '../useReconcileDeletedMonitors'; +import { useSettingsStore, DEFAULT_MONTAGE_GROUP_LAYOUT } from '../../stores/settings'; +import { useDashboardStore } from '../../stores/dashboard'; +import { getMonitors } from '../../api/monitors'; + +vi.mock('../../api/monitors', () => ({ getMonitors: vi.fn() })); +vi.mock('../useCurrentProfile', () => ({ + useCurrentProfile: () => ({ currentProfile: { id: 'p1' }, settings: {} }), +})); +vi.mock('../../stores/auth', () => ({ + useAuthStore: (selector: (s: { isAuthenticated: boolean }) => unknown) => + selector({ isAuthenticated: true }), +})); + +const mockGetMonitors = vi.mocked(getMonitors); + +function monitorList(ids: string[]) { + return { monitors: ids.map((Id) => ({ Monitor: { Id }, Monitor_Status: undefined })) } as never; +} + +function wrapper({ children }: { children: ReactNode }) { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return {children}; +} + +function seedProfile() { + useSettingsStore.setState({ + profileSettings: { + p1: { + excludedMonitorIds: ['1', '99'], + montageByGroup: { + all: { ...DEFAULT_MONTAGE_GROUP_LAYOUT, hiddenMonitorIds: ['99'] }, + }, + }, + } as never, + }); + useDashboardStore.setState({ + widgets: { + p1: [ + { + id: 'w1', + type: 'monitor', + settings: { monitorIds: ['1', '99'] }, + layout: { i: 'w1', x: 0, y: 0, w: 4, h: 4 }, + }, + ], + }, + }); +} + +function storedState() { + return { + excluded: useSettingsStore.getState().getProfileSettings('p1').excludedMonitorIds, + montageHidden: + useSettingsStore.getState().getProfileSettings('p1').montageByGroup?.all?.hiddenMonitorIds, + widgetIds: useDashboardStore.getState().widgets.p1?.[0].settings.monitorIds, + }; +} + +describe('useReconcileDeletedMonitors', () => { + beforeEach(() => { + mockGetMonitors.mockReset(); + seedProfile(); + }); + + it('drops ids for monitors ZoneMinder no longer has, everywhere they are stored', async () => { + mockGetMonitors.mockResolvedValue(monitorList(['1'])); + + renderHook(() => useReconcileDeletedMonitors(), { wrapper }); + + await waitFor(() => expect(storedState().excluded).toEqual(['1'])); + expect(storedState().montageHidden).toEqual([]); + expect(storedState().widgetIds).toEqual(['1']); + }); + + it('keeps a hidden monitor that still exists, which the ordinary monitor list omits', async () => { + // '99' is hidden, so getMonitors() without includeExcluded would not return + // it. Reconciling against that list would delete the user's own hidden + // entry, so the hook must ask for the list that includes it. + mockGetMonitors.mockResolvedValue(monitorList(['1', '99'])); + + renderHook(() => useReconcileDeletedMonitors(), { wrapper }); + + await waitFor(() => expect(mockGetMonitors).toHaveBeenCalled()); + expect(mockGetMonitors).toHaveBeenCalledWith({ includeExcluded: true }); + expect(storedState().excluded).toEqual(['1', '99']); + expect(storedState().widgetIds).toEqual(['1', '99']); + }); + + it('changes nothing when the fetch fails: an error is not proof of deletion', async () => { + mockGetMonitors.mockRejectedValue(new Error('offline')); + + renderHook(() => useReconcileDeletedMonitors(), { wrapper }); + + await waitFor(() => expect(mockGetMonitors).toHaveBeenCalled()); + expect(storedState().excluded).toEqual(['1', '99']); + expect(storedState().widgetIds).toEqual(['1', '99']); + }); + + it('changes nothing when the server returns no monitors at all', async () => { + mockGetMonitors.mockResolvedValue(monitorList([])); + + renderHook(() => useReconcileDeletedMonitors(), { wrapper }); + + await waitFor(() => expect(mockGetMonitors).toHaveBeenCalled()); + expect(storedState().excluded).toEqual(['1', '99']); + expect(storedState().montageHidden).toEqual(['99']); + expect(storedState().widgetIds).toEqual(['1', '99']); + }); +}); diff --git a/app/src/hooks/useReconcileDeletedMonitors.ts b/app/src/hooks/useReconcileDeletedMonitors.ts new file mode 100644 index 00000000..cc79bca7 --- /dev/null +++ b/app/src/hooks/useReconcileDeletedMonitors.ts @@ -0,0 +1,77 @@ +/** + * useReconcileDeletedMonitors Hook + * + * Drops stored references to monitors ZoneMinder no longer has, once per + * profile per successful monitor fetch. Deleting a monitor in ZoneMinder used + * to leave it behind forever: still counted in Settings' hidden monitors with + * no way to un-hide it, and still listed in a dashboard widget's display order + * as "Monitor 12" (refs #323, #324). + * + * The list has to be the one that includes excluded monitors. The ordinary + * monitors query has already dropped the hidden ones, and reconciling the + * hidden list against a list built by removing it would delete every entry. + * + * Nothing is pruned unless the fetch succeeded and returned at least one + * monitor. An empty or failed response is indistinguishable from "every + * monitor was deleted", and acting on it would take the user's configuration + * with it. + */ + +import { useEffect } from 'react'; +import { useQuery } from '@tanstack/react-query'; +import { getMonitors } from '../api/monitors'; +import { queryKeys } from '../lib/query/query-keys'; +import { useAuthStore } from '../stores/auth'; +import { useCurrentProfile } from './useCurrentProfile'; +import { useSettingsStore } from '../stores/settings'; +import { useDashboardStore } from '../stores/dashboard'; +import { log, LogLevel } from '../lib/logger'; +import { + pruneProfileSettingsMonitorIds, + pruneWidgetMonitorIds, +} from '../lib/monitor/prune-deleted-monitors'; + +export function useReconcileDeletedMonitors(): void { + const { currentProfile } = useCurrentProfile(); + const isAuthenticated = useAuthStore((state) => state.isAuthenticated); + const profileId = currentProfile?.id; + + // Same key the hidden-monitors setting uses, so this shares its cache rather + // than adding a second full monitor fetch. + const { data, isSuccess } = useQuery({ + queryKey: queryKeys.monitorsAllIncludingExcluded(profileId), + queryFn: () => getMonitors({ includeExcluded: true }), + enabled: !!profileId && isAuthenticated, + }); + + const monitors = data?.monitors; + + useEffect(() => { + if (!profileId || !isSuccess || !monitors?.length) return; + + const known = new Set(monitors.map(({ Monitor }) => Monitor.Id)); + + const settingsState = useSettingsStore.getState(); + const patch = pruneProfileSettingsMonitorIds( + settingsState.getProfileSettings(profileId), + known + ); + if (patch) { + settingsState.updateProfileSettings(profileId, patch); + log.monitor('Dropped deleted monitors from profile settings', LogLevel.INFO, { + keys: Object.keys(patch), + }); + } + + const dashboardState = useDashboardStore.getState(); + const widgetUpdates = pruneWidgetMonitorIds(dashboardState.widgets[profileId] ?? [], known); + for (const { id, settings } of widgetUpdates) { + dashboardState.updateWidget(profileId, id, { settings }); + } + if (widgetUpdates.length > 0) { + log.dashboard('Dropped deleted monitors from dashboard widgets', LogLevel.INFO, { + count: widgetUpdates.length, + }); + } + }, [profileId, isSuccess, monitors]); +} diff --git a/app/src/lib/monitor/__tests__/prune-deleted-monitors.test.ts b/app/src/lib/monitor/__tests__/prune-deleted-monitors.test.ts new file mode 100644 index 00000000..e18ac380 --- /dev/null +++ b/app/src/lib/monitor/__tests__/prune-deleted-monitors.test.ts @@ -0,0 +1,108 @@ +/** + * Tests for pruning references to monitors ZoneMinder no longer has + * (refs #323, #324). + */ + +import { describe, it, expect } from 'vitest'; +import { + pruneProfileSettingsMonitorIds, + pruneWidgetMonitorIds, +} from '../prune-deleted-monitors'; +import { DEFAULT_MONTAGE_GROUP_LAYOUT } from '../../../stores/settings'; +import type { DashboardWidget } from '../../../stores/dashboard'; + +const KNOWN = new Set(['1', '2']); + +function montageBucket(overrides: Partial) { + return { ...DEFAULT_MONTAGE_GROUP_LAYOUT, ...overrides }; +} + +function monitorWidget(settings: DashboardWidget['settings']): DashboardWidget { + return { + id: 'w1', + type: 'monitor', + settings, + layout: { i: 'w1', x: 0, y: 0, w: 4, h: 4 }, + }; +} + +describe('pruneProfileSettingsMonitorIds', () => { + it('drops a hidden monitor that no longer exists so the hidden count matches reality', () => { + const patch = pruneProfileSettingsMonitorIds({ excludedMonitorIds: ['1', '99'] }, KNOWN); + + expect(patch?.excludedMonitorIds).toEqual(['1']); + }); + + it('keeps every id when all of them still exist', () => { + expect(pruneProfileSettingsMonitorIds({ excludedMonitorIds: ['1', '2'] }, KNOWN)).toBeNull(); + }); + + it('drops deleted monitors from a montage group without touching the survivors', () => { + const patch = pruneProfileSettingsMonitorIds( + { + montageByGroup: { + all: montageBucket({ + hiddenMonitorIds: ['2', '99'], + workingLayout: [ + { i: '1', x: 0, y: 0, w: 6, h: 6 }, + { i: '99', x: 6, y: 0, w: 6, h: 6 }, + ], + }), + }, + }, + KNOWN + ); + + expect(patch?.montageByGroup?.all.hiddenMonitorIds).toEqual(['2']); + expect(patch?.montageByGroup?.all.workingLayout.map((l) => l.i)).toEqual(['1']); + }); + + it('leaves named saved layouts alone: they are user artifacts, not live state', () => { + const saved = [ + { name: 'Front', layout: [{ i: '99', x: 0, y: 0, w: 6, h: 6 }], displayCols: 2 }, + ]; + const patch = pruneProfileSettingsMonitorIds( + { montageByGroup: { all: montageBucket({ hiddenMonitorIds: ['99'], savedLayouts: saved }) } }, + KNOWN + ); + + expect(patch?.montageByGroup?.all.savedLayouts).toEqual(saved); + }); + + it('reports no change when a montage group holds only live monitors', () => { + const patch = pruneProfileSettingsMonitorIds( + { montageByGroup: { all: montageBucket({ hiddenMonitorIds: ['1'] }) } }, + KNOWN + ); + + expect(patch).toBeNull(); + }); +}); + +describe('pruneWidgetMonitorIds', () => { + it('drops a deleted monitor from a widget display order', () => { + const updates = pruneWidgetMonitorIds([monitorWidget({ monitorIds: ['1', '99', '2'] })], KNOWN); + + expect(updates).toEqual([{ id: 'w1', settings: { monitorIds: ['1', '2'] } }]); + }); + + it('clears a single-monitor widget whose monitor is gone', () => { + const updates = pruneWidgetMonitorIds([monitorWidget({ monitorId: '99' })], KNOWN); + + expect(updates[0].settings.monitorId).toBeUndefined(); + }); + + it('leaves widgets that reference only live monitors untouched', () => { + expect(pruneWidgetMonitorIds([monitorWidget({ monitorIds: ['1'], monitorId: '2' })], KNOWN)) + .toEqual([]); + }); + + it('keeps the rest of a widget settings object intact', () => { + const updates = pruneWidgetMonitorIds( + [monitorWidget({ monitorIds: ['99'], feedFit: 'cover', eventCount: 5 })], + KNOWN + ); + + expect(updates[0].settings).toEqual({ monitorIds: [], feedFit: 'cover', eventCount: 5 }); + }); +}); diff --git a/app/src/lib/monitor/prune-deleted-monitors.ts b/app/src/lib/monitor/prune-deleted-monitors.ts new file mode 100644 index 00000000..5bb02c41 --- /dev/null +++ b/app/src/lib/monitor/prune-deleted-monitors.ts @@ -0,0 +1,104 @@ +/** + * Pruning references to monitors ZoneMinder no longer has. + * + * Monitor ids are persisted in several places: the per-profile hidden list, + * each montage group's hidden list and working layout, and dashboard widget + * settings. Deleting a monitor in ZoneMinder removes it from the API but not + * from any of those, so it lingers as a ghost: counted as hidden while absent + * from the list that would let you un-hide it (refs #324), or listed as + * "Monitor 12" in a widget's display order (refs #323). + * + * These functions are pure and report "nothing to do" rather than returning a + * fresh copy, so a caller can write only when something actually changed. + * Deciding *when* it is safe to prune belongs to the caller: an incomplete + * monitor list would read as "everything was deleted" and take the user's + * configuration with it. + */ + +import type { Layout } from 'react-grid-layout'; +import type { MontageGroupLayout, ProfileSettings } from '../../stores/settings'; +import type { DashboardWidget } from '../../stores/dashboard'; + +/** Same array back when every id survives, so callers can compare by identity. */ +function keepKnown(ids: string[], known: Set): string[] { + const kept = ids.filter((id) => known.has(id)); + return kept.length === ids.length ? ids : kept; +} + +function keepKnownLayout(layout: Layout[], known: Set): Layout[] { + const kept = layout.filter((item) => known.has(item.i)); + return kept.length === layout.length ? layout : kept; +} + +function pruneGroup(bucket: MontageGroupLayout, known: Set): MontageGroupLayout | null { + const hiddenMonitorIds = keepKnown(bucket.hiddenMonitorIds, known); + const workingLayout = keepKnownLayout(bucket.workingLayout, known); + if (hiddenMonitorIds === bucket.hiddenMonitorIds && workingLayout === bucket.workingLayout) { + return null; + } + // savedLayouts stays as it is. Those are named arrangements the user made + // and may reload later; an entry for a monitor that is gone renders nothing + // and costs nothing, which is a better trade than editing saved work. + return { ...bucket, hiddenMonitorIds, workingLayout }; +} + +/** + * Build the settings patch that removes every reference to a monitor outside + * `known`, or null when the settings hold no stale ids. + */ +export function pruneProfileSettingsMonitorIds( + settings: Pick, 'excludedMonitorIds' | 'montageByGroup'>, + known: Set +): Partial | null { + const patch: Partial = {}; + + const excluded = settings.excludedMonitorIds ?? []; + const keptExcluded = keepKnown(excluded, known); + if (keptExcluded !== excluded) patch.excludedMonitorIds = keptExcluded; + + const byGroup = settings.montageByGroup; + if (byGroup) { + let groupsChanged = false; + const nextGroups: Record = {}; + for (const [groupKey, bucket] of Object.entries(byGroup)) { + const pruned = pruneGroup(bucket, known); + nextGroups[groupKey] = pruned ?? bucket; + if (pruned) groupsChanged = true; + } + if (groupsChanged) patch.montageByGroup = nextGroups; + } + + return Object.keys(patch).length > 0 ? patch : null; +} + +/** A widget id with the settings it should be updated to. */ +export interface WidgetSettingsUpdate { + id: string; + settings: DashboardWidget['settings']; +} + +/** + * Build the widget updates that remove every reference to a monitor outside + * `known`. Widgets left with no monitor are kept, not deleted: an empty widget + * is recoverable, a deleted one is not. + */ +export function pruneWidgetMonitorIds( + widgets: DashboardWidget[], + known: Set +): WidgetSettingsUpdate[] { + const updates: WidgetSettingsUpdate[] = []; + + for (const widget of widgets) { + const { monitorIds, monitorId } = widget.settings; + const keptIds = monitorIds ? keepKnown(monitorIds, known) : undefined; + const idIsStale = monitorId !== undefined && !known.has(monitorId); + if (keptIds === monitorIds && !idIsStale) continue; + + const settings = { ...widget.settings }; + if (keptIds) settings.monitorIds = keptIds; + if (idIsStale) delete settings.monitorId; + updates.push({ id: widget.id, settings }); + } + + return updates; +} diff --git a/docs/developer-guide/05-component-architecture.rst b/docs/developer-guide/05-component-architecture.rst index deb27fee..10c54450 100644 --- a/docs/developer-guide/05-component-architecture.rst +++ b/docs/developer-guide/05-component-architecture.rst @@ -1022,6 +1022,36 @@ profile, so every dependent view refetches with the new exclusion applied. **Test ids**: ``hidden-monitors-list``, ``hidden-monitors-count``, ``hidden-monitor-row-``, ``hidden-monitor-toggle-``. +Forgetting deleted monitors +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Location**: ``src/hooks/useReconcileDeletedMonitors.ts``, +``src/lib/monitor/prune-deleted-monitors.ts`` + +Monitor ids are persisted in four places: ``excludedMonitorIds``, each montage +group's ``hiddenMonitorIds`` and ``workingLayout``, and dashboard widget +settings. Deleting a monitor in ZoneMinder removes it from the API but from +none of those, so it lingers as a ghost. A hidden monitor that no longer exists +was the worst case: still counted in the hidden total, absent from the list +that would let you un-hide it, and therefore permanently stuck. + +``AppLayout`` mounts ``useReconcileDeletedMonitors`` once. It reads the same +``monitorsAllIncludingExcluded`` query the section above uses, so it adds no +second fetch, and drops any stored id the response does not contain. + +Two things about it are deliberate and easy to get wrong when editing it: + +- It must read the list that **includes** excluded monitors. The ordinary + monitors query has already removed the hidden ones, and reconciling the + hidden list against a list built by removing it would delete every entry. +- It prunes nothing unless the query succeeded and returned at least one + monitor. An empty or failed response looks exactly like "every monitor was + deleted", and acting on it would destroy the user's configuration. + +Named montage ``savedLayouts`` are left alone. They are arrangements the user +saved and may reload later; an entry for a monitor that is gone renders +nothing, which is a better trade than editing saved work. + Whole-app surfaces ------------------