fix(playlists): a rejected reorder goes back to the order the server accepted
Deep-review round 5, over round 4's own fixes. - Round 4 made a failed reorder roll the rows back to the CACHED detail, because a resync cannot do it (the server's list is unchanged, so the refetch is deep-equal, react-query keeps the same `data` object and the effect keyed on it never runs). But nothing wrote an ACCEPTED order into that cache — 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. `onApply` now writes the accepted order into the cache before invalidating. - `PlexPlaylistView.rename` had a `catch` no test could reach, and `commit` (reorder) was not gated by `busy` the way its siblings are — a reorder in flight while a removal completes is what makes a rollback ambiguous in the first place. The rename failure path is now covered; `commit` defers. - `putBack` read `q.data` from the render that bound the failing handler instead of the cache at rollback time, unlike the sibling `restoreFromCache`. Aligned. Stated plainly: I could NOT drive that staleness in jsdom — a mutation that restores the old shape leaves every candidate test green — so this one ships as an alignment with the proven sibling, not as a fix with a red test behind it. The scenario and the harness limit are on ROADMAP.md. Two regression tests, both mutation-checked red (the cache write, the rename catch). The drag-reorder failure path still has no test: dnd-kit's pointer and keyboard sensors do not activate under jsdom even with geometry and scrollIntoView stubbed, so a test there would have pinned the harness, not the code. Logged, not faked. vitest 1009.
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user