fix(plex): the collection page's watched toggle and Sort control survive the sort joining the query key

Deep-review R3. When 889d2acf put the sort into the collection detail's query key, two things on
the page did not follow it:

- The watched toggle's optimistic write kept the key's old spelling. setQueryData matches exactly,
  so it landed on an entry nothing reads, and its resync only invalidates the library root — the
  click succeeded server-side and changed nothing on screen (badge, icon, aria-label) until a
  reload; a second click toggled it back. One `detailKey` now serves the read and the write.
- Every sort change was a key with no data yet, so the whole page — hero, grid, and the Sort control
  with its open panel — was swapped for a spinner, and focus fell to <body>: key and direction could
  not be set in one visit. The previous page now stays on screen while the new order loads, for the
  SAME collection only (stepping to another must not show this one's hero meanwhile).

Verified on localdev (Rocky Collection): the toggle flips the poster; IMDb rating then Descending set
in one open panel, the hero never blanked, order reversed. 3 mutations, each red on its test.
This commit is contained in:
2026-09-11 01:12:51 +02:00
parent d2ba65e383
commit d20317e0f1
2 changed files with 71 additions and 2 deletions
@@ -20,6 +20,8 @@ beforeAll(() => {
}
}
globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver;
// …and the Sort control's popover watches its anchor with an IntersectionObserver.
globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver;
});
vi.mock("react-i18next", async (importOriginal) => {
@@ -126,3 +128,60 @@ describe("PlexCollectionView — Play-all", () => {
expect(onPlay).not.toHaveBeenCalled();
});
});
// Bug-hunt deep-review: the Sort control put the sort into the detail query's key, and two things
// on this page did not follow it.
describe("PlexCollectionView — after the sort joined the query key", () => {
it("flips a poster's watched state on the page it was clicked on", async () => {
// The optimistic write kept the key's OLD spelling — an entry nothing reads — and the resync
// only touches the library root, so the click did nothing visible until a reload.
vi.spyOn(api, "plexSetState").mockResolvedValue(undefined as never);
mount([card("movie1", "movie")]);
fireEvent.click(await screen.findByRole("button", { name: "plex.markWatched" }));
expect(await screen.findByRole("button", { name: "plex.markUnwatched" })).toBeTruthy();
});
it("keeps the page, and the open Sort panel, on screen while the new order loads", async () => {
// Every sort change was a key with no data yet: the whole page — the control and its open panel
// included — was swapped for a spinner, so key and direction could not be set in one visit.
vi.spyOn(api, "savePrefs").mockResolvedValue(undefined as never);
vi.spyOn(api, "plexCollection")
.mockResolvedValueOnce(detail([card("movie1", "movie"), card("movie2", "movie")]))
.mockReturnValueOnce(new Promise(() => {})); // the re-sorted page is still on its way
mount([card("movie1", "movie"), card("movie2", "movie")]);
fireEvent.click(await screen.findByRole("button", { name: /^plex\.filter\.sort:/ }));
// Popover rows are portalled and only exist while it is open, so `hidden: true`.
const other = (await screen.findAllByRole("menuitemradio", { hidden: true })).find(
(o) => o.getAttribute("aria-checked") !== "true",
);
fireEvent.click(other!);
await waitFor(() => expect(api.plexCollection).toHaveBeenCalledTimes(2));
expect(screen.queryByText("plex.loading")).toBeNull();
expect(screen.getAllByRole("menuitemradio", { hidden: true }).length).toBeGreaterThan(1);
expect(screen.getByText("plex.collection.playAll")).toBeTruthy();
});
it("does not show the previous collection while stepping to another one", async () => {
// Keeping the page on screen is for a re-sort of THIS collection only.
vi.spyOn(api, "plexCollection")
.mockResolvedValueOnce(detail([card("movie1", "movie")]))
.mockReturnValueOnce(new Promise(() => {}));
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
const view = (id: string) => (
<QueryClientProvider client={qc}>
<PlexCollectionView
collectionId={id}
onBack={() => {}}
onPlay={() => {}}
onOpenCard={() => {}}
onInfoCard={() => {}}
/>
</QueryClientProvider>
);
const { rerender } = render(view("col1"));
await screen.findByText("plex.collection.playAll");
rerender(view("col2"));
expect(await screen.findByText("plex.loading")).toBeTruthy();
expect(screen.queryByText("plex.collection.playAll")).toBeNull();
});
});
+12 -2
View File
@@ -69,9 +69,19 @@ export default function PlexCollectionView({
"plexCollectionSort",
COLLECTION_SORT_DEFAULT,
);
// ONE key for the read and every write into it: when the sort joined the key, the watched toggle's
// optimistic write kept the old spelling and landed on an entry nothing reads (setQueryData matches
// exactly), so the click did nothing visible until a reload.
const detailKey = qk.plexCollections("detail", collectionId, sortPref.key, sortPref.dir);
const q = useQuery({
queryKey: qk.plexCollections("detail", collectionId, sortPref.key, sortPref.dir),
queryKey: detailKey,
queryFn: () => api.plexCollection(collectionId, sortPref),
// A re-sort keeps the current page on screen while the new order loads. Without it every sort
// change was a key with no data yet, and the whole page — the Sort control and its open panel
// with it — was swapped for a spinner, so key and direction could not be set in one visit. Only
// within the SAME collection: stepping to another one must not show this one's hero meanwhile.
placeholderData: (prev, prevQuery) =>
prevQuery?.queryKey[2] === collectionId ? prev : undefined,
});
const d = q.data;
@@ -146,7 +156,7 @@ export default function PlexCollectionView({
const toggleWatched = (card: PlexCard) =>
toggleWatchState(card, t, {
optimistic: (next) => {
qc.setQueryData<PlexCollectionDetail>(qk.plexCollections("detail", collectionId), (old) =>
qc.setQueryData<PlexCollectionDetail>(detailKey, (old) =>
old
? {
...old,