fix(plex): resolve deep-review findings on the filter-bar overhaul
Self-review of the branch diff (feature/plex-filter-bar-overhaul) against main: - Surprise rail query key omitted `lang` while the backend /surprise now filters against the language-specific genre column → a HU/EN switch under an active genre filter left the rail stale. Key it by scope+lang+filters, matching plexDiscover. - Chip-removing a collection left its entry in `collectionTitles` (hashed into the plex query keys), splitting the cache from the pristine no-collection state and growing the titles map unboundedly. Prune the title on remove, like CollectionPicker. - The saved-views default-mirror effect gained `cfg` in its deps (re-memoized on every live filter edit) → a synchronous localStorage write per keystroke. Depend on the module-stable mirror fn instead. - Collections-browse search input had no accessible name (placeholder only). Add aria-label + type="search". - Extract the tile hue-hash + gradient duplicated between PlexDiscover and plexBrowseLists into a shared lib/hueTile so the two surfaces can't drift. - Document migration 0073 downgrade() as lossy and de-dup non-feed rows before recreating the narrow (user_id, name) constraint (verified by the drift test). - Add unit tests for coercePlexFilters (legacy single-collection migration) and plexFiltersSignature (order-independence, title-ignoring). Gate: tsc/lint/knip/prettier clean, 410 vitest; backend ruff + migration drift + saved-views tests green.
This commit is contained in:
@@ -29,6 +29,11 @@ def upgrade() -> None:
|
|||||||
|
|
||||||
|
|
||||||
def downgrade() -> None:
|
def downgrade() -> None:
|
||||||
|
# LOSSY / effectively forward-only: upgrade lets a user hold a feed view and a plex view of the same
|
||||||
|
# name, so re-creating the narrow (user_id, name) constraint can raise IntegrityError on such a pair.
|
||||||
|
# De-dup by dropping the non-feed rows first (they can't exist in the pre-0073 schema anyway) so the
|
||||||
|
# narrow constraint re-applies cleanly for the drift test's downgrade→re-upgrade cycle.
|
||||||
|
op.execute("DELETE FROM saved_views WHERE source <> 'feed'")
|
||||||
op.drop_constraint("uq_user_source_saved_view", "saved_views", type_="unique")
|
op.drop_constraint("uq_user_source_saved_view", "saved_views", type_="unique")
|
||||||
op.create_unique_constraint("uq_user_saved_view", "saved_views", ["user_id", "name"])
|
op.create_unique_constraint("uq_user_saved_view", "saved_views", ["user_id", "name"])
|
||||||
op.drop_column("saved_views", "source")
|
op.drop_column("saved_views", "source")
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { useQuery, useQueryClient } from "@tanstack/react-query";
|
|||||||
import { RotateCcw } from "lucide-react";
|
import { RotateCcw } from "lucide-react";
|
||||||
import { qk } from "../lib/queryKeys";
|
import { qk } from "../lib/queryKeys";
|
||||||
import { api, type PlexCard, type PlexDiscover as PlexDiscoverData } from "../lib/api";
|
import { api, type PlexCard, type PlexDiscover as PlexDiscoverData } from "../lib/api";
|
||||||
|
import { hueOf, hueTileGradient } from "../lib/hueTile";
|
||||||
import { plexLang } from "../i18n";
|
import { plexLang } from "../i18n";
|
||||||
import { toggleWatchState } from "../lib/plexWatch";
|
import { toggleWatchState } from "../lib/plexWatch";
|
||||||
import { usePlex } from "./PlexProvider";
|
import { usePlex } from "./PlexProvider";
|
||||||
@@ -18,14 +19,6 @@ import { LoadingState, StateMessage } from "./QueryState";
|
|||||||
// info / watched-toggle handlers come from the shell (PlexBrowse) so a rail card behaves exactly like
|
// info / watched-toggle handlers come from the shell (PlexBrowse) so a rail card behaves exactly like
|
||||||
// a grid card; the watched-toggle optimistically flips THIS surface's cache too.
|
// a grid card; the watched-toggle optimistically flips THIS surface's cache too.
|
||||||
|
|
||||||
// A stable, pleasant hue per genre string (no art to collage yet — colored tiles stand in). Hashing
|
|
||||||
// keeps a genre the same color across renders/sessions without storing anything.
|
|
||||||
function genreHue(name: string): number {
|
|
||||||
let h = 0;
|
|
||||||
for (let i = 0; i < name.length; i++) h = (h * 31 + name.charCodeAt(i)) % 360;
|
|
||||||
return h;
|
|
||||||
}
|
|
||||||
|
|
||||||
function CardRail({
|
function CardRail({
|
||||||
title,
|
title,
|
||||||
ariaLabel,
|
ariaLabel,
|
||||||
@@ -99,7 +92,7 @@ export default function PlexDiscover({
|
|||||||
});
|
});
|
||||||
// Surprise-me is its own query so the ↻ button can redraw it without refetching the whole surface.
|
// Surprise-me is its own query so the ↻ button can redraw it without refetching the whole surface.
|
||||||
const surpriseQ = useQuery({
|
const surpriseQ = useQuery({
|
||||||
queryKey: qk.plexSurprise(scope, filters),
|
queryKey: qk.plexSurprise(scope, lang, filters),
|
||||||
queryFn: () => api.plexSurprise(scope, undefined, filters, lang),
|
queryFn: () => api.plexSurprise(scope, undefined, filters, lang),
|
||||||
staleTime: 0,
|
staleTime: 0,
|
||||||
});
|
});
|
||||||
@@ -124,7 +117,7 @@ export default function PlexDiscover({
|
|||||||
: old,
|
: old,
|
||||||
);
|
);
|
||||||
qc.setQueryData<{ scope: string; items: PlexCard[] }>(
|
qc.setQueryData<{ scope: string; items: PlexCard[] }>(
|
||||||
qk.plexSurprise(scope, filters),
|
qk.plexSurprise(scope, lang, filters),
|
||||||
(old) => (old ? { ...old, items: flip(old.items) } : old),
|
(old) => (old ? { ...old, items: flip(old.items) } : old),
|
||||||
);
|
);
|
||||||
},
|
},
|
||||||
@@ -207,16 +200,14 @@ export default function PlexDiscover({
|
|||||||
<h2 className="mb-3 text-sm font-semibold">{t("plex.discover.byGenre")}</h2>
|
<h2 className="mb-3 text-sm font-semibold">{t("plex.discover.byGenre")}</h2>
|
||||||
<div className="grid grid-cols-2 gap-3 sm:grid-cols-3 md:grid-cols-4 lg:grid-cols-6">
|
<div className="grid grid-cols-2 gap-3 sm:grid-cols-3 md:grid-cols-4 lg:grid-cols-6">
|
||||||
{d.genres.map((g) => {
|
{d.genres.map((g) => {
|
||||||
const hue = genreHue(g.value);
|
const hue = hueOf(g.value);
|
||||||
return (
|
return (
|
||||||
<button
|
<button
|
||||||
key={g.value}
|
key={g.value}
|
||||||
type="button"
|
type="button"
|
||||||
onClick={() => onGenre(g.value)}
|
onClick={() => onGenre(g.value)}
|
||||||
className="glass-hover relative flex h-20 items-end overflow-hidden rounded-xl p-3 text-left transition"
|
className="glass-hover relative flex h-20 items-end overflow-hidden rounded-xl p-3 text-left transition"
|
||||||
style={{
|
style={{ background: hueTileGradient(hue) }}
|
||||||
background: `linear-gradient(135deg, hsl(${hue} 55% 32%), hsl(${(hue + 40) % 360} 60% 22%))`,
|
|
||||||
}}
|
|
||||||
>
|
>
|
||||||
<span className="text-sm font-semibold text-white drop-shadow">{g.value}</span>
|
<span className="text-sm font-semibold text-white drop-shadow">{g.value}</span>
|
||||||
<span className="absolute right-2 top-2 text-2xs text-white/70">{g.count}</span>
|
<span className="absolute right-2 top-2 text-2xs text-white/70">{g.count}</span>
|
||||||
|
|||||||
@@ -140,10 +140,13 @@ export function SavedViewsProvider({ children }: { children: ReactNode }) {
|
|||||||
setModalOpen(false);
|
setModalOpen(false);
|
||||||
}, [source]);
|
}, [source]);
|
||||||
|
|
||||||
// Feed only: keep the default-view mirror (read on load) in sync with the server state.
|
// Feed only: keep the default-view mirror (read on load) in sync with the server state. Depend on the
|
||||||
|
// mirror fn (a module-stable ref, or undefined for plex) NOT the whole `cfg` object — cfg re-memoizes
|
||||||
|
// on every live filter edit, which would fire this synchronous localStorage write far beyond its intent.
|
||||||
|
const syncMirror = cfg.syncDefaultMirror;
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (viewsQuery.data) cfg.syncDefaultMirror?.(viewsQuery.data);
|
if (viewsQuery.data) syncMirror?.(viewsQuery.data);
|
||||||
}, [viewsQuery.data, cfg]);
|
}, [viewsQuery.data, syncMirror]);
|
||||||
|
|
||||||
const currentKey = cfg.keyOf(cfg.filters);
|
const currentKey = cfg.keyOf(cfg.filters);
|
||||||
const activeView = useMemo(
|
const activeView = useMemo(
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { useQuery } from "@tanstack/react-query";
|
|||||||
import { Layers, ListMusic } from "lucide-react";
|
import { Layers, ListMusic } from "lucide-react";
|
||||||
import { qk } from "../lib/queryKeys";
|
import { qk } from "../lib/queryKeys";
|
||||||
import { api } from "../lib/api";
|
import { api } from "../lib/api";
|
||||||
|
import { hueOf, hueTileGradient } from "../lib/hueTile";
|
||||||
import { useDebounced } from "../lib/useDebounced";
|
import { useDebounced } from "../lib/useDebounced";
|
||||||
import { LoadingState, StateMessage } from "./QueryState";
|
import { LoadingState, StateMessage } from "./QueryState";
|
||||||
|
|
||||||
@@ -13,13 +14,6 @@ import { LoadingState, StateMessage } from "./QueryState";
|
|||||||
// union), so this is a pure front-end surface. Tiles are coloured by a title hash (no per-item art to
|
// union), so this is a pure front-end surface. Tiles are coloured by a title hash (no per-item art to
|
||||||
// fetch/proxy), matching the Discover genre tiles.
|
// fetch/proxy), matching the Discover genre tiles.
|
||||||
|
|
||||||
// A stable, pleasant hue per title string — same idea as PlexDiscover's genre tiles.
|
|
||||||
function hueOf(name: string): number {
|
|
||||||
let h = 0;
|
|
||||||
for (let i = 0; i < name.length; i++) h = (h * 31 + name.charCodeAt(i)) % 360;
|
|
||||||
return h;
|
|
||||||
}
|
|
||||||
|
|
||||||
function Tile({
|
function Tile({
|
||||||
title,
|
title,
|
||||||
count,
|
count,
|
||||||
@@ -38,9 +32,7 @@ function Tile({
|
|||||||
onClick={onClick}
|
onClick={onClick}
|
||||||
title={title}
|
title={title}
|
||||||
className="glass-hover group relative flex h-28 flex-col justify-end overflow-hidden rounded-xl p-3 text-left transition"
|
className="glass-hover group relative flex h-28 flex-col justify-end overflow-hidden rounded-xl p-3 text-left transition"
|
||||||
style={{
|
style={{ background: hueTileGradient(hue) }}
|
||||||
background: `linear-gradient(135deg, hsl(${hue} 55% 32%), hsl(${(hue + 40) % 360} 60% 22%))`,
|
|
||||||
}}
|
|
||||||
>
|
>
|
||||||
<span className="absolute right-2 top-2 text-white/70">{icon}</span>
|
<span className="absolute right-2 top-2 text-white/70">{icon}</span>
|
||||||
<span className="line-clamp-2 text-sm font-semibold text-white drop-shadow">{title}</span>
|
<span className="line-clamp-2 text-sm font-semibold text-white drop-shadow">{title}</span>
|
||||||
@@ -98,9 +90,11 @@ export function PlexCollectionsBrowse({ onPick }: { onPick: (id: string, title:
|
|||||||
return (
|
return (
|
||||||
<div>
|
<div>
|
||||||
<input
|
<input
|
||||||
|
type="search"
|
||||||
value={search}
|
value={search}
|
||||||
onChange={(e) => setSearch(e.target.value)}
|
onChange={(e) => setSearch(e.target.value)}
|
||||||
placeholder={t("plex.filter.collectionSearch")}
|
placeholder={t("plex.filter.collectionSearch")}
|
||||||
|
aria-label={t("plex.filter.collectionSearch")}
|
||||||
className="glass-card mb-4 w-full max-w-sm rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40"
|
className="glass-card mb-4 w-full max-w-sm rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40"
|
||||||
/>
|
/>
|
||||||
{q.isError && !q.data ? (
|
{q.isError && !q.data ? (
|
||||||
|
|||||||
@@ -121,7 +121,15 @@ export function buildPlexActiveFilters(
|
|||||||
out.push({
|
out.push({
|
||||||
key: `collection:${c}`,
|
key: `collection:${c}`,
|
||||||
label: filters.collectionTitles?.[c] ?? t("plex.filter.collection"),
|
label: filters.collectionTitles?.[c] ?? t("plex.filter.collection"),
|
||||||
remove: () => patch({ collections: filters.collections.filter((x) => x !== c) }),
|
// 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).
|
||||||
|
remove: () => {
|
||||||
|
const { [c]: _drop, ...restTitles } = filters.collectionTitles ?? {};
|
||||||
|
patch({
|
||||||
|
collections: filters.collections.filter((x) => x !== c),
|
||||||
|
collectionTitles: restTitles,
|
||||||
|
});
|
||||||
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
if (show !== "all") {
|
if (show !== "all") {
|
||||||
|
|||||||
@@ -0,0 +1,66 @@
|
|||||||
|
// @vitest-environment jsdom
|
||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import {
|
||||||
|
coercePlexFilters,
|
||||||
|
plexFiltersSignature,
|
||||||
|
EMPTY_PLEX_FILTERS,
|
||||||
|
type PlexFilters,
|
||||||
|
} from "./plex";
|
||||||
|
|
||||||
|
describe("coercePlexFilters", () => {
|
||||||
|
it("migrates the legacy single collection/collectionTitle into the multi-select shape", () => {
|
||||||
|
const out = coercePlexFilters({ collection: "A", collectionTitle: "Marvel" });
|
||||||
|
expect(out.collections).toEqual(["A"]);
|
||||||
|
expect(out.collectionTitles).toEqual({ A: "Marvel" });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("migrates a legacy collection with no title", () => {
|
||||||
|
const out = coercePlexFilters({ collection: "A" });
|
||||||
|
expect(out.collections).toEqual(["A"]);
|
||||||
|
expect(out.collectionTitles).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves an already-multi collections blob untouched (guard on !collections.length)", () => {
|
||||||
|
const out = coercePlexFilters({ collections: ["X", "Y"], collection: "A" });
|
||||||
|
expect(out.collections).toEqual(["X", "Y"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fills every default for an empty blob", () => {
|
||||||
|
expect(coercePlexFilters({})).toEqual(EMPTY_PLEX_FILTERS);
|
||||||
|
expect(coercePlexFilters({}).collections).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("drops the legacy sortDir field", () => {
|
||||||
|
const out = coercePlexFilters({ sortDir: "desc", genres: ["Drama"] });
|
||||||
|
expect((out as unknown as Record<string, unknown>).sortDir).toBeUndefined();
|
||||||
|
expect(out.genres).toEqual(["Drama"]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("plexFiltersSignature", () => {
|
||||||
|
const base = (): PlexFilters => ({
|
||||||
|
...EMPTY_PLEX_FILTERS,
|
||||||
|
genres: ["Drama"],
|
||||||
|
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"] };
|
||||||
|
expect(plexFiltersSignature(a)).toBe(plexFiltersSignature(b));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores display-only collectionTitles (not part of the wire params)", () => {
|
||||||
|
const a = base();
|
||||||
|
const b: PlexFilters = { ...base(), collectionTitles: { A: "Marvel" } };
|
||||||
|
expect(plexFiltersSignature(a)).toBe(plexFiltersSignature(b));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("differs when a real filter differs", () => {
|
||||||
|
const a = base();
|
||||||
|
const b: PlexFilters = { ...base(), genres: ["Comedy"] };
|
||||||
|
expect(plexFiltersSignature(a)).not.toBe(plexFiltersSignature(b));
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,13 @@
|
|||||||
|
// A stable, pleasant hue per string (genre/playlist/collection title) plus the matching tile gradient.
|
||||||
|
// Shared by the Discover genre tiles (PlexDiscover) and the header Playlist/Collection browse tiles
|
||||||
|
// (plexBrowseLists) so the two surfaces that are meant to look identical can't drift. Hashing keeps a
|
||||||
|
// name the same color across renders/sessions without storing anything.
|
||||||
|
export function hueOf(name: string): number {
|
||||||
|
let h = 0;
|
||||||
|
for (let i = 0; i < name.length; i++) h = (h * 31 + name.charCodeAt(i)) % 360;
|
||||||
|
return h;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function hueTileGradient(hue: number): string {
|
||||||
|
return `linear-gradient(135deg, hsl(${hue} 55% 32%), hsl(${(hue + 40) % 360} 60% 22%))`;
|
||||||
|
}
|
||||||
@@ -99,7 +99,8 @@ export const qk = {
|
|||||||
// both for a prefix invalidation after a watched-toggle. Surprise is its own key so the ↻ reshuffle
|
// both for a prefix invalidation after a watched-toggle. Surprise is its own key so the ↻ reshuffle
|
||||||
// refetches it alone.
|
// refetches it alone.
|
||||||
plexDiscover: (...rest: unknown[]) => ["plex-discover", ...rest],
|
plexDiscover: (...rest: unknown[]) => ["plex-discover", ...rest],
|
||||||
// Variadic like plexDiscover so it can key by scope + the active filters; `plexSurprise()` (no args)
|
// Variadic like plexDiscover so it can key by scope + lang + the active filters (lang because a genre
|
||||||
|
// filter narrows against the language-specific genre column, like the grid); `plexSurprise()` (no args)
|
||||||
// stays a prefix for the reshuffle/watched-toggle invalidations.
|
// stays a prefix for the reshuffle/watched-toggle invalidations.
|
||||||
plexSurprise: (...rest: unknown[]) => ["plex-surprise", ...rest],
|
plexSurprise: (...rest: unknown[]) => ["plex-surprise", ...rest],
|
||||||
plexSuggest: (...rest: unknown[]) => ["plex-suggest", ...rest],
|
plexSuggest: (...rest: unknown[]) => ["plex-suggest", ...rest],
|
||||||
|
|||||||
Reference in New Issue
Block a user