Merge chore/collection-review-tail: shared PlexLibraryPicker + scroll/create tests + 409->nameTaken sweep
gate / frontend (push) Failing after 36s
gate / backend (push) Successful in 2m3s

This commit is contained in:
2026-08-23 18:42:56 +02:00
9 changed files with 408 additions and 105 deletions
@@ -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).
@@ -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 <main> 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<typeof import("react-i18next")>();
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(<PageScroller scrollKey="e:k1">x</PageScroller>);
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(<PageScroller scrollKey="e10:a">x</PageScroller>);
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(<PageScroller scrollKey="e10:b">x</PageScroller>);
expect(main.scrollTop).toBe(0);
// Returning to the first key restores its offset.
rerender(<PageScroller scrollKey="e10:a">x</PageScroller>);
expect(main.scrollTop).toBe(300);
});
});
@@ -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);
}
+22 -47
View File
@@ -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<Set<string>>(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({
<X className="h-4 w-4" />
</button>
</div>
{debounced.length === 0 ? (
<p className="text-muted text-sm">{t("plex.collection.addHint")}</p>
) : q.isLoading ? (
<LoadingState label={t("plex.loading")} />
) : q.isError ? (
<p className="text-muted text-sm">{t("common.loadError")}</p>
) : results.length === 0 ? (
<p className="text-muted text-sm">{t("plex.collection.addNoResults")}</p>
) : (
<ul className="flex flex-col gap-1">
{results.map((c) => {
const inColl = existing.has(c.id) || added.has(c.id);
return (
<li key={c.id} className="flex items-center gap-3">
<img
src={c.thumb}
alt=""
className="h-12 w-8 shrink-0 rounded object-cover"
loading="lazy"
/>
<span className="min-w-0 flex-1 truncate text-sm">
{c.title}
{c.year != null && <span className="text-muted"> · {c.year}</span>}
</span>
<button
onClick={() => add(c)}
disabled={inColl}
className="glass-card glass-hover inline-flex items-center gap-1 rounded-lg px-2.5 py-1 text-xs disabled:opacity-50"
>
{inColl ? <Check className="h-3.5 w-3.5" /> : <Plus className="h-3.5 w-3.5" />}
{inColl ? t("plex.collection.added") : t("plex.collection.add")}
</button>
</li>
);
})}
</ul>
)}
<PlexLibraryPicker
scope={scope}
term={term}
emptyHint={<p className="text-muted text-sm">{t("plex.collection.addHint")}</p>}
renderAction={(c) => {
const inColl = existing.has(c.id) || added.has(c.id);
return (
<button
onClick={() => add(c)}
disabled={inColl}
className="glass-card glass-hover inline-flex items-center gap-1 rounded-lg px-2.5 py-1 text-xs disabled:opacity-50"
>
{inColl ? <Check className="h-3.5 w-3.5" /> : <Plus className="h-3.5 w-3.5" />}
{inColl ? t("plex.collection.added") : t("plex.collection.add")}
</button>
);
}}
/>
</div>
);
}
@@ -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<typeof import("react-i18next")>();
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<typeof card>[]) => ({
scope: "both",
total: items.length,
offset: 0,
limit: 12,
items,
episodes: [],
});
beforeEach(() => {
plexLibrary.mockReset();
plexLibrary.mockResolvedValue(result([]));
});
afterEach(cleanup);
const renderPicker = (props: Partial<ComponentProps<typeof PlexLibraryPicker>> = {}) => {
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
return render(
<QueryClientProvider client={qc}>
<PlexLibraryPicker
scope="both"
term=""
renderAction={(c) => <button>act:{c.id}</button>}
{...props}
/>
</QueryClientProvider>,
);
};
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: <p>EMPTY_HINT</p> });
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());
});
});
@@ -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 <LoadingState label={t("plex.loading")} />;
if (q.isError) return <p className="text-muted text-sm">{t("common.loadError")}</p>;
if (results.length === 0) return <p className="text-muted text-sm">{t("plex.collection.addNoResults")}</p>;
return (
<ul className="flex flex-col gap-1">
{results.map((c) => (
<li key={c.id} className="flex items-center gap-3">
<img src={c.thumb} alt="" className="h-12 w-8 shrink-0 rounded object-cover" loading="lazy" />
<span className="min-w-0 flex-1 truncate text-sm">
{c.title}
{c.year != null && <span className="text-muted"> · {c.year}</span>}
</span>
{renderAction(c)}
</li>
))}
</ul>
);
}
+24 -55
View File
@@ -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"
/>
</div>
{!nameOk || debounced.length === 0 ? null : q.isLoading ? (
<LoadingState label={t("plex.loading")} />
) : q.isError ? (
<p className="text-muted text-sm">{t("common.loadError")}</p>
) : results.length === 0 ? (
<p className="text-muted text-sm">{t("plex.collection.addNoResults")}</p>
) : (
<ul className="flex flex-col gap-1">
{results.map((c) => (
<li key={c.id} className="flex items-center gap-3">
<img
src={c.thumb}
alt=""
className="h-12 w-8 shrink-0 rounded object-cover"
loading="lazy"
/>
<span className="min-w-0 flex-1 truncate text-sm">
{c.title}
{c.year != null && <span className="text-muted"> · {c.year}</span>}
</span>
<button
onClick={() => create(c)}
disabled={busy}
className="glass-card glass-hover inline-flex items-center gap-1 rounded-lg px-2.5 py-1 text-xs disabled:opacity-50"
>
<Check className="h-3.5 w-3.5" />
{t("plex.collection.createWith")}
</button>
</li>
))}
</ul>
)}
<PlexLibraryPicker
scope="both"
term={term}
enabled={nameOk}
renderAction={(c) => (
<button
onClick={() => create(c)}
disabled={busy}
className="glass-card glass-hover inline-flex items-center gap-1 rounded-lg px-2.5 py-1 text-xs disabled:opacity-50"
>
<Check className="h-3.5 w-3.5" />
{t("plex.collection.createWith")}
</button>
)}
/>
</div>
);
}
+31
View File
@@ -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" });
});
});
+19
View File
@@ -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);
}