From 8f1309fa18004926a59aa04d4846369d8183bbc1 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 23 Aug 2026 01:53:33 +0200 Subject: [PATCH] fix(plex): round-2 deep-review follow-ups on the review fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review round, scoped to the round-1 fix commit (where round-1's own fixes introduced new code): - The collection chip-remove pruned collectionTitles to an empty {} on last removal, which still hashes differently from the pristine no-collection state (key absent) in the React Query keys → a residual cache split. Collapse an empty map to undefined so the key-shape matches the pristine one. - Replace the tautological "order-independent signature" test (both objects spread from EMPTY_PLEX_FILTERS inherited the same key order, so it proved nothing) with full literals in genuinely different insertion order that would fail an order-sensitive JSON.stringify signature. - Cover the collectionTitles chip-remove prune (usePlexActiveFilters.dom.test.tsx) and the plexSurprise/plexDiscover lang-key split (queryKeys.test.ts) with regression tests. Gate: tsc/lint/knip/prettier clean, 412 vitest. --- .../usePlexActiveFilters.dom.test.tsx | 24 +++++++++++++++- .../src/components/usePlexActiveFilters.ts | 4 ++- frontend/src/lib/api/plex.dom.test.ts | 28 +++++++++++++++---- frontend/src/lib/queryKeys.test.ts | 12 ++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/frontend/src/components/usePlexActiveFilters.dom.test.tsx b/frontend/src/components/usePlexActiveFilters.dom.test.tsx index d141f4bd..9e27da56 100644 --- a/frontend/src/components/usePlexActiveFilters.dom.test.tsx +++ b/frontend/src/components/usePlexActiveFilters.dom.test.tsx @@ -18,7 +18,12 @@ const plexState = { show: "watched", // A non-default sort is present in the shared state — the hook must IGNORE it (no sort chip). sort: { key: "year", dir: "asc" }, - filters: { ...EMPTY_PLEX_FILTERS, genres: ["Action"] }, + filters: { + ...EMPTY_PLEX_FILTERS, + genres: ["Action"], + collections: ["A"], + collectionTitles: { A: "Marvel" }, + }, }; const setShow = vi.fn(); const setFilters = vi.fn(); @@ -40,4 +45,21 @@ describe("usePlexActiveFilters", () => { expect(keys).not.toContain("sort"); expect(result.current.every((c) => !c.key.startsWith("sort"))).toBe(true); }); + + it("removing the last collection chip prunes its title and collapses collectionTitles to undefined", () => { + setFilters.mockClear(); + const { result } = renderHook(() => usePlexActiveFilters()); + const chip = result.current.find((c) => c.key === "collection:A"); + expect(chip).toBeDefined(); + chip!.remove(); + // patch() merges into the full filters object; assert the two touched fields directly. No residual + // { A: "Marvel" } and no empty {} (which would still split the query-key cache from the pristine + // no-collection state) — collectionTitles must be undefined. + const arg = setFilters.mock.calls[0]![0] as { + collections: string[]; + collectionTitles?: unknown; + }; + expect(arg.collections).toEqual([]); + expect(arg.collectionTitles).toBeUndefined(); + }); }); diff --git a/frontend/src/components/usePlexActiveFilters.ts b/frontend/src/components/usePlexActiveFilters.ts index 82e116a7..fb3ac03d 100644 --- a/frontend/src/components/usePlexActiveFilters.ts +++ b/frontend/src/components/usePlexActiveFilters.ts @@ -123,11 +123,13 @@ export function buildPlexActiveFilters( label: filters.collectionTitles?.[c] ?? t("plex.filter.collection"), // Prune the title too — collectionTitles is hashed into the plex query keys, so a stale entry // splits the cache from the logically-identical no-collection state (mirrors CollectionPicker). + // Collapse an empty map back to undefined so the last-removed state key-matches the pristine + // one (an empty `{}` still hashes differently from an absent key). remove: () => { const { [c]: _drop, ...restTitles } = filters.collectionTitles ?? {}; patch({ collections: filters.collections.filter((x) => x !== c), - collectionTitles: restTitles, + collectionTitles: Object.keys(restTitles).length ? restTitles : undefined, }); }, }); diff --git a/frontend/src/lib/api/plex.dom.test.ts b/frontend/src/lib/api/plex.dom.test.ts index 5bd6d79b..d54ea341 100644 --- a/frontend/src/lib/api/plex.dom.test.ts +++ b/frontend/src/lib/api/plex.dom.test.ts @@ -44,11 +44,29 @@ describe("plexFiltersSignature", () => { collections: ["A"], }); - it("is independent of key order for equivalent filters", () => { - // Same field values, keys assigned in a different literal order — appendPlexFilters emits by field, - // not by object key order, so the signatures must match. - const a: PlexFilters = { ...EMPTY_PLEX_FILTERS, genres: ["Drama"], collections: ["A"] }; - const b: PlexFilters = { ...EMPTY_PLEX_FILTERS, collections: ["A"], genres: ["Drama"] }; + it("is independent of object key insertion order", () => { + // Full literals with genuinely different key INSERTION order (NOT spread from a shared base, which + // would inherit that base's order and make this tautological). appendPlexFilters reads by field, so + // the signatures must match — this test FAILS against an order-sensitive JSON.stringify signature. + const a: PlexFilters = { + genres: ["Drama"], + contentRatings: [], + directors: [], + actors: [], + studios: [], + keywords: [], + collections: ["A"], + }; + const b: PlexFilters = { + collections: ["A"], + keywords: [], + studios: [], + actors: [], + directors: [], + contentRatings: [], + genres: ["Drama"], + }; + expect(JSON.stringify(Object.keys(a))).not.toBe(JSON.stringify(Object.keys(b))); // orders truly differ expect(plexFiltersSignature(a)).toBe(plexFiltersSignature(b)); }); diff --git a/frontend/src/lib/queryKeys.test.ts b/frontend/src/lib/queryKeys.test.ts index e48902a1..91bf121d 100644 --- a/frontend/src/lib/queryKeys.test.ts +++ b/frontend/src/lib/queryKeys.test.ts @@ -84,4 +84,16 @@ describe("qk — variadic composite plex keys", () => { { genres: [] }, ]); }); + + 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), + ); + // A bare call stays a prefix for the reshuffle/watched-toggle invalidations. + expect(qk.plexSurprise()).toEqual(["plex-surprise"]); + }); });