fix(e24): the review's seven frontend defects on the Home surface

Shift+W was dead whenever anything was open. The global handler called `setPage` directly,
so the page change unmounted the player (it lives inside the module column) and that
unmount's reconcile reclaimed the entry the navigation had just pushed — the player closed
and the page snapped back. `closeThenNavigate` cannot serve here: a global key does not
know which overlay is open, or whether one is. `navigateOverOverlays` is the same fix one
step shorter — bury the live overlay entries first, so the unmount consumes the marks
instead of issuing `history.back()`.

`q.isError` had no `&& !data` guard, alone among 33 call sites and against the convention
`ui/QueryState` documents: react-query flips status to error on a failed REFETCH while
keeping the payload, and every mutation here invalidates the discover prefix, so one blip
while marking a card watched blanked a fully populated landing page.

The reroll's key rooted at "discover-pick", outside the prefix every mutation invalidates,
so a rerolled hero outlived the page: play it, finish it, and it was still the answer with
a zeroed progress bar. It now sits under the same root. A FAILED reroll also rewound the
hero to the card the page loaded with, several presses back, because a new key per press
means `keepPreviousData` (pending-only) cannot cover an error; the last resolved pick is
held instead.

The bookmark on a rail tile never updated — `VideoCard` reads `video.saved` from this
payload and keeps no optimistic state — so the star stayed empty and the next click
re-sent the same call.

The shelf reorder wrote a captured snapshot back on failure rather than re-asking the
server, which its own comment promised: two overlapping moves each reverted to their own
snapshot and settled on an order neither side held, and since `before` is the whole
payload a rollback also discarded any rail refresh that had landed meanwhile. It now
invalidates the discover PREFIX on settle, which also fixes the sibling budgets keeping
the pre-move order.

The hero/rail de-duplication matched one key, so for a Plex series — the rail row is the
show, the hero is the episode it resolves to — the same series printed twice; it now
matches both keys, as `hero.pick`'s own exclusion already did. And when the hero empties
the resume rail, the rail is hidden rather than announcing "nothing half-finished right
now" under a hero captioned "where you left off".

Ten regression tests, all mutation-checked. Three of them were rewritten after the first
mutation round showed they could not go red: two asserted before the state under test had
settled, one matched a title the Plex tile does not render.
This commit is contained in:
2026-09-08 03:03:45 +02:00
parent 84f4b20e5a
commit fdc1eabbe0
7 changed files with 282 additions and 24 deletions
+141 -2
View File
@@ -11,8 +11,9 @@
// a Carousel (useHScroll → ResizeObserver), stubbed for jsdom.
import { StrictMode } from "react";
import { describe, it, expect, afterEach, beforeAll, beforeEach, vi } from "vitest";
import { render, screen, fireEvent, cleanup, waitFor } from "@testing-library/react";
import { act, render, screen, fireEvent, cleanup, waitFor } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { qk } from "../lib/queryKeys";
beforeAll(() => {
class Stub {
@@ -143,12 +144,20 @@ const payload = (heroId: string, tier = "resume") => ({
});
function mount() {
return mountWithClient().result;
}
/** Same mount, handing back the QueryClient — the tests that need to drive an invalidation (i.e. a
* background refetch, the only way to reach the has-data-and-errored state) cannot reach it
* otherwise. */
function mountWithClient() {
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
return render(
const result = render(
<QueryClientProvider client={qc}>
<Home />
</QueryClientProvider>,
);
return { qc, result };
}
beforeEach(() => {
@@ -178,6 +187,31 @@ describe("Home", () => {
expect(await screen.findByText("home.hero.resumeLeft:8:00")).toBeTruthy();
});
it("renders an error, never the empty state, when the payload cannot be loaded", async () => {
// The one thing this surface must not do: report a backend outage as "nothing to suggest yet"
// over a library full of things to suggest.
discover.mockRejectedValue(new Error("boom"));
mount();
expect(await screen.findByText("home.error")).toBeTruthy();
expect(screen.queryByText("home.hero.emptyAny")).toBeNull();
});
it("keeps the page on screen when a BACKGROUND refetch fails", async () => {
// react-query flips `status` to error on a failed refetch while keeping the cached payload, and
// every mutation here invalidates `qk.discover()` — so an unguarded `isError` blanked a fully
// populated landing page over one blip.
const { qc } = mountWithClient();
await screen.findByText("Hero v1");
discover.mockRejectedValue(new Error("blip"));
await act(async () => {
await qc.invalidateQueries({ queryKey: qk.discover() });
});
// The refetch must actually have been attempted, or this asserts nothing at all.
await waitFor(() => expect(discover).toHaveBeenCalledTimes(2));
expect(screen.queryByText("home.error")).toBeNull();
expect(screen.getByText("Hero v1")).toBeTruthy();
});
it("does not print the hero again as the first resume tile", async () => {
mount();
await screen.findByText("home.hero.resumeLeft:8:00");
@@ -186,6 +220,111 @@ describe("Home", () => {
expect(screen.getByText("Second thing")).toBeTruthy();
});
it("does not print a Plex series twice when the hero is the episode it resolves to", async () => {
// The commonest Plex resume case, and the one a single-key match misses: the rail row is the
// SHOW while the hero is the episode that show resolves to, so their `source:id` keys differ.
// Renamed on `plex` too, not only on the unified card: the rail tile is a `PlexPosterCard`, which
// takes its heading from the NATIVE payload — renaming just the outer title made this vacuous.
const s = showCard();
const show = { ...s, title: "The Series Tile", plex: { ...s.plex, title: "The Series Tile" } };
discover.mockResolvedValue({
budget: "any",
hero: { tier: "resume", card: show.next },
continue_watching: [show, ytCard("v2", "Second thing")],
comfort: [],
});
mount();
await screen.findByText("Second thing");
expect(screen.queryByText("The Series Tile")).toBeNull();
});
it("hides the resume rail rather than contradicting the hero that emptied it", async () => {
// One in-progress item: it becomes the hero, the dedupe filter takes it out of the rail, and the
// rail then announced "nothing half-finished right now" under a hero captioned "where you left
// off". The shelf's own empty state is untouched — it still has something to teach.
discover.mockResolvedValue({
budget: "any",
hero: { tier: "resume", card: ytCard("v1", "Hero v1") },
continue_watching: [ytCard("v1", "Hero v1")],
comfort: [],
});
mount();
await screen.findByText("Hero v1");
expect(screen.queryByText("home.continue.empty")).toBeNull();
expect(screen.getByText("home.comfort.empty")).toBeTruthy();
});
it("holds the hero it last found when a reroll fails", async () => {
// A failed pick has no data of its own (every press is a new key, and `keepPreviousData` is
// pending-only), so the hero used to rewind to the card the page LOADED with — several presses
// back — while `core.req`'s own dialog explained the failure.
// No resume rail: "Hero v1" then appears exactly where the hero is, so its reappearance is
// unambiguous evidence of a rewind rather than a rail tile that stopped being deduplicated.
discover.mockResolvedValue({
budget: "any",
hero: { tier: "resume", card: ytCard("v1", "Hero v1") },
continue_watching: [],
comfort: [],
});
discoverPick.mockResolvedValueOnce({
budget: "any",
hero: { tier: "surprise", card: ytCard("v9", "Rerolled") },
});
mount();
await screen.findByText("Hero v1");
fireEvent.click(screen.getByText("home.hero.reroll"));
await screen.findByText("Rerolled");
// Rejected on OUR schedule: asserting straight after the call goes out catches the query still
// pending, where `keepPreviousData` is legitimately holding the old hero — which is not the
// state under test and made this pass with or without the fix.
let failPick!: (e: Error) => void;
discoverPick.mockReturnValueOnce(
new Promise((_resolve, reject) => {
failPick = reject;
}),
);
fireEvent.click(screen.getByText("home.hero.reroll"));
await waitFor(() => expect(discoverPick).toHaveBeenCalledTimes(2));
await act(async () => {
failPick(new Error("boom"));
await new Promise((r) => setTimeout(r, 0));
});
expect(screen.getByText("Rerolled")).toBeTruthy();
expect(screen.queryByText("Hero v1")).toBeNull();
});
it("a mutation's invalidation reaches the rerolled hero too", async () => {
// The reroll has its own query key so pressing the button does not refetch the rails — but under
// the SAME root, or the prefix every mutation invalidates misses it and a rerolled hero outlives
// the page: play it, finish it, and it was still the answer, with a zeroed progress bar.
discoverPick.mockResolvedValue({
budget: "any",
hero: { tier: "surprise", card: ytCard("v9", "Rerolled") },
});
const { qc } = mountWithClient();
await screen.findByText("Hero v1");
fireEvent.click(screen.getByText("home.hero.reroll"));
await screen.findByText("Rerolled");
expect(discoverPick).toHaveBeenCalledTimes(1);
await act(async () => {
await qc.invalidateQueries({ queryKey: qk.discover() });
});
await waitFor(() => expect(discoverPick).toHaveBeenCalledTimes(2));
});
it("refreshes the payload the bookmark is read from", async () => {
mount();
await screen.findByText("Second thing");
expect(discover).toHaveBeenCalledTimes(1);
fireEvent.click(screen.getAllByTitle("card.saveForLater")[0]!);
// `VideoCard` derives the filled/empty star straight from `video.saved` in this payload and
// keeps no optimistic state of its own, so without re-asking, the icon never changed and the
// next click re-sent the same call — the control looked dead.
await waitFor(() => expect(discover).toHaveBeenCalledTimes(2));
});
it("asks for the chosen budget and keeps the previous answer on screen meanwhile", async () => {
mount();
await screen.findByText("Hero v1");