From 81b76602320f7ace10b83e244a4db076f5abec87 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 23 Aug 2026 08:08:34 +0200 Subject: [PATCH] test(plex): cover create write-through + the 409->nameTaken mapping (F-F) Backend: create-collection happy path (write-through + editable) and the create resync-failure 422 (honest reconcile detail), mirroring the rename/remove tests. Frontend: extract notifyCollectionEditError (the 409->nameTaken vs generic-fallback mapping that was inline in the New-collection create()) and unit-test it; wire create() through it. --- backend/tests/test_plex_collections_filter.py | 48 +++++++++++++++++++ frontend/src/components/plexBrowseLists.tsx | 24 ++++------ frontend/src/lib/actionErrors.test.ts | 31 ++++++++++++ frontend/src/lib/actionErrors.ts | 19 ++++++++ 4 files changed, 106 insertions(+), 16 deletions(-) create mode 100644 frontend/src/lib/actionErrors.test.ts 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/plexBrowseLists.tsx b/frontend/src/components/plexBrowseLists.tsx index c135ce25..f4fca68b 100644 --- a/frontend/src/components/plexBrowseLists.tsx +++ b/frontend/src/components/plexBrowseLists.tsx @@ -4,16 +4,9 @@ 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"; @@ -102,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); } } 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); +}