From 76b5f7c93e1a2ae90f0ba006a555ee2f3b6bd9b9 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Fri, 11 Sep 2026 18:05:19 +0200 Subject: [PATCH] fix(playlists): a failed background refetch keeps the list on screen, and the player outlives every branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review round 3 of bug hunt 2026-09. - The detail's error branch applies only when there is nothing to show (`isError && `, the QueryState convention): one failed refetch — on focus, after a move or a removal — replaced a loaded list with "couldn't load". The error line is announced (role=alert). - Tests for the two branches of "the player survives a change of selection" that round 2 left untested (a list that fails to load, no selection). --- .../src/components/Playlists.dom.test.tsx | 38 +++++++++++++++++++ frontend/src/components/Playlists.tsx | 8 ++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/Playlists.dom.test.tsx b/frontend/src/components/Playlists.dom.test.tsx index 7a9cf766..28a5860f 100644 --- a/frontend/src/components/Playlists.dom.test.tsx +++ b/frontend/src/components/Playlists.dom.test.tsx @@ -221,6 +221,23 @@ describe("Playlists — removing a video from inside the player", () => { }); }); +describe("Playlists — a failed refetch of a list that is on screen", () => { + it("keeps the list, instead of replacing it with the error", async () => { + // A background refetch — on focus, after a move or a removal — failing during an API restart + // replaced a loaded list with "couldn't load" (the `isError && ` convention, ignored). + mount({ 1: [vid("V", 0)] }); + await screen.findByRole("button", { name: "card.play — Video V" }); + vi.mocked(api.playlist).mockRejectedValue(new HttpError(502, "gateway")); + await act(async () => { + await qc.invalidateQueries({ queryKey: qk.playlist(1) }); + await new Promise((r) => setTimeout(r, 20)); // react-query notifies its observers on a timer + }); + expect(qc.getQueryState(qk.playlist(1))?.status).toBe("error"); + expect(screen.queryByText("playlists.loadFailed")).toBeNull(); + expect(screen.getByRole("button", { name: "card.play — Video V" })).toBeTruthy(); + }); +}); + describe("Playlists — the player outlives a change of selection", () => { it("stays mounted while a newly selected list is still loading", async () => { // The page swaps itself for "Loading…" until the new list arrives; with the player rendered only @@ -235,6 +252,27 @@ describe("Playlists — the player outlives a change of selection", () => { expect(h.mounted).toBe(true); }); + it("stays mounted when the newly selected list fails to load — a list deleted elsewhere", async () => { + const { rerender } = mount({ 1: [vid("V", 0)] }); + await play("Video V"); + await waitFor(() => expect(h.mounted).toBe(true)); + vi.mocked(api.playlist).mockRejectedValue(new HttpError(404, "gone")); + h.selectedId = 2; + rerender(); + await screen.findByText("playlists.loadFailed"); + expect(h.mounted).toBe(true); + }); + + it("stays mounted when nothing is selected any more", async () => { + const { rerender } = mount({ 1: [vid("V", 0)] }); + await play("Video V"); + await waitFor(() => expect(h.mounted).toBe(true)); + h.selectedId = null; + rerender(); + await screen.findByText("playlists.pickOne"); + expect(h.mounted).toBe(true); + }); + it("refreshes the list it was launched from when the viewer marks the video in it", async () => { // Invalidating the SELECTED list left the launch list's row showing the old status. vi.spyOn(api, "setState").mockResolvedValue(undefined as never); diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx index b006635a..9f7439aa 100644 --- a/frontend/src/components/Playlists.tsx +++ b/frontend/src/components/Playlists.tsx @@ -813,11 +813,13 @@ export default function Playlists() { ); } // The page used to render "Loading…" forever when the detail query failed — an error read as a - // slow network that never arrived. - if (detailQuery.isError) { + // slow network that never arrived. Only with nothing to show, though (the `isError && ` + // convention, see ui/QueryState): a background refetch that failed — on focus, after a move or a + // removal — replaced a list that was on screen with this message. + if (detailQuery.isError && !detail) { return ( <> -
+
{t("playlists.loadFailed")}