diff --git a/backend/app/plex/query.py b/backend/app/plex/query.py index c3fd6f7e..20abd28b 100644 --- a/backend/app/plex/query.py +++ b/backend/app/plex/query.py @@ -14,6 +14,22 @@ from app.models import PlexItem, PlexLibrary TS_CONFIG = "public.unaccent_simple" +# The ONE Plex sort vocabulary, shared by every surface that offers the Sort control (the browse grid +# and the collection info page) and mirrored by the frontend's `plexSort.ts`. It lives here rather than +# inside a router because the two surfaces order DIFFERENT things — browse orders a union subquery in +# SQL, a collection orders its already-loaded member list in Python — and a key that one of them cannot +# honour would put an option in the UI that silently does nothing. Tests pin both implementations to +# this tuple, so adding a key here fails the gate until every surface can order by it. +PLEX_SORT_KEYS: tuple[str, ...] = ( + "added", + "lastwatched", + "release", + "year", + "rating", + "duration", + "title", +) + # --- Language-aware column pickers (E5 S6) ---------------------------------- # # The HU-overlay twin (E5 S5) stores Hungarian metadata in `*_hu` columns on the SAME EN row. When diff --git a/backend/app/routes/plex/browse.py b/backend/app/routes/plex/browse.py index 5cdda59a..42d04506 100644 --- a/backend/app/routes/plex/browse.py +++ b/backend/app/routes/plex/browse.py @@ -54,6 +54,22 @@ from app.utils import to_tsquery_str router = APIRouter(prefix="/api/plex", tags=["plex"]) +def sort_column(u, sort: str): + """The browse grid's sort key -> orderable column on the union subquery. Module-level (not inline + in the handler) so a test can pin its vocabulary to `PLEX_SORT_KEYS` — a key the Sort control + offers but this cannot order would quietly fall back to `added_at` and look like a broken control. + An unknown key (a stale bookmark, a hand-edited URL) deliberately keeps that fallback.""" + return { + "added": u.c.added_at, + "title": func.lower(u.c.title), + "year": u.c.year, + "rating": u.c.rating, + "duration": u.c.duration_s, + "release": u.c.rel, + "lastwatched": u.c.last_watched, + }.get(sort, u.c.added_at) + + @router.get("/libraries") def list_libraries(user: User = Depends(current_user), db: Session = Depends(get_db)) -> dict: """The mirrored libraries the user can browse (drives the Plex scope selector). HU overlay @@ -221,14 +237,7 @@ def unified_library( if tsq is not None: order = [u.c.rank.desc()] else: - col = { - "title": func.lower(u.c.title), - "year": u.c.year, - "rating": u.c.rating, - "duration": u.c.duration_s, - "release": u.c.rel, - "lastwatched": u.c.last_watched, - }.get(sort, u.c.added_at) + col = sort_column(u, sort) order = [(col.asc() if sort_dir == "asc" else col.desc()).nullslast()] order.append(u.c.rk.desc()) rows = db.execute(select(u).order_by(*order).offset(offset).limit(limit)).all() diff --git a/backend/app/routes/plex/collections.py b/backend/app/routes/plex/collections.py index 862f5f9a..ffcfa397 100644 --- a/backend/app/routes/plex/collections.py +++ b/backend/app/routes/plex/collections.py @@ -1,5 +1,9 @@ """Plex collections: browse mirrored collections + the admin manual-collection editor.""" +from collections.abc import Callable +from datetime import date, datetime +from typing import Any, NamedTuple + from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel from sqlalchemy import func, select @@ -12,6 +16,7 @@ from app.models import ( PlexItem, PlexLibrary, PlexShow, + PlexState, User, ) from app.plex import rollup @@ -83,13 +88,20 @@ def _collection_card(c: PlexCollection) -> dict: def collections( library: str | None = None, q: str | None = None, + keys: str | None = None, user: User = Depends(current_user), db: Session = Depends(get_db), ) -> dict: """Mirrored collections as cards (most-populated first), optionally name-filtered by `q`. With a `library` plex_key → that one library's collections (used by the collection editor). WITHOUT one → the UNION across all enabled libraries (the unified-scope sidebar picker, which has no single - library selected).""" + library selected). + + `keys` (CSV of rating_keys) narrows to just those. It exists because a Plex link carries the + collection FILTER as bare rating_keys — nothing in the URL says what "17045" is called — so the + sidebar token and the applied-filter chip had nothing to render but the raw id. Resolving them + through this endpoint costs one small row each; pulling the whole list to find two titles is + ~200 KB.""" if library is not None: lib = ( db.execute(select(PlexLibrary).where(PlexLibrary.plex_key == str(library))) @@ -113,6 +125,13 @@ def collections( term = (q or "").strip() if term: query = query.where(PlexCollection.title.ilike(f"%{like_escape(term)}%")) + if keys is not None: + wanted = [k for k in (keys or "").split(",") if k] + # An explicit but empty `keys=` asks for nothing — answer nothing, rather than falling through + # to "every collection" (which is what an unguarded empty IN () would have to become). + if not wanted: + return {"collections": []} + query = query.where(PlexCollection.rating_key.in_(wanted)) rows = ( db.execute( query.order_by( @@ -149,14 +168,69 @@ def collections( } +class Member(NamedTuple): + """One collection member flattened to everything the Sort control can order by. The page's two + legs are different tables (movies and shows), so there is no single query to ORDER BY — the + ordering is a pure list sort over these, which is also what makes it unit-testable.""" + + kind: str # "movie" | "show" + obj: Any # PlexItem | PlexShow, carried through to the card serializer + title: str # lower-cased; never NULL (the column is not nullable) + year: int | None + rating: float | None + added: datetime | None + release: date | None + duration: int | None # movies only — a show has no single runtime + last_watched: datetime | None + + +# Sort key -> what to read off a Member. Pinned to `PLEX_SORT_KEYS` by a test: the collection page +# shows the SAME Sort control as the browse grid, so a key this map is missing would be an option +# that silently does nothing here. +MEMBER_SORT_KEYS: dict[str, Callable[[Member], Any]] = { + "added": lambda m: m.added, + "lastwatched": lambda m: m.last_watched, + "release": lambda m: m.release, + "year": lambda m: m.year, + "rating": lambda m: m.rating, + "duration": lambda m: m.duration, + "title": lambda m: m.title, +} + +# What the page ordered by before it had a control, and still its default: a film series wants to be +# read in release order, not by whenever Plex happened to ingest it. +DEFAULT_MEMBER_SORT = "year" + + +def sort_members(members: list[Member], sort: str, sort_dir: str) -> list[Member]: + """Order a collection's members by one of `PLEX_SORT_KEYS`. + + Two properties worth stating because they are easy to get wrong with a plain `reverse=True`: + members with no value for the key sink to the END in BOTH directions (matching the browse grid's + `nullslast()` — a movie with no rating is not "the worst rated"), and ties always resolve + title-ascending, so flipping the direction reverses the ranking rather than the alphabet inside + it. An unknown key falls back to the page's default instead of returning an arbitrary order.""" + key = MEMBER_SORT_KEYS.get(sort, MEMBER_SORT_KEYS[DEFAULT_MEMBER_SORT]) + ordered = sorted(members, key=lambda m: m.title) # stable secondary, applied first + have = [m for m in ordered if key(m) is not None] + missing = [m for m in ordered if key(m) is None] + have.sort(key=key, reverse=sort_dir != "asc") + return have + missing + + @router.get("/collections/{rating_key}") def collection_detail( rating_key: str, + sort: str = DEFAULT_MEMBER_SORT, + sort_dir: str = "asc", user: User = Depends(current_user), db: Session = Depends(get_db), ) -> dict: - """One collection's hero data + its members (movies + shows), ordered by release year then title — - the Collection info page. Read-only here; edits go through the admin mutation endpoints below.""" + """One collection's hero data + its members (movies + shows) in the requested order — the + Collection info page. The sort vocabulary is the browse grid's (`PLEX_SORT_KEYS`) so the same Sort + control means the same thing on both surfaces; the default is release year ascending, which is + what this page did before it had a control. Read-only here; edits go through the admin mutation + endpoints below.""" col = by_rating_key(db, PlexCollection, rating_key) if col is None: raise HTTPException(status_code=404, detail="Unknown collection") @@ -179,19 +253,63 @@ def collection_detail( .all() ) mstate = rollup.item_state_map(db, user.id, [m.id for m in movies]) - sstate = rollup.state_map(db, user.id, [s.id for s in shows]) - # Merge the two type-homogeneous lists into ONE member list ordered by (year, title). - tagged = [(m.year, (m.title or "").lower(), "movie", m) for m in movies] + [ - (s.year, (s.title or "").lower(), "show", s) for s in shows + # ONE rollup pass over the show members, for both the aggregate badge status and `resumed_at` — + # the browse grid's "last watched" for a show (the newest resume/watched across its episodes). The + # cheaper `rollup.state_map` gives only the status, which would leave `lastwatched` unorderable on + # a show collection while the control still offered it. + show_rows = ( + db.execute( + rollup.agg_subquery(db, user.id, include_resumed=True).where( + PlexItem.show_id.in_([s.id for s in shows]) + ) + ).all() + if shows + else [] + ) + sstate = {r.show_id: rollup.show_status(r.total, r.watched, r.inprog) for r in show_rows} + sresumed = {r.show_id: r.resumed_at for r in show_rows} + + def _movie_last_watched(st: PlexState | None) -> datetime | None: + # Same definition as the browse grid's: when it was last resumed, else when it was marked + # watched (coalesce(progress_updated_at, watched_at)). + return (st.progress_updated_at or st.watched_at) if st else None + + # Merge the two type-homogeneous lists into ONE member list, then order it. + members = [ + Member( + kind="movie", + obj=m, + title=(m.title or "").lower(), + year=m.year, + rating=m.rating, + added=m.added_at, + release=m.originally_available_at, + duration=m.duration_s, + last_watched=_movie_last_watched(mstate.get(m.id)), + ) + for m in movies + ] + [ + Member( + kind="show", + obj=sh, + title=(sh.title or "").lower(), + year=sh.year, + rating=sh.rating, + added=sh.added_at, + release=sh.originally_available_at, + duration=None, + last_watched=sresumed.get(sh.id), + ) + for sh in shows ] - tagged.sort(key=lambda t: (t[0] is None, t[0] or 0, t[1])) + members = sort_members(members, sort, sort_dir) items = [ - movie_card(o, mstate.get(o.id)) - if kind == "movie" - else show_card(o, sstate.get(o.id, "new")) - for _, _, kind, o in tagged + movie_card(m.obj, mstate.get(m.obj.id)) + if m.kind == "movie" + else show_card(m.obj, sstate.get(m.obj.id, "new")) + for m in members ] - years = [y for y, _, _, _ in tagged if y is not None] + years = [m.year for m in members if m.year is not None] certs = [m.content_rating for m in movies] + [s.content_rating for s in shows] return { **_collection_card(col), diff --git a/backend/tests/test_plex_collection_sort.py b/backend/tests/test_plex_collection_sort.py new file mode 100644 index 00000000..431e69f0 --- /dev/null +++ b/backend/tests/test_plex_collection_sort.py @@ -0,0 +1,291 @@ +"""The Collection info page's member ordering (`GET /api/plex/collections/{rk}?sort=&sort_dir=`) and +the `keys=` lookup that gives the sidebar token / filter chip a title instead of a raw rating_key. + +Reported from daily use: "Collectionben nem működik a rendezés". The browse grid sorts fine — the +collection page had no control at all and a hardcoded year-ascending order. Adding the control means +the SAME vocabulary must actually work here, hence the pin against `PLEX_SORT_KEYS`. + +Pure-logic tests run everywhere; the endpoint tests use the `db` fixture (DB lane). +""" + +from datetime import date, datetime, timedelta, timezone + +from fastapi.testclient import TestClient +from sqlalchemy import literal_column + +import app.state as state +from app.auth import current_user +from app.db import get_db +from app.main import app +from app.models import PlexCollection, PlexItem, PlexLibrary, PlexShow, PlexState, User +from app.plex.query import PLEX_SORT_KEYS +from app.routes.plex.browse import sort_column +from app.routes.plex.collections import ( + DEFAULT_MEMBER_SORT, + MEMBER_SORT_KEYS, + Member, + sort_members, +) + +NOW = datetime(2024, 6, 1, tzinfo=timezone.utc) + + +def _m(title: str, **over) -> Member: + base = dict( + kind="movie", + obj=None, + title=title, + year=None, + rating=None, + added=None, + release=None, + duration=None, + last_watched=None, + ) + base.update(over) + return Member(**base) # type: ignore[arg-type] + + +def _titles(members: list[Member]) -> list[str]: + return [m.title for m in members] + + +# --- the vocabulary is ONE vocabulary --------------------------------------------------------- + + +def test_every_sort_key_is_orderable_on_a_collection(): + """The collection page shows the same Sort control as the browse grid, so a key it cannot order + would be an option that silently does nothing.""" + assert set(MEMBER_SORT_KEYS) == set(PLEX_SORT_KEYS) + + +def test_every_sort_key_is_orderable_on_the_browse_grid(): + """The other half of the same pin: browse falls back to `added_at` for an unknown key, so a key + missing from its map would order by date while the control claimed otherwise.""" + + class _Cols: + title = literal_column("title") + year = literal_column("year") + rating = literal_column("rating") + duration_s = literal_column("duration_s") + rel = literal_column("rel") + last_watched = literal_column("last_watched") + added_at = literal_column("added_at") + + class _U: + c = _Cols() + + u = _U() + fallback = u.c.added_at + unhandled = [k for k in PLEX_SORT_KEYS if k != "added" and sort_column(u, k) is fallback] + assert unhandled == [] + # …and the fallback itself is still what an unknown key gets. + assert sort_column(u, "no-such-key") is fallback + + +# --- ordering behaviour ----------------------------------------------------------------------- + + +def test_orders_by_the_requested_key_in_both_directions(): + members = [_m("b", year=2006), _m("c", year=2017), _m("a", year=2003)] + assert _titles(sort_members(members, "year", "asc")) == ["a", "b", "c"] + assert _titles(sort_members(members, "year", "desc")) == ["c", "b", "a"] + + +def test_members_without_a_value_sink_to_the_end_in_both_directions(): + """A movie with no rating is not "the worst rated" — nullslast, like the browse grid.""" + members = [_m("rated-low", rating=6.5), _m("unrated"), _m("rated-high", rating=8.1)] + assert _titles(sort_members(members, "rating", "desc")) == [ + "rated-high", + "rated-low", + "unrated", + ] + assert _titles(sort_members(members, "rating", "asc")) == ["rated-low", "rated-high", "unrated"] + + +def test_ties_stay_title_ascending_when_the_direction_flips(): + """Flipping the direction must reverse the RANKING, not the alphabet inside a tie.""" + members = [_m("zulu", year=2003), _m("alpha", year=2003), _m("mike", year=2017)] + assert _titles(sort_members(members, "year", "asc")) == ["alpha", "zulu", "mike"] + assert _titles(sort_members(members, "year", "desc")) == ["mike", "alpha", "zulu"] + + +def test_an_unknown_key_falls_back_to_the_pages_default(): + members = [_m("b", year=2006), _m("a", year=2017)] + assert _titles(sort_members(members, "no-such-key", "asc")) == _titles( + sort_members(members, DEFAULT_MEMBER_SORT, "asc") + ) + + +def test_a_show_has_no_duration_and_therefore_sinks_on_that_key(): + members = [ + _m("short movie", duration=3600), + _m("a show", kind="show", duration=None), + _m("long movie", duration=9000), + ] + assert _titles(sort_members(members, "duration", "desc")) == [ + "long movie", + "short movie", + "a show", + ] + + +# --- the endpoint (DB lane) ------------------------------------------------------------------- + + +def _seed(db): + lib_m = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True) + lib_s = PlexLibrary(plex_key="2", title="Shows", kind="show", enabled=True) + db.add_all([lib_m, lib_s]) + db.flush() + + col = PlexCollection(rating_key="col1", library_id=lib_m.id, title="Franchise", child_count=3) + other = PlexCollection(rating_key="col2", library_id=lib_m.id, title="Elsewhere", child_count=1) + db.add_all([col, other]) + db.flush() + + # Titles deliberately alphabetical-opposite to the years, so a year sort can't accidentally pass + # by falling through to the title tiebreak. + old = PlexItem( + rating_key="mo_old", + library_id=lib_m.id, + kind="movie", + title="Zulu", + year=2003, + rating=8.1, + duration_s=8000, + added_at=NOW - timedelta(days=1), + originally_available_at=date(2003, 7, 9), + collection_keys=["col1"], + ) + mid = PlexItem( + rating_key="mo_mid", + library_id=lib_m.id, + kind="movie", + title="Mike", + year=2006, + rating=7.4, + duration_s=9000, + added_at=NOW - timedelta(days=3), + originally_available_at=date(2006, 7, 7), + collection_keys=["col1"], + ) + new = PlexItem( + rating_key="mo_new", + library_id=lib_m.id, + kind="movie", + title="Alpha", + year=2017, + rating=6.5, + duration_s=7000, + added_at=NOW, + originally_available_at=date(2017, 5, 26), + collection_keys=["col1"], + ) + # A show member with a resumed episode: `lastwatched` must work on this leg too, not just movies. + show = PlexShow( + rating_key="sh1", + library_id=lib_s.id, + title="Series", + year=2010, + child_count=1, + collection_keys=["col1"], + ) + db.add_all([old, mid, new, show]) + db.flush() + ep = PlexItem( + rating_key="sh1_e1", + library_id=lib_s.id, + kind="episode", + title="E1", + show_id=show.id, + season_number=1, + episode_number=1, + ) + db.add(ep) + db.flush() + + user = User(email="colsort@example.com") + db.add(user) + db.flush() + db.add_all( + [ + # The show's episode is the most recently touched thing in the collection… + PlexState( + user_id=user.id, + item_id=ep.id, + status="new", + position_seconds=60, + progress_updated_at=NOW, + ), + # …and one movie was watched earlier. + PlexState( + user_id=user.id, + item_id=old.id, + status="watched", + watched_at=NOW - timedelta(days=2), + ), + ] + ) + db.commit() + return user + + +def _get(db, user, monkeypatch, path): + monkeypatch.setattr(state, "_configured_cache", True) + app.dependency_overrides[get_db] = lambda: db + app.dependency_overrides[current_user] = lambda: user + try: + return TestClient(app).get(path) + finally: + app.dependency_overrides.clear() + + +def _order(r) -> list[str]: + return [i["title"] for i in r.json()["items"]] + + +def test_collection_detail_defaults_to_release_year_ascending(db, monkeypatch): + """The order the page had before it grew a control — a franchise reads in release order.""" + user = _seed(db) + r = _get(db, user, monkeypatch, "/api/plex/collections/col1") + assert r.status_code == 200 + assert _order(r) == ["Zulu", "Mike", "Series", "Alpha"] + + +def test_collection_detail_honours_sort_and_direction(db, monkeypatch): + user = _seed(db) + r = _get(db, user, monkeypatch, "/api/plex/collections/col1?sort=year&sort_dir=desc") + assert _order(r) == ["Alpha", "Series", "Mike", "Zulu"] + r = _get(db, user, monkeypatch, "/api/plex/collections/col1?sort=title&sort_dir=asc") + assert _order(r) == ["Alpha", "Mike", "Series", "Zulu"] + r = _get(db, user, monkeypatch, "/api/plex/collections/col1?sort=rating&sort_dir=desc") + # The show has no rating, so it sinks — it is not "rated 0". + assert _order(r) == ["Zulu", "Mike", "Alpha", "Series"] + + +def test_lastwatched_ranks_a_shows_episodes_alongside_the_movies(db, monkeypatch): + """The show leg needs the rollup's `resumed_at`; with only a status map this key would order + every show as never-watched.""" + user = _seed(db) + r = _get(db, user, monkeypatch, "/api/plex/collections/col1?sort=lastwatched&sort_dir=desc") + # Series (episode resumed NOW) > Zulu (watched 2 days ago) > the two never-touched, title-asc. + assert _order(r) == ["Series", "Zulu", "Alpha", "Mike"] + + +def test_collections_list_resolves_titles_by_key(db, monkeypatch): + """What a shared Plex link needs: the URL carries bare rating_keys, so the sidebar token and the + filter chip have nothing to show but `17045` until these are resolved.""" + user = _seed(db) + r = _get(db, user, monkeypatch, "/api/plex/collections?keys=col1") + assert r.status_code == 200 + assert [c["title"] for c in r.json()["collections"]] == ["Franchise"] + + +def test_an_empty_keys_param_asks_for_nothing(db, monkeypatch): + """`keys=` with nothing in it must not fall through to "every collection" — that would be a + 200 KB answer to a question about zero ids.""" + user = _seed(db) + r = _get(db, user, monkeypatch, "/api/plex/collections?keys=") + assert r.status_code == 200 + assert r.json()["collections"] == [] diff --git a/frontend/src/components/PlexCollectionView.tsx b/frontend/src/components/PlexCollectionView.tsx index 5feef76d..7aabdaba 100644 --- a/frontend/src/components/PlexCollectionView.tsx +++ b/frontend/src/components/PlexCollectionView.tsx @@ -8,6 +8,10 @@ import { formatRuntime } from "../lib/format"; import { notifyActionError, notifyCollectionEditError } from "../lib/actionErrors"; import { notify } from "../lib/notifications"; import { showEpisodeKeys } from "../lib/plexQueue"; +import { healPlexSort, plexSortOptions } from "../lib/plexSort"; +import { usePref } from "../lib/usePref"; +import SortControl from "./ui/SortControl"; +import { type SortState } from "../lib/columnSort"; import { useConfirm } from "./ConfirmProvider"; import { BackBtn, ActionBtn, DetailHero } from "../lib/plexDetailUi"; import { PlexPosterCard } from "./plexCards"; @@ -22,6 +26,11 @@ import { LoadingState, StateMessage } from "./ui/QueryState"; // admin-enabled collection — rename / delete / add / remove, all write-through to Plex (the mutation // endpoints round-trip to Plex, so each edit awaits and re-fetches; no optimism). Read-only ones (smart // / imdb / tmdb / … auto-lists) are marked with a lock. +// The order this page had before it grew a Sort control, and still its default: a franchise reads in +// release order. MUST match the endpoint's `DEFAULT_MEMBER_SORT` — the two only diverge if a request +// goes out without the params, which is why the control always sends them. +const COLLECTION_SORT_DEFAULT: NonNullable = { key: "year", dir: "asc" }; + function shuffled(a: string[]): string[] { const r = [...a]; for (let i = r.length - 1; i > 0; i--) { @@ -52,9 +61,17 @@ export default function PlexCollectionView({ // One comfort-pin menu for the member grid (see ui/ContextMenu). const pinMenu = useContextMenu(); const isAdmin = qc.getQueryData<{ role?: string }>(qk.me())?.role === "admin"; + // The member order is a PREFERENCE (server-synced, adopted on login), not an ephemeral filter — the + // same treatment the feed's view and the player's auto-advance get. It is deliberately its OWN key + // rather than the browse grid's sort: opening a collection must not silently re-sort the library + // behind it. + const [sortPref, setSortPref] = usePref>( + "plexCollectionSort", + COLLECTION_SORT_DEFAULT, + ); const q = useQuery({ - queryKey: qk.plexCollections("detail", collectionId), - queryFn: () => api.plexCollection(collectionId), + queryKey: qk.plexCollections("detail", collectionId, sortPref.key, sortPref.dir), + queryFn: () => api.plexCollection(collectionId, sortPref), }); const d = q.data; @@ -73,6 +90,11 @@ export default function PlexCollectionView({ }; const items = d?.items ?? []; + // A collection lives in ONE library, so its members are all movies or all shows. That decides both + // what the add-panel searches and which sort keys are on offer (only a movie has a runtime). + const scope = items.some((c) => c.type === "show") ? "show" : "movie"; + const sortState = healPlexSort(sortPref, scope); + const sortOptions = plexSortOptions(scope, (k) => t(`plex.sort.${k}`)); // Movies/episodes are playable leaves that queue as themselves; a show can't queue directly, so // Play-all expands it into its episodes (buildQueue). Either kind means there's something to play — // this is what a show-collection needs: before, `playable` held only leaves, so a show-collection's @@ -334,7 +356,7 @@ export default function PlexCollectionView({ {adding && canEdit && ( c.type === "show") ? "show" : "movie"} + scope={scope} existing={new Set(items.map((c) => c.id))} onAdded={invalidate} onClose={() => setAdding(false)} @@ -344,30 +366,46 @@ export default function PlexCollectionView({ {items.length === 0 ? (

{t("plex.collection.empty")}

) : ( -
- {items.map((c) => ( -
- onOpenCard(c)} - onInfo={() => onInfoCard(c)} - onToggleWatched={toggleWatched} - menuProps={pinMenu.triggerProps} + <> + {/* Sort sits directly above the grid, pinned right — the same placement (and the same + control, options and labels) as the browse grid, so the two read as one idea. */} +
+
+ setSortPref(next ?? COLLECTION_SORT_DEFAULT)} + label={t("plex.filter.sort")} + ascLabel={t("plex.filter.dir.asc")} + descLabel={t("plex.filter.dir.desc")} /> - {canEdit && ( - - )}
- ))} -
+
+
+ {items.map((c) => ( +
+ onOpenCard(c)} + onInfo={() => onInfoCard(c)} + onToggleWatched={toggleWatched} + menuProps={pinMenu.triggerProps} + /> + {canEdit && ( + + )} +
+ ))} +
+ )} )} diff --git a/frontend/src/components/PlexSidebar.tsx b/frontend/src/components/PlexSidebar.tsx index fcdb92d2..cda82882 100644 --- a/frontend/src/components/PlexSidebar.tsx +++ b/frontend/src/components/PlexSidebar.tsx @@ -1,5 +1,6 @@ import { useState } from "react"; import { qk } from "../lib/queryKeys"; +import { usePlexCollectionTitles } from "../lib/usePlexCollectionTitles"; import { useTranslation } from "react-i18next"; import { plexLang } from "../i18n"; import { useQuery, useQueryClient } from "@tanstack/react-query"; @@ -447,6 +448,10 @@ function CollectionPicker({ queryFn: () => api.plexCollections(undefined, searchDeb || undefined), enabled: open, // only fetch while the picker is open }); + // …but a selection restored from a LINK has ids and no titles, and the list above is only fetched + // once the picker is opened — so the token rendered the raw rating_key until then. Resolve just the + // selected ids (see usePlexCollectionTitles). + const resolved = usePlexCollectionTitles(values, titles); const options: ComboboxOption[] = (q.data?.collections ?? []).map((c) => ({ value: c.id, label: c.title, @@ -454,7 +459,7 @@ function CollectionPicker({ })); // Combine the fetched labels with the ones already stored so the trip through onChange keeps every // selected collection's title (the fetched list is narrowed by the current search). - const labelMap: Record = { ...titles }; + const labelMap: Record = { ...resolved }; for (const o of options) if (o.label) labelMap[o.value] = o.label; return ( @@ -463,7 +468,7 @@ function CollectionPicker({ clientFilter={false} // the server already narrows to `search` options={options} selected={values} - selectedLabels={titles} + selectedLabels={resolved} query={search} onQueryChange={setSearch} onOpenChange={setOpen} diff --git a/frontend/src/components/usePlexActiveFilters.dom.test.tsx b/frontend/src/components/usePlexActiveFilters.dom.test.tsx index 9e27da56..85da3b95 100644 --- a/frontend/src/components/usePlexActiveFilters.dom.test.tsx +++ b/frontend/src/components/usePlexActiveFilters.dom.test.tsx @@ -6,6 +6,8 @@ // the provider is mocked so the hook can be driven with a non-default sort present. import { describe, it, expect, vi } from "vitest"; import { renderHook } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { createElement, type ReactNode } from "react"; import { EMPTY_PLEX_FILTERS } from "../lib/api"; vi.mock("react-i18next", async (importOriginal) => { @@ -21,7 +23,9 @@ const plexState = { filters: { ...EMPTY_PLEX_FILTERS, genres: ["Action"], - collections: ["A"], + // "A" was picked in the sidebar (its title came along); "B" arrived from a shared LINK, which + // carries ids only — the case that used to render a raw rating_key. + collections: ["A", "B"], collectionTitles: { A: "Marvel" }, }, }; @@ -33,10 +37,17 @@ vi.mock("./PlexProvider", () => ({ })); import { usePlexActiveFilters } from "./usePlexActiveFilters"; +import { qk } from "../lib/queryKeys"; + +// The hook resolves collection titles it does not already have through react-query, so it needs a +// client. Every id here already HAS a title, so the query stays disabled and nothing is requested. +const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); +const wrapper = ({ children }: { children: ReactNode }) => + createElement(QueryClientProvider, { client: qc }, children); describe("usePlexActiveFilters", () => { it("emits filter + watch-state chips but never a sort chip", () => { - const { result } = renderHook(() => usePlexActiveFilters()); + const { result } = renderHook(() => usePlexActiveFilters(), { wrapper }); const keys = result.current.map((c) => c.key); // The in-panel narrowings still surface... expect(keys).toContain("genre:Action"); @@ -46,9 +57,22 @@ describe("usePlexActiveFilters", () => { expect(result.current.every((c) => !c.key.startsWith("sort"))).toBe(true); }); + it("labels a link-supplied collection with its resolved title, not the bare id", () => { + // Seed the resolution the hook would fetch, so this asserts the WIRING (hook -> builder -> label) + // rather than the network. Nothing is mocked. + qc.setQueryData(qk.plexCollections("titles", "B"), { + collections: [{ id: "B", title: "Pirates of the Caribbean Collection" }], + }); + const { result } = renderHook(() => usePlexActiveFilters(), { wrapper }); + const byKey = Object.fromEntries(result.current.map((c) => [c.key, c.label])); + expect(byKey["collection:B"]).toBe("Pirates of the Caribbean Collection"); + // …and the one the filter state already knew about is untouched. + expect(byKey["collection:A"]).toBe("Marvel"); + }); + it("removing the last collection chip prunes its title and collapses collectionTitles to undefined", () => { setFilters.mockClear(); - const { result } = renderHook(() => usePlexActiveFilters()); + const { result } = renderHook(() => usePlexActiveFilters(), { wrapper }); const chip = result.current.find((c) => c.key === "collection:A"); expect(chip).toBeDefined(); chip!.remove(); @@ -59,7 +83,7 @@ describe("usePlexActiveFilters", () => { collections: string[]; collectionTitles?: unknown; }; - expect(arg.collections).toEqual([]); + expect(arg.collections).toEqual(["B"]); expect(arg.collectionTitles).toBeUndefined(); }); }); diff --git a/frontend/src/components/usePlexActiveFilters.ts b/frontend/src/components/usePlexActiveFilters.ts index 7127fb52..514b1eb5 100644 --- a/frontend/src/components/usePlexActiveFilters.ts +++ b/frontend/src/components/usePlexActiveFilters.ts @@ -3,6 +3,7 @@ import { type ActiveFilter } from "../lib/useActiveFilters"; import { EMPTY_PLEX_FILTERS, type PlexFilters } from "../lib/api"; import { PLEX_SORT_FALLBACK } from "../lib/plexSort"; import { usePlex, usePlexActions } from "./PlexProvider"; +import { usePlexCollectionTitles } from "../lib/usePlexCollectionTitles"; // Plex's analogue of useActiveFilters: turns the shared Plex state (expanded filters + watch-state) // into the source-agnostic ActiveFilter[] the shared ActiveFilterChips renders above the grid. @@ -19,7 +20,10 @@ export function usePlexActiveFilters(): ActiveFilter[] { const { setShow, setFilters } = usePlexActions(); const { t } = useTranslation(); const patch = (p: Partial) => setFilters({ ...filters, ...p }); - return buildPlexActiveFilters(filters, show, patch, setShow, t); + // A link opened cold knows the collection ids but not their names — resolve the missing ones for + // display (see usePlexCollectionTitles for why they are NOT written back into the filter state). + const collectionTitles = usePlexCollectionTitles(filters.collections, filters.collectionTitles); + return buildPlexActiveFilters(filters, show, patch, setShow, t, collectionTitles); } // The pure builder behind the hook: turns a Plex filter set + watch-state into the source-agnostic @@ -31,6 +35,9 @@ export function buildPlexActiveFilters( patch: (p: Partial) => void, setShow: (s: string) => void, t: TFn, + /** Collection titles for the chip labels, resolved beyond what the filter state carries. Omitted by + * the saved-views manager, which describes a stored view and has only the captured titles. */ + collectionTitles?: Record, ): ActiveFilter[] { const out: ActiveFilter[] = []; @@ -127,7 +134,7 @@ export function buildPlexActiveFilters( for (const c of filters.collections) { out.push({ key: `collection:${c}`, - label: filters.collectionTitles?.[c] ?? t("plex.filter.collection"), + label: collectionTitles?.[c] ?? 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 diff --git a/frontend/src/lib/api/plex.ts b/frontend/src/lib/api/plex.ts index 647d5333..89aae403 100644 --- a/frontend/src/lib/api/plex.ts +++ b/frontend/src/lib/api/plex.ts @@ -479,16 +479,31 @@ export const plexApi = { }, // With a library plex_key → that library's collections (editor). Without one → the union across all // enabled libraries (the unified-scope sidebar picker). - plexCollections: (library?: string, q?: string): Promise<{ collections: PlexCollection[] }> => { + plexCollections: ( + library?: string, + q?: string, + keys?: string[], + ): Promise<{ collections: PlexCollection[] }> => { const u = new URLSearchParams(); if (library) u.set("library", library); if (q) u.set("q", q); + // `keys` resolves specific rating_keys to their titles — a shared Plex link carries the collection + // filter as bare ids, so the sidebar token and the filter chip have nothing else to render. + if (keys) u.set("keys", keys.join(",")); const qs = u.toString(); return req(`/api/plex/collections${qs ? `?${qs}` : ""}`); }, - // One collection's hero + ordered members (the Collection info page). - plexCollection: (rk: string): Promise => - req(`/api/plex/collections/${encodeURIComponent(rk)}`), + // One collection's hero + its members in the requested order (the Collection info page). The sort + // vocabulary is the browse grid's, so the same Sort control means the same thing on both. + plexCollection: (rk: string, sort?: SortState): Promise => { + const u = new URLSearchParams(); + if (sort) { + u.set("sort", sort.key); + u.set("sort_dir", sort.dir); + } + const qs = u.toString(); + return req(`/api/plex/collections/${encodeURIComponent(rk)}${qs ? `?${qs}` : ""}`); + }, // --- Collection editing (admin only; writes back to Plex) --- plexCreateCollection: ( library: string, diff --git a/frontend/src/lib/usePlexCollectionTitles.dom.test.tsx b/frontend/src/lib/usePlexCollectionTitles.dom.test.tsx new file mode 100644 index 00000000..1064a41a --- /dev/null +++ b/frontend/src/lib/usePlexCollectionTitles.dom.test.tsx @@ -0,0 +1,97 @@ +// @vitest-environment jsdom +// +// A shared Plex link carries the collection filter as bare rating_keys — `?pf=collections%3D17045` +// says nothing about what 17045 is called. Opened cold, the sidebar token rendered that raw id and +// the applied-filter chip could only say "Collection". This hook resolves the missing ones. +// +// Two things it must NOT do: ask the server for a title the filter state already carries (the common +// case — the user picked the collection in the sidebar), and pull the whole collection list to find +// one name (431 collections / ~200 KB on the owner's library). +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { createElement, type ReactNode } from "react"; + +const plexCollections = vi.fn(); +vi.mock("./api", () => ({ api: { plexCollections: (...a: unknown[]) => plexCollections(...a) } })); + +import { usePlexCollectionTitles } from "./usePlexCollectionTitles"; + +const wrap = () => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return ({ children }: { children: ReactNode }) => + createElement(QueryClientProvider, { client: qc }, children); +}; + +const CATALOG: Record = { + "17045": "Pirates of the Caribbean Collection", + "68231": "IMDb Top 250", +}; + +beforeEach(() => { + plexCollections.mockReset(); + // Answer like the endpoint does: only the requested keys come back. + plexCollections.mockImplementation((_lib: unknown, _q: unknown, keys: string[]) => + Promise.resolve({ + collections: (keys ?? []).map((id) => ({ id, title: CATALOG[id] })), + }), + ); +}); + +describe("usePlexCollectionTitles", () => { + it("resolves the ids the filter state has no title for", async () => { + const { result } = renderHook(() => usePlexCollectionTitles(["17045"], undefined), { + wrapper: wrap(), + }); + await waitFor(() => + expect(result.current["17045"]).toBe("Pirates of the Caribbean Collection"), + ); + // Only the missing ids are asked for — nothing else. + expect(plexCollections).toHaveBeenCalledWith(undefined, undefined, ["17045"]); + }); + + it("asks for nothing when every id already has a title", async () => { + const known = { "17045": "Pirates of the Caribbean Collection" }; + const { result } = renderHook(() => usePlexCollectionTitles(["17045"], known), { + wrapper: wrap(), + }); + expect(result.current["17045"]).toBe("Pirates of the Caribbean Collection"); + // Give a would-be fetch a chance to happen before asserting it did not. + await new Promise((r) => setTimeout(r, 20)); + expect(plexCollections).not.toHaveBeenCalled(); + }); + + it("asks only for the missing half of a mixed selection", async () => { + const { result } = renderHook( + () => usePlexCollectionTitles(["17045", "68231"], { "17045": "Already Known" }), + { wrapper: wrap() }, + ); + await waitFor(() => expect(result.current["68231"]).toBe("IMDb Top 250")); + // The known id is not in the request — and its stored title survives the merge. + expect(plexCollections).toHaveBeenCalledWith(undefined, undefined, ["68231"]); + expect(result.current["17045"]).toBe("Already Known"); + }); + + it("treats the same ids in a different order as one query", async () => { + const w = wrap(); + const a = renderHook(() => usePlexCollectionTitles(["68231", "17045"], undefined), { + wrapper: w, + }); + await waitFor(() => expect(a.result.current["17045"]).toBeDefined()); + const calls = plexCollections.mock.calls.length; + const b = renderHook(() => usePlexCollectionTitles(["17045", "68231"], undefined), { + wrapper: w, + }); + await waitFor(() => expect(b.result.current["17045"]).toBeDefined()); + expect(plexCollections.mock.calls.length).toBe(calls); + }); + + it("returns nothing extra while the lookup is still in flight", () => { + plexCollections.mockReturnValue(new Promise(() => {})); + const { result } = renderHook(() => usePlexCollectionTitles(["17045"], undefined), { + wrapper: wrap(), + }); + // The caller falls back to its own label; it must not be handed a half-built map. + expect(result.current["17045"]).toBeUndefined(); + }); +}); diff --git a/frontend/src/lib/usePlexCollectionTitles.ts b/frontend/src/lib/usePlexCollectionTitles.ts new file mode 100644 index 00000000..ec2c380c --- /dev/null +++ b/frontend/src/lib/usePlexCollectionTitles.ts @@ -0,0 +1,31 @@ +import { useQuery } from "@tanstack/react-query"; +import { api } from "./api"; +import { qk } from "./queryKeys"; + +// A Plex link carries the collection filter as bare rating_keys (`?pf=collections%3D17045`) — nothing +// in the URL says what "17045" is called. The title is normally picked up as a side effect of choosing +// the collection in the sidebar (`collectionTitles`), so a link opened cold had no name for it and the +// picker token rendered the raw id while the filter chip could only say "Collection". +// +// This resolves the missing ones from the server and merges them over what the filter state already +// knows. It stays OUT of `collectionTitles`: that map is hashed into the Plex query keys, so writing a +// resolved title back in would split the cache from the logically identical state and refetch the grid +// for a label. Titles are display-only, so they belong in a display-only read. +export function usePlexCollectionTitles( + ids: string[], + known: Record | undefined, +): Record { + // Sorted so the same set of ids in a different order is the same query. + const missing = ids.filter((id) => !known?.[id]).sort(); + const q = useQuery({ + queryKey: qk.plexCollections("titles", missing.join(",")), + queryFn: () => api.plexCollections(undefined, undefined, missing), + enabled: missing.length > 0, + // A collection's name is about as static as data gets, and this can be asked for on every render + // of the sidebar and the chip row. + staleTime: 60 * 60_000, + }); + const out: Record = { ...known }; + for (const c of q.data?.collections ?? []) out[c.id] = c.title; + return out; +}