test(plex): drive the playlist's reorder and rollback through a hook

The Plex playlist's reorder is a dnd-kit drag, and neither of its sensors activates under jsdom,
so `commit()`'s failure path could not be reached from a test. The query, the optimistic copy and
the two writes (reorder, remove) move into `usePlexPlaylistEdits`, unchanged, and are driven
directly: a failed reorder is said and the rows go back; an accepted order lands in the detail
cache; a failed removal comes back.

It also settles the `putBack` question that could not be driven before: with the refreshed list
taken in BEFORE the reorder fails, a rollback bound to the render that started the reorder puts a
removed row back, and the deep-equal refetch never clears it. The live cache read the code has now
is what keeps it out; the test fails if it goes back to the closure's `q.data`.

frontend/src/components/PlexPlaylistView.tsx

@@ -1,7 +1,7 @@
 import { useEffect, useMemo, useState } from "react";
 import { qk } from "../lib/queryKeys";
 import { useTranslation } from "react-i18next";
-import { useQuery, useQueryClient } from "@tanstack/react-query";
+import { useQueryClient } from "@tanstack/react-query";
 import {
   DndContext,
   KeyboardSensor,
@@ -40,6 +40,7 @@ import { Button } from "./ui/Button";
 import { LoadingState, StateMessage } from "./ui/QueryState";
 import ViewSwitcher, { type ViewOption } from "./ViewSwitcher";
 import { FocusSafeButton } from "./ui/FocusSafeButton";
+import { usePlexPlaylistEdits } from "./usePlexPlaylistEdits";
 
 // A playlist's items, grouped for readability: movies stay standalone, while a run of episodes from the
 // same show collapses into a season/show group so a whole series doesn't sprawl into an endless flat
@@ -381,7 +382,6 @@ export default function PlexPlaylistView({
   const confirm = useConfirm();
   const [renaming, setRenaming] = useState(false);
   const [name, setName] = useState("");
-  const [busy, setBusy] = useState(false);
   const [layout, setLayout] = useState<Layout>(() =>
     getAccountRaw(LS.plexPlaylistLayout) === "tree" ? "tree" : "accordion",
   );
@@ -393,15 +393,10 @@ export default function PlexPlaylistView({
   const [expandedShows, setExpandedShows] = useState<Set<string>>(new Set());
   const [collapsedSeasons, setCollapsedSeasons] = useState<Set<string>>(new Set());
 
-  const q = useQuery({
+  // The query, the optimistic copy and the two writes that edit it (reorder, remove) live in
-    queryKey: qk.plexPlaylist(playlistId),
+  // `usePlexPlaylistEdits`, where their failure paths can be driven directly: the drag that reaches
-    queryFn: () => api.plexPlaylist(playlistId),
+  // `commit` cannot be, since dnd-kit's sensors do not activate under jsdom.
-  });
+  const { q, items, busy, invalidate, commit, onRemove } = usePlexPlaylistEdits(playlistId);
-  // Local optimistic copy so drag/remove are instant; re-synced whenever the query returns.
-  const [items, setItems] = useState<PlexCard[]>([]);
-  useEffect(() => {
-    if (q.data) setItems(q.data.items);
-  }, [q.data]);
   const blocks = useMemo(() => buildBlocks(items), [items]);
   const order = useMemo(() => items.map((i) => i.id), [items]);
   // Total playtime of the whole playlist, mirroring the YouTube playlist header (E9 12D): items with
@@ -411,78 +406,6 @@ export default function PlexPlaylistView({
     [items],
   );
 
-  const invalidate = () => {
-    qc.invalidateQueries({ queryKey: qk.plexPlaylist(playlistId) });
-    qc.invalidateQueries({ queryKey: qk.plexPlaylists() });
-  };
-
-  // Every edit here is optimistic on the local copy, and none of them used to handle a failure: a
-  // removal that failed stayed removed on screen (the resync never ran), and a 404 — the playlist
-  // deleted on another device, now a structured `not_found` — was an unhandled rejection that said
-  // nothing at all (deep-review round 4; the YouTube side was fixed in round 3). A failure is said,
-  // and the rows are put back from what the server last sent — explicitly, because a resync that
-  // returns the same list keeps the same `q.data` and the re-sync effect above never runs.
-  // Read the cache AT ROLLBACK TIME, not the `q.data` of the render that bound the failing handler:
-  // a removal completing while a reorder was in flight refreshed the list, and the reorder's stale
-  // closure then put the removed row back on screen — where the deep-equal refetch could not clear
-  // it, and the next drag persisted an order containing it (deep-review round 5). `Playlists.tsx`'s
-  // sibling `restoreFromCache` reads it live for the same reason.
-  const putBack = () => {
-    const cur = qc.getQueryData<{ items: PlexCard[] }>(qk.plexPlaylist(playlistId));
-    if (cur) setItems(cur.items);
-    invalidate();
-  };
-
-  function commit(next: PlexCard[]) {
-    if (busy) return; // a reorder racing a removal is what makes a rollback ambiguous
-    setItems(next);
-    api
-      .plexReorderPlaylist(
-        playlistId,
-        next.map((i) => i.id),
-      )
-      .then(() => {
-        // Write the accepted order into the DETAIL cache, not just the list. That cache is what
-        // `putBack` rolls back to and what a re-entry inside the 30 s staleTime renders without
-        // refetching — so after an accepted reorder it showed the pre-reorder order again, and the
-        // next drag from there persisted it, silently undoing the one the server took. The
-        // YouTube sibling (`Playlists.tsx`) got this in round 5; this half was left behind (round
-        // 6). One accepted write, one cache entry that matches it.
-        qc.setQueryData<{ items: PlexCard[] }>(qk.plexPlaylist(playlistId), (old) =>
-          old ? { ...old, items: next } : old,
-        );
-        qc.invalidateQueries({ queryKey: qk.plexPlaylist(playlistId) });
-        // …and then REFETCH it, exactly as the sibling does. `next` is this render's membership, and
-        // the server only reorders the keys it was given — so anything that landed meanwhile (a row
-        // removed from another surface, an item added elsewhere) is clobbered by that write. With
-        // only the list key invalidated nothing corrected it: staleTime is 30 s and
-        // refetchOnWindowFocus is off, so the wrong membership stayed on screen for as long as the
-        // view was mounted and the next drag persisted an order built from it (round 7).
-        qc.invalidateQueries({ queryKey: qk.plexPlaylists() });
-      })
-      .catch((e: unknown) => {
-        notifyActionError(e, t("plex.playlist.reorderFailed"));
-        putBack();
-      });
-  }
-
-  async function onRemove(keys: string[]) {
-    if (busy || !keys.length) return;
-    setBusy(true);
-    const set = new Set(keys);
-    setItems((prev) => prev.filter((i) => !set.has(i.id)));
-    try {
-      if (keys.length === 1) await api.plexPlaylistRemoveItem(playlistId, keys[0]!);
-      else await api.plexPlaylistRemoveBulk(playlistId, keys);
-      invalidate();
-    } catch (e) {
-      notifyActionError(e, t("plex.playlist.removeFailed"));
-      putBack();
-    } finally {
-      setBusy(false);
-    }
-  }
-
   async function rename() {
     const title = name.trim();
     if (!title) return;

frontend/src/components/usePlexPlaylistEdits.dom.test.tsx

@@ -0,0 +1,125 @@
+// @vitest-environment jsdom
+//
+// The Plex playlist's reorder is a dnd-kit drag, and neither of dnd-kit's sensors activates under
+// jsdom — so `commit()`'s failure path, the one edit written as a promise chain (the easiest to lose
+// in an async/await refactor), was unreachable from any test (deep-review round 5). The writes live
+// in this hook now, and are driven directly.
+import { describe, it, expect, vi, afterEach } from "vitest";
+import { renderHook, waitFor, act, cleanup } from "@testing-library/react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import type { ReactNode } from "react";
+
+vi.mock("react-i18next", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("react-i18next")>()),
+  useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }),
+}));
+const { notify } = vi.hoisted(() => ({
+  notify: vi.fn((_n: { level: string; message: string }) => 0),
+}));
+vi.mock("../lib/notifications", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../lib/notifications")>()),
+  notify,
+}));
+
+import { usePlexPlaylistEdits } from "./usePlexPlaylistEdits";
+import { api, type PlexCard } from "../lib/api";
+import { qk } from "../lib/queryKeys";
+
+afterEach(() => {
+  cleanup();
+  vi.restoreAllMocks();
+  notify.mockClear();
+});
+
+const movie = (id: string) => ({ id, type: "movie", title: `Movie ${id}` }) as unknown as PlexCard;
+const ids = (items: PlexCard[]) => items.map((i) => i.id);
+// The failure is said under the action's own title (`notifyActionError(err, title)`).
+const errors = () =>
+  notify.mock.calls
+    .filter(([n]) => n.level === "error")
+    .map(([n]) => (n as { title?: string }).title);
+
+function setup(items: PlexCard[]) {
+  const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
+  const detail = vi
+    .spyOn(api, "plexPlaylist")
+    .mockResolvedValue({ id: 5, title: "F", items } as never);
+  const wrapper = ({ children }: { children: ReactNode }) => (
+    <QueryClientProvider client={qc}>{children}</QueryClientProvider>
+  );
+  const hook = renderHook(() => usePlexPlaylistEdits(5), { wrapper });
+  return { qc, hook, detail };
+}
+
+describe("usePlexPlaylistEdits · the reorder", () => {
+  it("says a failed reorder, and puts the rows back in the order the server holds", async () => {
+    const { hook } = setup([movie("1"), movie("2"), movie("3")]);
+    await waitFor(() => expect(ids(hook.result.current.items)).toEqual(["1", "2", "3"]));
+    vi.spyOn(api, "plexReorderPlaylist").mockRejectedValue(new Error("offline"));
+
+    act(() => hook.result.current.commit([movie("3"), movie("1"), movie("2")]));
+    expect(ids(hook.result.current.items)).toEqual(["3", "1", "2"]); // optimistic
+    await waitFor(() => expect(errors()).toEqual(["plex.playlist.reorderFailed"]));
+    expect(ids(hook.result.current.items)).toEqual(["1", "2", "3"]);
+  });
+
+  it("writes an accepted order into the detail cache, which a rollback and a re-entry read", async () => {
+    const { hook, qc, detail } = setup([movie("1"), movie("2")]);
+    await waitFor(() => expect(hook.result.current.items).toHaveLength(2));
+    // The follow-up refetch never answers, so only the accepted write can put the order there.
+    detail.mockReturnValue(new Promise(() => {}) as never);
+    const reorder = vi.spyOn(api, "plexReorderPlaylist").mockResolvedValue(undefined as never);
+
+    act(() => hook.result.current.commit([movie("2"), movie("1")]));
+    await waitFor(() => expect(reorder).toHaveBeenCalledWith(5, ["2", "1"]));
+    await waitFor(() =>
+      expect(ids(qc.getQueryData<{ items: PlexCard[] }>(qk.plexPlaylist(5))!.items)).toEqual([
+        "2",
+        "1",
+      ]),
+    );
+  });
+
+  // B2, settled here rather than in a browser: `putBack` must read the cache AT ROLLBACK TIME. A
+  // removal that landed while the reorder was in flight refreshed the list; a rollback bound to the
+  // render that STARTED the reorder put the removed row back on screen, where no refetch cleared it
+  // and the next drag persisted an order containing it (deep-review round 5).
+  it("rolls back to the list as it is NOW, not as it was when the reorder began", async () => {
+    const { hook, qc, detail } = setup([movie("1"), movie("2"), movie("3")]);
+    await waitFor(() => expect(hook.result.current.items).toHaveLength(3));
+    let fail: (e: unknown) => void = () => {};
+    vi.spyOn(api, "plexReorderPlaylist").mockReturnValue(
+      new Promise((_, reject) => (fail = reject)) as never,
+    );
+
+    act(() => hook.result.current.commit([movie("3"), movie("2"), movie("1")]));
+    // "2" was removed elsewhere meanwhile, and the refreshed list reached the cache. The server
+    // answers the same from now on — so the rollback's own refetch comes back deep-equal, keeps
+    // the same `q.data`, and could never correct a stale rollback.
+    detail.mockResolvedValue({ id: 5, title: "F", items: [movie("1"), movie("3")] } as never);
+    act(() =>
+      qc.setQueryData(qk.plexPlaylist(5), { id: 5, title: "F", items: [movie("1"), movie("3")] }),
+    );
+    // The view has taken the refreshed list in BEFORE the reorder fails — the race as it happens.
+    // (react-query batches its notification, so without this wait the refresh landed only after the
+    // rollback and quietly corrected a stale one, which proved nothing.)
+    await waitFor(() => expect(ids(hook.result.current.items)).toEqual(["1", "3"]));
+    await act(async () => fail(new Error("offline")));
+
+    await waitFor(() => expect(errors()).toEqual(["plex.playlist.reorderFailed"]));
+    expect(ids(hook.result.current.items)).toEqual(["1", "3"]);
+  });
+});
+
+describe("usePlexPlaylistEdits · the removal", () => {
+  it("puts a row back when its removal fails, and says why", async () => {
+    const { hook } = setup([movie("1"), movie("2")]);
+    await waitFor(() => expect(hook.result.current.items).toHaveLength(2));
+    vi.spyOn(api, "plexPlaylistRemoveItem").mockRejectedValue(new Error("offline"));
+
+    await act(() => hook.result.current.onRemove(["2"]));
+    expect(errors()).toEqual(["plex.playlist.removeFailed"]);
+    expect(ids(hook.result.current.items)).toEqual(["1", "2"]);
+    expect(hook.result.current.busy).toBe(false);
+  });
+});

frontend/src/components/usePlexPlaylistEdits.ts

@@ -0,0 +1,99 @@
+import { useEffect, useState } from "react";
+import { useTranslation } from "react-i18next";
+import { useQuery, useQueryClient } from "@tanstack/react-query";
+
+import { api, type PlexCard } from "../lib/api";
+import { notifyActionError } from "../lib/actionErrors";
+import { qk } from "../lib/queryKeys";
+
+/** A Plex playlist's items and the two writes that edit them in place — a reorder and a removal.
+ *  Split from `PlexPlaylistView` so the failure paths are reachable from a test: the reorder is
+ *  driven by a dnd-kit drag, and neither of its sensors activates under jsdom. */
+export function usePlexPlaylistEdits(playlistId: number) {
+  const { t } = useTranslation();
+  const qc = useQueryClient();
+  const [busy, setBusy] = useState(false);
+  const q = useQuery({
+    queryKey: qk.plexPlaylist(playlistId),
+    queryFn: () => api.plexPlaylist(playlistId),
+  });
+  // Local optimistic copy so drag/remove are instant; re-synced whenever the query returns.
+  const [items, setItems] = useState<PlexCard[]>([]);
+  useEffect(() => {
+    if (q.data) setItems(q.data.items);
+  }, [q.data]);
+
+  const invalidate = () => {
+    qc.invalidateQueries({ queryKey: qk.plexPlaylist(playlistId) });
+    qc.invalidateQueries({ queryKey: qk.plexPlaylists() });
+  };
+
+  // Every edit here is optimistic on the local copy, and none of them used to handle a failure: a
+  // removal that failed stayed removed on screen (the resync never ran), and a 404 — the playlist
+  // deleted on another device, now a structured `not_found` — was an unhandled rejection that said
+  // nothing at all (deep-review round 4; the YouTube side was fixed in round 3). A failure is said,
+  // and the rows are put back from what the server last sent — explicitly, because a resync that
+  // returns the same list keeps the same `q.data` and the re-sync effect above never runs.
+  // Read the cache AT ROLLBACK TIME, not the `q.data` of the render that bound the failing handler:
+  // a removal completing while a reorder was in flight refreshed the list, and the reorder's stale
+  // closure then put the removed row back on screen — where the deep-equal refetch could not clear
+  // it, and the next drag persisted an order containing it (deep-review round 5). `Playlists.tsx`'s
+  // sibling `restoreFromCache` reads it live for the same reason.
+  const putBack = () => {
+    const cur = qc.getQueryData<{ items: PlexCard[] }>(qk.plexPlaylist(playlistId));
+    if (cur) setItems(cur.items);
+    invalidate();
+  };
+
+  function commit(next: PlexCard[]) {
+    if (busy) return; // a reorder racing a removal is what makes a rollback ambiguous
+    setItems(next);
+    api
+      .plexReorderPlaylist(
+        playlistId,
+        next.map((i) => i.id),
+      )
+      .then(() => {
+        // Write the accepted order into the DETAIL cache, not just the list. That cache is what
+        // `putBack` rolls back to and what a re-entry inside the 30 s staleTime renders without
+        // refetching — so after an accepted reorder it showed the pre-reorder order again, and the
+        // next drag from there persisted it, silently undoing the one the server took. The
+        // YouTube sibling (`Playlists.tsx`) got this in round 5; this half was left behind (round
+        // 6). One accepted write, one cache entry that matches it.
+        qc.setQueryData<{ items: PlexCard[] }>(qk.plexPlaylist(playlistId), (old) =>
+          old ? { ...old, items: next } : old,
+        );
+        qc.invalidateQueries({ queryKey: qk.plexPlaylist(playlistId) });
+        // …and then REFETCH it, exactly as the sibling does. `next` is this render's membership, and
+        // the server only reorders the keys it was given — so anything that landed meanwhile (a row
+        // removed from another surface, an item added elsewhere) is clobbered by that write. With
+        // only the list key invalidated nothing corrected it: staleTime is 30 s and
+        // refetchOnWindowFocus is off, so the wrong membership stayed on screen for as long as the
+        // view was mounted and the next drag persisted an order built from it (round 7).
+        qc.invalidateQueries({ queryKey: qk.plexPlaylists() });
+      })
+      .catch((e: unknown) => {
+        notifyActionError(e, t("plex.playlist.reorderFailed"));
+        putBack();
+      });
+  }
+
+  async function onRemove(keys: string[]) {
+    if (busy || !keys.length) return;
+    setBusy(true);
+    const set = new Set(keys);
+    setItems((prev) => prev.filter((i) => !set.has(i.id)));
+    try {
+      if (keys.length === 1) await api.plexPlaylistRemoveItem(playlistId, keys[0]!);
+      else await api.plexPlaylistRemoveBulk(playlistId, keys);
+      invalidate();
+    } catch (e) {
+      notifyActionError(e, t("plex.playlist.removeFailed"));
+      putBack();
+    } finally {
+      setBusy(false);
+    }
+  }
+
+  return { q, items, busy, invalidate, commit, onRemove };
+}