diff --git a/backend/app/routes/plex/browse.py b/backend/app/routes/plex/browse.py index ba0b5d90..d5586fb1 100644 --- a/backend/app/routes/plex/browse.py +++ b/backend/app/routes/plex/browse.py @@ -1607,6 +1607,12 @@ def people_cards( return {"people": out} +# Cap each filmography section — a prolific person shouldn't return an unbounded list the client then +# mounts a card per row for (the grid paginates at PAGE; this page renders all at once). Ample for a +# personal library; the most-recent titles win via the year-desc order. +_PERSON_SECTION_MAX = 200 + + @router.get("/person/{name}") def person_detail( name: str, @@ -1615,8 +1621,9 @@ def person_detail( ) -> dict: """One cast/crew member's filmography: their cached headshot + the movies and shows (in the enabled libraries) whose cast OR directors include this name — the inverse of the cast-name → library-filter - link. The name string is the identity key (Plex cast carries no stable per-person id). An unknown - name simply returns empty sections.""" + link. The name string is the identity key (Plex cast carries no stable per-person id), so distinct + people who share a name share a page. Each section is capped at _PERSON_SECTION_MAX. An unknown name + simply returns empty sections.""" photo = db.scalar(select(PlexPerson.photo_thumb).where(PlexPerson.name == name)) movie_lib_ids, show_lib_ids = _content_libs(db) @@ -1633,6 +1640,7 @@ def person_detail( ), ) .order_by(PlexItem.year.desc().nullslast(), func.lower(PlexItem.title)) + .limit(_PERSON_SECTION_MAX) ).scalars().all() item_ids = [it.id for it in items] states = { @@ -1658,6 +1666,7 @@ def person_detail( ), ) .order_by(PlexShow.year.desc().nullslast(), func.lower(PlexShow.title)) + .limit(_PERSON_SECTION_MAX) ).scalars().all() statuses = rollup.state_map(db, user.id, [s.id for s in show_rows]) shows = [_show_card(sh, statuses.get(sh.id, "new")) for sh in show_rows] diff --git a/frontend/src/components/PlexBrowse.tsx b/frontend/src/components/PlexBrowse.tsx index f3c4d55d..81eb2b12 100644 --- a/frontend/src/components/PlexBrowse.tsx +++ b/frontend/src/components/PlexBrowse.tsx @@ -14,6 +14,7 @@ import { healPlexSort, plexSortOptions, PLEX_SORT_FALLBACK } from "../lib/plexSo import SortControl from "./SortControl"; import { createPortal } from "react-dom"; import { useTranslation } from "react-i18next"; +import type { TFunction } from "i18next"; import { useInfiniteQuery, useQuery, @@ -88,6 +89,25 @@ type Sub = const PAGE = 40; +// Persist a watch-state toggle from any Plex surface: flip via the caller's `optimistic` cache +// updater, call the API, surface a failure (the click used to look dead on 403/404/429), then +// `resync` (invalidate) — which corrects the optimistic flip to truth either way. Shared by the +// browse grid and the person page, each of which owns a different cache shape. +async function toggleWatchState( + card: PlexCard, + t: TFunction, + handlers: { optimistic: (next: string) => void; resync: () => void }, +) { + const next = card.status === "watched" ? "new" : "watched"; + handlers.optimistic(next); + try { + await api.plexSetState(card.id, next); + } catch (e) { + notifyActionError(e, t("plex.watchedFailed")); + } + handlers.resync(); +} + export default function PlexBrowse() { const { t } = useTranslation(); const qc = useQueryClient(); @@ -123,6 +143,7 @@ export default function PlexBrowse() { // info page like the card's ⓘ, a show's drill-down), then clear the hand-off so it doesn't re-open. useEffect(() => { if (jumpTo != null) { + if (jumpTo.type !== "show") infoScrollRef.current = 0; // a jumped-to info page starts at the top sub.open( jumpTo.type === "show" ? { kind: "show", id: jumpTo.id } : { kind: "info", id: jumpTo.id }, ); @@ -235,10 +256,12 @@ export default function PlexBrowse() { infoScrollRef.current = 0; // fresh info page starts at the top sub.open({ kind: "info", id: card.id }); } - // Open a cast/crew member's filmography page (from an info/show cast row). Remember the info-page - // scroll so browser Back to a cast row lands where it was, mirroring applyFilter's restore. + // Open a cast/crew member's filmography page (from an info/show cast row, or an actor filter-chip). + // Only when leaving an INFO page do we stash its scroll (so browser Back to that cast row lands where + // it was, like applyFilter) — from a show/grid context infoScrollRef must stay untouched, else a + // later info page would wrongly restore this stale offset. function onOpenPerson(name: string) { - infoScrollRef.current = scroller()?.scrollTop ?? 0; + if (sub.view.kind === "info") infoScrollRef.current = scroller()?.scrollTop ?? 0; sub.open({ kind: "person", name }); } // Optimistically flip a card's status in every cached library page — both the title grid (`items`) @@ -258,18 +281,11 @@ export default function PlexBrowse() { : old, ); } - async function toggleWatched(card: PlexCard) { - const next = card.status === "watched" ? "new" : "watched"; - optimisticStatus(card.id, next); - // On failure surface the server's reason (403/404/429) — the toggle used to fail silently, so the - // click looked dead. The invalidate below runs either way, resyncing the optimistic flip to truth. - try { - await api.plexSetState(card.id, next); - } catch (e) { - notifyActionError(e, t("plex.watchedFailed")); - } - qc.invalidateQueries({ queryKey: qk.plexLibrary() }); - } + const toggleWatched = (card: PlexCard) => + toggleWatchState(card, t, { + optimistic: (next) => optimisticStatus(card.id, next), + resync: () => qc.invalidateQueries({ queryKey: qk.plexLibrary() }), + }); // Apply a metadata filter (clicked on an info page / show hero / cast) then show the unified grid. // Array filters (genres/people/studios) UNION with what's set; scalars replace. Clicking a person // (cast/crew) widens to the 'both' scope so their movies AND shows show up together (mixed feed). @@ -715,7 +731,7 @@ function PlexInfoView({ // The inverse of the cast-name → library-filter link: opened from a cast row (info/show page), it // shows the person's headshot + name, a "filter the library by them" action, and their titles as two // poster-card sections (Movies / Shows). Mirrors PlexInfoView's fetch-wrapper + glass-hero shape. -function PlexPersonView({ +export function PlexPersonView({ name, onBack, onOpenCard, @@ -733,27 +749,25 @@ function PlexPersonView({ const q = useQuery({ queryKey: qk.plexPerson(name), queryFn: () => api.plexPerson(name) }); // Quick watch-toggle from a filmography card: optimistically flip in THIS person's cached payload - // (both the movies and shows arrays — an id matches only one), then persist + resync. Mirrors the - // grid's optimisticStatus on the person-detail shape; also invalidates the library so its grid agrees. - async function toggle(card: PlexCard) { - const next = card.status === "watched" ? "new" : "watched"; - qc.setQueryData(qk.plexPerson(name), (old) => - old - ? { - ...old, - movies: old.movies.map((m) => (m.id === card.id ? { ...m, status: next } : m)), - shows: old.shows.map((s) => (s.id === card.id ? { ...s, status: next } : s)), - } - : old, - ); - try { - await api.plexSetState(card.id, next); - } catch (e) { - notifyActionError(e, t("plex.watchedFailed")); - } - qc.invalidateQueries({ queryKey: qk.plexPerson(name) }); - qc.invalidateQueries({ queryKey: qk.plexLibrary() }); - } + // (both the movies and shows arrays — an id matches only one), then persist + resync via the shared + // helper; also invalidate the library so its grid agrees. + const toggle = (card: PlexCard) => + toggleWatchState(card, t, { + optimistic: (next) => + qc.setQueryData(qk.plexPerson(name), (old) => + old + ? { + ...old, + movies: old.movies.map((m) => (m.id === card.id ? { ...m, status: next } : m)), + shows: old.shows.map((s) => (s.id === card.id ? { ...s, status: next } : s)), + } + : old, + ), + resync: () => { + qc.invalidateQueries({ queryKey: qk.plexPerson(name) }); + qc.invalidateQueries({ queryKey: qk.plexLibrary() }); + }, + }); const section = (titleKey: string, cards: PlexCard[]) => cards.length > 0 ? ( @@ -985,7 +999,7 @@ function SeasonCard({ ); } -function CastStrip({ +export function CastStrip({ cast, onFilter, onOpenPerson, diff --git a/frontend/src/components/PlexPersonView.dom.test.tsx b/frontend/src/components/PlexPersonView.dom.test.tsx new file mode 100644 index 00000000..df255b2e --- /dev/null +++ b/frontend/src/components/PlexPersonView.dom.test.tsx @@ -0,0 +1,111 @@ +// @vitest-environment jsdom +// +// E5 S4: the actor detail page. Covers the two pieces the /code-review flagged as untested — the +// cast-row rewiring (a cast click now OPENS the person page via onOpenPerson, falling back to the old +// filter gesture only when no onOpenPerson is wired) and PlexPersonView's render (headshot/name hero, +// the Movies/Shows sections, and the "filter library by this person" action). i18n is key-passthrough +// and the api is mocked so the query resolves synchronously in the test. +import { describe, it, expect, afterEach, vi } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useTranslation: () => ({ t: (key: string) => key }) }; +}); + +const plexPerson = vi.fn(); +const plexSetState = vi.fn(() => Promise.resolve()); +vi.mock("../lib/api", () => ({ + api: { + plexPerson: (...a: unknown[]) => plexPerson(...a), + plexSetState: (...a: unknown[]) => plexSetState(...a), + }, + EMPTY_PLEX_FILTERS: {}, + plexFilterCount: () => 0, +})); + +import { CastStrip, PlexPersonView } from "./PlexBrowse"; + +afterEach(cleanup); + +const card = (id: string, title: string, type: "movie" | "show" = "movie") => ({ + id, + type, + title, + year: 2000, + duration_seconds: 3600, + thumb: `/api/plex/image/${id}`, + playable: "direct", + status: "new", + position_seconds: 0, +}); + +function wrap(ui: React.ReactElement) { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render({ui}); +} + +describe("CastStrip cast click", () => { + it("opens the person page (onOpenPerson) rather than filtering, when both are wired", () => { + const onOpenPerson = vi.fn(); + const onFilter = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByTitle("plex.info.viewPerson")); + expect(onOpenPerson).toHaveBeenCalledWith("Jane Doe"); + expect(onFilter).not.toHaveBeenCalled(); + }); + + it("falls back to the filter gesture when only onFilter is wired (e.g. the overlay)", () => { + const onFilter = vi.fn(); + render(); + fireEvent.click(screen.getByTitle("plex.info.filterActor")); + expect(onFilter).toHaveBeenCalledWith({ actors: ["Jane Doe"] }); + }); +}); + +describe("PlexPersonView", () => { + it("renders the Movies + Shows sections and the filter-by-person action", async () => { + plexPerson.mockResolvedValue({ + name: "Jane Doe", + photo: null, + movies: [card("m1", "Movie One")], + shows: [card("s1", "Show One", "show")], + }); + const onFilter = vi.fn(); + wrap( + {}} + onOpenCard={() => {}} + onInfoCard={() => {}} + onFilter={onFilter} + />, + ); + // Both section grids render their cards once the mocked query resolves. + await screen.findByText("Movie One"); + expect(screen.getByText("Show One")).toBeTruthy(); + expect(screen.getByText("plex.person.movies")).toBeTruthy(); // section headings (key passthrough) + expect(screen.getByText("plex.person.shows")).toBeTruthy(); + + fireEvent.click(screen.getByText("plex.person.filterBy")); + expect(onFilter).toHaveBeenCalledWith({ actors: ["Jane Doe"] }); + expect(plexPerson).toHaveBeenCalledWith("Jane Doe"); + }); + + it("shows the empty message when the person has no titles", async () => { + plexPerson.mockResolvedValue({ name: "Nobody", photo: null, movies: [], shows: [] }); + wrap( + {}} + onOpenCard={() => {}} + onInfoCard={() => {}} + onFilter={() => {}} + />, + ); + await screen.findByText("plex.person.empty"); + }); +});