diff --git a/frontend/src/components/PlaylistEditFailures.dom.test.tsx b/frontend/src/components/PlaylistEditFailures.dom.test.tsx index dec28ea6..304982b5 100644 --- a/frontend/src/components/PlaylistEditFailures.dom.test.tsx +++ b/frontend/src/components/PlaylistEditFailures.dom.test.tsx @@ -76,6 +76,23 @@ describe("the Plex playlist page", () => { expect(screen.getByText("Movie 1")).toBeTruthy(); }); + it("says so when a rename is refused, and keeps the field open", async () => { + vi.spyOn(api, "plexPlaylist").mockResolvedValue({ id: 5, title: "Films", items: [] }); + vi.spyOn(api, "plexRenamePlaylist").mockRejectedValue(gone("playlist")); + render( + + {}} onPlay={() => {}} /> + , + ); + await screen.findByText("Films"); + fireEvent.click(screen.getByTitle("plex.playlist.rename")); + const field = screen.getByDisplayValue("Films"); + fireEvent.change(field, { target: { value: "Movies" } }); + fireEvent.keyDown(field, { key: "Enter" }); + await waitFor(() => expect(errors()).toEqual(["gone playlist"])); + expect(screen.getByDisplayValue("Movies")).toBeTruthy(); // still open to try again + }); + it("leaves a playlist that is already gone when asked to delete it", async () => { vi.spyOn(api, "plexPlaylist").mockResolvedValue({ id: 5, title: "Films", items: [] }); vi.spyOn(api, "plexDeletePlaylist").mockRejectedValue(gone("playlist")); diff --git a/frontend/src/components/Playlists.dom.test.tsx b/frontend/src/components/Playlists.dom.test.tsx index c3b7262d..38ec9004 100644 --- a/frontend/src/components/Playlists.dom.test.tsx +++ b/frontend/src/components/Playlists.dom.test.tsx @@ -343,6 +343,31 @@ describe("Playlists — a change the server refused is taken back on screen", () expect(rowsOnScreen()).toEqual(["Video V", "Video W"]); }); + it("puts back the order the server ACCEPTED, not the one it had two reorders ago", async () => { + // The rollback reads the cached detail, and nothing wrote the accepted order into it — only the + // refetch that follows a successful reorder did. So a second reorder rejected while that refetch + // was still in flight rolled the rows back past the FIRST one, which the server had accepted. + // The membership guard cannot correct an order (its key is the sorted id set), so the wrong + // order stayed on screen for good and the next drag persisted it (deep-review round 5). + const reorder = vi + .spyOn(api, "reorderPlaylist") + .mockResolvedValueOnce(undefined as never) + .mockRejectedValue(new HttpError(500, "boom")); + mount({ 1: [vid("V", 0), vid("W", 0), vid("X", 0)] }); + await screen.findByRole("button", { name: "card.play — Video V" }); + // From here no refetch ever lands, so the cache is whatever the page itself put there. + vi.mocked(api.playlist).mockReturnValue(new Promise(() => {}) as never); + + await rowMenu("Video V", "playlists.orderDown"); + await waitFor(() => expect(reorder).toHaveBeenCalledTimes(1)); + expect(rowsOnScreen()).toEqual(["Video W", "Video V", "Video X"]); + + await rowMenu("Video X", "playlists.orderUp"); + await waitFor(() => expect(reorder).toHaveBeenCalledTimes(2)); + await act(async () => void (await new Promise((r) => setTimeout(r, 20)))); + expect(rowsOnScreen()).toEqual(["Video W", "Video V", "Video X"]); + }); + it("puts back a row whose removal failed, even when the resync fails too", async () => { // A removal during an API restart: the DELETE fails, and so does the refetch meant to put the row // back — the list kept showing it as removed while it was still in the playlist. diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx index 583b3175..f30f7d98 100644 --- a/frontend/src/components/Playlists.tsx +++ b/frontend/src/components/Playlists.tsx @@ -191,11 +191,19 @@ export default function Playlists() { next.map((v) => v.id), ) .then(() => { + // Write the accepted order into the cache BEFORE the refetch. The cache is what + // `restoreFromCache` rolls back to, and until this refetch landed it still held the order + // from before this reorder — so a SECOND reorder failing in that window rolled the rows + // back to an order the server does not have, permanently (the membership guard cannot + // correct an order). One accepted write, one cache entry that matches it (round 5). + qc.setQueryData(qk.playlist(listId), (old) => + old ? { ...old, items: next } : old, + ); // Refresh the sidebar (cover may change) and the detail (so the now-dirty state, // and the Reset/Unsynced indicators, show without an F5). The membership guard // keeps the refetch from wiping the undo history on a pure reorder. qc.invalidateQueries({ queryKey: qk.playlists() }); - qc.invalidateQueries({ queryKey: qk.playlist(selectedId) }); + qc.invalidateQueries({ queryKey: qk.playlist(listId) }); }) .catch((e) => { // Was a bare .then(): a failed reorder left the UI showing an order the server had diff --git a/frontend/src/components/PlexPlaylistView.tsx b/frontend/src/components/PlexPlaylistView.tsx index ffc97290..9e3f33f3 100644 --- a/frontend/src/components/PlexPlaylistView.tsx +++ b/frontend/src/components/PlexPlaylistView.tsx @@ -408,12 +408,19 @@ export default function PlexPlaylistView({ // 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 = () => { - if (q.data) setItems(q.data.items); + 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(