From 9f8e410033f56e5ea054c022ad417df7d518c587 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 11 Jul 2026 02:25:14 +0200 Subject: [PATCH] fix(plex): address code-review findings (F1-F3, F5, F9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1: refresh grid after playback/collection-edit — invalidate the renamed ["plex-library"] query key in PlexPlayer + PlexCollectionEditor (was stale ["plex-browse"], a dead no-op) - F2: exclude hidden movies from facet counts/bounds — the movie facet base now always joins watch-state and mirrors unified_library's status branches, so facets match the visible grid exactly - F3: show_detail/item_detail live-Plex enrichment degrades instead of 500ing — broaden the best-effort block to except Exception - F5: don't re-run the heavy facet computation on sort-direction toggles — key the facets query only on fields plexFacets actually sends - F9: optimistic watched-toggle also updates the search Episodes section and gives toggleEpisodeWatched an optimistic path (shared optimisticStatus helper) --- backend/app/routes/plex.py | 29 +++++++++++-------- frontend/src/components/PlexBrowse.tsx | 19 ++++++++---- .../src/components/PlexCollectionEditor.tsx | 2 +- frontend/src/components/PlexPlayer.tsx | 2 +- frontend/src/components/PlexSidebar.tsx | 6 +++- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/backend/app/routes/plex.py b/backend/app/routes/plex.py index 825cc8e..22afe1a 100644 --- a/backend/app/routes/plex.py +++ b/backend/app/routes/plex.py @@ -798,15 +798,18 @@ def facets( query = query.filter(model.duration_s >= pp["duration_min"]) if with_dur and pp.get("duration_max") is not None: query = query.filter(model.duration_s <= pp["duration_max"]) - if show in ("watched", "in_progress", "unwatched"): - st = aliased(PlexState) - query = query.outerjoin(st, and_(st.item_id == model.id, st.user_id == uid)) - if show == "watched": - query = query.filter(st.status == "watched") - elif show == "in_progress": - query = query.filter(st.position_seconds > 0, or_(st.status.is_(None), st.status != "watched")) - else: - query = query.filter(or_(st.status.is_(None), st.status.notin_(["watched", "hidden"]))) + # Always join watch-state and mirror the grid's status branches (unified_library) exactly — + # hidden movies never appear in the grid, so they must not inflate facet counts/bounds either. + st = aliased(PlexState) + query = query.outerjoin(st, and_(st.item_id == model.id, st.user_id == uid)) + if show == "watched": + query = query.filter(st.status == "watched") + elif show == "in_progress": + query = query.filter(st.position_seconds > 0, or_(st.status.is_(None), st.status != "watched")) + elif show == "unwatched": + query = query.filter(or_(st.status.is_(None), st.status.notin_(["watched", "hidden"]))) + else: + query = query.filter(or_(st.status.is_(None), st.status != "hidden")) else: if show in ("watched", "in_progress", "unwatched"): agg = _show_agg_subq(db, uid).subquery() @@ -1389,7 +1392,9 @@ def show_detail( meta = plex.metadata(sh.rating_key) or {} rich = _rich_meta(meta) related = _related_show_cards(db, user.id, plex.related(sh.rating_key), exclude=sh.id) - except (PlexError, PlexNotConfigured): + except Exception: + # Best-effort enrichment only — the local episode list is already assembled, so any live-Plex + # failure (not configured, HTTP/JSON shape, _rich_meta parse error) must degrade, not 500. pass return { @@ -1943,8 +1948,8 @@ def item_detail( "text": codec not in _BITMAP_SUBS, } ) - except (PlexError, PlexNotConfigured): - pass # metadata extras are best-effort; playback works without them + except Exception: + pass # metadata extras are best-effort; any live-Plex failure must degrade, not 500 — playback works without them prev_id, next_id = _episode_nav(db, it) show = db.get(PlexShow, it.show_id) if it.kind == "episode" and it.show_id else None diff --git a/frontend/src/components/PlexBrowse.tsx b/frontend/src/components/PlexBrowse.tsx index 5b4e478..ba67459 100644 --- a/frontend/src/components/PlexBrowse.tsx +++ b/frontend/src/components/PlexBrowse.tsx @@ -188,26 +188,33 @@ export default function PlexBrowse({ infoScrollRef.current = 0; // fresh info page starts at the top sub.open({ kind: "info", id: card.id }); } - async function toggleWatched(card: PlexCard) { - const next = card.status === "watched" ? "new" : "watched"; - // Optimistically flip the card in every cached library page so the quick toggle reacts instantly - // and reliably BOTH ways (mark and un-mark), then reconcile with the server. + // Optimistically flip a card's status in every cached library page — both the title grid (`items`) + // and the search "Episodes" section (`episodes`) — so the quick toggle reacts instantly and reliably + // BOTH ways (mark and un-mark). An id matches in only one array, so touching both is safe. + function optimisticStatus(id: string, next: string) { qc.setQueriesData>({ queryKey: ["plex-library"] }, (old) => old ? { ...old, pages: old.pages.map((pg) => ({ ...pg, - items: pg.items.map((it) => (it.id === card.id ? { ...it, status: next } : it)), + items: pg.items.map((it) => (it.id === id ? { ...it, status: next } : it)), + episodes: pg.episodes.map((ep) => (ep.id === id ? { ...ep, status: next } : ep)), })), } : old, ); + } + async function toggleWatched(card: PlexCard) { + const next = card.status === "watched" ? "new" : "watched"; + optimisticStatus(card.id, next); await api.plexSetState(card.id, next).catch(() => {}); qc.invalidateQueries({ queryKey: ["plex-library"] }); } async function toggleEpisodeWatched(ep: PlexCard) { - await api.plexSetState(ep.id, ep.status === "watched" ? "new" : "watched").catch(() => {}); + const next = ep.status === "watched" ? "new" : "watched"; + optimisticStatus(ep.id, next); + await api.plexSetState(ep.id, next).catch(() => {}); qc.invalidateQueries({ queryKey: ["plex-library"] }); } // Apply a metadata filter (clicked on an info page / show hero / cast) then show the unified grid. diff --git a/frontend/src/components/PlexCollectionEditor.tsx b/frontend/src/components/PlexCollectionEditor.tsx index 0f300b1..fe657e6 100644 --- a/frontend/src/components/PlexCollectionEditor.tsx +++ b/frontend/src/components/PlexCollectionEditor.tsx @@ -50,7 +50,7 @@ export default function PlexCollectionEditor({ const refresh = () => { qc.invalidateQueries({ queryKey: ["plex-collections"] }); qc.invalidateQueries({ queryKey: ["plex-item", item.id] }); - qc.invalidateQueries({ queryKey: ["plex-browse"] }); + qc.invalidateQueries({ queryKey: ["plex-library"] }); onChanged(); }; diff --git a/frontend/src/components/PlexPlayer.tsx b/frontend/src/components/PlexPlayer.tsx index 4fb7db2..68e4ddd 100644 --- a/frontend/src/components/PlexPlayer.tsx +++ b/frontend/src/components/PlexPlayer.tsx @@ -489,7 +489,7 @@ export default function PlexPlayer({ itemId, onClose, queue }: Props) { old ? { ...old, position_seconds: pos } : old, ); } - qc.invalidateQueries({ queryKey: ["plex-browse"] }); + qc.invalidateQueries({ queryKey: ["plex-library"] }); qc.invalidateQueries({ queryKey: ["plex-show"] }); }; }, [id, qc]); diff --git a/frontend/src/components/PlexSidebar.tsx b/frontend/src/components/PlexSidebar.tsx index 28bbfce..bfbb7b9 100644 --- a/frontend/src/components/PlexSidebar.tsx +++ b/frontend/src/components/PlexSidebar.tsx @@ -64,8 +64,12 @@ export default function PlexSidebar({ } // Facet values (genres/ratings/bounds) for the current scope, ADAPTED to the active filters // (faceted search) — so picking one filter narrows what the others offer. Refetches on any change. + // Key only on the fields plexFacets actually sends — sortDir/collectionTitle don't reach /facets, + // so keying on the whole `filters` object would re-run the heavy facet computation on every + // sort-direction toggle (or chip-title change) for an identical request. + const { sortDir: _sd, collectionTitle: _ct, ...facetKey } = filters; const facetsQ = useQuery({ - queryKey: ["plex-facets", scope, show, filters], + queryKey: ["plex-facets", scope, show, facetKey], queryFn: () => api.plexFacets(scope, filters, show), enabled: !!scope, placeholderData: (prev) => prev, // keep the old chips visible during the refetch (no flicker)