fix(plex): round-3 deep-review follow-ups

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.
This commit is contained in:
2026-08-23 02:09:14 +02:00
parent 8f1309fa18
commit 70a74f8629
3 changed files with 63 additions and 12 deletions
@@ -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(
<QueryClientProvider client={qc}>
<PlexDiscover
scope="both"
onOpenCard={noop}
onInfoCard={noop}
onGenre={noop}
onSeeAllRecent={noop}
/>
</QueryClientProvider>,
);
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 () => {
+5 -3
View File
@@ -413,7 +413,7 @@ function CollectionPicker({
values: string[];
titles: Record<string, string> | undefined;
mode: "any" | "all";
onChange: (values: string[], titles: Record<string, string>) => void;
onChange: (values: string[], titles: Record<string, string> | 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<string, string> = {};
for (const id of next) nextTitles[id] = labelMap[id] ?? titles?.[id] ?? id;
onChange(next, nextTitles);
onChange(next, next.length ? nextTitles : undefined);
}}
/>
);
+16 -8
View File
@@ -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"]);
});