app: useVideoExtras drops any request that is not the newest

Matching on the video id let a stale request land when the user left
and returned to the same video, or saved an edit while the page's
first description request was still in flight.  Each fetch now takes a
generation number and only the newest generation may write state,
per value, so a description refetch never invalidates a comments
request.

The edit dialog adopts a description that arrives while it is open
only if the user has not typed, so a draft written over the
placeholder is not wiped by the empty or failed value that follows.

Tests: 7 -> none -> 8 -> 7 with the first request rejecting last; a
save refetch beating the initial request; the draft surviving; the
pending placeholder asserted; the console spy restored in finally.

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

app/src/components/VideoEditMenu.js

@@ -379,14 +379,27 @@ export function EditVideoModal({open, onClose, fileGroupId, videoFile, descripti
     const [title, setTitle] = useState('');
     const [description, setDescription] = useState('');
     const [saving, setSaving] = useState(false);
+    // The description last copied into the textarea; the user has typed when the field differs.
+    const syncedDescription = useRef('');
 
     useEffect(() => {
         if (open) {
             // Start from the current details on every open, discarding an abandoned edit.
             setTitle(videoFile?.title || '');
             setDescription(currentDescription || '');
+            syncedDescription.current = currentDescription || '';
         }
-    }, [open, videoFile?.title, currentDescription]);
+    }, [open, videoFile?.title]);
+
+    useEffect(() => {
+        // The description may still be loading when the dialog opens (null), and it is refetched
+        // after a save.  Adopt what arrives only while the user has not typed, so a draft written
+        // over the placeholder is never wiped by the empty or failed value that follows it.
+        if (open && currentDescription != null && description === syncedDescription.current) {
+            setDescription(currentDescription);
+            syncedDescription.current = currentDescription;
+        }
+    }, [currentDescription]);
 
     const trimmed = title.trim();
     const titleChanged = trimmed !== (videoFile?.title || '');

app/src/components/VideoEditMenu.test.js

@@ -269,6 +269,26 @@ describe('EditVideoModal', () => {
         await waitFor(() => expect(updateVideo).toHaveBeenCalledWith(7, {description: ''}));
     });
 
+    test('a draft typed while the description was still loading survives its arrival', () => {
+        // The page passes null while the description is pending, then '' when the server has none.
+        const {rerender} = renderWithProviders(
+            <EditVideoModal open={true} onClose={jest.fn()} fileGroupId={7} videoFile={videoFile} description={null}/>,
+        );
+        fireEvent.change(screen.getByLabelText('Description'), {target: {value: 'my new words'}});
+        rerender(<EditVideoModal open={true} onClose={jest.fn()} fileGroupId={7} videoFile={videoFile} description=''/>);
+        expect(screen.getByLabelText('Description')).toHaveValue('my new words');
+    });
+
+    test('a description that arrives while the field is untouched is adopted', () => {
+        const {rerender} = renderWithProviders(
+            <EditVideoModal open={true} onClose={jest.fn()} fileGroupId={7} videoFile={videoFile} description={null}/>,
+        );
+        expect(screen.getByLabelText('Description')).toHaveValue('');
+        rerender(<EditVideoModal open={true} onClose={jest.fn()} fileGroupId={7} videoFile={videoFile}
+                                 description='From the server'/>);
+        expect(screen.getByLabelText('Description')).toHaveValue('From the server');
+    });
+
     test('WROL Mode disables saving', async () => {
         renderWithProviders(
             <EditVideoModal open={true} onClose={jest.fn()} fileGroupId={7} videoFile={videoFile}/>,

app/src/components/VideoExtras.test.js

@@ -12,8 +12,9 @@ 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}/>);
+    test('pending: shows a placeholder, not "no comments" or an error', () => {
+        const {container} = render(<Comments comments={null} video={video}/>);
+        expect(container.querySelector('.wrolpi-placeholder')).toBeInTheDocument();
         expect(screen.queryByText(NO_COMMENTS)).not.toBeInTheDocument();
         expect(screen.queryByText(/Could not/)).not.toBeInTheDocument();
         expect(screen.queryByRole('button', {name: 'Refresh'})).not.toBeInTheDocument();
@@ -39,8 +40,9 @@ describe('Comments', () => {
 });
 
 describe('VideoDescription', () => {
-    test('pending: shows neither "no description" nor an error', () => {
-        render(<VideoDescription description={null} setVideoTime={() => null}/>);
+    test('pending: shows a placeholder, not "no description" or an error', () => {
+        const {container} = render(<VideoDescription description={null} setVideoTime={() => null}/>);
+        expect(container.querySelector('.wrolpi-placeholder')).toBeInTheDocument();
         expect(screen.queryByText(NO_DESCRIPTION)).not.toBeInTheDocument();
         expect(screen.queryByText(/Could not/)).not.toBeInTheDocument();
     });

app/src/hooks/customHooks.js

@@ -773,40 +773,47 @@ export const useVideoExtras = (fileGroupId) => {
     // 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);
+    // Every fetch takes the next generation number; only the newest generation may write state.
+    // Matching on the video id is not enough: leaving and returning to the same video, or a save
+    // that refetches the description, must both beat an older request still in flight.
+    const commentsGen = useRef(0);
+    const descriptionGen = useRef(0);
 
     const fetchComments = async () => {
+        const gen = ++commentsGen.current;
         try {
             const result = await getVideoComments(fileGroupId);
-            if (currentIdRef.current === fileGroupId) {
+            if (gen === commentsGen.current) {
                 setComments(result.comments || []);
             }
         } catch (e) {
             console.error(e);
-            if (currentIdRef.current === fileGroupId) {
+            if (gen === commentsGen.current) {
                 setComments(undefined);
             }
         }
     }
 
     const fetchDescription = async () => {
+        const gen = ++descriptionGen.current;
         try {
             const result = await getVideoDescription(fileGroupId);
-            if (currentIdRef.current === fileGroupId) {
+            if (gen === descriptionGen.current) {
                 setDescription(result.description || '');
             }
         } catch (e) {
             console.error(e);
-            if (currentIdRef.current === fileGroupId) {
+            if (gen === descriptionGen.current) {
                 setDescription(undefined);
             }
         }
     }
 
     useEffect(() => {
-        // Whether switching to a new video or to none, the previous video's data must not linger.
-        currentIdRef.current = fileGroupId;
+        // Whether switching to a new video or to none, the previous video's data must not linger,
+        // and any request still in flight for it must not land.
+        commentsGen.current++;
+        descriptionGen.current++;
         setComments(null);
         setDescription(null);
         if (fileGroupId) {

app/src/hooks/customHooks.test.js

@@ -334,14 +334,17 @@ describe('useVideoExtras', () => {
     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 spy = 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();
+        try {
+            const {result} = renderHook(() => useVideoExtras(7));
+            await act(async () => {
+            });
+            expect(result.current.comments).toBeUndefined();
+            expect(result.current.description).toBeUndefined();
+        } finally {
+            spy.mockRestore();
+        }
     });
 
     test('switching video resets both to pending before the new responses arrive', async () => {
@@ -383,4 +386,58 @@ describe('useVideoExtras', () => {
         expect(result.current.comments).toEqual([{id: 'c8', parent: 'root'}]);
         expect(result.current.description).toBe('eighth');
     });
+
+    test('returning to a video does not let its first, still-pending request land', async () => {
+        // The route clears the video between pages, so the id goes 7 -> undefined -> 8 -> 7 while
+        // the very first request for 7 is still in flight (the API timeout is a minute).
+        let rejectComments, rejectDescription;
+        getVideoComments.mockReturnValueOnce(new Promise((_, rej) => rejectComments = rej));
+        getVideoDescription.mockReturnValueOnce(new Promise((_, rej) => rejectDescription = rej));
+        const {result, rerender} = renderHook(({id}) => useVideoExtras(id), {initialProps: {id: 7}});
+
+        rerender({id: undefined});
+        getVideoComments.mockResolvedValue({comments: [{id: 'c8', parent: 'root'}]});
+        getVideoDescription.mockResolvedValue({description: 'eighth'});
+        rerender({id: 8});
+        getVideoComments.mockResolvedValue({comments: [{id: 'c7b', parent: 'root'}]});
+        getVideoDescription.mockResolvedValue({description: 'seventh again'});
+        rerender({id: 7});
+        await act(async () => {
+        });
+        expect(result.current.comments).toEqual([{id: 'c7b', parent: 'root'}]);
+        expect(result.current.description).toBe('seventh again');
+
+        // The first request finally times out.  Same id, but it is not the newest request.
+        const spy = jest.spyOn(console, 'error').mockImplementation(() => {
+        });
+        try {
+            await act(async () => {
+                rejectComments(new Error('timeout'));
+                rejectDescription(new Error('timeout'));
+            });
+        } finally {
+            spy.mockRestore();
+        }
+        expect(result.current.comments).toEqual([{id: 'c7b', parent: 'root'}]);
+        expect(result.current.description).toBe('seventh again');
+    });
+
+    test('a refetch after saving beats the initial description request still in flight', async () => {
+        let resolveInitial;
+        getVideoComments.mockResolvedValue({comments: []});
+        getVideoDescription.mockReturnValueOnce(new Promise(res => resolveInitial = res));
+        const {result} = renderHook(() => useVideoExtras(7));
+
+        // The user edits and saves before the page's first description request returned.
+        getVideoDescription.mockResolvedValue({description: 'edited'});
+        await act(async () => {
+            await result.current.fetchDescription();
+        });
+        expect(result.current.description).toBe('edited');
+
+        await act(async () => {
+            resolveInitial({description: 'pre-edit'});
+        });
+        expect(result.current.description).toBe('edited');
+    });
 });