fix(playlists): one push or revert at a time for a list, whichever menu started it
The in-flight flag was per hook instance, and the rail row's menu and the list page header's "…" are two at once: a push from one left the other offering a second push. It is one app-wide store (`lib/store`), read with `get()` at the start so a second start refuses before a render; a revert re-reads it after its confirm dialog.
This commit is contained in:
@@ -155,3 +155,29 @@ describe("usePlaylistActions — a playlist's menu", () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// E28 review C1: the push / revert in-flight flag was per hook instance, and two instances run at
|
||||||
|
// once (the rail row's menu and the list page header's "…") — a push started from one left the other
|
||||||
|
// offering a second push of the same list.
|
||||||
|
describe("usePlaylistActions — one push at a time, across menus", () => {
|
||||||
|
it("a push started from one menu is off in the other until it ends", async () => {
|
||||||
|
let finish: (v: unknown) => void = () => {};
|
||||||
|
vi.spyOn(api, "playlistPushPlan").mockReturnValue(
|
||||||
|
new Promise((r) => (finish = r)) as ReturnType<typeof api.playlistPushPlan>,
|
||||||
|
);
|
||||||
|
const rail = mount();
|
||||||
|
const header = mount();
|
||||||
|
const dirty = { ...linked, dirty: true };
|
||||||
|
const push = (m: typeof rail) =>
|
||||||
|
m.result.current.items(dirty, { onRename: () => {}, pin: [] }).find((i) => i.id === "push")!;
|
||||||
|
act(() => push(rail).onSelect!());
|
||||||
|
expect(push(header).disabled).toBe(true);
|
||||||
|
// A second start (a keyboard or a stale menu) is refused, not run beside the first.
|
||||||
|
await act(async () => push(header).onSelect!());
|
||||||
|
expect(api.playlistPushPlan).toHaveBeenCalledTimes(1);
|
||||||
|
await act(async () => {
|
||||||
|
finish({ action: "update", affordable: true });
|
||||||
|
});
|
||||||
|
expect(push(header).disabled).toBeFalsy();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -1,4 +1,3 @@
|
|||||||
import { useState } from "react";
|
|
||||||
import { useQueryClient } from "@tanstack/react-query";
|
import { useQueryClient } from "@tanstack/react-query";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
import { ExternalLink, Pencil, RotateCcw, Trash2, Upload, Youtube } from "lucide-react";
|
import { ExternalLink, Pencil, RotateCcw, Trash2, Upload, Youtube } from "lucide-react";
|
||||||
@@ -12,6 +11,7 @@ import { useChoice, useConfirm } from "./confirmContext";
|
|||||||
import { usePlaylists, usePlaylistsActions } from "./playlistsContext";
|
import { usePlaylists, usePlaylistsActions } from "./playlistsContext";
|
||||||
import { useMe } from "../lib/useMe";
|
import { useMe } from "../lib/useMe";
|
||||||
import { type ActionMenuItem } from "./ui/ActionMenu";
|
import { type ActionMenuItem } from "./ui/ActionMenu";
|
||||||
|
import { createStore } from "../lib/store";
|
||||||
|
|
||||||
// A playlist's ONE action list (E28 C1) and the mutations behind it. Two entry points open it — the
|
// A playlist's ONE action list (E28 C1) and the mutations behind it. Two entry points open it — the
|
||||||
// rail row (right-click, long-press, its "…") and the list page header's "…" — so it lives here,
|
// rail row (right-click, long-press, its "…") and the list page header's "…" — so it lives here,
|
||||||
@@ -24,6 +24,15 @@ import { type ActionMenuItem } from "./ui/ActionMenu";
|
|||||||
* and `PlaylistDetail` (the detail pane), which is why neither is named here. */
|
* and `PlaylistDetail` (the detail pane), which is why neither is named here. */
|
||||||
type Menuable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id" | "dirty">;
|
type Menuable = Pick<Playlist, "id" | "name" | "kind" | "yt_playlist_id" | "dirty">;
|
||||||
|
|
||||||
|
// Which list a push / revert is running for, so its row is off (and says so) until it is done. ONE
|
||||||
|
// for the app, not per hook: the rail row's menu and the list page header's "…" are two instances at
|
||||||
|
// once, and a per-instance flag left the other offering a second push of the same list (E28 review
|
||||||
|
// C1). Read with `get()` at the start, so a second start refuses before any render has happened.
|
||||||
|
const inFlight = createStore<{ pushing: number | null; reverting: number | null }>({
|
||||||
|
pushing: null,
|
||||||
|
reverting: null,
|
||||||
|
});
|
||||||
|
|
||||||
export function usePlaylistActions() {
|
export function usePlaylistActions() {
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
const { t } = useTranslation();
|
const { t } = useTranslation();
|
||||||
@@ -32,9 +41,9 @@ export function usePlaylistActions() {
|
|||||||
const canWrite = useMe().can_write;
|
const canWrite = useMe().can_write;
|
||||||
const { selectedId } = usePlaylists();
|
const { selectedId } = usePlaylists();
|
||||||
const { setSelectedId } = usePlaylistsActions();
|
const { setSelectedId } = usePlaylistsActions();
|
||||||
// Which list a push / revert is running for, so its row is off (and says so) until it is done.
|
const { pushing, reverting } = inFlight.use();
|
||||||
const [pushing, setPushing] = useState<number | null>(null);
|
const setPushing = (id: number | null) => inFlight.set((s) => ({ ...s, pushing: id }));
|
||||||
const [reverting, setReverting] = useState<number | null>(null);
|
const setReverting = (id: number | null) => inFlight.set((s) => ({ ...s, reverting: id }));
|
||||||
|
|
||||||
const refresh = (id: number) => {
|
const refresh = (id: number) => {
|
||||||
qc.invalidateQueries({ queryKey: qk.playlists() });
|
qc.invalidateQueries({ queryKey: qk.playlists() });
|
||||||
@@ -96,14 +105,15 @@ export function usePlaylistActions() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function revert(pl: Menuable) {
|
async function revert(pl: Menuable) {
|
||||||
if (reverting != null) return;
|
if (inFlight.get().reverting != null) return;
|
||||||
const ok = await confirm({
|
const ok = await confirm({
|
||||||
title: t("playlists.revertTitle"),
|
title: t("playlists.revertTitle"),
|
||||||
message: t("playlists.revertMsg"),
|
message: t("playlists.revertMsg"),
|
||||||
confirmLabel: t("playlists.revertConfirm"),
|
confirmLabel: t("playlists.revertConfirm"),
|
||||||
danger: true,
|
danger: true,
|
||||||
});
|
});
|
||||||
if (!ok) return;
|
// Re-read after the dialog: another menu may have started one while this one asked.
|
||||||
|
if (!ok || inFlight.get().reverting != null) return;
|
||||||
setReverting(pl.id);
|
setReverting(pl.id);
|
||||||
try {
|
try {
|
||||||
await api.revertPlaylist(pl.id);
|
await api.revertPlaylist(pl.id);
|
||||||
@@ -120,7 +130,7 @@ export function usePlaylistActions() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function push(pl: Menuable) {
|
async function push(pl: Menuable) {
|
||||||
if (pushing != null) return;
|
if (inFlight.get().pushing != null) return;
|
||||||
const name = playlistName(pl, t);
|
const name = playlistName(pl, t);
|
||||||
setPushing(pl.id);
|
setPushing(pl.id);
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user