From b71b33654a21836ce016eee88a67fc74ee0f32fa Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 23 Aug 2026 05:51:48 +0200 Subject: [PATCH] feat(plex): forbid duplicate collection names + a "New collection" button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups after testing the collection editing against real Plex, which revealed that Plex CONFLATES same-named collections (two rating_keys resolve to one merged member list) — a genuine footgun. - Duplicate-name guard: create and rename now reject (409) a title that already exists in the target section (case-insensitive), before any Plex write. create() can also derive the library from the seed item, so the caller needn't know the section key. The create dialogs surface the 409, and the New-collection panel warns inline before submit. - A "New collection" button on the Collections browse surface (admin only) opens a panel: a name field (with the live duplicate check) + a search to pick the FIRST title — Plex can't create an empty collection, it needs a seed. Creating opens the new collection's detail page straight away, so you can keep adding titles there. This is the B-route the user wanted, vs. the only prior path (a movie's "add to collection"). Verified live: the 409 guard (creating an existing name is blocked, no Plex write), and the full New-collection flow (name + first title → create → lands on the detail page); test collections created and deleted, real account left clean. Gate: tsc/lint/knip/prettier clean, 424 vitest; backend ruff clean, 7 collections tests (+ a duplicate-name-rejection assertion). --- backend/app/routes/plex/collections.py | 29 ++- backend/tests/test_plex_collections_filter.py | 27 ++- .../src/components/PlexCollectionEditor.tsx | 4 + frontend/src/components/plexBrowseLists.tsx | 167 ++++++++++++++++-- frontend/src/i18n/locales/en/plex.json | 7 +- frontend/src/i18n/locales/hu/plex.json | 7 +- 6 files changed, 220 insertions(+), 21 deletions(-) diff --git a/backend/app/routes/plex/collections.py b/backend/app/routes/plex/collections.py index 4a4ceff2..58091e07 100644 --- a/backend/app/routes/plex/collections.py +++ b/backend/app/routes/plex/collections.py @@ -192,6 +192,22 @@ def _editable_collection_or_403(db: Session, rating_key: str) -> PlexCollection: return col +def _reject_duplicate_name( + db: Session, library_id: int, title: str, exclude_rk: str | None = None +) -> None: + """No two collections in a section may share a name: Plex CONFLATES same-named collections' + membership (both rating_keys resolve to one merged member list — a real footgun), so forbid it up + front on create + rename rather than mirror the mess.""" + dup = db.execute( + select(PlexCollection).where( + PlexCollection.library_id == library_id, + func.lower(PlexCollection.title) == title.lower(), + ) + ).scalars().first() + if dup is not None and dup.rating_key != exclude_rk: + raise HTTPException(status_code=409, detail="A collection with this name already exists") + + class CreateCollectionIn(BaseModel): library: str = "" title: str = "" @@ -212,11 +228,17 @@ def create_collection( raise HTTPException( status_code=400, detail="title and item_rating_key are required" ) - lib = db.execute( - select(PlexLibrary).where(PlexLibrary.plex_key == library) - ).scalars().first() + if library: + lib = db.execute( + select(PlexLibrary).where(PlexLibrary.plex_key == library) + ).scalars().first() + else: + # The "New collection" flow only knows the seed title → derive its library. + seed_item = by_rating_key(db, PlexItem, seed) or by_rating_key(db, PlexShow, seed) + lib = db.get(PlexLibrary, seed_item.library_id) if seed_item else None if lib is None: raise HTTPException(status_code=404, detail="Unknown library") + _reject_duplicate_name(db, lib.id, title) try: with PlexClient(db) as plex: new = plex.create_collection(lib.plex_key, title, seed) @@ -284,6 +306,7 @@ def collection_rename( title = body.title.strip() if not title: raise HTTPException(status_code=400, detail="title is required") + _reject_duplicate_name(db, col.library_id, title, exclude_rk=rating_key) lib = db.get(PlexLibrary, col.library_id) try: with PlexClient(db) as plex: diff --git a/backend/tests/test_plex_collections_filter.py b/backend/tests/test_plex_collections_filter.py index 1b028e1e..926655f1 100644 --- a/backend/tests/test_plex_collections_filter.py +++ b/backend/tests/test_plex_collections_filter.py @@ -7,7 +7,7 @@ from datetime import datetime, timezone from fastapi.testclient import TestClient import app.state as state -from app.auth import current_user +from app.auth import admin_user, current_user from app.db import get_db from app.main import app from app.models import PlexCollection, PlexItem, PlexLibrary, PlexShow, User @@ -94,6 +94,31 @@ def test_collection_cards_carry_montage_art_and_total_runtime(db, monkeypatch): assert card["total_runtime_s"] == 5400 # 3600 + 1800 +def test_create_collection_rejects_duplicate_name(db, monkeypatch): + # Same-named collections conflate in Plex, so a create with an existing name (case-insensitive) is + # rejected 409 — BEFORE any Plex call (the guard runs first, so the DB lane can assert it). + monkeypatch.setattr(state, "_configured_cache", True) + lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True) + db.add(lib) + db.flush() + db.add(PlexCollection(rating_key="A", library_id=lib.id, title="Marvel")) + db.add(PlexItem(rating_key="m1", library_id=lib.id, kind="movie", title="M1")) + user = User(email="dup@example.com") + db.add(user) + db.commit() + app.dependency_overrides[get_db] = lambda: db + app.dependency_overrides[current_user] = lambda: user + app.dependency_overrides[admin_user] = lambda: user + try: + r = TestClient(app).post( + "/api/plex/collections", + json={"library": "1", "title": "marvel", "item_rating_key": "m1"}, + ) + assert r.status_code == 409, r.text + finally: + app.dependency_overrides.clear() + + def test_collection_detail_hero_and_ordered_members(db, monkeypatch): # The Collection info page: hero aggregates (year range, most-restrictive cert, total runtime) + # members ordered by release year then title. diff --git a/frontend/src/components/PlexCollectionEditor.tsx b/frontend/src/components/PlexCollectionEditor.tsx index 4bb88a97..9b3df1ee 100644 --- a/frontend/src/components/PlexCollectionEditor.tsx +++ b/frontend/src/components/PlexCollectionEditor.tsx @@ -4,6 +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 { useScrollFade } from "../lib/useScrollFade"; import Modal from "./Modal"; import { useConfirm } from "./ConfirmProvider"; @@ -100,6 +101,9 @@ export default function PlexCollectionEditor({ setNewName(""); 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")); } finally { setBusy(null); } diff --git a/frontend/src/components/plexBrowseLists.tsx b/frontend/src/components/plexBrowseLists.tsx index 3a725550..9255a4bd 100644 --- a/frontend/src/components/plexBrowseLists.tsx +++ b/frontend/src/components/plexBrowseLists.tsx @@ -1,11 +1,12 @@ -import { useState } from "react"; +import { useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import type { TFunction } from "i18next"; -import { useQuery } from "@tanstack/react-query"; -import { ListMusic, Lock } from "lucide-react"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; +import { Check, ListMusic, Lock, Plus, Search, X } from "lucide-react"; import { qk } from "../lib/queryKeys"; -import { api, type PlexCollection } from "../lib/api"; +import { api, type PlexCard, type PlexCollection } from "../lib/api"; import { formatRuntime } from "../lib/format"; +import { notifyActionError } from "../lib/actionErrors"; import { useDebounced } from "../lib/useDebounced"; import { LoadingState, StateMessage } from "./QueryState"; import PosterTile from "./PosterTile"; @@ -68,28 +69,164 @@ export function PlexPlaylistsBrowse({ onOpen }: { onOpen: (id: number) => void } ); } -// A grid of Plex collections (searchable — the union can run to hundreds); a tile filters the title -// grid by that collection (via the parent's onPick → applyFilter). +// The "New collection" panel (admin, ask B-route): name (rejected if it duplicates an existing one — +// Plex conflates same-named collections) + a search to pick the FIRST title, since Plex can't create an +// empty collection. Creating opens the new collection's detail page (onCreated → onPick). +function NewCollectionPanel({ + existingNames, + onCreated, + onClose, +}: { + existingNames: Set; + onCreated: (rk: string) => void; + onClose: () => void; +}) { + const { t } = useTranslation(); + const [name, setName] = useState(""); + const [term, setTerm] = useState(""); + const [busy, setBusy] = useState(false); + const debounced = useDebounced(term.trim(), 300); + const trimmed = name.trim(); + const dup = trimmed.length > 0 && existingNames.has(trimmed.toLowerCase()); + const nameOk = trimmed.length > 0 && !dup; + 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; + setBusy(true); + try { + const col = await api.plexCreateCollection("", trimmed, card.id); // backend derives the library + onCreated(col.id); + } catch (e) { + notifyActionError(e, t("plex.collection.editFailed")); + setBusy(false); + } + } + + return ( +
+
+

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

+ +
+ setName(e.target.value)} + placeholder={t("plex.collection.namePlaceholder")} + aria-label={t("plex.collection.newCollection")} + className="glass-card w-full rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40" + /> + {dup ? ( +

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

+ ) : ( +

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

+ )} +
+ + setTerm(e.target.value)} + disabled={!nameOk} + placeholder={t("plex.collection.addSearch")} + aria-label={t("plex.collection.addSearch")} + 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 ? ( + + ) : results.length === 0 ? ( +

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

+ ) : ( +
    + {results.map((c) => ( +
  • + + + {c.title} + {c.year != null && · {c.year}} + + +
    • + ))} +
+ )} +
+ ); +} + +// A grid of Plex collections (searchable — the union can run to hundreds); a tile opens its info page. export function PlexCollectionsBrowse({ onPick }: { onPick: (id: string, title: string) => void }) { const { t } = useTranslation(); + const qc = useQueryClient(); + const isAdmin = qc.getQueryData<{ role?: string }>(qk.me())?.role === "admin"; const [search, setSearch] = useState(""); + const [creating, setCreating] = useState(false); const searchDeb = useDebounced(search.trim(), 300); const q = useQuery({ queryKey: qk.plexCollections("union", searchDeb), queryFn: () => api.plexCollections(undefined, searchDeb || undefined), }); - const collections = q.data?.collections ?? []; + const collections = useMemo(() => q.data?.collections ?? [], [q.data?.collections]); + const existingNames = useMemo( + () => new Set(collections.map((c) => c.title.trim().toLowerCase())), + [collections], + ); return (
- setSearch(e.target.value)} - placeholder={t("plex.filter.collectionSearch")} - aria-label={t("plex.filter.collectionSearch")} - className="glass-card mb-4 w-full max-w-sm rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40" - /> +
+ setSearch(e.target.value)} + placeholder={t("plex.filter.collectionSearch")} + aria-label={t("plex.filter.collectionSearch")} + className="glass-card w-full max-w-sm rounded-lg px-3 py-1.5 text-sm focus:outline-hidden focus:ring-2 focus:ring-accent/40" + /> + {isAdmin && !creating && ( + + )} +
+ {creating && ( + { + setCreating(false); + onPick(rk, ""); + }} + onClose={() => setCreating(false)} + /> + )} {q.isError && !q.data ? ( q.refetch()}> {t("common.loadError")} diff --git a/frontend/src/i18n/locales/en/plex.json b/frontend/src/i18n/locales/en/plex.json index 48ee9e5d..e78bd428 100644 --- a/frontend/src/i18n/locales/en/plex.json +++ b/frontend/src/i18n/locales/en/plex.json @@ -20,7 +20,12 @@ "add": "Add", "added": "Added", "deleteConfirm": "Delete the collection \"{{title}}\"? This removes it from Plex too.", - "editFailed": "Collection edit failed" + "editFailed": "Collection edit failed", + "newCollection": "New collection", + "namePlaceholder": "Collection name", + "nameTaken": "A collection with this name already exists.", + "newHint": "Pick the first title to add — a collection needs at least one.", + "createWith": "Create" }, "searchPlaceholder": "Search Plex…", "searchField": { diff --git a/frontend/src/i18n/locales/hu/plex.json b/frontend/src/i18n/locales/hu/plex.json index ad2869b4..6c765462 100644 --- a/frontend/src/i18n/locales/hu/plex.json +++ b/frontend/src/i18n/locales/hu/plex.json @@ -20,7 +20,12 @@ "add": "Hozzáad", "added": "Hozzáadva", "deleteConfirm": "Törlöd a(z) „{{title}}” kollekciót? Ez a Plexből is eltávolítja.", - "editFailed": "A kollekció szerkesztése sikertelen" + "editFailed": "A kollekció szerkesztése sikertelen", + "newCollection": "Új kollekció", + "namePlaceholder": "Kollekció neve", + "nameTaken": "Már létezik ilyen nevű kollekció.", + "newHint": "Válaszd ki az első címet — egy kollekcióhoz legalább egy elem kell.", + "createWith": "Létrehozás" }, "searchPlaceholder": "Keresés a Plexben…", "searchField": {