mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(panel): settings preferences become real client prefs — no more 422 save, no more theater toggles (#487)
The Settings page PUT four keys (notifications_enabled, sound_enabled, auto_refresh, refresh_interval) the backend's settings allowlist never accepted — Save died on the first 422 and had never persisted these cards. Worse, nothing consumed the prefs anywhere: no auto-refresh timer, no notification toast, no sound system existed. Pure theater. - the four prefs move into the persisted UI store (client-only, same idiom as theme/sidebar) and the cards apply instantly; the dead server plumbing and the global Save button are gone — the backend allowlist stays strict and untouched - AutoRefreshDriver (new): when Auto Refresh is on, ticks the page-refresh registry every N seconds — skips while nothing is registered or a refresh is in flight; default-off so no background poller starts unasked - NotificationAlerts (new): toasts each newly-arrived WS notification (subject + priority) when notifications are enabled, with an optional ~120ms Web-Audio chime — initial backlog on connect never toasts, one chime per batch, autoplay blocks never throw - tests: settings page rewritten store-driven; fake-timer coverage for the driver; stream/store/sonner/AudioContext-mocked coverage for alerts Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -11,6 +11,7 @@ import {
|
||||
DropdownMenuTrigger,
|
||||
} from "@/components/ui/dropdown-menu";
|
||||
import { NotificationBell } from "@/components/notifications/notification-bell";
|
||||
import { NotificationAlerts } from "@/components/notifications/notification-alerts";
|
||||
import { ConnectionStatus } from "./connection-status";
|
||||
import { MobileSidebar } from "./mobile-sidebar";
|
||||
import {
|
||||
@@ -102,6 +103,7 @@ export function Header() {
|
||||
|
||||
{/* Notifications with WebSocket */}
|
||||
<NotificationBell />
|
||||
<NotificationAlerts />
|
||||
|
||||
{/* User */}
|
||||
<div className="flex items-center gap-2 ml-2 pl-4 border-l">
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render } from "@testing-library/react";
|
||||
import { toast } from "sonner";
|
||||
import { NotificationAlerts } from "../notification-alerts";
|
||||
import type { NotificationMessage } from "@/hooks/use-websocket";
|
||||
|
||||
// Mutable stand-in for the WS notification stream — reassign (never mutate in
|
||||
// place) `list` between renders so the effect's dependency actually changes.
|
||||
const mockStream = vi.hoisted(() => ({
|
||||
list: [] as NotificationMessage[],
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/use-websocket", () => ({
|
||||
useNotificationStream: () => ({ notifications: mockStream.list }),
|
||||
}));
|
||||
|
||||
// Non-reactive stand-in for the persisted UI store (zustand selector form).
|
||||
const mockUiStore = vi.hoisted(() => ({
|
||||
notificationsEnabled: true,
|
||||
soundEnabled: true,
|
||||
}));
|
||||
|
||||
vi.mock("@/store", () => ({
|
||||
useUIStore: (selector: (s: typeof mockUiStore) => unknown) =>
|
||||
selector(mockUiStore),
|
||||
}));
|
||||
|
||||
vi.mock("sonner", () => ({ toast: vi.fn() }));
|
||||
|
||||
function notification(overrides: Partial<NotificationMessage>) {
|
||||
return { type: "notification", ...overrides } as NotificationMessage;
|
||||
}
|
||||
|
||||
// AudioContext doesn't exist in jsdom; stub a constructible fake so playChime
|
||||
// can run its real path instead of hitting the "no AudioContext" early return.
|
||||
function stubAudioContext() {
|
||||
const ctor = vi.fn(function FakeAudioContext() {
|
||||
return {
|
||||
currentTime: 0,
|
||||
createOscillator: () => ({
|
||||
type: "sine",
|
||||
frequency: { value: 0 },
|
||||
connect: (dest: unknown) => dest,
|
||||
start: vi.fn(),
|
||||
stop: vi.fn(),
|
||||
onended: null,
|
||||
}),
|
||||
createGain: () => ({
|
||||
gain: { value: 0 },
|
||||
connect: (dest: unknown) => dest,
|
||||
}),
|
||||
close: vi.fn(),
|
||||
};
|
||||
});
|
||||
vi.stubGlobal("AudioContext", ctor);
|
||||
return ctor;
|
||||
}
|
||||
|
||||
describe("NotificationAlerts", () => {
|
||||
beforeEach(() => {
|
||||
mockStream.list = [];
|
||||
mockUiStore.notificationsEnabled = true;
|
||||
mockUiStore.soundEnabled = true;
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("ignores whatever the stream already has on mount (no backlog toast)", () => {
|
||||
mockStream.list = [
|
||||
notification({ notification_id: "1", subject: "Old one" }),
|
||||
];
|
||||
render(<NotificationAlerts />);
|
||||
expect(toast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("toasts a newly-arrived notification while enabled", () => {
|
||||
const { rerender } = render(<NotificationAlerts />);
|
||||
expect(toast).not.toHaveBeenCalled();
|
||||
|
||||
mockStream.list = [
|
||||
...mockStream.list,
|
||||
notification({ notification_id: "2", subject: "New task", priority: "high" }),
|
||||
];
|
||||
rerender(<NotificationAlerts />);
|
||||
|
||||
expect(toast).toHaveBeenCalledTimes(1);
|
||||
expect(toast).toHaveBeenCalledWith(
|
||||
"New task",
|
||||
expect.objectContaining({ description: expect.stringContaining("high") }),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not toast when notifications are disabled", () => {
|
||||
mockUiStore.notificationsEnabled = false;
|
||||
const { rerender } = render(<NotificationAlerts />);
|
||||
|
||||
mockStream.list = [
|
||||
...mockStream.list,
|
||||
notification({ notification_id: "3", subject: "Silenced" }),
|
||||
];
|
||||
rerender(<NotificationAlerts />);
|
||||
|
||||
expect(toast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("plays a chime via Web Audio when sound is enabled", () => {
|
||||
const ctor = stubAudioContext();
|
||||
const { rerender } = render(<NotificationAlerts />);
|
||||
|
||||
mockStream.list = [
|
||||
...mockStream.list,
|
||||
notification({ notification_id: "4", subject: "Ping" }),
|
||||
];
|
||||
rerender(<NotificationAlerts />);
|
||||
|
||||
expect(ctor).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("skips the chime when sound is disabled, but still toasts", () => {
|
||||
const ctor = stubAudioContext();
|
||||
mockUiStore.soundEnabled = false;
|
||||
const { rerender } = render(<NotificationAlerts />);
|
||||
|
||||
mockStream.list = [
|
||||
...mockStream.list,
|
||||
notification({ notification_id: "5", subject: "Quiet" }),
|
||||
];
|
||||
rerender(<NotificationAlerts />);
|
||||
|
||||
expect(toast).toHaveBeenCalledTimes(1);
|
||||
expect(ctor).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("never crashes when AudioContext is unavailable (autoplay-blocked browsers)", () => {
|
||||
vi.stubGlobal("AudioContext", undefined);
|
||||
const { rerender } = render(<NotificationAlerts />);
|
||||
|
||||
mockStream.list = [
|
||||
...mockStream.list,
|
||||
notification({ notification_id: "6", subject: "Still fine" }),
|
||||
];
|
||||
expect(() => rerender(<NotificationAlerts />)).not.toThrow();
|
||||
expect(toast).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,63 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useRef } from "react";
|
||||
import { toast } from "sonner";
|
||||
import { useNotificationStream } from "@/hooks/use-websocket";
|
||||
import { useUIStore } from "@/store";
|
||||
|
||||
/** ~120ms, low-volume beep via Web Audio — no audio asset. Never throws. */
|
||||
function playChime() {
|
||||
try {
|
||||
const AudioCtx =
|
||||
window.AudioContext ??
|
||||
(window as unknown as { webkitAudioContext?: typeof AudioContext })
|
||||
.webkitAudioContext;
|
||||
if (!AudioCtx) return;
|
||||
const ctx = new AudioCtx();
|
||||
const osc = ctx.createOscillator();
|
||||
const gain = ctx.createGain();
|
||||
osc.type = "sine";
|
||||
osc.frequency.value = 880;
|
||||
gain.gain.value = 0.05;
|
||||
osc.connect(gain).connect(ctx.destination);
|
||||
osc.start();
|
||||
osc.stop(ctx.currentTime + 0.12);
|
||||
osc.onended = () => void ctx.close();
|
||||
} catch {
|
||||
// Autoplay/permission blocks are expected on some browsers — no-op.
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Watches the live notification stream and toasts + optionally chimes for
|
||||
* newly-arrived entries. Mount exactly once (next to the bell). Renders
|
||||
* nothing.
|
||||
*/
|
||||
export function NotificationAlerts() {
|
||||
const { notifications } = useNotificationStream();
|
||||
const notificationsEnabled = useUIStore((s) => s.notificationsEnabled);
|
||||
const soundEnabled = useUIStore((s) => s.soundEnabled);
|
||||
|
||||
// null = not yet initialized; primes on first render so a page load never
|
||||
// toasts a backlog the stream replays on connect.
|
||||
const seenCountRef = useRef<number | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (seenCountRef.current === null) {
|
||||
seenCountRef.current = notifications.length;
|
||||
return;
|
||||
}
|
||||
const newOnes = notifications.slice(seenCountRef.current);
|
||||
seenCountRef.current = notifications.length;
|
||||
if (newOnes.length === 0 || !notificationsEnabled) return;
|
||||
|
||||
for (const n of newOnes) {
|
||||
toast(n.subject ?? "New notification", {
|
||||
description: n.priority ? `Priority: ${n.priority}` : undefined,
|
||||
});
|
||||
}
|
||||
if (soundEnabled) playChime();
|
||||
}, [notifications, notificationsEnabled, soundEnabled]);
|
||||
|
||||
return null;
|
||||
}
|
||||
@@ -0,0 +1,119 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { render, act } from "@testing-library/react";
|
||||
import { useState } from "react";
|
||||
import { AutoRefreshDriver } from "../auto-refresh-driver";
|
||||
import { PageRefreshProvider } from "../page-refresh-provider";
|
||||
import { usePageRefresh } from "@/hooks";
|
||||
|
||||
// Non-reactive stand-in for the persisted UI store: AutoRefreshDriver reads it
|
||||
// via the zustand selector form (`useUIStore((s) => s.foo)`), so the mock must
|
||||
// accept and apply a selector. Mutate the fields directly and re-render to
|
||||
// simulate a store update (matches the settings-page test idiom).
|
||||
const mockStore = vi.hoisted(() => ({
|
||||
autoRefresh: false,
|
||||
refreshIntervalSeconds: 10,
|
||||
}));
|
||||
|
||||
vi.mock("@/store", () => ({
|
||||
useUIStore: (selector: (s: typeof mockStore) => unknown) =>
|
||||
selector(mockStore),
|
||||
}));
|
||||
|
||||
function Registrator({ callback }: { callback: () => void | Promise<void> }) {
|
||||
const { register } = usePageRefresh();
|
||||
const [registered, setRegistered] = useState(false);
|
||||
if (!registered) {
|
||||
register(callback);
|
||||
setRegistered(true);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function Harness({
|
||||
callback,
|
||||
mountRegistrator = true,
|
||||
}: {
|
||||
callback: () => void | Promise<void>;
|
||||
mountRegistrator?: boolean;
|
||||
}) {
|
||||
return (
|
||||
<PageRefreshProvider>
|
||||
{mountRegistrator && <Registrator callback={callback} />}
|
||||
<AutoRefreshDriver />
|
||||
</PageRefreshProvider>
|
||||
);
|
||||
}
|
||||
|
||||
describe("AutoRefreshDriver", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
mockStore.autoRefresh = false;
|
||||
mockStore.refreshIntervalSeconds = 10;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("arms no interval when disabled (nothing registered), even if the pref is on", () => {
|
||||
mockStore.autoRefresh = true;
|
||||
render(<Harness callback={vi.fn()} mountRegistrator={false} />);
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
});
|
||||
|
||||
it("arms no interval when Auto Refresh is off, even with a callback registered", () => {
|
||||
render(<Harness callback={vi.fn()} />);
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
});
|
||||
|
||||
it("fires refresh at the configured interval when enabled + registered", async () => {
|
||||
mockStore.autoRefresh = true;
|
||||
const callback = vi.fn();
|
||||
render(<Harness callback={callback} />);
|
||||
|
||||
// Async act so the refresh() promise chain (setLoading true -> ... ->
|
||||
// false) drains between ticks — otherwise the second tick's
|
||||
// loadingRef.current read races the still-pending microtask.
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(10_000);
|
||||
});
|
||||
expect(callback).toHaveBeenCalledTimes(1);
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(10_000);
|
||||
});
|
||||
expect(callback).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("respects a changed interval", () => {
|
||||
mockStore.autoRefresh = true;
|
||||
mockStore.refreshIntervalSeconds = 5;
|
||||
const callback = vi.fn();
|
||||
render(<Harness callback={callback} />);
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(5_000);
|
||||
});
|
||||
expect(callback).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("stops ticking once the pref turns off (cleans up the interval)", () => {
|
||||
mockStore.autoRefresh = true;
|
||||
const callback = vi.fn();
|
||||
const { rerender } = render(<Harness callback={callback} />);
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(10_000);
|
||||
});
|
||||
expect(callback).toHaveBeenCalledTimes(1);
|
||||
|
||||
mockStore.autoRefresh = false;
|
||||
rerender(<Harness callback={callback} />);
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(10_000);
|
||||
});
|
||||
expect(callback).toHaveBeenCalledTimes(1); // unchanged
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,32 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useRef } from "react";
|
||||
import { usePageRefresh } from "@/hooks";
|
||||
import { useUIStore } from "@/store";
|
||||
|
||||
/**
|
||||
* Ticks the page-refresh registry on an interval when the user's Auto Refresh
|
||||
* preference is on. Must render inside `PageRefreshProvider`. Renders nothing.
|
||||
*/
|
||||
export function AutoRefreshDriver() {
|
||||
const { refresh, disabled, loading } = usePageRefresh();
|
||||
const autoRefresh = useUIStore((s) => s.autoRefresh);
|
||||
const refreshIntervalSeconds = useUIStore((s) => s.refreshIntervalSeconds);
|
||||
|
||||
// Read via ref inside the tick so an in-flight refresh only skips that one
|
||||
// tick, instead of tearing down/re-arming the interval on every loading flip.
|
||||
const loadingRef = useRef(loading);
|
||||
useEffect(() => {
|
||||
loadingRef.current = loading;
|
||||
}, [loading]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!autoRefresh || disabled) return;
|
||||
const id = setInterval(() => {
|
||||
if (!loadingRef.current) void refresh();
|
||||
}, refreshIntervalSeconds * 1000);
|
||||
return () => clearInterval(id);
|
||||
}, [autoRefresh, disabled, refreshIntervalSeconds, refresh]);
|
||||
|
||||
return null;
|
||||
}
|
||||
Reference in New Issue
Block a user