Skip to content

Commit 3950047

Browse files
pliablepixelsclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 2d19a31 commit 3950047

6 files changed

Lines changed: 437 additions & 0 deletions

File tree

app/src/components/layout/AppLayout.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import { KioskOverlay } from '../kiosk/KioskOverlay';
3838
import { SidebarContent } from './SidebarContent';
3939
import { DeveloperNoticeBanner } from './DeveloperNoticeBanner';
4040
import { OfflineBanner } from './OfflineBanner';
41+
import { useReconcileDeletedMonitors } from '../../hooks/useReconcileDeletedMonitors';
4142
import { CertTrustBanner } from '../CertTrustBanner';
4243
import { DeleteBatchBar } from '../events/DeleteBatchBar';
4344
import { AssistantWidget } from '../assistant/AssistantWidget';
@@ -105,6 +106,9 @@ export default function AppLayout() {
105106
// Apply global insomnia setting
106107
useInsomnia({ enabled: settings.insomnia });
107108

109+
// Forget monitors ZoneMinder no longer has (refs #323, #324)
110+
useReconcileDeletedMonitors();
111+
108112
const { isLocked, previousInsomniaState } = useKioskStore(
109113
useShallow((state) => ({
110114
isLocked: state.isLocked,
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { describe, it, expect, beforeEach, vi } from 'vitest';
2+
import { renderHook, waitFor } from '@testing-library/react';
3+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
4+
import type { ReactNode } from 'react';
5+
import { useReconcileDeletedMonitors } from '../useReconcileDeletedMonitors';
6+
import { useSettingsStore, DEFAULT_MONTAGE_GROUP_LAYOUT } from '../../stores/settings';
7+
import { useDashboardStore } from '../../stores/dashboard';
8+
import { getMonitors } from '../../api/monitors';
9+
10+
vi.mock('../../api/monitors', () => ({ getMonitors: vi.fn() }));
11+
vi.mock('../useCurrentProfile', () => ({
12+
useCurrentProfile: () => ({ currentProfile: { id: 'p1' }, settings: {} }),
13+
}));
14+
vi.mock('../../stores/auth', () => ({
15+
useAuthStore: (selector: (s: { isAuthenticated: boolean }) => unknown) =>
16+
selector({ isAuthenticated: true }),
17+
}));
18+
19+
const mockGetMonitors = vi.mocked(getMonitors);
20+
21+
function monitorList(ids: string[]) {
22+
return { monitors: ids.map((Id) => ({ Monitor: { Id }, Monitor_Status: undefined })) } as never;
23+
}
24+
25+
function wrapper({ children }: { children: ReactNode }) {
26+
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
27+
return <QueryClientProvider client={client}>{children}</QueryClientProvider>;
28+
}
29+
30+
function seedProfile() {
31+
useSettingsStore.setState({
32+
profileSettings: {
33+
p1: {
34+
excludedMonitorIds: ['1', '99'],
35+
montageByGroup: {
36+
all: { ...DEFAULT_MONTAGE_GROUP_LAYOUT, hiddenMonitorIds: ['99'] },
37+
},
38+
},
39+
} as never,
40+
});
41+
useDashboardStore.setState({
42+
widgets: {
43+
p1: [
44+
{
45+
id: 'w1',
46+
type: 'monitor',
47+
settings: { monitorIds: ['1', '99'] },
48+
layout: { i: 'w1', x: 0, y: 0, w: 4, h: 4 },
49+
},
50+
],
51+
},
52+
});
53+
}
54+
55+
function storedState() {
56+
return {
57+
excluded: useSettingsStore.getState().getProfileSettings('p1').excludedMonitorIds,
58+
montageHidden:
59+
useSettingsStore.getState().getProfileSettings('p1').montageByGroup?.all?.hiddenMonitorIds,
60+
widgetIds: useDashboardStore.getState().widgets.p1?.[0].settings.monitorIds,
61+
};
62+
}
63+
64+
describe('useReconcileDeletedMonitors', () => {
65+
beforeEach(() => {
66+
mockGetMonitors.mockReset();
67+
seedProfile();
68+
});
69+
70+
it('drops ids for monitors ZoneMinder no longer has, everywhere they are stored', async () => {
71+
mockGetMonitors.mockResolvedValue(monitorList(['1']));
72+
73+
renderHook(() => useReconcileDeletedMonitors(), { wrapper });
74+
75+
await waitFor(() => expect(storedState().excluded).toEqual(['1']));
76+
expect(storedState().montageHidden).toEqual([]);
77+
expect(storedState().widgetIds).toEqual(['1']);
78+
});
79+
80+
it('keeps a hidden monitor that still exists, which the ordinary monitor list omits', async () => {
81+
// '99' is hidden, so getMonitors() without includeExcluded would not return
82+
// it. Reconciling against that list would delete the user's own hidden
83+
// entry, so the hook must ask for the list that includes it.
84+
mockGetMonitors.mockResolvedValue(monitorList(['1', '99']));
85+
86+
renderHook(() => useReconcileDeletedMonitors(), { wrapper });
87+
88+
await waitFor(() => expect(mockGetMonitors).toHaveBeenCalled());
89+
expect(mockGetMonitors).toHaveBeenCalledWith({ includeExcluded: true });
90+
expect(storedState().excluded).toEqual(['1', '99']);
91+
expect(storedState().widgetIds).toEqual(['1', '99']);
92+
});
93+
94+
it('changes nothing when the fetch fails: an error is not proof of deletion', async () => {
95+
mockGetMonitors.mockRejectedValue(new Error('offline'));
96+
97+
renderHook(() => useReconcileDeletedMonitors(), { wrapper });
98+
99+
await waitFor(() => expect(mockGetMonitors).toHaveBeenCalled());
100+
expect(storedState().excluded).toEqual(['1', '99']);
101+
expect(storedState().widgetIds).toEqual(['1', '99']);
102+
});
103+
104+
it('changes nothing when the server returns no monitors at all', async () => {
105+
mockGetMonitors.mockResolvedValue(monitorList([]));
106+
107+
renderHook(() => useReconcileDeletedMonitors(), { wrapper });
108+
109+
await waitFor(() => expect(mockGetMonitors).toHaveBeenCalled());
110+
expect(storedState().excluded).toEqual(['1', '99']);
111+
expect(storedState().montageHidden).toEqual(['99']);
112+
expect(storedState().widgetIds).toEqual(['1', '99']);
113+
});
114+
});
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* useReconcileDeletedMonitors Hook
3+
*
4+
* Drops stored references to monitors ZoneMinder no longer has, once per
5+
* profile per successful monitor fetch. Deleting a monitor in ZoneMinder used
6+
* to leave it behind forever: still counted in Settings' hidden monitors with
7+
* no way to un-hide it, and still listed in a dashboard widget's display order
8+
* as "Monitor 12" (refs #323, #324).
9+
*
10+
* The list has to be the one that includes excluded monitors. The ordinary
11+
* monitors query has already dropped the hidden ones, and reconciling the
12+
* hidden list against a list built by removing it would delete every entry.
13+
*
14+
* Nothing is pruned unless the fetch succeeded and returned at least one
15+
* monitor. An empty or failed response is indistinguishable from "every
16+
* monitor was deleted", and acting on it would take the user's configuration
17+
* with it.
18+
*/
19+
20+
import { useEffect } from 'react';
21+
import { useQuery } from '@tanstack/react-query';
22+
import { getMonitors } from '../api/monitors';
23+
import { queryKeys } from '../lib/query/query-keys';
24+
import { useAuthStore } from '../stores/auth';
25+
import { useCurrentProfile } from './useCurrentProfile';
26+
import { useSettingsStore } from '../stores/settings';
27+
import { useDashboardStore } from '../stores/dashboard';
28+
import { log, LogLevel } from '../lib/logger';
29+
import {
30+
pruneProfileSettingsMonitorIds,
31+
pruneWidgetMonitorIds,
32+
} from '../lib/monitor/prune-deleted-monitors';
33+
34+
export function useReconcileDeletedMonitors(): void {
35+
const { currentProfile } = useCurrentProfile();
36+
const isAuthenticated = useAuthStore((state) => state.isAuthenticated);
37+
const profileId = currentProfile?.id;
38+
39+
// Same key the hidden-monitors setting uses, so this shares its cache rather
40+
// than adding a second full monitor fetch.
41+
const { data, isSuccess } = useQuery({
42+
queryKey: queryKeys.monitorsAllIncludingExcluded(profileId),
43+
queryFn: () => getMonitors({ includeExcluded: true }),
44+
enabled: !!profileId && isAuthenticated,
45+
});
46+
47+
const monitors = data?.monitors;
48+
49+
useEffect(() => {
50+
if (!profileId || !isSuccess || !monitors?.length) return;
51+
52+
const known = new Set(monitors.map(({ Monitor }) => Monitor.Id));
53+
54+
const settingsState = useSettingsStore.getState();
55+
const patch = pruneProfileSettingsMonitorIds(
56+
settingsState.getProfileSettings(profileId),
57+
known
58+
);
59+
if (patch) {
60+
settingsState.updateProfileSettings(profileId, patch);
61+
log.monitor('Dropped deleted monitors from profile settings', LogLevel.INFO, {
62+
keys: Object.keys(patch),
63+
});
64+
}
65+
66+
const dashboardState = useDashboardStore.getState();
67+
const widgetUpdates = pruneWidgetMonitorIds(dashboardState.widgets[profileId] ?? [], known);
68+
for (const { id, settings } of widgetUpdates) {
69+
dashboardState.updateWidget(profileId, id, { settings });
70+
}
71+
if (widgetUpdates.length > 0) {
72+
log.dashboard('Dropped deleted monitors from dashboard widgets', LogLevel.INFO, {
73+
count: widgetUpdates.length,
74+
});
75+
}
76+
}, [profileId, isSuccess, monitors]);
77+
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* Tests for pruning references to monitors ZoneMinder no longer has
3+
* (refs #323, #324).
4+
*/
5+
6+
import { describe, it, expect } from 'vitest';
7+
import {
8+
pruneProfileSettingsMonitorIds,
9+
pruneWidgetMonitorIds,
10+
} from '../prune-deleted-monitors';
11+
import { DEFAULT_MONTAGE_GROUP_LAYOUT } from '../../../stores/settings';
12+
import type { DashboardWidget } from '../../../stores/dashboard';
13+
14+
const KNOWN = new Set(['1', '2']);
15+
16+
function montageBucket(overrides: Partial<typeof DEFAULT_MONTAGE_GROUP_LAYOUT>) {
17+
return { ...DEFAULT_MONTAGE_GROUP_LAYOUT, ...overrides };
18+
}
19+
20+
function monitorWidget(settings: DashboardWidget['settings']): DashboardWidget {
21+
return {
22+
id: 'w1',
23+
type: 'monitor',
24+
settings,
25+
layout: { i: 'w1', x: 0, y: 0, w: 4, h: 4 },
26+
};
27+
}
28+
29+
describe('pruneProfileSettingsMonitorIds', () => {
30+
it('drops a hidden monitor that no longer exists so the hidden count matches reality', () => {
31+
const patch = pruneProfileSettingsMonitorIds({ excludedMonitorIds: ['1', '99'] }, KNOWN);
32+
33+
expect(patch?.excludedMonitorIds).toEqual(['1']);
34+
});
35+
36+
it('keeps every id when all of them still exist', () => {
37+
expect(pruneProfileSettingsMonitorIds({ excludedMonitorIds: ['1', '2'] }, KNOWN)).toBeNull();
38+
});
39+
40+
it('drops deleted monitors from a montage group without touching the survivors', () => {
41+
const patch = pruneProfileSettingsMonitorIds(
42+
{
43+
montageByGroup: {
44+
all: montageBucket({
45+
hiddenMonitorIds: ['2', '99'],
46+
workingLayout: [
47+
{ i: '1', x: 0, y: 0, w: 6, h: 6 },
48+
{ i: '99', x: 6, y: 0, w: 6, h: 6 },
49+
],
50+
}),
51+
},
52+
},
53+
KNOWN
54+
);
55+
56+
expect(patch?.montageByGroup?.all.hiddenMonitorIds).toEqual(['2']);
57+
expect(patch?.montageByGroup?.all.workingLayout.map((l) => l.i)).toEqual(['1']);
58+
});
59+
60+
it('leaves named saved layouts alone: they are user artifacts, not live state', () => {
61+
const saved = [
62+
{ name: 'Front', layout: [{ i: '99', x: 0, y: 0, w: 6, h: 6 }], displayCols: 2 },
63+
];
64+
const patch = pruneProfileSettingsMonitorIds(
65+
{ montageByGroup: { all: montageBucket({ hiddenMonitorIds: ['99'], savedLayouts: saved }) } },
66+
KNOWN
67+
);
68+
69+
expect(patch?.montageByGroup?.all.savedLayouts).toEqual(saved);
70+
});
71+
72+
it('reports no change when a montage group holds only live monitors', () => {
73+
const patch = pruneProfileSettingsMonitorIds(
74+
{ montageByGroup: { all: montageBucket({ hiddenMonitorIds: ['1'] }) } },
75+
KNOWN
76+
);
77+
78+
expect(patch).toBeNull();
79+
});
80+
});
81+
82+
describe('pruneWidgetMonitorIds', () => {
83+
it('drops a deleted monitor from a widget display order', () => {
84+
const updates = pruneWidgetMonitorIds([monitorWidget({ monitorIds: ['1', '99', '2'] })], KNOWN);
85+
86+
expect(updates).toEqual([{ id: 'w1', settings: { monitorIds: ['1', '2'] } }]);
87+
});
88+
89+
it('clears a single-monitor widget whose monitor is gone', () => {
90+
const updates = pruneWidgetMonitorIds([monitorWidget({ monitorId: '99' })], KNOWN);
91+
92+
expect(updates[0].settings.monitorId).toBeUndefined();
93+
});
94+
95+
it('leaves widgets that reference only live monitors untouched', () => {
96+
expect(pruneWidgetMonitorIds([monitorWidget({ monitorIds: ['1'], monitorId: '2' })], KNOWN))
97+
.toEqual([]);
98+
});
99+
100+
it('keeps the rest of a widget settings object intact', () => {
101+
const updates = pruneWidgetMonitorIds(
102+
[monitorWidget({ monitorIds: ['99'], feedFit: 'cover', eventCount: 5 })],
103+
KNOWN
104+
);
105+
106+
expect(updates[0].settings).toEqual({ monitorIds: [], feedFit: 'cover', eventCount: 5 });
107+
});
108+
});

0 commit comments

Comments
 (0)