diff --git a/app/src/api.errorAsEmpty.test.js b/app/src/api.errorAsEmpty.test.js new file mode 100644 index 00000000..cb7c693c --- /dev/null +++ b/app/src/api.errorAsEmpty.test.js @@ -0,0 +1,84 @@ +import { + flasherSearch, getConfigBackups, getDownloaders, getInventoryBackups, getMapSearchStatus, searchMap, +} from './api'; + +// A helper that fails must say so. These used to turn a non-OK response into an empty value +// ([], {dates: []}, {results: []}, ...), so every consumer rendered "nothing here" for a server +// error. They now return undefined; an empty value only ever comes from the server. + +jest.mock('./components/ui', () => ({ + ...jest.requireActual('./components/ui'), + toast: jest.fn(), +})); + +const response = (ok, body = {}) => { + const r = { + ok, + status: ok ? 200 : 500, + json: async () => body, + text: async () => JSON.stringify(body), + }; + r.clone = () => r; + return r; +}; + +let originalFetch; +beforeEach(() => { + originalFetch = global.fetch; +}); +afterEach(() => { + global.fetch = originalFetch; +}); + +const failWith = () => { + global.fetch = jest.fn(() => Promise.resolve(response(false, {error: 'boom'}))); +}; +const succeedWith = (body) => { + global.fetch = jest.fn(() => Promise.resolve(response(true, body))); +}; + +describe('on a non-OK response, the helper returns undefined', () => { + test.each([ + ['getConfigBackups', () => getConfigBackups('wrolpi.yaml')], + ['getInventoryBackups', () => getInventoryBackups('food')], + ['getDownloaders', () => getDownloaders()], + ['searchMap', () => searchMap('camp')], + ['getMapSearchStatus', () => getMapSearchStatus()], + ['flasherSearch', () => flasherSearch(null, null)], + ])('%s', async (_name, call) => { + failWith(); + expect(await call()).toBeUndefined(); + }); +}); + +describe('a confirmed empty result is still empty', () => { + test('getConfigBackups', async () => { + succeedWith({dates: []}); + expect(await getConfigBackups('wrolpi.yaml')).toEqual({dates: []}); + }); + + test('getInventoryBackups', async () => { + succeedWith({dates: []}); + expect(await getInventoryBackups('food')).toEqual([]); + }); + + test('getDownloaders', async () => { + succeedWith({downloaders: []}); + expect(await getDownloaders()).toEqual({downloaders: []}); + }); + + test('searchMap', async () => { + succeedWith({results: [], total: 0}); + expect(await searchMap('camp')).toEqual({results: [], total: 0}); + }); + + test('getMapSearchStatus', async () => { + succeedWith({indexed: [], missing: []}); + expect(await getMapSearchStatus()).toEqual({indexed: [], missing: []}); + }); + + test('flasherSearch', async () => { + succeedWith({file_groups: [], totals: {file_groups: 0}}); + expect(await flasherSearch(null, null)).toEqual([[], 0]); + }); +}); diff --git a/app/src/api.js b/app/src/api.js index 57f5d255..907d6012 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -522,7 +522,7 @@ export async function getConfigBackups(fileName) { if (response.ok) { return await response.json(); } - return {dates: []}; + // Undefined, not {dates: []}, so the modal can tell a failed fetch from having no backups. } export async function postConfigBackupPreview(fileName, backupDate, mode) { @@ -720,7 +720,7 @@ export async function getInventoryBackups(slug) { if (response.ok) { return (await response.json())['dates']; } - return []; + // Undefined, not [], so the modal can tell a failed fetch from having no backups. } export async function postInventoryRestorePreview(slug, backupDate, mode) { @@ -1259,12 +1259,15 @@ export async function startDownloads() { } export async function getDownloaders() { + // Not toasting because this will happen often. Undefined on any failure, not an empty list, so + // the form can say the downloaders could not be fetched. try { const response = await apiGet(`${API_URI}/downloaders`); - // Not toasting because this will happen often. - return await response.json(); + if (response.ok) { + return await response.json(); + } } catch (e) { - return {downloaders: []}; + // The API is down; reported as undefined below. } } @@ -1351,7 +1354,7 @@ export async function flasherSearch(chip, path) { } const message = await getErrorMessage(response, 'Cannot search firmware. See server logs.'); toast({type: 'error', title: 'Unable to search firmware', description: message, time: 5000}); - return [null, null]; + // Undefined, not [null, null], so the picker can tell a failed search from finding nothing. } export async function getFlasherConfigs() { @@ -1658,7 +1661,8 @@ export async function searchMap(query, limit = 12, offset = 0, lat = null, lon = } const response = await apiGet(url); if (!response.ok) { - return {results: [], total: 0}; + // Undefined, not {results: []}, so a failed search is not shown as finding nothing. + return undefined; } return await response.json(); } @@ -1666,7 +1670,8 @@ export async function searchMap(query, limit = 12, offset = 0, lat = null, lon = export async function getMapSearchStatus() { const response = await apiGet(`${API_URI}/map/search/status`); if (!response.ok) { - return {indexed: [], missing: []}; + // Undefined, not {indexed: [], missing: []}, which read as "no maps are installed". + return undefined; } return await response.json(); } diff --git a/app/src/components/Download.js b/app/src/components/Download.js index f8f4bc5e..8f115071 100644 --- a/app/src/components/Download.js +++ b/app/src/components/Download.js @@ -247,28 +247,38 @@ export function DownloadFrequencySelector({ } export function DownloaderSelector({form, name = 'sub_downloader', path = 'sub_downloader'}) { - const [downloaders, setDownloaders] = React.useState([]); + // null = pending, undefined = could not be fetched, a list = loaded. Until loaded, the select is + // disabled and says why, rather than sitting empty as though there were no downloaders. + const [downloaders, setDownloaders] = React.useState(null); const fetchDownloaders = async () => { - let {downloaders: downloaders_} = await getDownloaders(); - downloaders_ = downloaders_.map((i) => { + const result = await getDownloaders(); + if (!result || !Array.isArray(result.downloaders)) { + setDownloaders(undefined); + return; + } + setDownloaders(result.downloaders.map((i) => { return {key: i.name, text: i.pretty_name || i.name, value: i.name} - }) - setDownloaders(downloaders_); + })); } React.useEffect(() => { fetchDownloaders(); }, []); + const placeholder = downloaders === null ? 'Loading downloaders…' + : downloaders === undefined ? 'Could not fetch downloaders' + : 'Select a downloader'; + return } diff --git a/app/src/components/DownloaderSelector.test.js b/app/src/components/DownloaderSelector.test.js new file mode 100644 index 00000000..7bc65ee5 --- /dev/null +++ b/app/src/components/DownloaderSelector.test.js @@ -0,0 +1,43 @@ +import React from 'react'; +import {act, render, screen} from '../test-utils'; +import {createTestForm} from '../test-utils'; +import {DownloaderSelector} from './Download'; +import {getDownloaders} from '../api'; + +// The RSS form's Downloader select is filled from the API. Until the list arrives, or if it could +// not be fetched, the select must say so rather than sit empty as though there were no downloaders. + +jest.mock('../api', () => ({ + ...jest.requireActual('../api'), + getDownloaders: jest.fn(), +})); + +const makeForm = () => createTestForm({}, { + overrides: {getSelectionProps: () => [{value: null, onChange: jest.fn(), disabled: false, error: null}]}, +}); + +beforeEach(() => getDownloaders.mockReset()); + +test('pending: the select says it is loading and is disabled', () => { + getDownloaders.mockReturnValue(new Promise(() => { + })); + render(); + const input = screen.getByPlaceholderText('Loading downloaders…'); + expect(input).toBeDisabled(); +}); + +test('failed: the select says the downloaders could not be fetched', async () => { + getDownloaders.mockResolvedValue(undefined); + render(); + await act(async () => { + }); + expect(screen.getByPlaceholderText('Could not fetch downloaders')).toBeDisabled(); +}); + +test('loaded: the select offers the downloaders', async () => { + getDownloaders.mockResolvedValue({downloaders: [{name: 'video', pretty_name: 'Videos'}]}); + render(); + await act(async () => { + }); + expect(screen.getByPlaceholderText('Select a downloader')).toBeEnabled(); +}); diff --git a/app/src/components/Flasher.js b/app/src/components/Flasher.js index 304c4374..542428cf 100644 --- a/app/src/components/Flasher.js +++ b/app/src/components/Flasher.js @@ -270,7 +270,8 @@ export function FlasherPage() { // Media-directory firmware picker. const [mediaFilter, setMediaFilter] = useState(''); - const [mediaResults, setMediaResults] = useState([]); + // null = not searched yet, undefined = the search failed, a list = the server's results. + const [mediaResults, setMediaResults] = useState(null); const [mediaLoading, setMediaLoading] = useState(false); // When set (e.g. "ESP32-S2"), the picker only shows firmware for that chip (detected from the device). const [deviceChip, setDeviceChip] = useState(null); @@ -393,14 +394,15 @@ export function FlasherPage() { const fetchMediaFirmware = React.useCallback(async (pathFilter, chip) => { setMediaLoading(true); try { - const [fileGroups] = await flasherSearch(chip || null, pathFilter || null); - setMediaResults(fileGroups || []); + const result = await flasherSearch(chip || null, pathFilter || null); + // The api helper returns undefined (and toasts) on a non-OK response. + setMediaResults(result ? result[0] || [] : undefined); } catch (e) { // Surface connectivity errors instead of showing the empty "no firmware found" state, which would // be indistinguishable from genuinely having no firmware. setError(e && e.message ? e.message : String(e)); setBootHint(false); - setMediaResults([]); + setMediaResults(undefined); } finally { setMediaLoading(false); } @@ -767,8 +769,10 @@ export function FlasherPage() { onChange={(e) => handleMediaFilterChange(e.currentTarget.value)} />
- {mediaLoading + {mediaLoading || mediaResults === null ? + : mediaResults === undefined + ? Could not search firmware. : (mediaResults.length === 0 ?

No .bin firmware found diff --git a/app/src/components/Flasher.test.js b/app/src/components/Flasher.test.js index 2873c8e7..7d69b48d 100644 --- a/app/src/components/Flasher.test.js +++ b/app/src/components/Flasher.test.js @@ -290,6 +290,18 @@ describe('FlasherPage', () => { expect((await screen.findAllByText('No saved configurations yet.')).length).toBeGreaterThan(0); }); + // "No .bin firmware found" may only appear once the server searched and found none. + it('shows an error, not "no firmware found", when the firmware search failed', async () => { + Object.defineProperty(global.navigator, 'serial', {value: {}, configurable: true}); + // The api helper returns undefined on a non-OK response. + flasherSearch.mockResolvedValue(undefined); + render(); + await waitFor(() => expect(flasherSearch).toHaveBeenCalled()); + switchTab('Choose from your WROLPi'); + expect((await screen.findAllByText(/Could not search firmware/)).length).toBeGreaterThan(0); + expect(screen.queryAllByText(/firmware found/)).toHaveLength(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/MapSearchView.js b/app/src/components/MapSearchView.js index 6df499ff..bd9ed8c8 100644 --- a/app/src/components/MapSearchView.js +++ b/app/src/components/MapSearchView.js @@ -2,7 +2,7 @@ import React, {useEffect, useState} from "react"; import {Link, useNavigate} from "react-router"; import {getMapSearchStatus, searchMap} from "../api"; import {Card, CardGroup, Header, Loading, Panel} from "./ui"; -import {Paginator} from "./Common"; +import {ErrorMessage, Paginator} from "./Common"; import {useLatestRequest, usePages} from "../hooks/customHooks"; import {QueryContext} from "../contexts/contexts"; import {MAP_VIEWER_URI} from "./Vars"; @@ -68,16 +68,24 @@ export function MapSearchView() { const {offset, limit, activePage, setPage, totalPages, setTotal} = usePages(12); // useLatestRequest discards stale responses and catches errors, so rapid pagination // can't show out-of-order results and a network failure can't leave the spinner stuck. - const {data, sendRequest, loading} = useLatestRequest(300); + // Loading from the first render: a query of two or more characters always sends a request (below), + // so this settles, and the shorter-query branches render before the loading check. + const {data, sendRequest, loading} = useLatestRequest(300, true); + // null = unknown (pending, or the status could not be fetched), otherwise whether any map is + // installed. Only a real status may claim that no maps are installed. const [hasMaps, setHasMaps] = useState(null); useEffect(() => { let cancelled = false; (async () => { - const status = await getMapSearchStatus(); - if (!cancelled) { - const installed = (status.indexed || []).length + (status.missing || []).length; - setHasMaps(installed > 0); + try { + const status = await getMapSearchStatus(); + if (!cancelled && status) { + const installed = (status.indexed || []).length + (status.missing || []).length; + setHasMaps(installed > 0); + } + } catch (e) { + console.error(e); } })(); return () => { @@ -106,12 +114,22 @@ export function MapSearchView() { ; } + if (!searchStr) { + return

Enter a search query to find map locations.
; + } + + if (searchStr.length < 2) { + return
Enter at least two characters to find map locations.
; + } + if (loading) { return ; } - if (!searchStr) { - return
Enter a search query to find map locations.
; + // Settled with no data: the request failed (the api helper returns undefined on a non-OK + // response, and useLatestRequest leaves null when it throws). + if (data == null) { + return Could not search the map.; } if (results.length === 0) { diff --git a/app/src/components/MapSearchView.test.js b/app/src/components/MapSearchView.test.js new file mode 100644 index 00000000..01e02884 --- /dev/null +++ b/app/src/components/MapSearchView.test.js @@ -0,0 +1,61 @@ +import React from 'react'; +import {render, screen} from '../test-utils'; +import {MapSearchView} from './MapSearchView'; +import {getMapSearchStatus, searchMap} from '../api'; + +// The map search tab says "No map locations found" only when the server searched and found none, +// and "No maps are installed" only when the server said so. A failed request is an error. + +jest.mock('../api', () => ({ + ...jest.requireActual('../api'), + getMapSearchStatus: jest.fn(), + searchMap: jest.fn(), +})); + +const NOT_FOUND = /No map locations found/; +const NO_MAPS = /No maps are installed/; +const FAILED = /Could not search the map/; + +const renderAt = (q) => render(, {query: {searchParams: new URLSearchParams({q})}}); + +beforeEach(() => { + getMapSearchStatus.mockReset(); + searchMap.mockReset(); + getMapSearchStatus.mockResolvedValue({indexed: ['utah'], missing: []}); +}); + +test('found nothing: says so', async () => { + searchMap.mockResolvedValue({results: [], total: 0}); + renderAt('camp'); + expect(await screen.findByText(NOT_FOUND)).toBeInTheDocument(); +}); + +test('a failed search shows an error, not "No map locations found"', async () => { + searchMap.mockResolvedValue(undefined); + renderAt('camp'); + expect(await screen.findByText(FAILED)).toBeInTheDocument(); + expect(screen.queryByText(NOT_FOUND)).not.toBeInTheDocument(); +}); + +test('a failed status check does not claim no maps are installed', async () => { + getMapSearchStatus.mockResolvedValue(undefined); + searchMap.mockResolvedValue({results: [], total: 0}); + renderAt('camp'); + expect(await screen.findByText(NOT_FOUND)).toBeInTheDocument(); + expect(screen.queryByText(NO_MAPS)).not.toBeInTheDocument(); +}); + +test('a thrown status check does not claim no maps are installed', async () => { + getMapSearchStatus.mockRejectedValue(new Error('down')); + searchMap.mockResolvedValue({results: [], total: 0}); + renderAt('camp'); + expect(await screen.findByText(NOT_FOUND)).toBeInTheDocument(); + expect(screen.queryByText(NO_MAPS)).not.toBeInTheDocument(); +}); + +test('the server saying no maps are installed is shown', async () => { + getMapSearchStatus.mockResolvedValue({indexed: [], missing: []}); + searchMap.mockResolvedValue({results: [], total: 0}); + renderAt('camp'); + expect(await screen.findByText(NO_MAPS)).toBeInTheDocument(); +}); diff --git a/app/src/components/MapViewer.js b/app/src/components/MapViewer.js index f00b519d..888bb78a 100644 --- a/app/src/components/MapViewer.js +++ b/app/src/components/MapViewer.js @@ -421,10 +421,13 @@ function MapSearch({map}) { lon = center.lng; } + // A failed search (undefined) shows no dropdown, same as finding nothing: this is an + // as-you-type suggestion list, not a results page. searchMap(q, 8, 0, lat, lon).then(data => { - setResults(data.results || []); - setShowResults((data.results || []).length > 0); - }); + const found = data?.results || []; + setResults(found); + setShowResults(found.length > 0); + }).catch(e => console.error(e)); }, [map]); const handleInput = useCallback((e) => { diff --git a/app/src/components/admin/ConfigBackups.test.js b/app/src/components/admin/ConfigBackups.test.js new file mode 100644 index 00000000..6dd8a4ad --- /dev/null +++ b/app/src/components/admin/ConfigBackups.test.js @@ -0,0 +1,43 @@ +import React from 'react'; +import {render, screen} from '../../test-utils'; +import {BackupsModal} from './Configs'; +import {getConfigBackups} from '../../api'; + +// The config backups modal says "No backups available" only once the server confirmed there are +// none; a failed fetch is an error, and a thrown one must not leave the loader up forever. + +jest.mock('../../api', () => ({ + ...jest.requireActual('../../api'), + getConfigBackups: jest.fn(), + postConfigBackupPreview: jest.fn(() => new Promise(() => { + })), +})); + +const NONE = /No backups available/; +const FAILED = /Could not fetch backups/; + +const renderModal = () => render( + , +); + +beforeEach(() => getConfigBackups.mockReset()); + +test('empty: says there are no backups', async () => { + getConfigBackups.mockResolvedValue({dates: []}); + renderModal(); + expect(await screen.findByText(NONE)).toBeInTheDocument(); +}); + +test('failed: shows an error, not "No backups available"', async () => { + getConfigBackups.mockResolvedValue(undefined); + renderModal(); + expect(await screen.findByText(FAILED)).toBeInTheDocument(); + expect(screen.queryByText(NONE)).not.toBeInTheDocument(); +}); + +test('thrown: shows an error, not a loader forever', async () => { + getConfigBackups.mockRejectedValue(new Error('down')); + renderModal(); + expect(await screen.findByText(FAILED)).toBeInTheDocument(); + expect(screen.queryByText(/Loading backups/)).not.toBeInTheDocument(); +}); diff --git a/app/src/components/admin/Configs.js b/app/src/components/admin/Configs.js index 106921eb..98cbe8ae 100644 --- a/app/src/components/admin/Configs.js +++ b/app/src/components/admin/Configs.js @@ -1,4 +1,4 @@ -import {APIButton} from "../Common"; +import {APIButton, ErrorMessage} from "../Common"; import {Button, Header, Icon, IconButton, Loader, Loading, Modal, Placeholder, Table, Tooltip} from "../ui"; import React, {useEffect, useState} from "react"; import {getConfigText, getConfigBackups, postConfigBackupImport, postConfigBackupPreview} from "../../api"; @@ -117,9 +117,11 @@ function BackupDateRow({date, previews, previewsLoaded, onSelect}) { ; } -function BackupsModal({open, onClose, fileName, fetchConfigs}) { +export function BackupsModal({open, onClose, fileName, fetchConfigs}) { const [dates, setDates] = useState([]); const [datesLoading, setDatesLoading] = useState(true); + // The backups could not be fetched; distinct from the server confirming there are none. + const [datesFailed, setDatesFailed] = useState(false); const [previews, setPreviews] = useState({}); const [previewsLoaded, setPreviewsLoaded] = useState({}); const [selectedAction, setSelectedAction] = useState(null); // {date, mode, preview} @@ -130,17 +132,30 @@ function BackupsModal({open, onClose, fileName, fetchConfigs}) { if (!open) return; let cancelled = false; setDatesLoading(true); + setDatesFailed(false); setDates([]); setPreviews({}); setPreviewsLoaded({}); setSelectedAction(null); setApplying(false); - getConfigBackups(fileName).then(result => { - if (cancelled) return; - setDates(result?.dates || []); - setDatesLoading(false); - }); + getConfigBackups(fileName) + .then(result => { + if (cancelled) return; + // The api helper returns undefined on a non-OK response. + if (result) { + setDates(result.dates || []); + } else { + setDatesFailed(true); + } + }) + .catch(e => { + console.error(e); + if (!cancelled) setDatesFailed(true); + }) + .finally(() => { + if (!cancelled) setDatesLoading(false); + }); return () => { cancelled = true; }; }, [open, fileName]); @@ -253,6 +268,8 @@ function BackupsModal({open, onClose, fileName, fetchConfigs}) { {datesLoading ? Loading backups... + : datesFailed + ? Could not fetch backups for this config. : dates.length === 0 ?

No backups available for this config.

:
diff --git a/app/src/components/inventory/InventoryImportModal.js b/app/src/components/inventory/InventoryImportModal.js index 7f7aa18d..ce20d27c 100644 --- a/app/src/components/inventory/InventoryImportModal.js +++ b/app/src/components/inventory/InventoryImportModal.js @@ -25,7 +25,7 @@ function ItemNameList({items}) { * `onChanged` refreshes the page's inventory state after a successful action. */ export function InventoryImportModal({open, onClose, slug, name, onChanged}) { - const [dates, setDates] = useState(null); // null while loading + const [dates, setDates] = useState(null); // null while loading, undefined if the fetch failed const [reimporting, setReimporting] = useState(false); const [selected, setSelected] = useState(null); // {date, mode, preview} confirmation view const [previewing, setPreviewing] = useState(null); // `${date}:${mode}` currently loading a preview @@ -41,11 +41,19 @@ export function InventoryImportModal({open, onClose, slug, name, onChanged}) { setReimporting(false); setApplying(false); setPreviewing(null); - getInventoryBackups(slug).then(result => { - if (!cancelled) { - setDates(result || []); - } - }); + getInventoryBackups(slug) + .then(result => { + if (!cancelled) { + // The api helper returns undefined on a non-OK response. + setDates(result === undefined ? undefined : result || []); + } + }) + .catch(e => { + console.error(e); + if (!cancelled) { + setDates(undefined); + } + }); return () => { cancelled = true; }; @@ -153,6 +161,8 @@ export function InventoryImportModal({open, onClose, slug, name, onChanged}) {

{dates === null ? Loading backups… + : dates === undefined + ?

Could not fetch backups for this inventory.

: dates.length === 0 ?

No backups yet. A backup is saved automatically whenever this inventory changes.

:
diff --git a/app/src/components/inventory/InventoryImportModal.test.js b/app/src/components/inventory/InventoryImportModal.test.js index 6ec7d604..751deecf 100644 --- a/app/src/components/inventory/InventoryImportModal.test.js +++ b/app/src/components/inventory/InventoryImportModal.test.js @@ -65,4 +65,25 @@ describe('InventoryImportModal', () => { renderModal(); await waitFor(() => expect(getInventoryBackups).toHaveBeenCalledWith('food-storage')); }); + + // "No backups yet" may only appear once the server confirmed there are none. + test('says there are no backups when the server returned none', async () => { + getInventoryBackups.mockResolvedValue([]); + renderModal(); + expect(await screen.findByText(/No backups yet/)).toBeInTheDocument(); + }); + + test('a failed backups fetch shows an error, not "No backups yet"', async () => { + getInventoryBackups.mockResolvedValue(undefined); + renderModal(); + expect(await screen.findByText(/Could not fetch backups/)).toBeInTheDocument(); + expect(screen.queryByText(/No backups yet/)).not.toBeInTheDocument(); + }); + + test('a thrown backups fetch shows an error, not a loader forever', async () => { + getInventoryBackups.mockRejectedValue(new Error('down')); + renderModal(); + expect(await screen.findByText(/Could not fetch backups/)).toBeInTheDocument(); + expect(screen.queryByText(/Loading backups/)).not.toBeInTheDocument(); + }); });