diff --git a/backend/tests/test_plex_collections_filter.py b/backend/tests/test_plex_collections_filter.py index b512839a..67707360 100644 --- a/backend/tests/test_plex_collections_filter.py +++ b/backend/tests/test_plex_collections_filter.py @@ -303,6 +303,54 @@ def test_remove_item_writes_through_to_plex(db, monkeypatch): app.dependency_overrides.clear() +def _resync_create(db, plex, lib, rk): + """Create-path resync stub: the new rating_key has no local row yet, so materialize it.""" + col = PlexCollection(rating_key=rk, library_id=lib.id, title="Fresh Coll", child_count=1) + db.add(col) + db.flush() + return col + + +def test_create_writes_through_to_plex(db, monkeypatch): + _lib, user = _seed_editable(db) + stub = _CollStub() + client = _admin_client(db, user, monkeypatch, stub, resync=_resync_create) + try: + r = client.post( + "/api/plex/collections", + json={"library": "1", "title": "Fresh Coll", "item_rating_key": "m1"}, + ) + assert r.status_code == 200, r.text + assert "create_collection" in stub.calls + body = r.json() + assert body["id"] == "NEW" # the stub's new ratingKey + assert body["editable"] is True # create marks the new collection editable + finally: + app.dependency_overrides.clear() + + +def test_create_resync_failure_is_422(db, monkeypatch): + # The collection was created in Plex (wrote=True) but the follow-up resync read failed → 422 with + # the honest "reconcile on the next sync" detail, not a false "write failed". + _lib, user = _seed_editable(db) + stub = _CollStub() + + def _boom(db_, plex, lib_, rk): + raise PlexError("read failed") + + client = _admin_client(db, user, monkeypatch, stub, resync=_boom) + try: + r = client.post( + "/api/plex/collections", + json={"library": "1", "title": "Fresh Coll", "item_rating_key": "m1"}, + ) + assert r.status_code == 422, r.text + assert "reconcile" in r.json()["detail"].lower() + assert "create_collection" in stub.calls + finally: + app.dependency_overrides.clear() + + def test_mutation_failure_returns_422_write_failed(db, monkeypatch): # The Plex mutation itself raised → the write never landed → 422 "Plex write failed", and resync # is never reached (stub.calls holds only the mutating call). diff --git a/frontend/src/components/PageScroller.dom.test.tsx b/frontend/src/components/PageScroller.dom.test.tsx new file mode 100644 index 00000000..872857be --- /dev/null +++ b/frontend/src/components/PageScroller.dom.test.tsx @@ -0,0 +1,99 @@ +// @vitest-environment jsdom +// +// The scroll-restoration contract PageScroller implements: offsets are remembered per +// (history entry id + content key), so Back to an entry restores where you left it while a forward +// navigation (a new entry id) or a new content key opens at the top. jsdom lays nothing out, so we +// feed the
fake metrics, drive restoreScroll's rAF loop synchronously, and steer the entry id +// through window.history.state (what ensureNavId/peekNavId read) — firing the nav bus (notifyNav) +// stands in for a Back/Forward that doesn't re-render the scroller. +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vitest"; +import { render, fireEvent, cleanup } from "@testing-library/react"; + +vi.mock("react-i18next", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useTranslation: () => ({ t: (k: string) => k }) }; +}); + +import PageScroller from "./PageScroller"; +import { notifyNav } from "../lib/history"; + +beforeAll(() => { + // useScrollFade wires a ResizeObserver (watch-the-children); jsdom ships none. + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; +}); + +beforeEach(() => { + // restoreScroll re-applies the offset over a few rAF frames (the feed needs time to reach height); + // run rAF synchronously so a restore lands within the test tick. + vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => { + cb(0); + return 0; + }); + vi.stubGlobal("cancelAnimationFrame", () => {}); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +function setEntry(id: number) { + window.history.replaceState({ sfScroll: id }, ""); +} + +function sizedMain(container: HTMLElement): HTMLElement { + const main = container.querySelector("main")!; + // Enough scrollable room for a 300px offset (restoreScroll only sets when scrollHeight-clientHeight + // >= the target), and a writable scrollTop jsdom otherwise no-ops. + Object.defineProperty(main, "scrollHeight", { configurable: true, value: 1000 }); + Object.defineProperty(main, "clientHeight", { configurable: true, value: 500 }); + Object.defineProperty(main, "scrollTop", { configurable: true, writable: true, value: 0 }); + return main; +} + +describe("PageScroller scroll memory", () => { + it("restores per history entry: a forward nav opens at the top, Back restores the offset", () => { + setEntry(1); + const { container } = render(x); + const main = sizedMain(container); + + // The user scrolls entry 1 to 200 — recorded live on scroll. + main.scrollTop = 200; + fireEvent.scroll(main); + + // Forward navigation → a fresh entry id, same content key → the view opens at the top. + setEntry(2); + notifyNav(); + expect(main.scrollTop).toBe(0); + + // Back to entry 1 → its saved offset is restored. + setEntry(1); + notifyNav(); + expect(main.scrollTop).toBe(200); + }); + + it("restores per content key within one entry: a new key resets to top, returning restores", () => { + setEntry(10); + const { container, rerender } = render(x); + const main = sizedMain(container); + + main.scrollTop = 300; + fireEvent.scroll(main); + + // A new content key (e.g. a fresh feed filter) in the SAME entry starts at the top. + rerender(x); + expect(main.scrollTop).toBe(0); + + // Returning to the first key restores its offset. + rerender(x); + expect(main.scrollTop).toBe(300); + }); +}); diff --git a/frontend/src/components/PlexCollectionEditor.tsx b/frontend/src/components/PlexCollectionEditor.tsx index 9b3df1ee..17ba8b76 100644 --- a/frontend/src/components/PlexCollectionEditor.tsx +++ b/frontend/src/components/PlexCollectionEditor.tsx @@ -4,7 +4,7 @@ import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { Check, Plus } from "lucide-react"; import { api } from "../lib/api"; -import { notifyActionError } from "../lib/actionErrors"; +import { notifyCollectionEditError } from "../lib/actionErrors"; import { useScrollFade } from "../lib/useScrollFade"; import Modal from "./Modal"; import { useConfirm } from "./ConfirmProvider"; @@ -102,8 +102,9 @@ export default function PlexCollectionEditor({ setMembers((prev) => new Set(prev).add(col.id)); refresh(); } catch (e) { - // Surfaces the 409 when the name duplicates an existing collection (Plex conflates same names). - notifyActionError(e, t("plex.collection.editFailed")); + // The backend rejects a duplicate name with 409 (Plex conflates same names) — localize it to + // nameTaken rather than surfacing the server's raw English detail. + notifyCollectionEditError(e, t("plex.collection.editFailed"), t("plex.collection.nameTaken")); } finally { setBusy(null); } diff --git a/frontend/src/components/PlexCollectionView.tsx b/frontend/src/components/PlexCollectionView.tsx index e7496b67..052680d3 100644 --- a/frontend/src/components/PlexCollectionView.tsx +++ b/frontend/src/components/PlexCollectionView.tsx @@ -5,11 +5,11 @@ import { Play, Shuffle, Pencil, Trash2, Plus, X, Lock, Check, Search } from "luc import { qk } from "../lib/queryKeys"; import { api, isIntrinsicReadOnly, type PlexCard, type PlexCollectionDetail } from "../lib/api"; import { formatRuntime } from "../lib/format"; -import { notifyActionError } from "../lib/actionErrors"; -import { useDebounced } from "../lib/useDebounced"; +import { notifyActionError, notifyCollectionEditError } from "../lib/actionErrors"; import { useConfirm } from "./ConfirmProvider"; import { BackBtn, ActionBtn, DetailHero } from "../lib/plexDetailUi"; import { PlexPosterCard } from "./plexCards"; +import PlexLibraryPicker from "./PlexLibraryPicker"; import { toggleWatchState } from "../lib/plexWatch"; import { LoadingState, StateMessage } from "./QueryState"; @@ -99,7 +99,8 @@ export default function PlexCollectionView({ invalidate(); setRenaming(false); } catch (e) { - notifyActionError(e, t("plex.collection.editFailed")); + // The backend enforces per-library name uniqueness — localize its 409 to nameTaken. + notifyCollectionEditError(e, t("plex.collection.editFailed"), t("plex.collection.nameTaken")); } finally { setBusy(false); } @@ -342,14 +343,7 @@ function AddItemPanel({ }) { const { t } = useTranslation(); const [term, setTerm] = useState(""); - const debounced = useDebounced(term.trim(), 300); const [added, setAdded] = useState>(new Set()); - const q = useQuery({ - queryKey: qk.plexLibrary("collection-add", scope, debounced), - queryFn: () => api.plexLibrary({ scope, q: debounced, limit: 12 }), - enabled: debounced.length > 0, - }); - const results = q.data?.items ?? []; async function add(card: PlexCard) { try { @@ -381,43 +375,24 @@ function AddItemPanel({ - {debounced.length === 0 ? ( -

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

- ) : q.isLoading ? ( - - ) : q.isError ? ( -

{t("common.loadError")}

- ) : results.length === 0 ? ( -

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

- ) : ( -
    - {results.map((c) => { - const inColl = existing.has(c.id) || added.has(c.id); - return ( -
  • - - - {c.title} - {c.year != null && · {c.year}} - - -
    • - ); - })} -
- )} + {t("plex.collection.addHint")}

} + renderAction={(c) => { + const inColl = existing.has(c.id) || added.has(c.id); + return ( + + ); + }} + /> ); } diff --git a/frontend/src/components/PlexLibraryPicker.dom.test.tsx b/frontend/src/components/PlexLibraryPicker.dom.test.tsx new file mode 100644 index 00000000..4d6887b3 --- /dev/null +++ b/frontend/src/components/PlexLibraryPicker.dom.test.tsx @@ -0,0 +1,101 @@ +// @vitest-environment jsdom +// +// Contract of the shared library search-and-pick list (F6): it owns the debounced query and the +// full loading/error/empty/no-results/list state machine so the two callers (AddItemPanel in +// PlexCollectionView, NewCollectionPanel in plexBrowseLists) can't drift. The two things a caller +// steers — the `enabled` gate (NewCollectionPanel holds it off until the collection name is valid) +// and the `emptyHint` shown before a term is typed — are the pieces with no test before this file, so +// they're covered here. i18n is key-passthrough and api/debounce are mocked so the test drives state +// directly. +import { type ComponentProps } from "react"; +import { describe, it, expect, afterEach, beforeEach, vi } from "vitest"; +import { render, screen, cleanup, waitFor } 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 }) }; +}); +// Identity debounce so a passed term is reflected immediately. +vi.mock("../lib/useDebounced", () => ({ useDebounced: (v: unknown) => v })); + +// api.plexLibrary stub — returns whatever the current test queued. +const plexLibrary = vi.fn(); +vi.mock("../lib/api", () => ({ api: { plexLibrary: (...a: unknown[]) => plexLibrary(...a) } })); + +import PlexLibraryPicker from "./PlexLibraryPicker"; + +const card = (id: string, title: string, year = 2000) => ({ + id, + type: "movie" as const, + title, + year, + thumb: `/api/plex/image/${id}`, +}); +const result = (items: ReturnType[]) => ({ + scope: "both", + total: items.length, + offset: 0, + limit: 12, + items, + episodes: [], +}); + +beforeEach(() => { + plexLibrary.mockReset(); + plexLibrary.mockResolvedValue(result([])); +}); +afterEach(cleanup); + +const renderPicker = (props: Partial> = {}) => { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + } + {...props} + /> + , + ); +}; + +describe("PlexLibraryPicker", () => { + it("renders nothing and fires no query while disabled, even with a term", () => { + const { container } = renderPicker({ enabled: false, term: "matrix" }); + expect(container.firstChild).toBeNull(); + expect(plexLibrary).not.toHaveBeenCalled(); + }); + + it("shows the emptyHint (and fires no query) before a term is typed", () => { + renderPicker({ term: "", emptyHint:

EMPTY_HINT

}); + expect(screen.getByText("EMPTY_HINT")).toBeTruthy(); + expect(plexLibrary).not.toHaveBeenCalled(); + }); + + it("queries the library once enabled + a term are both present and renders the rows via renderAction", async () => { + plexLibrary.mockResolvedValue(result([card("m1", "The Matrix"), card("m2", "Reloaded")])); + renderPicker({ term: "matrix" }); + + await waitFor(() => expect(screen.getByText("act:m1")).toBeTruthy()); + expect(screen.getByText("act:m2")).toBeTruthy(); + expect(screen.getByText("The Matrix")).toBeTruthy(); + expect(plexLibrary).toHaveBeenCalledWith({ scope: "both", q: "matrix", limit: 12 }); + }); + + it("renders the honest error state (not an empty/no-results state) when the query fails", async () => { + plexLibrary.mockRejectedValue(new Error("boom")); + renderPicker({ term: "matrix" }); + + await waitFor(() => expect(screen.getByText("common.loadError")).toBeTruthy()); + expect(screen.queryByText("plex.collection.addNoResults")).toBeNull(); + }); + + it("shows the no-results message when the query returns an empty list", async () => { + plexLibrary.mockResolvedValue(result([])); + renderPicker({ term: "zzzz" }); + + await waitFor(() => expect(screen.getByText("plex.collection.addNoResults")).toBeTruthy()); + }); +}); diff --git a/frontend/src/components/PlexLibraryPicker.tsx b/frontend/src/components/PlexLibraryPicker.tsx new file mode 100644 index 00000000..25c0ba51 --- /dev/null +++ b/frontend/src/components/PlexLibraryPicker.tsx @@ -0,0 +1,60 @@ +import { type ReactNode } from "react"; +import { useTranslation } from "react-i18next"; +import { useQuery } from "@tanstack/react-query"; +import { qk } from "../lib/queryKeys"; +import { api, type PlexCard } from "../lib/api"; +import { useDebounced } from "../lib/useDebounced"; +import { LoadingState } from "./QueryState"; + +// A debounced library search-and-pick list, shared by the "Add title" panel (PlexCollectionView) and +// the "New collection" first-title panel (plexBrowseLists) — both search the library the same way and +// render the same result rows, differing only in the per-row action. Callers keep their own chrome +// (search input, name field, close button) and supply `term` + `renderAction`; this owns the query, +// the loading/error/empty states, and the row markup so those can't drift between the two surfaces. +export default function PlexLibraryPicker({ + scope, + term, + enabled = true, + emptyHint = null, + renderAction, +}: { + scope: string; + /** The raw search term; debounced internally. */ + term: string; + /** Gate the query off (e.g. until a collection name is valid). When false, renders nothing. */ + enabled?: boolean; + /** Shown while the (debounced) term is empty — a hint, or nothing. */ + emptyHint?: ReactNode; + /** The trailing control for each result row (Add / Create-with / …). */ + renderAction: (card: PlexCard) => ReactNode; +}) { + const { t } = useTranslation(); + const debounced = useDebounced(term.trim(), 300); + const q = useQuery({ + queryKey: qk.plexLibrary("collection-pick", scope, debounced), + queryFn: () => api.plexLibrary({ scope, q: debounced, limit: 12 }), + enabled: enabled && debounced.length > 0, + }); + const results = q.data?.items ?? []; + + if (!enabled) return null; + if (debounced.length === 0) return <>{emptyHint}; + if (q.isLoading) return ; + if (q.isError) return

{t("common.loadError")}

; + if (results.length === 0) return

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

; + + return ( +
    + {results.map((c) => ( +
  • + + + {c.title} + {c.year != null && · {c.year}} + + {renderAction(c)} +
    • + ))} +
+ ); +} diff --git a/frontend/src/components/plexBrowseLists.tsx b/frontend/src/components/plexBrowseLists.tsx index e8f54a3a..f4fca68b 100644 --- a/frontend/src/components/plexBrowseLists.tsx +++ b/frontend/src/components/plexBrowseLists.tsx @@ -4,19 +4,13 @@ import type { TFunction } from "i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { Check, ListMusic, Lock, Plus, Search, X } from "lucide-react"; import { qk } from "../lib/queryKeys"; -import { - api, - HttpError, - isIntrinsicReadOnly, - type PlexCard, - type PlexCollection, -} from "../lib/api"; +import { api, isIntrinsicReadOnly, type PlexCard, type PlexCollection } from "../lib/api"; import { formatRuntime } from "../lib/format"; -import { notifyActionError } from "../lib/actionErrors"; -import { notify } from "../lib/notifications"; +import { notifyCollectionEditError } from "../lib/actionErrors"; import { useDebounced } from "../lib/useDebounced"; import { LoadingState, StateMessage } from "./QueryState"; import PosterTile from "./PosterTile"; +import PlexLibraryPicker from "./PlexLibraryPicker"; // A collection tile's sub-line: item count · total runtime (each dropped when unknown, e.g. show-only // collections have no movie runtime). @@ -86,7 +80,6 @@ function NewCollectionPanel({ const [name, setName] = useState(""); const [term, setTerm] = useState(""); const [busy, setBusy] = useState(false); - const debounced = useDebounced(term.trim(), 300); const trimmed = name.trim(); // Advisory only: `existingNames` is the search-scoped, all-library collection union, whereas the // backend enforces uniqueness PER library (Plex only conflates same-named collections within one @@ -94,12 +87,6 @@ function NewCollectionPanel({ // same-name collection in another library would be un-creatable. The backend's 409 is authoritative. const dupHint = trimmed.length > 0 && existingNames.has(trimmed.toLowerCase()); const nameOk = trimmed.length > 0; - const q = useQuery({ - queryKey: qk.plexLibrary("collection-new", debounced), - queryFn: () => api.plexLibrary({ scope: "both", q: debounced, limit: 12 }), - enabled: nameOk && debounced.length > 0, - }); - const results = q.data?.items ?? []; async function create(card: PlexCard) { if (!nameOk || busy) return; @@ -108,13 +95,12 @@ function NewCollectionPanel({ const col = await api.plexCreateCollection("", trimmed, card.id); // backend derives the library onCreated(col.id); } catch (e) { - // The backend is authoritative on per-library name uniqueness — localize its 409 to nameTaken - // (notifyActionError would otherwise surface the server's raw English detail). - if (e instanceof HttpError && e.status === 409) { - notify({ level: "error", message: t("plex.collection.nameTaken") }); - } else { - notifyActionError(e, t("plex.collection.editFailed")); - } + // The backend is authoritative on per-library name uniqueness — its 409 is localized to nameTaken. + notifyCollectionEditError( + e, + t("plex.collection.editFailed"), + t("plex.collection.nameTaken"), + ); setBusy(false); } } @@ -155,38 +141,21 @@ function NewCollectionPanel({ className="glass-card flex-1 rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40 disabled:opacity-50" /> - {!nameOk || debounced.length === 0 ? null : q.isLoading ? ( - - ) : q.isError ? ( -

{t("common.loadError")}

- ) : results.length === 0 ? ( -

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

- ) : ( -
    - {results.map((c) => ( -
  • - - - {c.title} - {c.year != null && · {c.year}} - - -
    • - ))} -
- )} + ( + + )} + /> ); } diff --git a/frontend/src/lib/actionErrors.test.ts b/frontend/src/lib/actionErrors.test.ts new file mode 100644 index 00000000..7142f0f4 --- /dev/null +++ b/frontend/src/lib/actionErrors.test.ts @@ -0,0 +1,31 @@ +// @vitest-environment jsdom +// +// jsdom (not node): importing ../api pulls in i18n, which reads navigator.language at module load. +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Spy on the toast sink so we can assert exactly what each error maps to. +vi.mock("./notifications", () => ({ notify: vi.fn(() => 0) })); + +import { HttpError } from "./api"; +import { notify } from "./notifications"; +import { notifyCollectionEditError } from "./actionErrors"; + +const notifyMock = vi.mocked(notify); +beforeEach(() => notifyMock.mockClear()); + +describe("notifyCollectionEditError", () => { + it("maps a backend 409 to the friendly nameTaken message (not the raw server detail)", () => { + notifyCollectionEditError(new HttpError(409, "already exists"), "EDIT_FAILED", "NAME_TAKEN"); + expect(notifyMock).toHaveBeenCalledWith({ level: "error", message: "NAME_TAKEN" }); + }); + + it("falls back to the server detail for a non-409 HttpError", () => { + notifyCollectionEditError(new HttpError(500, "server boom"), "EDIT_FAILED", "NAME_TAKEN"); + expect(notifyMock).toHaveBeenCalledWith({ level: "error", message: "server boom" }); + }); + + it("falls back to the generic edit-failed message for a non-HttpError", () => { + notifyCollectionEditError(new Error("network"), "EDIT_FAILED", "NAME_TAKEN"); + expect(notifyMock).toHaveBeenCalledWith({ level: "error", message: "EDIT_FAILED" }); + }); +}); diff --git a/frontend/src/lib/actionErrors.ts b/frontend/src/lib/actionErrors.ts index 3b94c5bb..f59b099c 100644 --- a/frontend/src/lib/actionErrors.ts +++ b/frontend/src/lib/actionErrors.ts @@ -12,3 +12,22 @@ export function notifyActionError(err: unknown, fallbackMessage: string): void { const detail = err instanceof HttpError ? err.detail : null; notify({ level: "error", message: detail || fallbackMessage }); } + +/** + * Collection write errors. The backend enforces per-library name uniqueness and returns 409 on a + * duplicate; localize that specific case to the friendly `nameTaken` copy (notifyActionError would + * otherwise surface the server's raw English detail). Everything else falls back to the generic + * edit-failed handling (which still prefers the server's reason when present). Both args are already + * translated so this stays i18n-agnostic and unit-testable. + */ +export function notifyCollectionEditError( + err: unknown, + editFailedMessage: string, + nameTakenMessage: string, +): void { + if (err instanceof HttpError && err.status === 409) { + notify({ level: "error", message: nameTakenMessage }); + return; + } + notifyActionError(err, editFailedMessage); +}