app: search Other tab distinguishes pending channels from none (#728)

* app: search Other tab distinguishes pending channels from none

useSearchChannels initialised channels to [] and loading to false, and
only set loading inside its mount effect, so the Other tab's first
render painted "No Channels" with a count of 0 before the search had
run.  A failed search was rendered the same way, since the hook had no
catch and the API helper returns undefined on a non-OK response.

Invariants now enforced and tested:
* null = pending, undefined = fetch failed, [] = no channels; loading
  is true from the first render because the search starts on mount.
* Changing tags resets to pending, and only the newest request may
  write state, so a slow response for previous tags is dropped.
* The tab shows a loader while pending, an error on failure, and
  "No Channels" only once the server confirmed there are none.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* app: useSearchChannels follows its caller's tags

The hook read its tag argument only as the initial state, so the
Other tab never refetched when the filter modal changed the URL's tags
while the tab stayed mounted; the list, including a stale "No
Channels", was whatever the mount fetched.

The internal tag state now follows the argument's contents, keyed on
the serialized list because the caller builds a new array every
render.  Tests change the hook argument after a loaded result and
expect a second search and a reset to pending, check that a new array
with the same tags does not refetch, and keep the slow-response drop.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

app/src/components/OtherSearchView.test.js

@@ -0,0 +1,56 @@
+import React from 'react';
+import {act, render, screen} from '../test-utils';
+import {OtherSearchView} from './Search';
+import {searchChannels} from '../api';
+
+// What the search page's Other tab shows for each state of useSearchChannels.  "No Channels"
+// must only appear once the server has confirmed there are none.
+
+jest.mock('../api', () => ({
+    ...jest.requireActual('../api'),
+    searchChannels: jest.fn(),
+}));
+
+const NO_CHANNELS = /No Channels/;
+
+beforeEach(() => searchChannels.mockReset());
+
+test('pending: shows a loader, not "No Channels"', () => {
+    searchChannels.mockReturnValue(new Promise(() => {
+    }));
+    const {container} = render(<OtherSearchView loading={false}/>);
+    expect(container.querySelector('.wrolpi-loading')).toBeInTheDocument();
+    expect(screen.queryByText(NO_CHANNELS)).not.toBeInTheDocument();
+});
+
+test('failed: shows an error, not "No Channels"', async () => {
+    searchChannels.mockResolvedValue(undefined);
+    const spy = jest.spyOn(console, 'error').mockImplementation(() => {
+    });
+    try {
+        render(<OtherSearchView loading={false}/>);
+        await act(async () => {
+        });
+        expect(screen.getByText(/Could not fetch the channels/)).toBeInTheDocument();
+        expect(screen.queryByText(NO_CHANNELS)).not.toBeInTheDocument();
+    } finally {
+        spy.mockRestore();
+    }
+});
+
+test('empty: shows "No Channels"', async () => {
+    searchChannels.mockResolvedValue({channels: []});
+    render(<OtherSearchView loading={false}/>);
+    await act(async () => {
+    });
+    expect(screen.getByText(NO_CHANNELS)).toBeInTheDocument();
+});
+
+test('loaded: lists the channels', async () => {
+    searchChannels.mockResolvedValue({channels: [{id: 1, name: 'Cooking'}]});
+    render(<OtherSearchView loading={false}/>);
+    await act(async () => {
+    });
+    expect(screen.getByText('Cooking')).toBeInTheDocument();
+    expect(screen.queryByText(NO_CHANNELS)).not.toBeInTheDocument();
+});

app/src/components/Search.js

@@ -6,7 +6,7 @@ import {ShortcutHint} from "./ShortcutHint";
 import {ZimSearchView} from "./Zim";
 import {MapSearchView} from "./MapSearchView";
 import {searchEstimateFiles, searchEstimateMap, searchEstimateOthers, searchEstimateZims, searchSuggestions} from "../api";
-import {filterToMimetypes, fuzzyMatch, normalizeEstimate, SearchResultsInput, TabLinks} from "./Common";
+import {ErrorMessage, filterToMimetypes, fuzzyMatch, normalizeEstimate, SearchResultsInput, TabLinks} from "./Common";
 import _ from "lodash";
 import {TagsContext} from "../Tags";
 import {Accordion, Header, Icon, Label, Loading, Panel, Stack} from "./ui";
@@ -541,14 +541,17 @@ function SearchChannelPreview({channel}) {
     </div>
 }
 
-function OtherSearchView({loading}) {
+// `channels` follows useSearchChannels: null = pending, undefined = fetch failed, [] = none.
+export function OtherSearchView({loading}) {
     const {searchParams} = React.useContext(QueryContext);
     const [activeValue, setActiveValue] = React.useState('channels');
     const activeTags = searchParams.getAll('tag');
     const {channels, loading: channelsLoading} = useSearchChannels(activeTags);
 
-    if (loading || channelsLoading) {
+    if (loading || channelsLoading || channels === null) {
         return <Panel><Loading/></Panel>
+    } else if (channels === undefined) {
+        return <Panel><ErrorMessage>Could not fetch the channels.</ErrorMessage></Panel>
     }
 
     return <Accordion value={activeValue} onChange={setActiveValue}>

app/src/hooks/customHooks.js

@@ -2079,19 +2079,42 @@ export const useCalcQuery = () => {
 
 export const useSearchChannels = (defaultTagNames) => {
     const [tagNames, setTagNames] = useState(defaultTagNames || []);
-    const [channels, setChannels] = useState([]);
-    const [loading, setLoading] = useState(false);
+    // null = pending, undefined = fetch failed, [] = no channels.  A search starts on mount, so the
+    // first render is already loading; consumers must never see [] for a search that has not run.
+    const [channels, setChannels] = useState(null);
+    const [loading, setLoading] = useState(true);
+    // Only the newest request may write state; a slow response for previous tags is dropped.
+    const requestGen = useRef(0);
 
     const localSearchChannels = async () => {
+        const gen = ++requestGen.current;
+        setChannels(null);
         setLoading(true);
         try {
             const {channels: newChannels} = await searchChannels(tagNames);
-            setChannels(newChannels);
+            if (gen === requestGen.current) {
+                setChannels(newChannels || []);
+            }
+        } catch (e) {
+            console.error(e);
+            if (gen === requestGen.current) {
+                setChannels(undefined);
+            }
         } finally {
-            setLoading(false);
+            if (gen === requestGen.current) {
+                setLoading(false);
+            }
         }
     };
 
+    // The caller's tags come from the URL and change in place while the view stays mounted, so
+    // follow their contents.  Keyed on the serialized list because the caller builds a new array
+    // every render; adopting it by reference would refetch forever.
+    const tagKey = JSON.stringify(defaultTagNames || []);
+    useEffect(() => {
+        setTagNames(prev => JSON.stringify(prev) === tagKey ? prev : JSON.parse(tagKey));
+    }, [tagKey]);
+
     useEffect(() => {
         localSearchChannels();
     }, [tagNames]);

app/src/hooks/customHooks.test.js

@@ -1,8 +1,8 @@
 import React from 'react';
 import {act, renderHook} from '@testing-library/react';
 import {render, screen} from '../test-utils';
-import {usePages, useDriveTemperature, useDriveHealth, useVideoExtras} from './customHooks';
-import {getVideoComments, getVideoDescription} from '../api';
+import {usePages, useDriveTemperature, useDriveHealth, useSearchChannels, useVideoExtras} from './customHooks';
+import {getVideoComments, getVideoDescription, searchChannels} from '../api';
 import {QueryContext, StatusContext} from '../contexts/contexts';
 import {Paginator} from '../components/Common';
 
@@ -38,6 +38,7 @@ jest.mock('../api', () => ({
     ...jest.requireActual('../api'),
     getVideoComments: jest.fn(),
     getVideoDescription: jest.fn(),
+    searchChannels: jest.fn(),
 }));
 
 // Mock Media so Paginator renders both mobile + tablet variants synchronously.
@@ -441,3 +442,95 @@ describe('useVideoExtras', () => {
         expect(result.current.description).toBe('edited');
     });
 });
+
+describe('useSearchChannels', () => {
+    // State contract: null = pending, undefined = fetch failed, [] = no channels.  The Other tab
+    // shows "No Channels" only for the last of those; `loading` is true from the first render.
+    beforeEach(() => searchChannels.mockReset());
+
+    test('is loading with no channels on the first render', () => {
+        searchChannels.mockReturnValue(new Promise(() => {
+        }));
+        const {result} = renderHook(() => useSearchChannels(['a']));
+        expect(result.current.loading).toBe(true);
+        expect(result.current.channels).toBeNull();
+    });
+
+    test('resolves to the channels and stops loading', async () => {
+        searchChannels.mockResolvedValue({channels: [{id: 1, name: 'One'}]});
+        const {result} = renderHook(() => useSearchChannels(['a']));
+        await act(async () => {
+        });
+        expect(searchChannels).toHaveBeenCalledWith(['a']);
+        expect(result.current.channels).toEqual([{id: 1, name: 'One'}]);
+        expect(result.current.loading).toBe(false);
+    });
+
+    test('a failed fetch yields undefined, not "no channels"', async () => {
+        // The api helper returns undefined on a non-OK response, which the hook cannot destructure.
+        searchChannels.mockResolvedValue(undefined);
+        const spy = jest.spyOn(console, 'error').mockImplementation(() => {
+        });
+        try {
+            const {result} = renderHook(() => useSearchChannels(['a']));
+            await act(async () => {
+            });
+            expect(result.current.channels).toBeUndefined();
+            expect(result.current.loading).toBe(false);
+        } finally {
+            spy.mockRestore();
+        }
+    });
+
+    test('a changed tag argument refetches and resets a loaded result to pending', async () => {
+        // The Other tab passes the URL's tags on every render; the filter modal changes them in
+        // place while the tab stays mounted.
+        searchChannels.mockResolvedValue({channels: [{id: 1, name: 'Old'}]});
+        const {result, rerender} = renderHook(({tags}) => useSearchChannels(tags), {initialProps: {tags: ['old']}});
+        await act(async () => {
+        });
+        expect(result.current.channels).toEqual([{id: 1, name: 'Old'}]);
+        expect(searchChannels).toHaveBeenCalledTimes(1);
+
+        searchChannels.mockReturnValue(new Promise(() => {
+        }));
+        rerender({tags: ['new']});
+        expect(searchChannels).toHaveBeenCalledTimes(2);
+        expect(searchChannels).toHaveBeenLastCalledWith(['new']);
+        expect(result.current.channels).toBeNull();
+        expect(result.current.loading).toBe(true);
+    });
+
+    test('a new array with the same tags does not refetch', async () => {
+        searchChannels.mockResolvedValue({channels: []});
+        const {rerender} = renderHook(({tags}) => useSearchChannels(tags), {initialProps: {tags: ['a']}});
+        await act(async () => {
+        });
+        rerender({tags: ['a']});
+        await act(async () => {
+        });
+        expect(searchChannels).toHaveBeenCalledTimes(1);
+    });
+
+    test('a slow response for the old tags is dropped after the tags change', async () => {
+        let resolveOld;
+        searchChannels.mockReturnValueOnce(new Promise(res => resolveOld = res));
+        const {result, rerender} = renderHook(({tags}) => useSearchChannels(tags), {initialProps: {tags: ['old']}});
+        expect(result.current.loading).toBe(true);
+
+        searchChannels.mockResolvedValue({channels: [{id: 2, name: 'New'}]});
+        rerender({tags: ['new']});
+        expect(result.current.channels).toBeNull();
+        expect(result.current.loading).toBe(true);
+        await act(async () => {
+        });
+        expect(result.current.channels).toEqual([{id: 2, name: 'New'}]);
+        expect(result.current.loading).toBe(false);
+
+        await act(async () => {
+            resolveOld({channels: [{id: 1, name: 'Old'}]});
+        });
+        expect(result.current.channels).toEqual([{id: 2, name: 'New'}]);
+        expect(result.current.loading).toBe(false);
+    });
+});