feat(plex): forbid duplicate collection names + a "New collection" button
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).
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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<string>;
|
||||
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 (
|
||||
<div className="glass-card mb-6 rounded-2xl p-4">
|
||||
<div className="mb-2 flex items-center justify-between gap-2">
|
||||
<h3 className="text-sm font-semibold">{t("plex.collection.newCollection")}</h3>
|
||||
<button
|
||||
onClick={onClose}
|
||||
aria-label={t("common.close")}
|
||||
className="glass-hover grid h-8 w-8 place-items-center rounded-lg"
|
||||
>
|
||||
<X className="h-4 w-4" />
|
||||
</button>
|
||||
</div>
|
||||
<input
|
||||
autoFocus
|
||||
value={name}
|
||||
onChange={(e) => 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 ? (
|
||||
<p className="mt-1 mb-2 text-xs text-danger">{t("plex.collection.nameTaken")}</p>
|
||||
) : (
|
||||
<p className="mt-1 mb-2 text-xs text-muted">{t("plex.collection.newHint")}</p>
|
||||
)}
|
||||
<div className="mb-3 flex items-center gap-2">
|
||||
<Search className="h-4 w-4 shrink-0 text-muted" />
|
||||
<input
|
||||
value={term}
|
||||
onChange={(e) => 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"
|
||||
/>
|
||||
</div>
|
||||
{!nameOk || debounced.length === 0 ? null : q.isLoading ? (
|
||||
<LoadingState label={t("plex.loading")} />
|
||||
) : 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>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// 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 (
|
||||
<div>
|
||||
<input
|
||||
type="search"
|
||||
value={search}
|
||||
onChange={(e) => 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"
|
||||
/>
|
||||
<div className="mb-4 flex flex-wrap items-center gap-2">
|
||||
<input
|
||||
type="search"
|
||||
value={search}
|
||||
onChange={(e) => 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 && (
|
||||
<button
|
||||
onClick={() => setCreating(true)}
|
||||
className="glass-card glass-hover inline-flex shrink-0 items-center gap-1.5 rounded-lg px-3 py-1.5 text-sm"
|
||||
>
|
||||
<Plus className="h-4 w-4" />
|
||||
{t("plex.collection.newCollection")}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
{creating && (
|
||||
<NewCollectionPanel
|
||||
existingNames={existingNames}
|
||||
onCreated={(rk) => {
|
||||
setCreating(false);
|
||||
onPick(rk, "");
|
||||
}}
|
||||
onClose={() => setCreating(false)}
|
||||
/>
|
||||
)}
|
||||
{q.isError && !q.data ? (
|
||||
<StateMessage tone="error" onRetry={() => q.refetch()}>
|
||||
{t("common.loadError")}
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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": {
|
||||
|
||||
Reference in New Issue
Block a user