diff --git a/backend/app/plex/rollup.py b/backend/app/plex/rollup.py index 9994df06..11e173fd 100644 --- a/backend/app/plex/rollup.py +++ b/backend/app/plex/rollup.py @@ -42,27 +42,32 @@ def rollup(cards: list[dict]) -> dict: } -def agg_subquery(db: Session, user_id: int): +def agg_subquery(db: Session, user_id: int, include_resumed: bool = False): """Per-show episode counts for a user: total, watched, in-progress. Reused to (a) filter the show - grid by aggregate watch-state and (b) compute each card's badge.""" + grid by aggregate watch-state, (b) compute each card's badge, and (c) drive the Discover + continue-watching rail. `include_resumed` adds `resumed_at` = max(progress_updated_at) across the + show's episodes, so the rail can order shows by how recently they were last progressed.""" ps = aliased(PlexState) - return ( - select( - PlexItem.show_id.label("show_id"), - func.count(PlexItem.id).label("total"), - func.count(case((ps.status == "watched", 1))).label("watched"), - func.count( - case( - ( - and_( - ps.position_seconds > 0, - or_(ps.status.is_(None), ps.status != "watched"), - ), - 1, - ) + cols = [ + PlexItem.show_id.label("show_id"), + func.count(PlexItem.id).label("total"), + func.count(case((ps.status == "watched", 1))).label("watched"), + func.count( + case( + ( + and_( + ps.position_seconds > 0, + or_(ps.status.is_(None), ps.status != "watched"), + ), + 1, ) - ).label("inprog"), - ) + ) + ).label("inprog"), + ] + if include_resumed: + cols.append(func.max(ps.progress_updated_at).label("resumed_at")) + return ( + select(*cols) .outerjoin(ps, and_(ps.item_id == PlexItem.id, ps.user_id == user_id)) .where(PlexItem.kind == "episode", PlexItem.show_id.isnot(None)) .group_by(PlexItem.show_id) diff --git a/backend/app/plex/tmdb.py b/backend/app/plex/tmdb.py index cbc05c71..abf30f6d 100644 --- a/backend/app/plex/tmdb.py +++ b/backend/app/plex/tmdb.py @@ -434,13 +434,16 @@ def parse_person(payload: dict) -> dict: """TMDB /person/{id} (+combined_credits) → the persisted bio fields. `known_for` = the person's most-popular movie/tv credits (deduped by id), top `_KNOWN_FOR_MAX`, resolved to library later.""" credits = (payload.get("combined_credits") or {}).get("cast") or [] - seen: set[int] = set() + # Dedup on (id, media_type), not the bare id: TMDB movie ids and tv ids are independent sequences + # (id 603 can be both a movie and a show — see resolve_library_guids), so a same-id movie+show pair + # is two distinct credits and both must survive. + seen: set[tuple[int, str]] = set() known: list[dict] = [] for c in sorted(credits, key=lambda c: c.get("popularity") or 0, reverse=True): tid, mt = c.get("id"), c.get("media_type") - if not tid or mt not in ("movie", "tv") or tid in seen: + if not tid or mt not in ("movie", "tv") or (tid, mt) in seen: continue - seen.add(tid) + seen.add((tid, mt)) known.append({"tmdb_id": tid, "media_type": mt, "title": c.get("title") or c.get("name")}) if len(known) >= _KNOWN_FOR_MAX: break diff --git a/backend/app/routes/plex/discover.py b/backend/app/routes/plex/discover.py index 3bea64fc..65e2c695 100644 --- a/backend/app/routes/plex/discover.py +++ b/backend/app/routes/plex/discover.py @@ -10,15 +10,15 @@ card paths); the HU overlay coalesces only in the main grid. """ from fastapi import APIRouter, Depends, Query -from sqlalchemy import and_, case, func, or_, select -from sqlalchemy.orm import Session, aliased +from sqlalchemy import and_, func, or_, select +from sqlalchemy.orm import Session from app.auth import current_user from app.db import get_db from app.models import PlexItem, PlexShow, PlexState, User from app.plex import rollup from app.plex.cards import movie_card, show_card -from app.plex.query import genres_col +from app.plex.query import apply_show_state_filter, genres_col from app.plex.tmdb import resolve_recs from app.routes.plex._shared import _content_libs, _norm_lang @@ -123,39 +123,19 @@ def _continue_watching( rows.append((st.progress_updated_at, movie_card(it, st))) if show_ids: - ps = aliased(PlexState) - prog = ( - select( - PlexItem.show_id.label("show_id"), - func.max(ps.progress_updated_at).label("resumed_at"), - func.count(PlexItem.id).label("total"), - func.count(case((ps.status == "watched", 1))).label("watched"), - func.count( - case( - ( - and_( - ps.position_seconds > 0, - or_(ps.status.is_(None), ps.status != "watched"), - ), - 1, - ) - ) - ).label("inprog"), - ) - .outerjoin(ps, and_(ps.item_id == PlexItem.id, ps.user_id == uid)) - .where(PlexItem.kind == "episode", PlexItem.show_id.isnot(None)) - .group_by(PlexItem.show_id) - .subquery() - ) - # in-progress == started but not fully watched (mirrors apply_show_state_filter's `in_progress`). + # Reuse the shared per-show episode rollup (total/watched/inprog) rather than re-inlining it, plus + # its `resumed_at` = max(progress_updated_at); the in-progress predicate comes from the same shared + # `apply_show_state_filter` the grid uses, so the rail can't drift from the grid's definition. + prog = rollup.agg_subquery(db, uid, include_resumed=True).subquery() srows = db.execute( - select(PlexShow, prog.c.resumed_at) - .join(prog, prog.c.show_id == PlexShow.id) - .where( - PlexShow.library_id.in_(show_ids), - or_(prog.c.watched > 0, prog.c.inprog > 0), - prog.c.watched < prog.c.total, + apply_show_state_filter( + select(PlexShow, prog.c.resumed_at).join(prog, prog.c.show_id == PlexShow.id), + prog.c.total, + prog.c.watched, + prog.c.inprog, + "in_progress", ) + .where(PlexShow.library_id.in_(show_ids)) .order_by(prog.c.resumed_at.desc().nullslast()) .limit(RAIL_LIMIT) ).all() diff --git a/backend/app/routes/plex/people.py b/backend/app/routes/plex/people.py index cc7afea2..6370e373 100644 --- a/backend/app/routes/plex/people.py +++ b/backend/app/routes/plex/people.py @@ -1,6 +1,8 @@ """Plex people: cast/crew filter-chip cards + a single person's filmography page.""" import logging +import threading +import time from fastapi import APIRouter, BackgroundTasks, Depends from sqlalchemy import func, or_, select @@ -112,6 +114,32 @@ def people_cards( _PERSON_SECTION_MAX = 200 +# A person's bio is enriched at most once per this window: it dedups concurrent opens of the same person +# (each would otherwise schedule its own 8s TMDB task) AND rate-limits re-triggering when TMDB is +# down/slow (a transient failure leaves tmdb_enriched_at NULL, so without this every reload would spawn a +# fresh task and pile up on the shared threadpool). In-process + best-effort: a restart just clears it, at +# worst costing a few extra fetches. A successful enrich stamps the watermark, so the reservation only +# ever expires harmlessly. +_ENRICH_COOLDOWN_S = 60.0 +_enrich_inflight: dict[int, float] = {} +_enrich_lock = threading.Lock() + + +def _should_schedule_enrich(person_id: int) -> bool: + """True (and reserves a cooldown) if this person isn't already being enriched / recently attempted.""" + now = time.monotonic() + with _enrich_lock: + deadline = _enrich_inflight.get(person_id) + if deadline is not None and now < deadline: + return False + _enrich_inflight[person_id] = now + _ENRICH_COOLDOWN_S + # Bound the map: drop expired reservations once it grows (a large library has many people). + if len(_enrich_inflight) > 1024: + for pid in [p for p, d in _enrich_inflight.items() if d <= now]: + del _enrich_inflight[pid] + return True + + def _enrich_person_bg(person_id: int) -> None: """Fetch + persist one person's TMDB bio OFF the request path — a FastAPI background task that runs AFTER the response is sent, so the outbound TMDB call never sits on the serve latency (the module's @@ -150,7 +178,14 @@ def person_detail( """ person = db.execute(select(PlexPerson).where(PlexPerson.name == name)).scalars().first() # Lazy one-time bio enrich, scheduled AFTER the response so the TMDB call is never on the serve path. - if person is not None and person.tmdb_person_id and person.tmdb_enriched_at is None: + # Gated by a per-person cooldown so concurrent/rapid opens (or repeated opens while TMDB is degraded) + # can't pile background tasks onto the shared threadpool. + if ( + person is not None + and person.tmdb_person_id + and person.tmdb_enriched_at is None + and _should_schedule_enrich(person.id) + ): background_tasks.add_task(_enrich_person_bg, person.id) movie_lib_ids, show_lib_ids = _content_libs(db) diff --git a/backend/tests/test_plex_discover.py b/backend/tests/test_plex_discover.py index 8f9bbf25..bc77d2cd 100644 --- a/backend/tests/test_plex_discover.py +++ b/backend/tests/test_plex_discover.py @@ -140,6 +140,30 @@ def test_continue_watching_null_status_in_hidden_out(db, monkeypatch): assert "mh" not in ids # hidden is excluded +def test_continue_watching_excludes_fully_watched_show(db, monkeypatch): + # A show whose every episode is watched is DONE, not in-progress: the `watched < total` guard must + # keep it off the Continue-watching rail (a bare `watched > 0` would wrongly resurface it). + lib_s = PlexLibrary(plex_key="2", title="Shows", kind="show", enabled=True) + db.add(lib_s) + db.flush() + sh = PlexShow(rating_key="shw", library_id=lib_s.id, title="All Watched", child_count=1) + db.add(sh) + db.flush() + ep = PlexItem(rating_key="shwe1", library_id=lib_s.id, kind="episode", title="E1", + show_id=sh.id, season_number=1, episode_number=1) + db.add(ep) + db.flush() + user = User(email="fullywatched@example.com") + db.add(user) + db.flush() + db.add(PlexState(user_id=user.id, item_id=ep.id, status="watched", position_seconds=0, + progress_updated_at=NOW)) + db.commit() + + ids = _ids(_get(db, user, monkeypatch, "/api/plex/discover?scope=show").json()["continue_watching"]) + assert "shw" not in ids # fully watched → excluded from continue-watching + + def test_because_you_watched_resolves_in_library_recs(db, monkeypatch): user = _seed(db) d = _get(db, user, monkeypatch, "/api/plex/discover?scope=both").json() diff --git a/backend/tests/test_plex_person_bio.py b/backend/tests/test_plex_person_bio.py index fe93ce9e..329a79c5 100644 --- a/backend/tests/test_plex_person_bio.py +++ b/backend/tests/test_plex_person_bio.py @@ -47,6 +47,22 @@ def test_parse_person_empty_payload(): assert p["biography"] is None and p["known_for"] is None +def test_parse_person_keeps_same_id_across_media_types(): + # TMDB movie ids and tv ids are independent, so a movie-42 and a tv-42 credit are two distinct works + # and BOTH must survive the dedup (keyed on (id, media_type), not the bare id). + payload = { + "combined_credits": { + "cast": [ + {"id": 42, "media_type": "movie", "title": "Movie 42", "popularity": 90}, + {"id": 42, "media_type": "tv", "name": "Show 42", "popularity": 80}, + {"id": 42, "media_type": "movie", "title": "dup movie 42", "popularity": 5}, # real dup + ] + } + } + kf = plex_tmdb.parse_person(payload)["known_for"] + assert [(k["tmdb_id"], k["media_type"]) for k in kf] == [(42, "movie"), (42, "tv")] # both kept, dup dropped + + class _FakeTmdb: def __init__(self, payload): self.payload = payload @@ -73,6 +89,49 @@ def test_enrich_person_persists_and_stamps(db, monkeypatch): assert [k["tmdb_id"] for k in person.known_for] == [20, 10, 30] +class _FakeTmdbResult: + """Context-manager TmdbClient stand-in whose fetch_person raises / returns a fixed payload.""" + + def __init__(self, *, raises=False, payload=None): + self._raises = raises + self._payload = payload + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def fetch_person(self, pid): + if self._raises: + raise plex_tmdb.TmdbError("429 rate limited") + return self._payload + + +def test_enrich_person_transient_error_leaves_watermark_null(db, monkeypatch): + # A transient TmdbError must NOT stamp the watermark, so the next open retries (never permanently + # blank a person after one blip). + person = PlexPerson(name="Transient P", tmdb_person_id=555) + db.add(person) + db.commit() + monkeypatch.setattr(plex_tmdb.sysconfig, "get_str", lambda d, k: "key" if k == "tmdb_api_key" else "") + monkeypatch.setattr(plex_tmdb, "TmdbClient", lambda d, timeout=None: _FakeTmdbResult(raises=True)) + assert plex_tmdb.enrich_person(db, person) is False + assert person.tmdb_enriched_at is None # left NULL → retried next open + + +def test_enrich_person_404_stamps_watermark_without_bio(db, monkeypatch): + # A 404 (deleted person → None payload) stamps the watermark so it isn't refetched every open. + person = PlexPerson(name="Gone P", tmdb_person_id=555) + db.add(person) + db.commit() + monkeypatch.setattr(plex_tmdb.sysconfig, "get_str", lambda d, k: "key" if k == "tmdb_api_key" else "") + monkeypatch.setattr(plex_tmdb, "TmdbClient", lambda d, timeout=None: _FakeTmdbResult(payload=None)) + assert plex_tmdb.enrich_person(db, person) is True + assert person.tmdb_enriched_at is not None # stamped + assert person.biography is None # nothing fetched + + def test_enrich_person_noop_without_key(db, monkeypatch): person = PlexPerson(name="Actor Y", tmdb_person_id=555) db.add(person) @@ -116,6 +175,7 @@ def test_person_detail_schedules_enrich_off_request_path(db, monkeypatch): scheduled: list[int] = [] monkeypatch.setattr(people, "_enrich_person_bg", lambda pid: scheduled.append(pid)) + people._enrich_inflight.clear() # ensure the per-person cooldown guard doesn't suppress this open resp = _get(db, user, monkeypatch, "/api/plex/person/Lazy P") assert resp.status_code == 200, resp.text @@ -123,6 +183,62 @@ def test_person_detail_schedules_enrich_off_request_path(db, monkeypatch): assert len(scheduled) == 1 # the enrich was scheduled off the request path +def test_enrich_person_bg_closes_session_and_isolates_errors(monkeypatch): + # The background-task body must open a fresh session, fetch the person, and ALWAYS close it — rolling + # back (never propagating) if enrich_person raises. Pure-unit via a fake session (no DB needed). + from app.routes.plex import people + + calls = {"rollback": 0, "close": 0, "enriched": 0} + + class _FakeSession: + def __init__(self, person): + self._person = person + + def get(self, model, pid): + return self._person + + def rollback(self): + calls["rollback"] += 1 + + def close(self): + calls["close"] += 1 + + # Success path: person found → enrich called, session closed, no rollback. + monkeypatch.setattr(people, "SessionLocal", lambda: _FakeSession(object())) + monkeypatch.setattr( + people.plex_tmdb, "enrich_person", lambda db, p: calls.__setitem__("enriched", calls["enriched"] + 1) + ) + people._enrich_person_bg(5) + assert calls == {"rollback": 0, "close": 1, "enriched": 1} + + # Error path: enrich raises → rollback + close, no exception escapes. + calls.update({"rollback": 0, "close": 0, "enriched": 0}) + + def _boom(db, p): + raise RuntimeError("enrich blew up") + + monkeypatch.setattr(people.plex_tmdb, "enrich_person", _boom) + people._enrich_person_bg(5) # must not raise + assert calls["rollback"] == 1 and calls["close"] == 1 + + # Missing person (deleted id): None-guard skips enrich, session still closed. + calls.update({"rollback": 0, "close": 0, "enriched": 0}) + monkeypatch.setattr(people, "SessionLocal", lambda: _FakeSession(None)) + people._enrich_person_bg(9) + assert calls == {"rollback": 0, "close": 1, "enriched": 0} + + +def test_should_schedule_enrich_dedups_within_cooldown(): + # Concurrent/rapid opens of the same person must schedule the enrich only once per cooldown window. + from app.routes.plex import people + + people._enrich_inflight.clear() + assert people._should_schedule_enrich(12345) is True # first open reserves + assert people._should_schedule_enrich(12345) is False # within cooldown → deduped + assert people._should_schedule_enrich(99999) is True # a different person is independent + people._enrich_inflight.clear() + + def test_person_detail_no_enrich_when_already_enriched(db, monkeypatch): # An already-enriched person (tmdb_enriched_at set) must NOT schedule another background fetch. from datetime import datetime, timezone diff --git a/backend/tests/test_plex_tmdb.py b/backend/tests/test_plex_tmdb.py index 4200255f..0adca455 100644 --- a/backend/tests/test_plex_tmdb.py +++ b/backend/tests/test_plex_tmdb.py @@ -293,6 +293,36 @@ def test_enrich_batch_skips_shows_without_guid(db): assert sh.tmdb_enriched_at is None +def test_resolve_library_guids_scopes_to_given_libraries(db): + # The scoped path (person page) must keep a credit that lives in a NON-scoped library (disabled or an + # HU-overlay twin) OUT of the result; the unscoped path (detail recs) matches across all libraries; + # and an explicit empty list skips that media side entirely (distinct from None = match all). + lib_on = _lib(db, key="on", kind="movie") + lib_off = _lib(db, key="off", kind="movie") + m_on = PlexItem(rating_key="on1", library_id=lib_on.id, kind="movie", title="In Scope", guid="tmdb:100") + m_off = PlexItem(rating_key="off1", library_id=lib_off.id, kind="movie", title="Out", guid="tmdb:200") + db.add_all([m_on, m_off]) + db.commit() + entries = [{"tmdb_id": 100, "media_type": "movie"}, {"tmdb_id": 200, "media_type": "movie"}] + + # Scoped to lib_on: the row in lib_off is excluded. + movies, shows = tmdb.resolve_library_guids( + db, entries, movie_lib_ids=[lib_on.id], show_lib_ids=[] + ) + assert set(movies) == {"tmdb:100"} + assert shows == {} + + # Unscoped (None defaults): matches across all libraries. + movies_all, _ = tmdb.resolve_library_guids(db, entries) + assert set(movies_all) == {"tmdb:100", "tmdb:200"} + + # Empty list for a media side skips its bucket even with candidates present (vs None = match all). + _, shows_empty = tmdb.resolve_library_guids( + db, [{"tmdb_id": 100, "media_type": "tv"}], show_lib_ids=[] + ) + assert shows_empty == {} + + def test_resolve_recs_matches_by_media_type_not_bare_id(db): # TMDB movie ids and tv ids are independent, so tmdb:500 can be BOTH a show (in our library) and a # movie recommendation. The match must respect media_type — a bare-id match would deep-link the movie diff --git a/frontend/src/components/AnchoredPopover.dom.test.tsx b/frontend/src/components/AnchoredPopover.dom.test.tsx new file mode 100644 index 00000000..51afe911 --- /dev/null +++ b/frontend/src/components/AnchoredPopover.dom.test.tsx @@ -0,0 +1,60 @@ +// @vitest-environment jsdom +// +// Gates the shared PopoverItem ARIA contract: aria-checked for menuitemradio/menuitemcheckbox and +// aria-selected for option are rendered as explicit attributes AFTER the floating-ui item props (so they +// win over getItemProps' single-select guess). Six live menus (LanguageSwitcher, ViewSwitcher, +// AddToPlaylist, ChannelTags, tableHeader, plexDetailUi) depend on this; without a test, a regression in +// the shared path would silently drop the checked indicator for screen-reader users. Floating-ui recipe: +// stub the two observers autoUpdate wires, wrap in a FloatingTree, query the hidden surface. +import { describe, it, expect, beforeAll, afterEach } from "vitest"; +import { useState } from "react"; +import { render, screen, cleanup } from "@testing-library/react"; +import { FloatingTree } from "@floating-ui/react"; +import { useAnchoredPopover, PopoverSurface, PopoverItem } from "./AnchoredPopover"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); + +afterEach(cleanup); + +function Menu() { + const [open, setOpen] = useState(true); + const pop = useAnchoredPopover({ open, onOpenChange: setOpen, mode: "menu" }); + return ( + + {}}> + Newest + + {}}> + Oldest + + {}}> + Show shorts + + + ); +} + +describe("PopoverItem ARIA contract", () => { + it("exposes aria-checked on menuitemradio and menuitemcheckbox items", () => { + render( + + + , + ); + const radios = screen.getAllByRole("menuitemradio", { hidden: true }); + expect(radios.map((r) => r.getAttribute("aria-checked"))).toEqual(["true", "false"]); + const checkbox = screen.getByRole("menuitemcheckbox", { hidden: true }); + expect(checkbox.getAttribute("aria-checked")).toBe("true"); + }); +}); diff --git a/frontend/src/components/Combobox.dom.test.tsx b/frontend/src/components/Combobox.dom.test.tsx index 5721ae26..7fb2a690 100644 --- a/frontend/src/components/Combobox.dom.test.tsx +++ b/frontend/src/components/Combobox.dom.test.tsx @@ -127,6 +127,71 @@ describe("Combobox", () => { expect(screen.getByText("plex.filter.moreRefine")).toBeTruthy(); // 10 hidden behind search }); + it("Enter commits the pointer-highlighted option, not the arrow-active one", () => { + // The visible highlight follows the pointer (hoverIndex); Enter must commit THAT row, not the bare + // keyboard activeIndex — otherwise arrow-then-hover-then-Enter toggles a different option than shown. + const onChange = vi.fn(); + wrap(); + const input = screen.getByRole("combobox"); + input.focus(); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: "ArrowDown" }); // activeIndex → 0 (Drama) + const action = screen + .getAllByRole("option", { hidden: true }) + .find((el) => el.textContent?.includes("Action"))!; + fireEvent.mouseMove(action); // hoverIndex → Action (the visibly highlighted row) + fireEvent.keyDown(input, { key: "Enter" }); + expect(onChange).toHaveBeenCalledWith(["Action"]); // the highlighted (hovered) row, not Drama + }); + + it("Enter commits a hover-highlighted option even with no prior arrow key", () => { + const onChange = vi.fn(); + wrap(); + const input = screen.getByRole("combobox"); + input.focus(); + fireEvent.focus(input); + const comedy = screen + .getAllByRole("option", { hidden: true }) + .find((el) => el.textContent?.includes("Comedy"))!; + fireEvent.mouseMove(comedy); // highlight via hover, activeIndex still null + fireEvent.keyDown(input, { key: "Enter" }); + expect(onChange).toHaveBeenCalledWith(["Comedy"]); // no longer a silent no-op + }); + + it("async mode: controlled query + onQueryChange/onOpenChange, no client filter, selectedLabels chip", () => { + const onQueryChange = vi.fn(); + const onOpenChange = vi.fn(); + const onChange = vi.fn(); + const serverOpts = [{ value: "id1", label: "Harry Potter", count: 8 }]; + wrap( + , + ); + // The selected value's option isn't in the (server-narrowed) list, so its chip uses selectedLabels. + expect(screen.getByText("Stored Collection")).toBeTruthy(); + const input = screen.getByRole("combobox"); + fireEvent.focus(input); + expect(onOpenChange).toHaveBeenCalledWith(true); + fireEvent.change(input, { target: { value: "harr" } }); + expect(onQueryChange).toHaveBeenCalledWith("harr"); // controlled query propagates out + // clientFilter=false: the single server option renders verbatim (not re-filtered against "har"). + const opts = screen.getAllByRole("option", { hidden: true }); + expect(opts.length).toBe(1); + expect(opts[0]?.textContent).toContain("Harry Potter"); + fireEvent.click(screen.getByText("Harry Potter")); + expect(onChange).toHaveBeenCalledWith(["id1"]); // single-select replaces + }); + it("single-select: picking replaces the value and closes the list", () => { const onChange = vi.fn(); wrap( diff --git a/frontend/src/components/Combobox.tsx b/frontend/src/components/Combobox.tsx index 1f1d7822..8cfab27b 100644 --- a/frontend/src/components/Combobox.tsx +++ b/frontend/src/components/Combobox.tsx @@ -100,7 +100,11 @@ export default function Combobox({ onOpenChange?.(next); }; const pop = useAnchoredPopover({ open, onOpenChange: setOpenAnd, mode: "combobox" }); - const { activeIndex } = pop; + // `highlightIndex` = the row the user actually SEES highlighted (pointer hover, else keyboard); Enter + // commits THAT, not the bare keyboard `activeIndex` — otherwise arrow-then-hover-then-Enter would toggle + // a different option than the highlighted one, and a hover with no prior arrow (activeIndex still null) + // would make Enter a no-op over a highlighted row. + const { activeIndex, highlightIndex } = pop; // Clear the typed query when the list closes. useEffect(() => { @@ -176,9 +180,10 @@ export default function Combobox({ }, onFocus: () => setOpenAnd(true), onKeyDown: (e) => { - if (e.key === "Enter" && open && activeIndex != null && visible[activeIndex]) { + const idx = highlightIndex ?? activeIndex; + if (e.key === "Enter" && open && idx != null && visible[idx]) { e.preventDefault(); // select the highlighted option, don't submit a surrounding form - if (!visible[activeIndex].disabled) commit(visible[activeIndex].value); + if (!visible[idx].disabled) commit(visible[idx].value); } else if (e.key === "Backspace" && query === "" && selected.length > 0) { // Backspace on an empty field removes the last chip (react-select behaviour). onChange(selected.slice(0, -1)); diff --git a/frontend/src/components/PlexInfo.dom.test.tsx b/frontend/src/components/PlexInfo.dom.test.tsx index dcf1b629..13d6b4a6 100644 --- a/frontend/src/components/PlexInfo.dom.test.tsx +++ b/frontend/src/components/PlexInfo.dom.test.tsx @@ -109,6 +109,24 @@ describe("PlexInfo enriched detail (page)", () => { expect(onOpenPerson).toHaveBeenCalledWith("Jane Director"); }); + it("RatingsRow: with no IMDb score, the audience score becomes the filterable one", () => { + const onFilter = vi.fn(); + const noImdb = { ...detail, imdb_rating: null, imdb_id: null, imdb_url: null }; + wrap(); + expect(screen.queryByText("IMDb")).toBeNull(); + // audience (8.4) is now the primary/filterable score → clicking it filters ratingMin. + fireEvent.click(screen.getByText("8.4")); + expect(onFilter).toHaveBeenCalledWith({ ratingMin: 8 }); + }); + + it("RatingsRow: suppresses the duplicate audience chip when it equals the IMDb score", () => { + const dupe = { ...detail, rating: 7.8 }; // audience == imdb_rating (7.8) + wrap(); + expect(screen.getByText("IMDb")).toBeTruthy(); + expect(screen.queryByText("plex.info.audience")).toBeNull(); // duplicate audience chip suppressed + expect(screen.getAllByText("7.8")).toHaveLength(1); // the score shows once (IMDb only) + }); + it("the overlay variant stays lean (no facts / keywords / crew panels)", () => { wrap(); expect(screen.queryByText("plex.info.facts.title")).toBeNull(); diff --git a/frontend/src/components/PlexInfo.tsx b/frontend/src/components/PlexInfo.tsx index ca799d19..362d5d48 100644 --- a/frontend/src/components/PlexInfo.tsx +++ b/frontend/src/components/PlexInfo.tsx @@ -15,6 +15,7 @@ import { useDetailPrefs, } from "../lib/plexDetailUi"; import { usePref } from "../lib/usePref"; +import i18n from "../i18n"; import { Carousel } from "./Carousel"; import PlexCollectionEditor from "./PlexCollectionEditor"; import PlexPlaylistAdd from "./PlexPlaylistAdd"; @@ -27,9 +28,13 @@ import PlexPlaylistAdd from "./PlexPlaylistAdd"; // The TMDB-enriched pieces (multi-source ratings row, facts, keyword chips, full crew — E5 S5) render // on the page variant only; the overlay stays lean. -const compact = (n: number) => new Intl.NumberFormat("en", { notation: "compact" }).format(n); +// Localize the grouping/compact notation to the active language (the app convention, lib/api/core.ts); +// `i18n.language` is read at call time, and the component re-renders on a language switch. Currency stays +// USD — TMDB budget/revenue figures are USD regardless of the viewer's language. +const compact = (n: number) => + new Intl.NumberFormat(i18n.language, { notation: "compact" }).format(n); const money = (n: number) => - new Intl.NumberFormat("en-US", { + new Intl.NumberFormat(i18n.language, { notation: "compact", style: "currency", currency: "USD", diff --git a/frontend/src/i18n/locales/en/plex.json b/frontend/src/i18n/locales/en/plex.json index 0f46cba2..1949c832 100644 --- a/frontend/src/i18n/locales/en/plex.json +++ b/frontend/src/i18n/locales/en/plex.json @@ -72,7 +72,6 @@ "collectionEmpty": "No collections", "matchLabel": "Match", "studio": "Studio", - "add": "Add", "moreRefine": "{{count}} more — refine with search" }, "count": "{{count}} titles", diff --git a/frontend/src/i18n/locales/hu/plex.json b/frontend/src/i18n/locales/hu/plex.json index 110a5066..40f19229 100644 --- a/frontend/src/i18n/locales/hu/plex.json +++ b/frontend/src/i18n/locales/hu/plex.json @@ -72,7 +72,6 @@ "collectionEmpty": "Nincs kollekció", "matchLabel": "Egyezés", "studio": "Stúdió", - "add": "Hozzáadás", "moreRefine": "még {{count}} — finomíts kereséssel" }, "count": "{{count}} cím",