diff --git a/backend/app/discover/cards.py b/backend/app/discover/cards.py index 336e917f..eb481364 100644 --- a/backend/app/discover/cards.py +++ b/backend/app/discover/cards.py @@ -112,7 +112,7 @@ def from_plex(row: dict) -> dict: def from_channel(ch: Channel, unwatched: int) -> dict: """A pinned channel. It is not itself playable — the shelf resolves it to its newest unwatched - video on click (see `resolve.py`) — so the tile advertises what is waiting there instead of a + video on click (see `pins._shelf_card`) — so the tile advertises what is waiting there instead of a runtime it does not have.""" card = _base( source="youtube", diff --git a/backend/app/routes/discover.py b/backend/app/routes/discover.py index 7493359a..3ff5465d 100644 --- a/backend/app/routes/discover.py +++ b/backend/app/routes/discover.py @@ -148,7 +148,11 @@ def add_pin( ) ).scalar_one_or_none() if winner is None: - raise + # Not the unique constraint, then: the only other way in is the target row being pruned + # between `resolve_ref` confirming it and this INSERT (a Plex sync dropping a removed + # item), which trips the FK. Semantically that is the same "no such thing" the pre-check + # returns above — re-raising handed the user a 500 they could do nothing with. + raise HTTPException(status_code=404, detail="Unknown item") return {"ok": True, "id": winner.id, "created": False} return {"ok": True, "id": pin.id, "created": True} diff --git a/backend/tests/test_discover.py b/backend/tests/test_discover.py index 0ae2d56e..a2ecf648 100644 --- a/backend/tests/test_discover.py +++ b/backend/tests/test_discover.py @@ -488,6 +488,20 @@ def test_a_pin_must_reference_exactly_one_thing(db): db.rollback() +def test_the_same_thing_cannot_be_pinned_twice(db): + """The per-kind unique constraints are what make the route's concurrent-tab path safe (it catches + `IntegrityError` precisely because of them) — and the migration drift check filters constraint + diffs out as noise, so nothing else would notice them going missing. The route's own dedupe + returns early without ever reaching the DB, so only this test exercises them.""" + user, _ = _seed_yt(db) + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=0)) + db.commit() + db.add(ComfortPin(user_id=user.id, video_id="vmed", position=1)) + with pytest.raises(IntegrityError): + db.flush() + db.rollback() + + def test_deleting_the_pinned_row_removes_the_pin(db): """A film dropped from the library must not leave a dead tile on the one surface whose whole job is to be reliable — which is why the shelf holds foreign keys, not strings.""" diff --git a/frontend/src/components/Home.dom.test.tsx b/frontend/src/components/Home.dom.test.tsx index 00c41e09..9490feb2 100644 --- a/frontend/src/components/Home.dom.test.tsx +++ b/frontend/src/components/Home.dom.test.tsx @@ -50,6 +50,7 @@ vi.mock("../lib/api", () => ({ watchLaterAdd: () => Promise.resolve(), watchLaterRemove: () => Promise.resolve(), }, + DISCOVER_BUDGETS: ["short", "medium", "long", "any"], discoverCardKey: (c: { source: string; id: string }) => `${c.source}:${c.id}`, })); diff --git a/frontend/src/components/Home.pin.dom.test.tsx b/frontend/src/components/Home.pin.dom.test.tsx index 0cd3e386..2a91e8ce 100644 --- a/frontend/src/components/Home.pin.dom.test.tsx +++ b/frontend/src/components/Home.pin.dom.test.tsx @@ -59,6 +59,7 @@ vi.mock("../lib/api", () => ({ reorderComfortPins: (...a: unknown[]) => reorderComfortPins(...a), }, HttpError: class extends Error {}, + DISCOVER_BUDGETS: ["short", "medium", "long", "any"], discoverCardKey: (c: { source: string; id: string }) => `${c.source}:${c.id}`, })); diff --git a/frontend/src/components/Home.tsx b/frontend/src/components/Home.tsx index 93eee986..2958bcdc 100644 --- a/frontend/src/components/Home.tsx +++ b/frontend/src/components/Home.tsx @@ -13,6 +13,7 @@ import { import { qk } from "../lib/queryKeys"; import { api, + DISCOVER_BUDGETS, discoverCardKey, type DiscoverBudget, type DiscoverCard, @@ -50,12 +51,16 @@ import { usePlexActions } from "./PlexProvider"; // // Everything domain-shaped is already built: the cards are the feed's `VideoCard` and Plex's // `PlexPosterCard`, rendered from the native payload each unified card carries, and the rails are -// the platform `Carousel`. Nothing here is a third card component. +// the platform `Carousel`. The two PLAYABLE card faces are reused rather than re-drawn; the only +// card this file defines is `ShelfTile`, because a pinned channel or playlist has no card anywhere +// in the app and `PosterTile`'s art is hardcoded to a Plex image URL. // Lazy, exactly as the feed does it: the YouTube IFrame player loads only once something is played. const PlayerModal = lazy(() => import("./PlayerModal")); -const BUDGETS: readonly DiscoverBudget[] = ["short", "medium", "long", "any"]; +// From the api slice's own list, not a second copy: `readonly DiscoverBudget[]` accepts any +// SUBSET, so a band added server-side and to `DISCOVER_BUDGETS` typechecked green while this row +// silently kept offering four. // A rail tile's slot width. `cardsSmall` is the feed's own dense card (its column bottoms out at // 180px), so a 200px slot is the same object the feed shows — not a Home-only variant that would @@ -139,6 +144,9 @@ function Rail({ label: t("home.pin.remove"), icon: , separatorBefore: true, + // The only row here that removes a curated entry, and it sat in the same colour as + // the four harmless reorders. Every other destructive row in the app is `danger`. + danger: true, // The pin id travels on the shelf card, so this needs no lookup — and unlike every // other surface it can never be in the "not known yet" state. onSelect: () => pin.remove(target.pin_id as number, target.title), @@ -153,7 +161,7 @@ function Rail({ return (

{title}

- {emptyLabel} + {emptyLabel}
); } @@ -288,7 +296,7 @@ function Hero({ className={`relative block w-full overflow-hidden rounded-lg border border-border bg-card ${ card.aspect === "poster" ? "aspect-[2/3] max-w-[12rem]" : "aspect-video" }`} - aria-label={t("home.hero.play")} + aria-label={`${started ? t("home.hero.continue") : t("home.hero.play")}: ${card.title}`} > {card.thumb ? : null} {card.duration_seconds != null && ( @@ -347,8 +355,9 @@ export default function Home() { const [press, setPress] = useState(0); // The cards the reroll has already offered this session, newest last. A ROLLING window rather than // the whole history: an unbounded list would eventually exclude everything and leave the button - // with nothing to give, whereas a window of 8 can never exhaust a library and only repeats after - // eight presses. Kept here (not in the URL or a pref) because it is a property of this sitting. + // with nothing to give, whereas a bounded one cannot. Its size is `SEEN_WINDOW` — see there for + // why it is 32 and not the 8 this comment used to quote. Kept here (not in the URL or a pref) + // because it is a property of this sitting. const [seen, setSeen] = useState([]); const [player, setPlayer] = useState<{ video: Video; startAt: number | null } | null>(null); @@ -415,24 +424,31 @@ export default function Home() { // A full refetch, unlike the feed's stale-mark: these rails are two dozen rows, and marking // something watched genuinely changes what the page should answer (it can retire the hero). // The feed avoids this because it would refetch ten infinite-query pages per click. - void api.setState(id, status).then(() => qc.invalidateQueries({ queryKey: qk.discover() })); + void api + .setState(id, status) + .then(() => qc.invalidateQueries({ queryKey: qk.discover() })) + // Without this the rejection escapes unhandled and the click just does nothing — the + // feed's equivalents both catch, and the reorder on this same page already does. + .catch((e: unknown) => notifyActionError(e, t("home.error"))); }, - [qc], + [qc, t], ); const onToggleSave = useCallback( (id: string, saved: boolean) => { - void (saved ? api.watchLaterAdd(id) : api.watchLaterRemove(id)).then(() => { - qc.invalidateQueries({ queryKey: qk.playlists() }); - qc.invalidateQueries({ queryKey: qk.playlist() }); - // And the payload the star is READ from. `VideoCard` derives the filled/empty bookmark - // straight from `video.saved` and keeps no optimistic state of its own (the feed instead - // holds a `savedOverrides` map), so without this the icon never changed and a second click - // re-sent the same call — the control looked dead. - qc.invalidateQueries({ queryKey: qk.discover() }); - }); + void (saved ? api.watchLaterAdd(id) : api.watchLaterRemove(id)) + .then(() => { + qc.invalidateQueries({ queryKey: qk.playlists() }); + qc.invalidateQueries({ queryKey: qk.playlist() }); + // And the payload the star is READ from. `VideoCard` derives the filled/empty bookmark + // straight from `video.saved` and keeps no optimistic state of its own (the feed instead + // holds a `savedOverrides` map), so without this the icon never changed and a second click + // re-sent the same call — the control looked dead. + qc.invalidateQueries({ queryKey: qk.discover() }); + }) + .catch((e: unknown) => notifyActionError(e, t("home.error"))); }, - [qc], + [qc, t], ); // One click plays. A shelf tile resolves through its `next` (a show → the episode that would @@ -581,7 +597,7 @@ export default function Home() { setSeen([]); // a new question deserves the whole library again setPress(0); // …and the tier rotation starts from the page's own priority order }} - options={BUDGETS.map((b) => ({ value: b, label: t(`home.budget.${b}`) }))} + options={DISCOVER_BUDGETS.map((b) => ({ value: b, label: t(`home.budget.${b}`) }))} /> @@ -597,9 +613,19 @@ export default function Home() { // Honest: nothing in the library fits what was asked. Offering something that does not fit // would make the one control on this page meaningless.
- + {/* Not `StateMessage`'s `onRetry` slot: that button is labelled "Retry", which promises + a re-run of something that failed. Nothing failed — the library simply has no answer + this short, and the honest offer is to ask a different question. */} + {budget === "any" ? t("home.hero.emptyAny") : t("home.hero.emptyBudget")} + {budget !== "any" && ( +
+ +
+ )}
)} diff --git a/frontend/src/components/comfortPin.tsx b/frontend/src/components/comfortPin.tsx index a40f1f0a..aac38117 100644 --- a/frontend/src/components/comfortPin.tsx +++ b/frontend/src/components/comfortPin.tsx @@ -75,11 +75,9 @@ export function shelfMoveTarget(from: number, to: ShelfMove, length: number): nu const targetKey = (target: PinTarget): string => `${target.kind}:${target.ref}`; export interface ComfortPin { - /** The pin's id when the target is on the shelf, `null` when it is not, `undefined` while the - * shelf is still being fetched — three states, because "not pinned" and "not known yet" must not - * render as the same row. */ - pinIdOf: (target: PinTarget) => number | null | undefined; - toggle: (target: PinTarget, title: string) => void; + /** `pinIdOf` and `toggle` stay module-local until a second consumer needs them: publishing them + * invites the next surface to re-derive the three-state loading dance (pinned / not pinned / + * not known yet) that `item` already encapsulates, which is what this file exists to prevent. */ remove: (pinId: number, title: string) => void; /** The pin/unpin row, ready to spread into any `ActionMenuItem[]`. Empty for a null target. */ item: (target: PinTarget | null, title: string) => ActionMenuItem[]; @@ -201,5 +199,5 @@ export function useComfortPin(open: boolean): ComfortPin { [pinIdOf, t, toggle], ); - return { pinIdOf, toggle, remove, item }; + return { remove, item }; } diff --git a/frontend/src/components/ui/QueryState.tsx b/frontend/src/components/ui/QueryState.tsx index 2aa85523..2b18abb2 100644 --- a/frontend/src/components/ui/QueryState.tsx +++ b/frontend/src/components/ui/QueryState.tsx @@ -29,17 +29,24 @@ export function StateMessage({ children, onRetry, tone = "muted", + compact = false, className = "", }: { children: ReactNode; onRetry?: () => void; tone?: "muted" | "error"; + /** A rung down in vertical rhythm, for a message that sits INSIDE a section that already has its + * own heading and padding (a rail's empty state) rather than standing alone on a page. Here + * rather than at the call site because the only two consumers so far had to reach for `!py-6` to + * beat this component's own `py-10` — the first `!important` overrides in the frontend, and the + * copyable idiom for the next surface that wants a compact empty state. */ + compact?: boolean; className?: string; }) { const { t } = useTranslation(); return (