app: video comments and description distinguish pending from empty

useVideoExtras initialised both values to null and the page treated
null the same as "none", so every video open (and every prev/next)
showed "No comments have been downloaded. Refresh the video?" with a
live Refresh button until the comments arrived.  The Description tab
showed "No description available." the same way.  A failed fetch was
also rendered as "none", and the previous video's data lingered until
the next video's responses arrived.

Invariants now enforced and tested:
* null = pending, undefined = fetch failed, [] / '' = loaded and empty.
  The API serialises a missing info_json field as null, so the hook
  normalises that to empty; consumers never see null for "none".
* Switching video resets both to pending, and a slow response for the
  previous video is dropped.
* The page renders a placeholder while pending, an error message on
  failure, and offers Refresh only once the server confirmed there are
  no comments.

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

app/src/components/VideoExtras.test.js

@@ -0,0 +1,64 @@
+import React from 'react';
+import {render, screen} from '../test-utils';
+import {Comments, VideoDescription} from './VideoPlayer';
+
+// What the video page shows for each state of useVideoExtras.  The states are null (pending),
+// undefined (fetch failed), and an empty value (loaded, nothing to show).  "No comments have been
+// downloaded" carries a Refresh button that re-downloads the video, so it must only appear once
+// the server has confirmed there are none.
+
+const video = {id: 7, url: 'https://example.com/v'};
+const NO_COMMENTS = /No comments have been/;
+const NO_DESCRIPTION = /No description available/;
+
+describe('Comments', () => {
+    test('pending: shows neither "no comments" nor an error', () => {
+        render(<Comments comments={null} video={video}/>);
+        expect(screen.queryByText(NO_COMMENTS)).not.toBeInTheDocument();
+        expect(screen.queryByText(/Could not/)).not.toBeInTheDocument();
+        expect(screen.queryByRole('button', {name: 'Refresh'})).not.toBeInTheDocument();
+    });
+
+    test('failed: shows an error, not "no comments"', () => {
+        render(<Comments comments={undefined} video={video}/>);
+        expect(screen.getByText(/Could not fetch the comments/)).toBeInTheDocument();
+        expect(screen.queryByText(NO_COMMENTS)).not.toBeInTheDocument();
+    });
+
+    test('empty: shows "no comments" with the Refresh button', () => {
+        render(<Comments comments={[]} video={video}/>);
+        expect(screen.getByText(NO_COMMENTS)).toBeInTheDocument();
+        expect(screen.getByRole('button', {name: 'Refresh'})).toBeInTheDocument();
+    });
+
+    test('loaded: renders the comments', () => {
+        render(<Comments comments={[{id: 'c1', parent: 'root', text: 'first!', author: 'a'}]} video={video}/>);
+        expect(screen.getByText('first!')).toBeInTheDocument();
+        expect(screen.queryByText(NO_COMMENTS)).not.toBeInTheDocument();
+    });
+});
+
+describe('VideoDescription', () => {
+    test('pending: shows neither "no description" nor an error', () => {
+        render(<VideoDescription description={null} setVideoTime={() => null}/>);
+        expect(screen.queryByText(NO_DESCRIPTION)).not.toBeInTheDocument();
+        expect(screen.queryByText(/Could not/)).not.toBeInTheDocument();
+    });
+
+    test('failed: shows an error, not "no description"', () => {
+        render(<VideoDescription description={undefined} setVideoTime={() => null}/>);
+        expect(screen.getByText(/Could not fetch the description/)).toBeInTheDocument();
+        expect(screen.queryByText(NO_DESCRIPTION)).not.toBeInTheDocument();
+    });
+
+    test('empty: shows "no description"', () => {
+        render(<VideoDescription description='' setVideoTime={() => null}/>);
+        expect(screen.getByText(NO_DESCRIPTION)).toBeInTheDocument();
+    });
+
+    test('loaded: renders the description', () => {
+        render(<VideoDescription description='A fine video.' setVideoTime={() => null}/>);
+        expect(screen.getByText('A fine video.')).toBeInTheDocument();
+        expect(screen.queryByText(NO_DESCRIPTION)).not.toBeInTheDocument();
+    });
+});

app/src/components/VideoPlayer.js

@@ -8,6 +8,7 @@ import {
     BackButton,
     DirectoryLink,
     encodeMediaPath,
+    ErrorMessage,
     humanFileSize,
     humanNumber,
     isoDatetimeToAgoPopup,
@@ -19,7 +20,7 @@ import {
 } from "./Common";
 import {VideoPlaceholder} from "./Placeholder";
 import {useChannel, useVideoCaptions, useVideoExtras} from "../hooks/customHooks";
-import {Button, Grid, Header, Icon, Label, Loading, Panel, Tabs, TextInput} from "./ui";
+import {Button, Grid, Header, Icon, Label, Loading, Panel, Placeholder, Tabs, TextInput} from "./ui";
 import {VideoCard} from "./Videos";
 import {TagsSelector} from "../Tags";
 import {AddToPlaylistButton} from "./AddToPlaylist";
@@ -170,8 +171,14 @@ const VideoComment = ({comment, children}) => {
 }
 
 
-const Comments = ({comments, video}) => {
-    if (!comments || _.isEmpty(comments)) {
+// `comments` follows useVideoExtras: null = pending, undefined = fetch failed, [] = none downloaded.
+// Only the last of those may offer the Refresh button.
+export const Comments = ({comments, video}) => {
+    if (comments === null) {
+        return <Placeholder lines={3}/>
+    } else if (comments === undefined) {
+        return <ErrorMessage>Could not fetch the comments.</ErrorMessage>
+    } else if (_.isEmpty(comments)) {
         return <NoComments video={video}/>
     }
 
@@ -343,12 +350,6 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) {
         caption_files.filter(i => i['mimetype'] === 'text/vtt').map(i => `${MEDIA_PATH}/${encodeMediaPath(i['path'])}`)
         : [];
 
-    let description = 'No description available.';
-    if (videoDescription) {
-        // Only replace empty description if there is one available.
-        description = formatVideoDescription(videoDescription, setVideoTime)
-    }
-
     const getFile = (suffix) => videoFile.files.find(i => i.path.toLowerCase().endsWith(suffix));
     const fileSize = (file) => file && file.size
         ? <span style={{marginLeft: '1em', color: 'var(--muted)'}}>({humanFileSize(file.size)})</span>
@@ -550,9 +551,7 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) {
                 </Tabs.Panel>
 
                 <Tabs.Panel value='description' style={{paddingTop: '1em'}}>
-                    <pre className="wrap-text">
-                        {description}
-                    </pre>
+                    <VideoDescription description={videoDescription} setVideoTime={setVideoTime}/>
                 </Tabs.Panel>
 
                 <Tabs.Panel value='files' style={{paddingTop: '1em'}}>
@@ -717,6 +716,18 @@ export default VideoPage;
 
 const chapterRegex = new RegExp('^(\\(?(?:((\\d?\\d):)?(?:(\\d?\\d):(\\d\\d)))\\)?)\.?\\s+(.*)$', 'i');
 
+// `description` follows useVideoExtras: null = pending, undefined = fetch failed, '' = none.
+export const VideoDescription = ({description, setVideoTime}) => {
+    if (description === null) {
+        return <Placeholder lines={3}/>
+    } else if (description === undefined) {
+        return <ErrorMessage>Could not fetch the description.</ErrorMessage>
+    }
+    return <pre className="wrap-text">
+        {description ? formatVideoDescription(description, setVideoTime) : 'No description available.'}
+    </pre>
+}
+
 function formatVideoDescription(description, setVideoTime) {
     // Convert timestamps to links which change the video's playback location.  Change hashtags to search links.
     if (description && description.length > 0) {

app/src/hooks/customHooks.js

@@ -767,34 +767,51 @@ export const useVideo = (fileGroupId) => {
 export const useVideoExtras = (fileGroupId) => {
     // Fetches extra data to display on a Video's page.  These are served separately from the
     // Video itself so list responses stay small and never read files from disk.
+    //
+    // Each value is null while pending, undefined when the fetch failed, and empty ([] or '') when
+    // the server confirmed there is nothing.  The API serializes a missing info_json field as null,
+    // so a null response is normalized to empty here; consumers must never see null for "none".
     const [comments, setComments] = useState(null);
     const [description, setDescription] = useState(null);
+    // The video currently shown.  A response for any other video is stale and is dropped.
+    const currentIdRef = useRef(fileGroupId);
 
     const fetchComments = async () => {
         try {
             const result = await getVideoComments(fileGroupId);
-            setComments(result.comments);
+            if (currentIdRef.current === fileGroupId) {
+                setComments(result.comments || []);
+            }
         } catch (e) {
             console.error(e);
+            if (currentIdRef.current === fileGroupId) {
+                setComments(undefined);
+            }
         }
     }
 
     const fetchDescription = async () => {
         try {
             const result = await getVideoDescription(fileGroupId);
-            setDescription(result.description);
+            if (currentIdRef.current === fileGroupId) {
+                setDescription(result.description || '');
+            }
         } catch (e) {
             console.error(e);
+            if (currentIdRef.current === fileGroupId) {
+                setDescription(undefined);
+            }
         }
     }
 
     useEffect(() => {
+        // Whether switching to a new video or to none, the previous video's data must not linger.
+        currentIdRef.current = fileGroupId;
+        setComments(null);
+        setDescription(null);
         if (fileGroupId) {
             fetchComments();
             fetchDescription();
-        } else {
-            setComments(null);
-            setDescription(null);
         }
     }, [fileGroupId]);
 

app/src/hooks/customHooks.test.js

@@ -306,4 +306,81 @@ describe('useVideoExtras', () => {
         expect(result.current.comments).toBeNull();
         expect(result.current.description).toBeNull();
     });
+
+    // State contract, matching the rest of the hooks: null = pending, undefined = error,
+    // [] / '' = loaded and empty.  The page branches on these to choose between a placeholder,
+    // an error, and "No comments have been downloaded".
+    test('both are pending (null) until the responses arrive', () => {
+        getVideoComments.mockReturnValue(new Promise(() => {
+        }));
+        getVideoDescription.mockReturnValue(new Promise(() => {
+        }));
+        const {result} = renderHook(() => useVideoExtras(7));
+        expect(result.current.comments).toBeNull();
+        expect(result.current.description).toBeNull();
+    });
+
+    test('a video with no comments or description loads as empty, not pending', async () => {
+        // The API serializes a missing info_json field as null.
+        getVideoComments.mockResolvedValue({comments: null});
+        getVideoDescription.mockResolvedValue({description: null});
+        const {result} = renderHook(() => useVideoExtras(7));
+        await act(async () => {
+        });
+        expect(result.current.comments).toEqual([]);
+        expect(result.current.description).toBe('');
+    });
+
+    test('a failed fetch yields undefined so the page can show an error instead of "no comments"', async () => {
+        getVideoComments.mockRejectedValue(new Error('boom'));
+        getVideoDescription.mockRejectedValue(new Error('boom'));
+        jest.spyOn(console, 'error').mockImplementation(() => {
+        });
+        const {result} = renderHook(() => useVideoExtras(7));
+        await act(async () => {
+        });
+        expect(result.current.comments).toBeUndefined();
+        expect(result.current.description).toBeUndefined();
+        console.error.mockRestore();
+    });
+
+    test('switching video resets both to pending before the new responses arrive', async () => {
+        getVideoComments.mockResolvedValue({comments: [{id: 'c1', parent: 'root'}]});
+        getVideoDescription.mockResolvedValue({description: 'first'});
+        const {result, rerender} = renderHook(({id}) => useVideoExtras(id), {initialProps: {id: 7}});
+        await act(async () => {
+        });
+        expect(result.current.description).toBe('first');
+
+        // The next video's responses are slow: the previous video's data must not linger.
+        getVideoComments.mockReturnValue(new Promise(() => {
+        }));
+        getVideoDescription.mockReturnValue(new Promise(() => {
+        }));
+        rerender({id: 8});
+        expect(result.current.comments).toBeNull();
+        expect(result.current.description).toBeNull();
+    });
+
+    test('a slow response for the previous video does not overwrite the current one', async () => {
+        // Video 7's responses are slow; the user moves on to video 8 before they arrive.
+        let resolveComments, resolveDescription;
+        getVideoComments.mockReturnValueOnce(new Promise(res => resolveComments = res));
+        getVideoDescription.mockReturnValueOnce(new Promise(res => resolveDescription = res));
+        const {result, rerender} = renderHook(({id}) => useVideoExtras(id), {initialProps: {id: 7}});
+
+        getVideoComments.mockResolvedValue({comments: [{id: 'c8', parent: 'root'}]});
+        getVideoDescription.mockResolvedValue({description: 'eighth'});
+        rerender({id: 8});
+        await act(async () => {
+        });
+        expect(result.current.description).toBe('eighth');
+
+        await act(async () => {
+            resolveComments({comments: [{id: 'c7', parent: 'root'}]});
+            resolveDescription({description: 'seventh'});
+        });
+        expect(result.current.comments).toEqual([{id: 'c8', parent: 'root'}]);
+        expect(result.current.description).toBe('eighth');
+    });
 });