From a33083cc27e5e46f46a0e18d04bf070768c60676 Mon Sep 17 00:00:00 2001 From: Roland Date: Sat, 26 Sep 2026 17:01:47 -0600 Subject: [PATCH] app: Flasher saved configs and Statistics distinguish pending from empty The Flasher's saved configurations started as [] with no loading flag, and the Saved Firmwares tab is the default tab, so every load painted "No saved configurations yet." until the fetch returned. The api helper returned [] on a non-OK response too, so a failed fetch read as an empty list. useStatistics used useState() with no initial value, so the Statistics page's first render saw undefined and flashed "Failed to fetch statistics" for one frame before the effect set {} and the spinner appeared. Invariants now enforced and tested: * null = pending, undefined = fetch failed, [] / object = loaded. The api helper returns undefined (and toasts) on a non-OK response. * The Saved Firmwares tab shows a placeholder while pending, an error on failure, and "No saved configurations yet." only once the server confirmed there are none. Only the newest configs request may write state, since save and delete refetch. * The Statistics page shows its loader while pending and its error only for a failed fetch. Co-Authored-By: Claude Fable 5.1 --- app/src/api.js | 4 +- app/src/components/Apps.js | 5 ++- app/src/components/Flasher.js | 31 +++++++++++--- app/src/components/Flasher.test.js | 25 ++++++++++++ app/src/components/StatisticsPage.test.js | 49 ++++++++++++++++++++++ app/src/hooks/customHooks.js | 8 ++-- app/src/hooks/customHooks.test.js | 50 ++++++++++++++++++++++- 7 files changed, 158 insertions(+), 14 deletions(-) create mode 100644 app/src/components/StatisticsPage.test.js diff --git a/app/src/api.js b/app/src/api.js index f80d9ced..57f5d255 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -1361,7 +1361,9 @@ export async function getFlasherConfigs() { const data = await response.json(); return data['configurations'] || []; } - return []; + // Undefined, not [], so the page can tell a failed fetch from an empty list. + const message = await getErrorMessage(response, 'Could not get saved firmware configurations'); + toast({type: 'error', title: 'Flasher Error', description: message, time: 5000}); } export async function saveFlasherConfig(name, files, eraseAll) { diff --git a/app/src/components/Apps.js b/app/src/components/Apps.js index 3360afac..9e547f3c 100644 --- a/app/src/components/Apps.js +++ b/app/src/components/Apps.js @@ -14,7 +14,8 @@ import {useStatistics} from "../hooks/customHooks"; import {CalculatorsPage} from "./Calculators"; import {BookmarksPage} from "./Bookmarks"; -function StatisticsPage() { +// `statistics` follows useStatistics: null = pending, undefined = fetch failed. +export function StatisticsPage() { useTitle('Statistics'); const {statistics} = useStatistics(); @@ -26,7 +27,7 @@ function StatisticsPage() { } - if (statistics['global_statistics']) { + if (statistics && statistics['global_statistics']) { let {global_statistics, file_statistics} = statistics; const { archive_count, diff --git a/app/src/components/Flasher.js b/app/src/components/Flasher.js index 9e6f1111..304c4374 100644 --- a/app/src/components/Flasher.js +++ b/app/src/components/Flasher.js @@ -3,7 +3,7 @@ import {Route, Routes} from "react-router"; import {ESPLoader, Transport} from "esptool-js"; import {useDropzone} from "react-dropzone"; import _ from "lodash"; -import {encodeMediaPath, humanFileSize, PageContainer, useTitle} from "./Common"; +import {encodeMediaPath, ErrorMessage, humanFileSize, PageContainer, useTitle} from "./Common"; import { Button, Checkbox, @@ -17,6 +17,7 @@ import { Message, Modal, Panel, + Placeholder, Progress, Select, Stack, @@ -297,8 +298,12 @@ export function FlasherPage() { // Which firmware-source tab is active (controlled so a file drop can switch to "Add from computer"). const [activeTab, setActiveTab] = useState(TAB_SAVED); - // Saved firmware configurations (flasher.yaml). - const [savedConfigs, setSavedConfigs] = useState([]); + // Saved firmware configurations (flasher.yaml). null = pending, undefined = fetch failed, + // [] = none saved; the "No saved configurations yet." message is only for the last. + const [savedConfigs, setSavedConfigs] = useState(null); + // Only the newest configs request may write state: a save or delete refetches while the mount + // request may still be in flight. + const savedConfigsGen = useRef(0); const [saveModalOpen, setSaveModalOpen] = useState(false); const [saveName, setSaveName] = useState(''); @@ -424,7 +429,19 @@ export function FlasherPage() { // Saved firmware configurations. const fetchSavedConfigs = React.useCallback(async () => { - setSavedConfigs(await getFlasherConfigs()); + const gen = ++savedConfigsGen.current; + try { + const configs = await getFlasherConfigs(); + if (gen === savedConfigsGen.current) { + // The api helper returns undefined on a non-OK response. + setSavedConfigs(configs ?? undefined); + } + } catch (e) { + console.error(e); + if (gen === savedConfigsGen.current) { + setSavedConfigs(undefined); + } + } }, []); useEffect(() => { @@ -812,7 +829,9 @@ export function FlasherPage() { Configure the firmware files and offsets, then save them as a named set to re-flash later (e.g. a Meshtastic T-Deck: firmware at 0x0, littlefs at 0xc90000).

- {savedConfigs.length > 0 + {savedConfigs === null && } + {savedConfigs === undefined && Could not fetch saved configurations.} + {Array.isArray(savedConfigs) && (savedConfigs.length > 0 ? {savedConfigs.map((configuration, index) =>
handleDeleteConfig(configuration.name)}/>
)} - :

No saved configurations yet.

} + :

No saved configurations yet.

)} ; return
diff --git a/app/src/components/Flasher.test.js b/app/src/components/Flasher.test.js index 34ca2301..2873c8e7 100644 --- a/app/src/components/Flasher.test.js +++ b/app/src/components/Flasher.test.js @@ -265,6 +265,31 @@ describe('FlasherPage', () => { } }); + // The Saved Firmwares tab is the default tab. "No saved configurations yet." may only appear once the + // server confirmed there are none; a pending or failed fetch must not read as an empty list. + it('shows a placeholder, not "no saved configurations", while the configs load', () => { + Object.defineProperty(global.navigator, 'serial', {value: {}, configurable: true}); + getFlasherConfigs.mockReturnValue(new Promise(() => { + })); + const {container} = render(); + expect(container.querySelector('.wrolpi-placeholder')).toBeInTheDocument(); + expect(screen.queryAllByText('No saved configurations yet.')).toHaveLength(0); + }); + + it('shows an error, not "no saved configurations", when the configs fetch failed', async () => { + Object.defineProperty(global.navigator, 'serial', {value: {}, configurable: true}); + getFlasherConfigs.mockResolvedValue(undefined); + render(); + expect((await screen.findAllByText(/Could not fetch saved configurations/)).length).toBeGreaterThan(0); + expect(screen.queryAllByText('No saved configurations yet.')).toHaveLength(0); + }); + + it('shows "no saved configurations" once the server confirmed there are none', async () => { + Object.defineProperty(global.navigator, 'serial', {value: {}, configurable: true}); + render(); + expect((await screen.findAllByText('No saved configurations yet.')).length).toBeGreaterThan(0); + }); + // PageContainer renders both the mobile and desktop breakpoints (fresnel Media), so each label appears twice. it('warns when Web Serial is unavailable', () => { delete global.navigator.serial; diff --git a/app/src/components/StatisticsPage.test.js b/app/src/components/StatisticsPage.test.js new file mode 100644 index 00000000..8aaae49e --- /dev/null +++ b/app/src/components/StatisticsPage.test.js @@ -0,0 +1,49 @@ +import React from 'react'; +import {act, render, screen} from '../test-utils'; +import {StatisticsPage} from './Apps'; +import {getStatistics} from '../api'; + +// What the Statistics page shows while the numbers are still loading, when the fetch failed, and +// once they arrive. "Failed to fetch statistics" may only appear for a failed fetch. + +jest.mock('../api', () => ({ + ...jest.requireActual('../api'), + getStatistics: jest.fn(), +})); + +const FAILED = /Failed to fetch statistics/; +const STATS = { + global_statistics: {db_size: 1024}, + file_statistics: { + archive_count: 1, audio_count: 2, ebook_count: 3, image_count: 4, pdf_count: 5, total_count: 21, + video_count: 6, zip_count: 0, total_size: 4096, tagged_files: 7, tagged_zims: 0, tags_count: 2, + }, +}; + +beforeEach(() => getStatistics.mockReset()); + +test('pending: shows a loader, not the error', () => { + getStatistics.mockReturnValue(new Promise(() => { + })); + const {container} = render(); + expect(container.querySelector('.wrolpi-loading')).toBeInTheDocument(); + expect(screen.queryByText(FAILED)).not.toBeInTheDocument(); +}); + +test('failed: shows the error', async () => { + // The api helper toasts and returns undefined on a non-OK response. + getStatistics.mockResolvedValue(undefined); + render(); + await act(async () => { + }); + expect(screen.getByText(FAILED)).toBeInTheDocument(); +}); + +test('loaded: shows the numbers', async () => { + getStatistics.mockResolvedValue(STATS); + render(); + await act(async () => { + }); + expect(screen.getByText('All Files')).toBeInTheDocument(); + expect(screen.queryByText(FAILED)).not.toBeInTheDocument(); +}); diff --git a/app/src/hooks/customHooks.js b/app/src/hooks/customHooks.js index 8836aab2..94c1c03f 100644 --- a/app/src/hooks/customHooks.js +++ b/app/src/hooks/customHooks.js @@ -1697,12 +1697,15 @@ export const collectLocations = (inventories) => { }; export const useStatistics = () => { - const [statistics, setStatistics] = useState(); + // null = pending, undefined = fetch failed (the api helper returns undefined on a non-OK + // response), an object = loaded. Never undefined on the first render: the page shows its + // error for undefined. + const [statistics, setStatistics] = useState(null); const fetchFileStatistics = async () => { try { const s = await getStatistics(); - setStatistics(s); + setStatistics(s ?? undefined); } catch (e) { console.error(e); setStatistics(undefined); @@ -1710,7 +1713,6 @@ export const useStatistics = () => { } useEffect(() => { - setStatistics({}); fetchFileStatistics(); }, []); diff --git a/app/src/hooks/customHooks.test.js b/app/src/hooks/customHooks.test.js index 0df9eaab..987467af 100644 --- a/app/src/hooks/customHooks.test.js +++ b/app/src/hooks/customHooks.test.js @@ -1,8 +1,10 @@ import React from 'react'; import {act, renderHook} from '@testing-library/react'; import {render, screen} from '../test-utils'; -import {usePages, useDriveTemperature, useDriveHealth, useSearchChannels, useVideoExtras} from './customHooks'; -import {getVideoComments, getVideoDescription, searchChannels} from '../api'; +import { + usePages, useDriveTemperature, useDriveHealth, useSearchChannels, useStatistics, useVideoExtras, +} from './customHooks'; +import {getStatistics, getVideoComments, getVideoDescription, searchChannels} from '../api'; import {QueryContext, StatusContext} from '../contexts/contexts'; import {Paginator} from '../components/Common'; @@ -39,6 +41,7 @@ jest.mock('../api', () => ({ getVideoComments: jest.fn(), getVideoDescription: jest.fn(), searchChannels: jest.fn(), + getStatistics: jest.fn(), })); // Mock Media so Paginator renders both mobile + tablet variants synchronously. @@ -534,3 +537,46 @@ describe('useSearchChannels', () => { expect(result.current.loading).toBe(false); }); }); + +describe('useStatistics', () => { + // null = pending, undefined = fetch failed, an object = loaded. The Statistics page shows its + // error only for undefined, so the first render must not be undefined. + beforeEach(() => getStatistics.mockReset()); + + test('is pending (null) on the first render', () => { + getStatistics.mockReturnValue(new Promise(() => { + })); + const {result} = renderHook(() => useStatistics()); + expect(result.current.statistics).toBeNull(); + }); + + test('resolves to the statistics', async () => { + getStatistics.mockResolvedValue({global_statistics: {}, file_statistics: {}}); + const {result} = renderHook(() => useStatistics()); + await act(async () => { + }); + expect(result.current.statistics).toEqual({global_statistics: {}, file_statistics: {}}); + }); + + test('a non-OK response yields undefined', async () => { + getStatistics.mockResolvedValue(undefined); + const {result} = renderHook(() => useStatistics()); + await act(async () => { + }); + expect(result.current.statistics).toBeUndefined(); + }); + + test('a thrown request yields undefined', async () => { + getStatistics.mockRejectedValue(new Error('boom')); + const spy = jest.spyOn(console, 'error').mockImplementation(() => { + }); + try { + const {result} = renderHook(() => useStatistics()); + await act(async () => { + }); + expect(result.current.statistics).toBeUndefined(); + } finally { + spy.mockRestore(); + } + }); +});