From eead78007d0578c3bfbf2b4dea97a4884927b9cc Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Sat, 26 Sep 2026 17:25:46 -0600 Subject: [PATCH] app: status and settings contexts say whether their first poll landed (#734) Both contexts start as {} and nothing distinguished that from a server that reported nothing, so for up to one poll interval after a page load consumers rendered the negative case as fact: the Settings form appeared with blank values and an "undefined/config/wrolpi.yaml" path (the ready check `settings ? true : undefined` was always true), the Downloads toggle was clickable and read "Downloading Disabled", the upgrade panel said "up to date, vundefined", and the five hardware toggles on the Controller page claimed "not supported on this server". Invariants now enforced and tested: * StatusContext.loaded is true once a poll returned a real status; the API being down does not count. SettingsContext.loaded is true once settings were fetched, and `failed` when a fetch returned nothing before that; a failed refresh after a load keeps the last known values and `loaded`. * The Settings page shows its loader until loaded and its error only when failed. The upgrade panel shows a loader until status loaded. The Downloads toggle is disabled with a neutral label until then. * A subsystem toggle is disabled without claiming "unsupported" while status is unknown (undefined); "unsupported" (null) is reserved for a loaded status that lacks the field. A call in flight is unknown, not unsupported. The context fixtures declare loaded so existing specs render as before. Co-authored-by: Claude Fable 5.1 --- app/src/components/Common.js | 15 ++-- app/src/components/StatusLoaded.test.js | 69 ++++++++++++++++++ app/src/components/admin/Settings.js | 21 ++++-- app/src/contexts/contexts.js | 5 ++ app/src/hooks/customHooks.js | 51 ++++++++++--- app/src/hooks/customHooks.test.js | 93 +++++++++++++++++++++++- app/src/hooks/useSubsystemToggle.test.js | 2 +- app/src/test-fixtures.js | 4 + 8 files changed, 235 insertions(+), 25 deletions(-) create mode 100644 app/src/components/StatusLoaded.test.js diff --git a/app/src/components/Common.js b/app/src/components/Common.js index 9e93e039..3803a962 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -928,7 +928,10 @@ export function HelpHeader({ // why it cannot be used. `confirmStop` asks before switching off. function SubsystemToggle({label, on, onChange, unsupportedMessage, info = null, disabled = null, confirmStop = null}) { const [confirmOpen, setConfirmOpen] = React.useState(false); + // null = the status has loaded without this subsystem; undefined = the status has not loaded + // yet (or a call is in flight). Both disable the toggle; only the first claims unsupported. const unsupported = on === null; + const unknown = on === undefined; const handleChange = (checked) => { if (!checked && confirmStop) { @@ -961,11 +964,11 @@ function SubsystemToggle({label, on, onChange, unsupportedMessage, info = null, } - {popup && } + {popup && } ; } @@ -1065,9 +1068,10 @@ export function Toggle({label, checked, disabled, onChange, icon, popupContent = export function DisableDownloadsToggle() { const [pending, setPending] = React.useState(false); - const {status, fetchStatus} = React.useContext(StatusContext); + const {status, fetchStatus, loaded} = React.useContext(StatusContext); - const {downloads} = status ? status : {downloads: null}; + // Until the first status poll lands there is no downloads state to show, only a disabled toggle. + const downloads = loaded ? status?.downloads : null; const wrolModeEnabled = useWROLMode(); const setDownloads = async (enable) => { @@ -1082,8 +1086,9 @@ export function DisableDownloadsToggle() { } const on = downloads && downloads['disabled'] === false && downloads['stopped'] === false; + const label = downloads === null ? 'Downloading' : on === true ? 'Downloading Enabled' : 'Downloading Disabled'; return ({ + ...jest.requireActual('../api'), + getConfigs: jest.fn(() => new Promise(() => { + })), +})); + +const unloadedStatus = {status: {}, loaded: false}; + +describe('DisableDownloadsToggle before status has loaded', () => { + test('is disabled and does not claim downloading is disabled', () => { + render(, {status: unloadedStatus}); + expect(screen.getByTestId('toggle')).toBeDisabled(); + expect(screen.queryByText('Downloading Disabled')).not.toBeInTheDocument(); + }); + + test('once loaded, reads the downloads state', () => { + render(, {status: {status: {downloads: {disabled: true, stopped: false}}, loaded: true}}); + expect(screen.getByText('Downloading Disabled')).toBeInTheDocument(); + expect(screen.getByTestId('toggle')).toBeEnabled(); + }); +}); + +describe('HotspotToggle before status has loaded', () => { + test('is disabled without claiming the hotspot is unsupported', () => { + render(, {status: unloadedStatus}); + expect(screen.getByTestId('toggle')).toBeDisabled(); + expect(screen.queryByTestId('subsystem-popup')).not.toBeInTheDocument(); + }); + + test('once loaded without a hotspot field, is unsupported', () => { + render(, {status: {status: {}, loaded: true}}); + expect(screen.getByTestId('toggle')).toBeDisabled(); + expect(screen.getByTestId('subsystem-popup')).toBeInTheDocument(); + }); +}); + +describe('UpgradeSegment before status has loaded', () => { + test('does not claim the WROLPi is up to date', () => { + render(, {status: unloadedStatus}); + expect(screen.queryByText(/up to date/)).not.toBeInTheDocument(); + }); + + test('once loaded with no update, says so', () => { + render(, {status: {status: {update_available: false, version: '1.0'}, loaded: true}}); + expect(screen.getByText(/up to date/)).toBeInTheDocument(); + }); +}); + +describe('SettingsPage before settings have loaded', () => { + test('shows a loader, not a form over blank values', () => { + const {container} = render(, {settings: {settings: {}, loaded: false, failed: false}}); + expect(container.querySelector('.wrolpi-loading')).toBeInTheDocument(); + expect(screen.queryByText(/Any changes will be written/)).not.toBeInTheDocument(); + }); + + test('shows an error when the settings could not be fetched', () => { + render(, {settings: {settings: {}, loaded: false, failed: true}}); + expect(screen.getByText('Unable to fetch settings')).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/admin/Settings.js b/app/src/components/admin/Settings.js index bceba925..be5e7ebe 100644 --- a/app/src/components/admin/Settings.js +++ b/app/src/components/admin/Settings.js @@ -101,8 +101,8 @@ export function ShutdownButton() { } -function UpgradeSegment() { - const {status, fetchStatus} = React.useContext(StatusContext); +export function UpgradeSegment() { + const {status, fetchStatus, loaded} = React.useContext(StatusContext); const dockerized = useDockerized(); const [upgrading, setUpgrading] = React.useState(false); const [checking, setChecking] = React.useState(false); @@ -144,6 +144,14 @@ function UpgradeSegment() { ; } + // Nothing is known until the first status poll lands; "up to date" is a claim about the server. + if (!loaded) { + return +
System Upgrade
+ + ; + } + // No update available if (!status?.update_available) { return @@ -325,7 +333,9 @@ export function SettingsPage() { const [isDevMode, setIsDevMode] = React.useState(false); // Get current settings from the API. - const {settings, saveSettings, fetchSettings} = React.useContext(SettingsContext); + const { + settings, saveSettings, fetchSettings, loaded: settingsLoaded, failed: settingsFailed, + } = React.useContext(SettingsContext); const {status} = React.useContext(StatusContext); // Used to track changes to the settings form between saves/loads. const [state, setState] = React.useState({}); @@ -463,7 +473,8 @@ export function SettingsPage() { React.useEffect(() => { console.debug('settings changed, replacing state...'); - setReady(settings ? true : undefined); + // true = show the form, false = still loading, undefined = could not fetch. + setReady(settingsLoaded ? true : settingsFailed ? undefined : false); setState({ // Only these settings can be changed on the SettingsPage. archive_destination: settings.archive_destination, @@ -490,7 +501,7 @@ export function SettingsPage() { playlists_destination: settings.playlists_destination, save_ffprobe_json: settings.save_ffprobe_json, }); - }, [JSON.stringify(settings)]); + }, [JSON.stringify(settings), settingsLoaded, settingsFailed]); const handleInputChange = async (name, value) => { setState({...state, [name]: value}); diff --git a/app/src/contexts/contexts.js b/app/src/contexts/contexts.js index 0e224ce7..619280a6 100644 --- a/app/src/contexts/contexts.js +++ b/app/src/contexts/contexts.js @@ -46,6 +46,8 @@ export const ThemeContext = React.createContext({ export const StatusContext = React.createContext({ status: {}, fetchStatus: null, + // True once the first status poll has landed. + loaded: false, }); export const SettingsContext = React.createContext({ @@ -53,6 +55,9 @@ export const SettingsContext = React.createContext({ fetchSettings: null, saveSettings: null, pending: false, + // True once settings have been fetched; `failed` when a fetch returned nothing before that. + loaded: false, + failed: false, }); // `useQuery` diff --git a/app/src/hooks/customHooks.js b/app/src/hooks/customHooks.js index 94c1c03f..af7f9384 100644 --- a/app/src/hooks/customHooks.js +++ b/app/src/hooks/customHooks.js @@ -1154,7 +1154,8 @@ export const useDownloads = () => { // Every hardware toggle works the same way: one field of the shared status object // says whether the subsystem is on, and starting/stopping it is a Controller call. -// `on` is null when the status is unknown, which the UI renders as unsupported. +// `on` is undefined until the status has loaded (unknown, the toggle is disabled), null once it +// has loaded without the field (unsupported on this server), and otherwise a boolean. const useSubsystemToggle = ({ statusField, onValue = 'on', @@ -1165,23 +1166,26 @@ const useSubsystemToggle = ({ errorDescription, refetchStatus = false, }) => { - const [on, setOn] = useState(null); - const {status, fetchStatus} = useContext(StatusContext); + const [on, setOn] = useState(undefined); + const {status, fetchStatus, loaded} = useContext(StatusContext); const value = status?.[statusField]; useEffect(() => { - if (value === onValue) { + if (!loaded) { + setOn(undefined); + } else if (value === onValue) { setOn(true); } else if (offValues.includes(value)) { setOn(false); } else { setOn(null); } - }, [value]); + }, [value, loaded]); const setSubsystem = async (enable) => { const previous = on; - setOn(null); + // Unknown while the call is in flight, not unsupported. + setOn(undefined); try { if (enable) { await start(); @@ -1331,8 +1335,16 @@ export const useSearchDirectories = (value) => { } export const useSettings = () => { + // `settings` is {} until the first fetch lands, and a consumer cannot tell that from real + // settings by looking at it. `loaded` is true once a fetch returned settings; `failed` is + // true when a fetch returned nothing before anything loaded. A failed refresh after a load + // keeps the last known good values and `loaded`. const [settings, setSettings] = useState({}); + const [loaded, setLoaded] = useState(false); + const [failed, setFailed] = useState(false); const [pending, setPending] = useState(false); + // Read by fetchSettings after an await, when the closed-over `loaded` may be stale. + const loadedRef = useRef(false); const fetchSettings = async () => { if (window.apiDown) { @@ -1343,9 +1355,17 @@ export const useSettings = () => { const newSettings = await getSettings(); if (newSettings) { setSettings(newSettings); + loadedRef.current = true; + setLoaded(true); + setFailed(false); + } else if (!loadedRef.current) { + setFailed(true); } } catch (e) { // Don't clear settings on error — keep the last known good values. + if (!loadedRef.current) { + setFailed(true); + } } finally { setPending(false); } @@ -1355,15 +1375,15 @@ export const useSettings = () => { fetchSettings(); }, []); - return {settings, pending, fetchSettings, saveSettings}; + return {settings, loaded, failed, pending, fetchSettings, saveSettings}; } export const useSettingsInterval = () => { - const {settings, pending, fetchSettings, saveSettings} = useSettings(); + const {settings, loaded, failed, pending, fetchSettings, saveSettings} = useSettings(); useRecurringTimeout(fetchSettings, 1000 * 10); - return {settings, pending, fetchSettings, saveSettings}; + return {settings, loaded, failed, pending, fetchSettings, saveSettings}; } export const useMediaDirectory = () => { @@ -1373,7 +1393,11 @@ export const useMediaDirectory = () => { } export const useStatus = () => { + // `status` is {} until the first poll lands, and a consumer cannot tell that from a server + // that reported nothing. `loaded` is true once a poll returned a real status; the API being + // down does not count. const [status, setStatus] = useState({}); + const [loaded, setLoaded] = useState(false); const fetchStatus = async () => { try { @@ -1398,6 +1422,9 @@ export const useStatus = () => { // Merge both responses - controller stats override app status for system info setStatus({...appStatus, ...controllerStats}); + if (!window.apiDown) { + setLoaded(true); + } } catch (e) { if (e instanceof ApiDownError) { // API is down, do not log this error. @@ -1417,15 +1444,15 @@ export const useStatus = () => { window.apiDown = false; }, []); - return {status, fetchStatus} + return {status, loaded, fetchStatus} } export const useStatusInterval = () => { - const {status, fetchStatus} = useStatus(); + const {status, loaded, fetchStatus} = useStatus(); useRecurringTimeout(fetchStatus, 1000 * 3); - return {status, fetchStatus}; + return {status, loaded, fetchStatus}; } export const StatusProvider = (props) => { diff --git a/app/src/hooks/customHooks.test.js b/app/src/hooks/customHooks.test.js index 987467af..9f26d925 100644 --- a/app/src/hooks/customHooks.test.js +++ b/app/src/hooks/customHooks.test.js @@ -2,9 +2,13 @@ import React from 'react'; import {act, renderHook} from '@testing-library/react'; import {render, screen} from '../test-utils'; import { - usePages, useDriveTemperature, useDriveHealth, useSearchChannels, useStatistics, useVideoExtras, + usePages, useDriveTemperature, useDriveHealth, useSearchChannels, useSettings, useStatistics, useStatus, + useVideoExtras, } from './customHooks'; -import {getStatistics, getVideoComments, getVideoDescription, searchChannels} from '../api'; +import { + ApiDownError, getSettings, getStatistics, getStatus, getVideoComments, getVideoDescription, searchChannels, +} from '../api'; +import {getControllerStats} from '../api/controller'; import {QueryContext, StatusContext} from '../contexts/contexts'; import {Paginator} from '../components/Common'; @@ -42,6 +46,13 @@ jest.mock('../api', () => ({ getVideoDescription: jest.fn(), searchChannels: jest.fn(), getStatistics: jest.fn(), + getStatus: jest.fn(), + getSettings: jest.fn(), +})); + +jest.mock('../api/controller', () => ({ + ...jest.requireActual('../api/controller'), + getControllerStats: jest.fn(), })); // Mock Media so Paginator renders both mobile + tablet variants synchronously. @@ -580,3 +591,81 @@ describe('useStatistics', () => { } }); }); + +describe('useStatus loaded', () => { + // `loaded` is false until the first poll returns a real status, so consumers can tell "not + // polled yet" from "the server reported nothing". + beforeEach(() => { + getStatus.mockReset(); + getControllerStats.mockReset(); + getControllerStats.mockResolvedValue({}); + }); + + test('is false before any poll', () => { + const {result} = renderHook(() => useStatus()); + expect(result.current.loaded).toBe(false); + expect(result.current.status).toEqual({}); + }); + + test('is true once a poll returned a status', async () => { + getStatus.mockResolvedValue({version: '1.0'}); + const {result} = renderHook(() => useStatus()); + await act(async () => { + await result.current.fetchStatus(); + }); + expect(result.current.loaded).toBe(true); + expect(result.current.status.version).toBe('1.0'); + }); + + test('stays false while the API is down', async () => { + getStatus.mockRejectedValue(new ApiDownError('down')); + const {result} = renderHook(() => useStatus()); + await act(async () => { + await result.current.fetchStatus(); + }); + expect(result.current.loaded).toBe(false); + }); +}); + +describe('useSettings loaded', () => { + beforeEach(() => { + getSettings.mockReset(); + window.apiDown = false; + }); + + test('is false before the fetch returns, then true', async () => { + let resolve; + getSettings.mockReturnValue(new Promise(res => resolve = res)); + const {result} = renderHook(() => useSettings()); + expect(result.current.loaded).toBe(false); + expect(result.current.failed).toBe(false); + await act(async () => { + resolve({media_directory: '/media/wrolpi'}); + }); + expect(result.current.loaded).toBe(true); + expect(result.current.settings.media_directory).toBe('/media/wrolpi'); + }); + + test('a fetch that returns nothing before anything loaded marks failed', async () => { + getSettings.mockResolvedValue(undefined); + const {result} = renderHook(() => useSettings()); + await act(async () => { + }); + expect(result.current.loaded).toBe(false); + expect(result.current.failed).toBe(true); + }); + + test('a failed refresh after a successful load keeps loaded and the last settings', async () => { + getSettings.mockResolvedValueOnce({media_directory: '/media/wrolpi'}); + const {result} = renderHook(() => useSettings()); + await act(async () => { + }); + getSettings.mockRejectedValue(new Error('boom')); + await act(async () => { + await result.current.fetchSettings(); + }); + expect(result.current.loaded).toBe(true); + expect(result.current.failed).toBe(false); + expect(result.current.settings.media_directory).toBe('/media/wrolpi'); + }); +}); diff --git a/app/src/hooks/useSubsystemToggle.test.js b/app/src/hooks/useSubsystemToggle.test.js index ec8c9be6..f84d5177 100644 --- a/app/src/hooks/useSubsystemToggle.test.js +++ b/app/src/hooks/useSubsystemToggle.test.js @@ -23,7 +23,7 @@ const controllerApi = require('../api/controller'); const fetchStatus = jest.fn(); const makeWrapper = (status) => ({children}) => ( - {children} + {children} ); beforeEach(() => { diff --git a/app/src/test-fixtures.js b/app/src/test-fixtures.js index e01259da..f8f8242f 100644 --- a/app/src/test-fixtures.js +++ b/app/src/test-fixtures.js @@ -188,9 +188,11 @@ export const themeContextFixture = (overrides = {}) => ({ ...overrides, }); +// `loaded` says the first poll has landed; the fixtures carry real values, so they are loaded. export const statusContextFixture = (overrides = {}) => ({ status: statusFixture(), fetchStatus: () => Promise.resolve(), + loaded: true, ...overrides, }); @@ -199,6 +201,8 @@ export const settingsContextFixture = (overrides = {}) => ({ fetchSettings: () => Promise.resolve(), saveSettings: () => Promise.resolve(), pending: false, + loaded: true, + failed: false, ...overrides, });