From 70a74f8629031e7219a523fa2bc6dad5ce609b44 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 23 Aug 2026 02:09:14 +0200 Subject: [PATCH] fix(plex): round-3 deep-review follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review round, scoped to the round-2 fix commit. The production ternary was confirmed correct; two sibling-surface findings: - The sidebar CollectionPicker's clear-all still wrote collectionTitles:{} while the chip-row remove (fixed in round 2) wrote undefined — a divergence that leaves the {}-vs-absent cache split on the picker path. Collapse an empty map to undefined there too, so both no-collection routes key-match the pristine state. - The queryKeys lang test was near-tautological (a variadic-spread factory can't drop a passed arg), and nothing asserted the CALL SITE keys carry lang. Add a real guard in PlexDiscover.dom.test.tsx that inspects the registered query keys for scope+lang, and reword the factory test's comment to say it only shape-pins the segment pass-through. Gate: tsc/lint/knip/prettier clean, 413 vitest. --- .../src/components/PlexDiscover.dom.test.tsx | 43 ++++++++++++++++++- frontend/src/components/PlexSidebar.tsx | 8 ++-- frontend/src/lib/queryKeys.test.ts | 24 +++++++---- 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/frontend/src/components/PlexDiscover.dom.test.tsx b/frontend/src/components/PlexDiscover.dom.test.tsx index b1966a85..4845ebea 100644 --- a/frontend/src/components/PlexDiscover.dom.test.tsx +++ b/frontend/src/components/PlexDiscover.dom.test.tsx @@ -114,7 +114,48 @@ describe("PlexDiscover", () => { expect(screen.getAllByText("Random Pick").length).toBeGreaterThan(0); expect(screen.getAllByText("Rec One").length).toBeGreaterThan(0); // Now called with the active filters too (empty default here, no provider mounted). - expect(plexDiscover).toHaveBeenCalledWith("both", "en", expect.objectContaining({ genres: [] })); + expect(plexDiscover).toHaveBeenCalledWith( + "both", + "en", + expect.objectContaining({ genres: [] }), + ); + }); + + it("registers the discover + surprise queries keyed by the active lang", async () => { + // The real cache-separation guard (the queryKeys factory test only pins that a lang ARG survives the + // spread): inspect the actual keys the component registered. A regression dropping lang from either + // qk call would collide HU/EN in the cache even while the queryFn still fetches the right language. + plexDiscover.mockResolvedValue(fullPayload); + plexSurprise.mockResolvedValue({ scope: "both", items: [] }); + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + await screen.findByText("plex.discover.continueWatching"); + const keys = qc + .getQueryCache() + .getAll() + .map((q) => q.queryKey); + expect(keys).toContainEqual([ + "plex-discover", + "both", + "en", + expect.objectContaining({ genres: [] }), + ]); + expect(keys).toContainEqual([ + "plex-surprise", + "both", + "en", + expect.objectContaining({ genres: [] }), + ]); }); it("fires onGenre with the tile's value", async () => { diff --git a/frontend/src/components/PlexSidebar.tsx b/frontend/src/components/PlexSidebar.tsx index a12cf31c..64d665cd 100644 --- a/frontend/src/components/PlexSidebar.tsx +++ b/frontend/src/components/PlexSidebar.tsx @@ -413,7 +413,7 @@ function CollectionPicker({ values: string[]; titles: Record | undefined; mode: "any" | "all"; - onChange: (values: string[], titles: Record) => void; + onChange: (values: string[], titles: Record | undefined) => void; onModeChange: (mode: "any" | "all") => void; }) { const { t } = useTranslation(); @@ -451,10 +451,12 @@ function CollectionPicker({ matchMode={mode === "all" ? "and" : "or"} onMatchModeChange={(m) => onModeChange(m === "and" ? "all" : "any")} onChange={(next) => { - // Carry only the selected ids' titles forward (drop titles for de-selected ones). + // Carry only the selected ids' titles forward (drop titles for de-selected ones). Collapse an + // empty map to undefined so a clear-all key-matches the pristine state (an empty {} still hashes + // differently from an absent key), mirroring the chip-row remove handler. const nextTitles: Record = {}; for (const id of next) nextTitles[id] = labelMap[id] ?? titles?.[id] ?? id; - onChange(next, nextTitles); + onChange(next, next.length ? nextTitles : undefined); }} /> ); diff --git a/frontend/src/lib/queryKeys.test.ts b/frontend/src/lib/queryKeys.test.ts index 91bf121d..88b035d2 100644 --- a/frontend/src/lib/queryKeys.test.ts +++ b/frontend/src/lib/queryKeys.test.ts @@ -85,14 +85,22 @@ describe("qk — variadic composite plex keys", () => { ]); }); - it("keys the Discover/Surprise rails by lang so a language switch caches separately", () => { - // Both surfaces filter against the language-specific genre column, so HU and EN must not collide. - expect(qk.plexDiscover("movie", "hu", FILTERS)).not.toEqual( - qk.plexDiscover("movie", "en", FILTERS), - ); - expect(qk.plexSurprise("movie", "hu", FILTERS)).not.toEqual( - qk.plexSurprise("movie", "en", FILTERS), - ); + it("preserves scope+lang+filters as distinct Discover/Surprise key segments", () => { + // Shape-pinning only: these lock that the variadic factory carries every arg through (a "tidy-up" + // that drops a param would break cache separation). That the CALL SITE actually passes lang is + // guarded separately in PlexDiscover.dom.test.tsx (it inspects the registered query keys). + expect(qk.plexDiscover("movie", "hu", FILTERS)).toEqual([ + "plex-discover", + "movie", + "hu", + FILTERS, + ]); + expect(qk.plexSurprise("movie", "hu", FILTERS)).toEqual([ + "plex-surprise", + "movie", + "hu", + FILTERS, + ]); // A bare call stays a prefix for the reshuffle/watched-toggle invalidations. expect(qk.plexSurprise()).toEqual(["plex-surprise"]); });