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, });