From ee94d1949ad1b26397f26943d6a9f3a76241f353 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 9 Oct 2026 00:44:46 +0200 Subject: [PATCH] fix(feed): a video marked watched in the player stays out of an Unwatched view Marked watched in the player (by hand, or by the player at its end), a card left an Unwatched list at once, then came back as "new" when the player closed, until a reload. The player's closing position patch writes into the cached feed; the feed took the bumped dataUpdatedAt for fresh server data, dropped its optimistic overrides, and nothing refetched. The overrides now clear only when a refetch lands, and a player session that changed a state refetches the list and its count by the view's filters on close, once its writes have landed. --- .../Feed.playerSession.dom.test.tsx | 192 ++++++++++++++++++ frontend/src/components/Feed.tsx | 59 ++++-- 2 files changed, 235 insertions(+), 16 deletions(-) create mode 100644 frontend/src/components/Feed.playerSession.dom.test.tsx diff --git a/frontend/src/components/Feed.playerSession.dom.test.tsx b/frontend/src/components/Feed.playerSession.dom.test.tsx new file mode 100644 index 00000000..c4e1bae9 --- /dev/null +++ b/frontend/src/components/Feed.playerSession.dom.test.tsx @@ -0,0 +1,192 @@ +// @vitest-environment jsdom +// +// A video marked watched in the player (by hand, or by the player at its end) left an Unwatched view +// at once, then came back as "new" when the player closed, until a reload (owner, on a channel page). +// The player's closing position patch writes into the cached feed, which bumped `dataUpdatedAt`; the +// feed took that for fresh server data and dropped its optimistic overrides, and nothing refetched. +// The grid and the player are stubs that show what Feed hands them. +import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; +import { render, screen, fireEvent, waitFor, cleanup, act } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), +})); +vi.mock("./wizardContext", () => ({ useWizard: () => ({ openWizard: vi.fn() }) })); +// What the grid was handed for its card menu's channel-filter row. +const grid: { channelFilter?: { ids: string[]; set: (ids: string[]) => void } } = {}; +vi.mock("./VirtualFeed", () => ({ + default: ({ + items, + onOpen, + channelFilter, + }: { + items: { id: string }[]; + onOpen: (v: unknown) => void; + channelFilter?: { ids: string[]; set: (ids: string[]) => void }; + }) => ( + (grid.channelFilter = channelFilter), + ( +
    + {items.map((v) => ( +
  1. + {v.id} + +
    2. + ))} +
+ ) + ), +})); +vi.mock("./PlayerModal", () => ({ + default: ({ + video, + onState, + onClose, + }: { + video: { id: string }; + onState: (id: string, status: string) => void; + onClose: () => void; + }) => ( +
+ + +
+ ), +})); + +import Feed from "./Feed"; +import { api, DEFAULT_TAG_MODES, type FeedFilters, type Video } from "../lib/api"; +import { qk } from "../lib/queryKeys"; +import { patchCachedProgress } from "../lib/progressCache"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const FILTERS: FeedFilters = { + tags: [], + tagModes: DEFAULT_TAG_MODES, + channelIds: ["UCx"], + q: "", + sort: "newest", + scope: "my", + includeNormal: true, + includeShorts: false, + includeLive: false, + show: "unwatched", +}; + +const video = (status: string, position = 0) => + ({ + id: "v1", + title: "v1", + channel_id: "UCx", + channel_title: "Channel", + status, + position_seconds: position, + duration_seconds: 600, + published_at: "2026-01-01T00:00:00Z", + watch_url: "https://www.youtube.com/watch?v=v1", + }) as unknown as Video; + +function mount({ + channelScoped = true, + setFilters = () => {}, +}: { channelScoped?: boolean; setFilters?: (f: FeedFilters) => void } = {}) { + const feed = vi + .spyOn(api, "feed") + .mockResolvedValueOnce({ items: [video("new")], next_cursor: null } as never) + // The server after the mark: watched, so an Unwatched view leaves it out. + .mockResolvedValue({ items: [], next_cursor: null } as never); + const summary = vi.spyOn(api, "feedSummary").mockResolvedValue({ count: 1, facets: {} } as never); + vi.spyOn(api, "setState").mockResolvedValue({ ok: true } as never); + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + qc.setQueryData(qk.me(), { id: 1, can_write: true, prefs: {} }); + render( + + {}} + canRead + ytSearch={null} + onYtSearch={() => {}} + onExitYtSearch={() => {}} + channelScoped={channelScoped} + /> + , + ); + return { qc, feed, summary }; +} + +describe("Feed — a video marked watched in the player", () => { + it("stays out of an Unwatched view when the player writes its position into the cache", async () => { + const { qc } = mount(); + fireEvent.click(await screen.findByText("open v1")); + fireEvent.click(await screen.findByText("mark")); + await waitFor(() => expect(screen.queryByText("open v1")).toBeNull()); + // What the player does as it closes: the server's stored position, written into the cached list. + act(() => patchCachedProgress(qc, new Map([["v1", 10]]))); + await new Promise((r) => setTimeout(r, 20)); + expect(screen.queryByText("open v1")).toBeNull(); + }); + + it("refetches the list and its count by the view's filters when the player closes", async () => { + const { feed, summary } = mount(); + fireEvent.click(await screen.findByText("open v1")); + fireEvent.click(await screen.findByText("mark")); + const before = { feed: feed.mock.calls.length, summary: summary.mock.calls.length }; + fireEvent.click(screen.getByText("close")); + await waitFor(() => expect(feed.mock.calls.length).toBeGreaterThan(before.feed)); + await waitFor(() => expect(summary.mock.calls.length).toBeGreaterThan(before.summary)); + // The refetch is the channel's Unwatched list, and the server's answer keeps the card out. + expect(feed.mock.calls[feed.mock.calls.length - 1]?.[0]).toMatchObject({ + channelIds: ["UCx"], + show: "unwatched", + }); + await new Promise((r) => setTimeout(r, 20)); + expect(screen.queryByText("open v1")).toBeNull(); + }); + + it("does not refetch on a close with nothing changed", async () => { + const { feed } = mount(); + fireEvent.click(await screen.findByText("open v1")); + const before = feed.mock.calls.length; + fireEvent.click(await screen.findByText("close")); + await new Promise((r) => setTimeout(r, 30)); + expect(feed.mock.calls.length).toBe(before); + }); +}); + +describe("Feed — the card menu's channel filter", () => { + it("is offered on the feed, as its own channel list, and sets it", async () => { + const setFilters = vi.fn(); + mount({ channelScoped: false, setFilters }); + await screen.findByText("open v1"); + expect(grid.channelFilter?.ids).toEqual(["UCx"]); + grid.channelFilter?.set(["UCx", "UCy"]); + expect(setFilters).toHaveBeenCalledWith({ ...FILTERS, channelIds: ["UCx", "UCy"] }); + }); + + it("is not offered on a channel page, whose view is that channel already", async () => { + mount({ channelScoped: true }); + await screen.findByText("open v1"); + expect(grid.channelFilter).toBeUndefined(); + }); +}); diff --git a/frontend/src/components/Feed.tsx b/frontend/src/components/Feed.tsx index 09b4e057..6333df32 100644 --- a/frontend/src/components/Feed.tsx +++ b/frontend/src/components/Feed.tsx @@ -157,15 +157,34 @@ export default function Feed({ ); const qc = useQueryClient(); - const openVideo = useCallback( - (video: Video, startAt: number | null = null) => setActiveVideo({ video, startAt }), - [], - ); + // What changed state while the player was open, and the writes still in flight: closing it + // refetches the list by its filters once those land. Marked watched (by hand, or by the player at + // the end) a card leaves an Unwatched view through the override at once, but the list, its count + // and the cache stayed the pre-session server's until a reload (owner). + const playerSession = useRef({ + open: false, + changed: false, + pending: new Set>(), + }); + const openVideo = useCallback((video: Video, startAt: number | null = null) => { + playerSession.current.open = true; + setActiveVideo({ video, startAt }); + }, []); // Stable identity, like Playlists' `closePlayer`: PlayerModal's keydown effect // depends on `onClose`, so a fresh closure every Feed render re-subscribed those listeners, // re-ran focusModal() under the open player, dropped its in-flight timers and flushed the // debounced volume write on each pass. - const closePlayer = useCallback(() => setActiveVideo(null), []); + const closePlayer = useCallback(() => { + setActiveVideo(null); + const session = playerSession.current; + session.open = false; + if (!session.changed) return; + session.changed = false; + void Promise.allSettled([...session.pending]).then(() => { + void qc.invalidateQueries({ queryKey: qk.feed() }); + void qc.invalidateQueries({ queryKey: qk.feedSummary() }); + }); + }, [qc]); const ytActive = !!ytSearch; // How many live-search results to gather (the count selector replaces a manual "load more"; @@ -246,20 +265,21 @@ export default function Feed({ setSavedOverrides({}); setResetIds({}); } - // ...but NOT on infinite-scroll appends: fetchNextPage also bumps dataUpdatedAt while page 1's - // rows are unchanged, so clearing overrides there would make a just-hidden card flash back. - // Only a real refetch (page count doesn't grow) is authoritative. - const pagesLenRef = useRef(0); - const pagesLen = query.data?.pages?.length ?? 0; + // ...but only when a refetch LANDS, the server's own answer. Not on an infinite-scroll append (its + // page 1 rows are unchanged, so a just-hidden card would flash back), and not on a local write into + // the cache: the player's closing position patch bumped `dataUpdatedAt` too, cleared the overrides, + // and put a card marked watched back into an Unwatched view as "new", with nothing to refetch it. + const refetching = query.isFetching && !query.isFetchingNextPage; + const wasRefetching = useRef(false); useEffect(() => { - const appended = pagesLen > pagesLenRef.current; - pagesLenRef.current = pagesLen; - if (!appended) { + const landed = wasRefetching.current && !refetching && query.status === "success"; + wasRefetching.current = refetching; + if (landed) { setOverrides({}); setSavedOverrides({}); setResetIds({}); } - }, [query.dataUpdatedAt, pagesLen]); + }, [refetching, query.status]); // One shared query with the sidebar (same key + filters -> React Query dedupes to a single request): // the feed reads `.count`, the sidebar reads `.facets` (PERF-13, replaces feedCount + facets). @@ -283,13 +303,20 @@ export default function Feed({ const onState = useCallback( (id: string, status: VideoStatus) => { setOverrides((o) => ({ ...o, [id]: status })); + const write = api.setState(id, status); + const session = playerSession.current; + if (session.open) { + session.changed = true; + session.pending.add(write); + const settled = () => void session.pending.delete(write); + write.then(settled, settled); + } // Refetch once the server has the change so other views (e.g. Hidden) are in sync. // Announce the change only AFTER the server confirms it: a "Marked watched"/"Hidden" // notice (or resolving a stale one) is a claim of success, so firing it optimistically // would falsely report a change that the .catch below is about to roll back (e.g. when // the API is unreachable). - api - .setState(id, status) + write .then(() => { // Mark stale WITHOUT refetching: the optimistic `overrides` layer + the matchesView filter // already show the correct state (a hidden/watched card drops out immediately), so refetching