fix(playlists): the player's Remove follows the list it was launched from, the player survives a change of selection, and a gone list is said in the reader's language
- D3 snapshotted the removal's TARGET, but whether the player offered Remove still followed the
selected list: launched from Watch later (no Remove, by design), selecting another list under it
made Remove appear — and delete from Watch later. The snapshot now carries the launch list's
editability too.
- Selecting another list under an open player (the command palette works over it) put the page into
"Loading…", and the player was rendered only in the loaded branch — it vanished mid-playback and
came back at its launch video. It now sits at one place in the tree whatever the page is doing.
- A removal takes the video out of the CACHED list as well as the rows: the player's progress
checkpoint rewrote that entry before the refetch landed, handing the membership guard a list that
still held the video — the row came back and the undo history went.
- The watched-state callback refreshed the selected list, not the launch list (D3's sibling).
- `_own_playlist`'s 404 is structured (`{"code": "playlist", "reason": "not_found"}`) and localised:
the bare "Unknown playlist" reached the Hungarian UI in English, which also made round 1's new
`removeFailed` string unreachable. Both Remove and Move report through `notifyActionError` now.
- The "viewer moved on under the confirm" test stepped back only once, which lands on the same video
whether or not the removed one left the queue; it now takes the second step that tells them apart.
This commit is contained in:
@@ -50,7 +50,10 @@ def _check_pull_cooldown(user: User) -> None:
|
|||||||
def _own_playlist(db: Session, user: User, playlist_id: int) -> Playlist:
|
def _own_playlist(db: Session, user: User, playlist_id: int) -> Playlist:
|
||||||
pl = db.get(Playlist, playlist_id)
|
pl = db.get(Playlist, playlist_id)
|
||||||
if pl is None or pl.user_id != user.id:
|
if pl is None or pl.user_id != user.id:
|
||||||
raise HTTPException(status_code=404, detail="Unknown playlist")
|
# Structured, so the UI can say it in the reader's language: this 404 is exactly the one a
|
||||||
|
# user meets in practice (the list was deleted in another tab), and a bare "Unknown
|
||||||
|
# playlist" reached the Hungarian UI in English. One reason for "gone" and "not yours" alike.
|
||||||
|
raise HTTPException(status_code=404, detail={"code": "playlist", "reason": "not_found"})
|
||||||
return pl
|
return pl
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -128,6 +128,22 @@ def test_move_into_someone_elses_list_404s_and_changes_nothing(db, make_user, as
|
|||||||
assert _ids(db, theirs.id) == []
|
assert _ids(db, theirs.id) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_list_that_is_not_yours_answers_with_a_reason_the_ui_can_translate(
|
||||||
|
db, make_user, as_user
|
||||||
|
):
|
||||||
|
# The one 404 a user meets in practice (the list went away in another tab). It used to be a bare
|
||||||
|
# English "Unknown playlist", which the Hungarian UI then showed verbatim.
|
||||||
|
user, other = make_user(), make_user()
|
||||||
|
_seed_videos(db, ["v1"])
|
||||||
|
theirs = _playlist(db, other, "Theirs", ["v1"])
|
||||||
|
|
||||||
|
r = as_user(user).delete(f"/api/playlists/{theirs.id}/items/v1")
|
||||||
|
assert r.status_code == 404
|
||||||
|
assert r.json()["detail"] == {"code": "playlist", "reason": "not_found"}
|
||||||
|
db.expire_all()
|
||||||
|
assert _ids(db, theirs.id) == ["v1"]
|
||||||
|
|
||||||
|
|
||||||
def test_move_marks_both_linked_sides_dirty(db, make_user, as_user):
|
def test_move_marks_both_linked_sides_dirty(db, make_user, as_user):
|
||||||
"""A move edits two YouTube mirrors at once — the source lost an item, the target gained one.
|
"""A move edits two YouTube mirrors at once — the source lost an item, the target gained one.
|
||||||
Marking only the source is the asymmetric fix this repo keeps re-learning."""
|
Marking only the source is the asymmetric fix this repo keeps re-learning."""
|
||||||
|
|||||||
@@ -216,6 +216,11 @@ describe("PlayerModal — removing the playing video from its playlist", () => {
|
|||||||
expect(lastStart()?.videoId).toBe("C");
|
expect(lastStart()?.videoId).toBe("C");
|
||||||
prev();
|
prev();
|
||||||
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
await waitFor(() => expect(lastStart()?.videoId).toBe("B"));
|
||||||
|
// …and A is gone from the queue, not just skipped over: B is its head now, so stepping back
|
||||||
|
// again goes nowhere. (One step back lands on B either way — it took the second to tell.)
|
||||||
|
prev();
|
||||||
|
await settle();
|
||||||
|
expect(lastStart()?.videoId).toBe("B");
|
||||||
expect(started.filter((s) => s.videoId === "A")).toHaveLength(1); // never played again
|
expect(started.filter((s) => s.videoId === "A")).toHaveLength(1); // never played again
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -10,9 +10,11 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|||||||
const h = vi.hoisted(() => ({
|
const h = vi.hoisted(() => ({
|
||||||
notify: vi.fn(),
|
notify: vi.fn(),
|
||||||
selectedId: 1 as number | null,
|
selectedId: 1 as number | null,
|
||||||
|
mounted: false,
|
||||||
player: null as null | {
|
player: null as null | {
|
||||||
video: { id: string; position_seconds: number };
|
video: { id: string; position_seconds: number };
|
||||||
onClose: () => void;
|
onClose: () => void;
|
||||||
|
onState: (id: string, status: string) => void;
|
||||||
onRemoveFromList?: (v: unknown) => Promise<boolean>;
|
onRemoveFromList?: (v: unknown) => Promise<boolean>;
|
||||||
},
|
},
|
||||||
}));
|
}));
|
||||||
@@ -34,12 +36,21 @@ vi.mock("../lib/notifications", async (importOriginal) => ({
|
|||||||
vi.mock("./PlaylistsProvider", () => ({ usePlaylists: () => ({ selectedId: h.selectedId }) }));
|
vi.mock("./PlaylistsProvider", () => ({ usePlaylists: () => ({ selectedId: h.selectedId }) }));
|
||||||
vi.mock("./ConfirmProvider", () => ({ useConfirm: () => async () => true }));
|
vi.mock("./ConfirmProvider", () => ({ useConfirm: () => async () => true }));
|
||||||
vi.mock("./usePlaylistActions", () => ({ usePlaylistActions: () => ({ remove: vi.fn() }) }));
|
vi.mock("./usePlaylistActions", () => ({ usePlaylistActions: () => ({ remove: vi.fn() }) }));
|
||||||
vi.mock("./PlayerModal", () => ({
|
vi.mock("./PlayerModal", async () => {
|
||||||
default: (props: NonNullable<typeof h.player>) => {
|
const { useEffect } = await import("react");
|
||||||
|
return {
|
||||||
|
default: function PlayerStandIn(props: NonNullable<typeof h.player>) {
|
||||||
h.player = props;
|
h.player = props;
|
||||||
|
useEffect(() => {
|
||||||
|
h.mounted = true;
|
||||||
|
return () => {
|
||||||
|
h.mounted = false;
|
||||||
|
};
|
||||||
|
}, []);
|
||||||
return null;
|
return null;
|
||||||
},
|
},
|
||||||
}));
|
};
|
||||||
|
});
|
||||||
|
|
||||||
import Playlists from "./Playlists";
|
import Playlists from "./Playlists";
|
||||||
import { api, HttpError, type Video } from "../lib/api";
|
import { api, HttpError, type Video } from "../lib/api";
|
||||||
@@ -72,10 +83,11 @@ const vid = (id: string, position_seconds: number): Video =>
|
|||||||
watch_url: `https://www.youtube.com/watch?v=${id}`,
|
watch_url: `https://www.youtube.com/watch?v=${id}`,
|
||||||
}) as unknown as Video;
|
}) as unknown as Video;
|
||||||
|
|
||||||
|
const WATCH_LATER = 9;
|
||||||
const list = (id: number, items: Video[]) => ({
|
const list = (id: number, items: Video[]) => ({
|
||||||
id,
|
id,
|
||||||
name: `List ${id}`,
|
name: `List ${id}`,
|
||||||
kind: "user",
|
kind: id === WATCH_LATER ? "watch_later" : "user",
|
||||||
yt_playlist_id: null,
|
yt_playlist_id: null,
|
||||||
item_count: items.length,
|
item_count: items.length,
|
||||||
items,
|
items,
|
||||||
@@ -107,6 +119,7 @@ const play = async (title: string) =>
|
|||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
h.selectedId = 1;
|
h.selectedId = 1;
|
||||||
h.player = null;
|
h.player = null;
|
||||||
|
h.mounted = false;
|
||||||
h.notify.mockClear();
|
h.notify.mockClear();
|
||||||
});
|
});
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -171,4 +184,70 @@ describe("Playlists — removing a video from inside the player", () => {
|
|||||||
expect(done).toBe(false);
|
expect(done).toBe(false);
|
||||||
expect(h.notify).toHaveBeenCalledWith(expect.objectContaining({ level: "error" }));
|
expect(h.notify).toHaveBeenCalledWith(expect.objectContaining({ level: "error" }));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("offers Remove by the list it was launched from, not by whichever list is selected now", async () => {
|
||||||
|
// Watch later deliberately has no Remove. Gated on the SELECTED list, it appeared on a player
|
||||||
|
// launched from Watch later as soon as another list was selected under it — and deleted from
|
||||||
|
// Watch later, since the removal itself targets the launch list.
|
||||||
|
h.selectedId = WATCH_LATER;
|
||||||
|
const { rerender } = mount({ [WATCH_LATER]: [vid("V", 0)], 2: [vid("W", 0)] });
|
||||||
|
await play("Video V");
|
||||||
|
await waitFor(() => expect(h.player).not.toBeNull());
|
||||||
|
expect(h.player!.onRemoveFromList).toBeUndefined();
|
||||||
|
h.selectedId = 2;
|
||||||
|
rerender();
|
||||||
|
await screen.findByRole("button", { name: "card.play — Video W" });
|
||||||
|
expect(h.player!.onRemoveFromList).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("takes the removed video out of the cached list, so a progress checkpoint cannot bring it back", async () => {
|
||||||
|
// The player steps to the next item after a removal and stores the last one's position into
|
||||||
|
// every cached list. Landing before the refetch, that write handed the page a list still holding
|
||||||
|
// the video, and the page — seeing a different set of ids — reset: the row came back.
|
||||||
|
vi.spyOn(api, "removeFromPlaylist").mockResolvedValue(undefined as never);
|
||||||
|
mount({ 1: [vid("V", 0), vid("W", 0)] });
|
||||||
|
await play("Video V");
|
||||||
|
await waitFor(() => expect(h.player).not.toBeNull());
|
||||||
|
vi.mocked(api.playlist).mockReturnValue(new Promise(() => {})); // the refetch is still out
|
||||||
|
await act(async () => {
|
||||||
|
await h.player!.onRemoveFromList!(vid("V", 0));
|
||||||
|
});
|
||||||
|
await act(async () => {
|
||||||
|
patchCachedProgress(qc, new Map([["V", 99]]));
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
});
|
||||||
|
expect(screen.queryByRole("button", { name: "card.play — Video V" })).toBeNull();
|
||||||
|
expect(screen.getByRole("button", { name: "card.play — Video W" })).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
|
||||||
|
// in the loaded branch, that took it down mid-playback and put it back at its launch video.
|
||||||
|
const { rerender } = mount({ 1: [vid("V", 0)] });
|
||||||
|
await play("Video V");
|
||||||
|
await waitFor(() => expect(h.mounted).toBe(true));
|
||||||
|
vi.mocked(api.playlist).mockReturnValue(new Promise(() => {}));
|
||||||
|
h.selectedId = 2;
|
||||||
|
rerender();
|
||||||
|
await screen.findByText("playlists.loading");
|
||||||
|
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);
|
||||||
|
const { rerender } = mount({ 1: [vid("V", 0)], 2: [vid("W", 0)] });
|
||||||
|
await play("Video V");
|
||||||
|
await waitFor(() => expect(h.player).not.toBeNull());
|
||||||
|
h.selectedId = 2;
|
||||||
|
rerender();
|
||||||
|
await screen.findByRole("button", { name: "card.play — Video W" });
|
||||||
|
await act(async () => {
|
||||||
|
h.player!.onState("V", "watched");
|
||||||
|
await new Promise((r) => setTimeout(r, 0));
|
||||||
|
});
|
||||||
|
expect(qc.getQueryState(qk.playlist(1))?.isInvalidated).toBe(true);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -40,8 +40,9 @@ import {
|
|||||||
X,
|
X,
|
||||||
Youtube,
|
Youtube,
|
||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import { api, HttpError, type Playlist, type Video } from "../lib/api";
|
import { api, HttpError, type Playlist, type PlaylistDetail, type Video } from "../lib/api";
|
||||||
import { notify } from "../lib/notifications";
|
import { notify } from "../lib/notifications";
|
||||||
|
import { notifyActionError } from "../lib/actionErrors";
|
||||||
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
|
import { notifyYouTubeActionError } from "../lib/youtubeErrors";
|
||||||
import { playlistName } from "../lib/playlistName";
|
import { playlistName } from "../lib/playlistName";
|
||||||
import { useUndoable } from "../lib/useUndoable";
|
import { useUndoable } from "../lib/useUndoable";
|
||||||
@@ -227,12 +228,15 @@ export default function Playlists() {
|
|||||||
// normal case. Only onClose takes the player down now. `listId` is part of that snapshot for the
|
// normal case. Only onClose takes the player down now. `listId` is part of that snapshot for the
|
||||||
// same reason: the selection can change under an open player (the command palette opens over it),
|
// same reason: the selection can change under an open player (the command palette opens over it),
|
||||||
// and "remove this from the playlist" means the list it was LAUNCHED from, not whichever is
|
// and "remove this from the playlist" means the list it was LAUNCHED from, not whichever is
|
||||||
// selected by the time the button is pressed.
|
// selected by the time the button is pressed. So does whether that button exists at all
|
||||||
|
// (`editable`): gating it on the current selection offered Remove on a player launched from
|
||||||
|
// Watch later — which deliberately has none — as soon as another list was selected under it.
|
||||||
const [playing, setPlaying] = useState<{
|
const [playing, setPlaying] = useState<{
|
||||||
video: Video;
|
video: Video;
|
||||||
queue: Video[];
|
queue: Video[];
|
||||||
index: number;
|
index: number;
|
||||||
listId: number | null;
|
listId: number | null;
|
||||||
|
editable: boolean;
|
||||||
} | null>(null);
|
} | null>(null);
|
||||||
// Where the last move went. `LS.playlist` is the SELECTED (source) list and `updated_at` moves on
|
// Where the last move went. `LS.playlist` is the SELECTED (source) list and `updated_at` moves on
|
||||||
// any edit, so neither answers "where do I keep sending things"; this does, and it is a per-account
|
// any edit, so neither answers "where do I keep sending things"; this does, and it is a per-account
|
||||||
@@ -301,6 +305,8 @@ export default function Playlists() {
|
|||||||
// push/delete-on-YouTube paths. Folding it in here would have taken drag and remove away from
|
// push/delete-on-YouTube paths. Folding it in here would have taken drag and remove away from
|
||||||
// every account without the scope, which they have always had.
|
// every account without the scope, which they have always had.
|
||||||
const canEdit = editable;
|
const canEdit = editable;
|
||||||
|
const canEditRef = useRef(canEdit);
|
||||||
|
canEditRef.current = canEdit;
|
||||||
const [pushing, setPushing] = useState(false);
|
const [pushing, setPushing] = useState(false);
|
||||||
const [reverting, setReverting] = useState(false);
|
const [reverting, setReverting] = useState(false);
|
||||||
|
|
||||||
@@ -310,7 +316,13 @@ export default function Playlists() {
|
|||||||
const queue = shownRef.current;
|
const queue = shownRef.current;
|
||||||
const index = queue.findIndex((v) => v.id === id);
|
const index = queue.findIndex((v) => v.id === id);
|
||||||
if (index < 0) return;
|
if (index < 0) return;
|
||||||
setPlaying({ video: queue[index]!, queue, index, listId: selectedRef.current });
|
setPlaying({
|
||||||
|
video: queue[index]!,
|
||||||
|
queue,
|
||||||
|
index,
|
||||||
|
listId: selectedRef.current,
|
||||||
|
editable: canEditRef.current,
|
||||||
|
});
|
||||||
}, []);
|
}, []);
|
||||||
const onPlay = useCallback((v: Video) => openPlayer(v.id), [openPlayer]);
|
const onPlay = useCallback((v: Video) => openPlayer(v.id), [openPlayer]);
|
||||||
const onRemove = useCallback((v: Video) => {
|
const onRemove = useCallback((v: Video) => {
|
||||||
@@ -551,6 +563,14 @@ export default function Playlists() {
|
|||||||
order.rebase((list) => list.filter((v) => v.id !== video.id));
|
order.rebase((list) => list.filter((v) => v.id !== video.id));
|
||||||
lastSetRef.current = `${listId}:${idsKey(next)}`;
|
lastSetRef.current = `${listId}:${idsKey(next)}`;
|
||||||
}
|
}
|
||||||
|
// …and out of the CACHED list too, not just the rows. Anything that rewrites that cache entry
|
||||||
|
// before the refetch lands handed the membership guard a list that still held the video — the
|
||||||
|
// player's own progress checkpoint does exactly that when a removal steps it to the next item —
|
||||||
|
// so the guard reset the order: the removed row came back and the undo history the rebase had
|
||||||
|
// kept went with it. (A failure resyncs below, which puts the row back where it belongs.)
|
||||||
|
qc.setQueryData<PlaylistDetail>(qk.playlist(listId), (old) =>
|
||||||
|
old ? { ...old, items: old.items.filter((v) => v.id !== video.id) } : old,
|
||||||
|
);
|
||||||
try {
|
try {
|
||||||
await api.removeFromPlaylist(listId, video.id);
|
await api.removeFromPlaylist(listId, video.id);
|
||||||
refreshAll(listId);
|
refreshAll(listId);
|
||||||
@@ -561,7 +581,7 @@ export default function Playlists() {
|
|||||||
// confirmed removal fails in silence (inside the full-screen player there is not even a row
|
// confirmed removal fails in silence (inside the full-screen player there is not even a row
|
||||||
// snapping back to show it).
|
// snapping back to show it).
|
||||||
if (e instanceof HttpError && (e.status === 404 || e.status === 403))
|
if (e instanceof HttpError && (e.status === 404 || e.status === 403))
|
||||||
notify({ level: "error", message: e.detail || t("playlists.removeFailed") });
|
notifyActionError(e, t("playlists.removeFailed"));
|
||||||
refreshAll(listId); // …and resync, which restores any row the optimistic removal took out
|
refreshAll(listId); // …and resync, which restores any row the optimistic removal took out
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
@@ -597,7 +617,7 @@ export default function Playlists() {
|
|||||||
// error dialog for 400/409/422 and 5xx and raises the connectivity notice for a bad gateway,
|
// error dialog for 400/409/422 and 5xx and raises the connectivity notice for a bad gateway,
|
||||||
// so toasting unconditionally reported a transport fault twice (E10 review round 2).
|
// so toasting unconditionally reported a transport fault twice (E10 review round 2).
|
||||||
if (e instanceof HttpError && (e.status === 404 || e.status === 403))
|
if (e instanceof HttpError && (e.status === 404 || e.status === 403))
|
||||||
notify({ level: "error", message: e.detail || t("playlists.moveFailed") });
|
notifyActionError(e, t("playlists.moveFailed"));
|
||||||
}
|
}
|
||||||
// BOTH sides changed, and so did this video's membership. `qk.playlist(selectedId)` is an exact
|
// BOTH sides changed, and so did this video's membership. `qk.playlist(selectedId)` is an exact
|
||||||
// key, so it cannot cover the target — invalidate the whole `["playlist"]` prefix, exactly as
|
// key, so it cannot cover the target — invalidate the whole `["playlist"]` prefix, exactly as
|
||||||
@@ -636,7 +656,10 @@ export default function Playlists() {
|
|||||||
|
|
||||||
function playerState(id: string, status: string) {
|
function playerState(id: string, status: string) {
|
||||||
api.setState(id, status).then(() => {
|
api.setState(id, status).then(() => {
|
||||||
qc.invalidateQueries({ queryKey: qk.playlist(selectedId) });
|
// Every cached list, not `qk.playlist(selectedId)`: the player may have been launched from a
|
||||||
|
// list that is no longer the selected one (see `playing`), and that list holds the row whose
|
||||||
|
// status just changed. Only lists actually on screen refetch.
|
||||||
|
qc.invalidateQueries({ queryKey: qk.playlist() });
|
||||||
qc.invalidateQueries({ queryKey: qk.feed() });
|
qc.invalidateQueries({ queryKey: qk.feed() });
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -767,13 +790,37 @@ export default function Playlists() {
|
|||||||
: []),
|
: []),
|
||||||
];
|
];
|
||||||
|
|
||||||
|
// The open player, at ONE place in the tree whichever state the page below it is in. Selecting
|
||||||
|
// another list under an open player (the command palette works over it) puts the page into
|
||||||
|
// "Loading…" until that list arrives; with the player rendered only in the loaded branch, that
|
||||||
|
// unmounted it mid-playback and remounted it at the video it was launched with.
|
||||||
|
const player = playing && (
|
||||||
|
<Suspense fallback={null}>
|
||||||
|
<PlayerModal
|
||||||
|
video={playing.video}
|
||||||
|
queue={playing.queue}
|
||||||
|
startIndex={playing.index}
|
||||||
|
startAt={null}
|
||||||
|
onClose={closePlayer}
|
||||||
|
onState={playerState}
|
||||||
|
onRemoveFromList={playing.editable ? removeFromPlayer : undefined}
|
||||||
|
/>
|
||||||
|
</Suspense>
|
||||||
|
);
|
||||||
|
|
||||||
if (selectedId == null) {
|
if (selectedId == null) {
|
||||||
return <div className="text-muted text-sm p-4">{t("playlists.pickOne")}</div>;
|
return (
|
||||||
|
<>
|
||||||
|
<div className="text-muted text-sm p-4">{t("playlists.pickOne")}</div>
|
||||||
|
{player}
|
||||||
|
</>
|
||||||
|
);
|
||||||
}
|
}
|
||||||
// The page used to render "Loading…" forever when the detail query failed — an error read as a
|
// The page used to render "Loading…" forever when the detail query failed — an error read as a
|
||||||
// slow network that never arrived.
|
// slow network that never arrived.
|
||||||
if (detailQuery.isError) {
|
if (detailQuery.isError) {
|
||||||
return (
|
return (
|
||||||
|
<>
|
||||||
<div className="text-sm p-4 flex flex-col items-start gap-3">
|
<div className="text-sm p-4 flex flex-col items-start gap-3">
|
||||||
<span className="inline-flex items-center gap-2 text-muted">
|
<span className="inline-flex items-center gap-2 text-muted">
|
||||||
<AlertTriangle className="w-4 h-4 text-danger" />
|
<AlertTriangle className="w-4 h-4 text-danger" />
|
||||||
@@ -783,16 +830,24 @@ export default function Playlists() {
|
|||||||
{t("common.retry")}
|
{t("common.retry")}
|
||||||
</Button>
|
</Button>
|
||||||
</div>
|
</div>
|
||||||
|
{player}
|
||||||
|
</>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (!detail) {
|
if (!detail) {
|
||||||
return <div className="text-muted text-sm p-4">{t("playlists.loading")}</div>;
|
return (
|
||||||
|
<>
|
||||||
|
<div className="text-muted text-sm p-4">{t("playlists.loading")}</div>
|
||||||
|
{player}
|
||||||
|
</>
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
|
||||||
// No top padding: the sticky column header must sit flush at the scroller top so it doesn't
|
// No top padding: the sticky column header must sit flush at the scroller top so it doesn't
|
||||||
// shift up when `position: sticky` engages on the first scroll (the fixed toolbar band above
|
// shift up when `position: sticky` engages on the first scroll (the fixed toolbar band above
|
||||||
// gives the breathing room). Same reason as the channel table — see Channels.tsx.
|
// gives the breathing room). Same reason as the channel table — see Channels.tsx.
|
||||||
|
return (
|
||||||
|
<>
|
||||||
<div className="px-4 pb-4">
|
<div className="px-4 pb-4">
|
||||||
{/* Header (title, actions, view controls) is fixed chrome — it portals into the shell's
|
{/* Header (title, actions, view controls) is fixed chrome — it portals into the shell's
|
||||||
fixed band, so only the video list scrolls. */}
|
fixed band, so only the video list scrolls. */}
|
||||||
@@ -874,7 +929,9 @@ export default function Playlists() {
|
|||||||
<button
|
<button
|
||||||
onClick={pushToYoutube}
|
onClick={pushToYoutube}
|
||||||
disabled={pushing}
|
disabled={pushing}
|
||||||
title={linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube")}
|
title={
|
||||||
|
linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube")
|
||||||
|
}
|
||||||
aria-label={
|
aria-label={
|
||||||
linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube")
|
linked ? t("playlists.pushToYoutube") : t("playlists.exportToYoutube")
|
||||||
}
|
}
|
||||||
@@ -1040,20 +1097,8 @@ export default function Playlists() {
|
|||||||
|
|
||||||
{/* One surface for the whole table, outside the rows — it anchors to the pointer, not the row. */}
|
{/* One surface for the whole table, outside the rows — it anchors to the pointer, not the row. */}
|
||||||
<ContextMenu menu={rowMenu} ariaLabel={t("playlists.rowMenu")} items={ctxItems} />
|
<ContextMenu menu={rowMenu} ariaLabel={t("playlists.rowMenu")} items={ctxItems} />
|
||||||
|
|
||||||
{playing && (
|
|
||||||
<Suspense fallback={null}>
|
|
||||||
<PlayerModal
|
|
||||||
video={playing.video}
|
|
||||||
queue={playing.queue}
|
|
||||||
startIndex={playing.index}
|
|
||||||
startAt={null}
|
|
||||||
onClose={closePlayer}
|
|
||||||
onState={playerState}
|
|
||||||
onRemoveFromList={canEdit ? removeFromPlayer : undefined}
|
|
||||||
/>
|
|
||||||
</Suspense>
|
|
||||||
)}
|
|
||||||
</div>
|
</div>
|
||||||
|
{player}
|
||||||
|
</>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -77,6 +77,9 @@
|
|||||||
"movedToast": "“{{title}}” moved to “{{name}}” ✓",
|
"movedToast": "“{{title}}” moved to “{{name}}” ✓",
|
||||||
"moveFailed": "Couldn't move the video.",
|
"moveFailed": "Couldn't move the video.",
|
||||||
"removeFailed": "Couldn't remove the video.",
|
"removeFailed": "Couldn't remove the video.",
|
||||||
|
"errors": {
|
||||||
|
"not_found": "This playlist doesn't exist any more."
|
||||||
|
},
|
||||||
"moveErrors": {
|
"moveErrors": {
|
||||||
"not_in_playlist": "That video isn't in this playlist any more — it changed somewhere else.",
|
"not_in_playlist": "That video isn't in this playlist any more — it changed somewhere else.",
|
||||||
"same_playlist": "That video is already in this playlist."
|
"same_playlist": "That video is already in this playlist."
|
||||||
|
|||||||
@@ -77,6 +77,9 @@
|
|||||||
"movedToast": "„{{title}}” áthelyezve ide: „{{name}}” ✓",
|
"movedToast": "„{{title}}” áthelyezve ide: „{{name}}” ✓",
|
||||||
"moveFailed": "Nem sikerült áthelyezni a videót.",
|
"moveFailed": "Nem sikerült áthelyezni a videót.",
|
||||||
"removeFailed": "Nem sikerült eltávolítani a videót.",
|
"removeFailed": "Nem sikerült eltávolítani a videót.",
|
||||||
|
"errors": {
|
||||||
|
"not_found": "Ez a lejátszási lista már nem létezik."
|
||||||
|
},
|
||||||
"moveErrors": {
|
"moveErrors": {
|
||||||
"not_in_playlist": "Ez a videó már nincs ebben a listában — időközben máshol megváltozott.",
|
"not_in_playlist": "Ez a videó már nincs ebben a listában — időközben máshol megváltozott.",
|
||||||
"same_playlist": "Ez a videó már ebben a listában van."
|
"same_playlist": "Ez a videó már ebben a listában van."
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ vi.mock("../activeAccount", () => ({
|
|||||||
notifyActiveAccountChange: vi.fn(),
|
notifyActiveAccountChange: vi.fn(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
import { uploadReq, setUnauthorizedHandler, HttpError } from "./core";
|
import { req, uploadReq, setUnauthorizedHandler, HttpError } from "./core";
|
||||||
|
|
||||||
// A fake XHR whose lifecycle the test drives: capture the instance, set status/responseText, then
|
// A fake XHR whose lifecycle the test drives: capture the instance, set status/responseText, then
|
||||||
// invoke onload/onerror/abort by hand.
|
// invoke onload/onerror/abort by hand.
|
||||||
@@ -117,3 +117,21 @@ describe("uploadReq", () => {
|
|||||||
expect(reportError).not.toHaveBeenCalled();
|
expect(reportError).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("req — a structured refusal reads in the user's language", () => {
|
||||||
|
it("names a playlist that is gone instead of passing the server's English through", async () => {
|
||||||
|
// The 404 every playlist route answers when the list was deleted in another tab. It used to be a
|
||||||
|
// bare "Unknown playlist" string, shown verbatim under the Hungarian UI.
|
||||||
|
vi.spyOn(globalThis, "fetch").mockResolvedValue(
|
||||||
|
new Response(JSON.stringify({ detail: { code: "playlist", reason: "not_found" } }), {
|
||||||
|
status: 404,
|
||||||
|
headers: { "content-type": "application/json" },
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await expect(req("/api/playlists/7/items/v1", { method: "DELETE" })).rejects.toMatchObject({
|
||||||
|
status: 404,
|
||||||
|
detail: "playlists.errors.not_found",
|
||||||
|
});
|
||||||
|
expect(reportError).not.toHaveBeenCalled(); // a 404 is the caller's to report
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -82,6 +82,8 @@ function localizeDetail(d: unknown): string {
|
|||||||
// unknown reason falls back to the plain failure rather than printing a key.
|
// unknown reason falls back to the plain failure rather than printing a key.
|
||||||
if (o?.code === "playlist_move")
|
if (o?.code === "playlist_move")
|
||||||
return t(`playlists.moveErrors.${o.reason}`, t("playlists.moveFailed"));
|
return t(`playlists.moveErrors.${o.reason}`, t("playlists.moveFailed"));
|
||||||
|
// Any playlist route whose list is gone (deleted in another tab) — or was never the caller's.
|
||||||
|
if (o?.code === "playlist") return t(`playlists.errors.${o.reason}`, t("errors.generic"));
|
||||||
// SEC-14: a manual sync trigger on cooldown. Not an error the user caused so much as a pace
|
// SEC-14: a manual sync trigger on cooldown. Not an error the user caused so much as a pace
|
||||||
// limit, and the copy says so.
|
// limit, and the copy says so.
|
||||||
if (o?.code === "sync_cooldown") return t("errors.syncCooldown");
|
if (o?.code === "sync_cooldown") return t("errors.syncCooldown");
|
||||||
|
|||||||
Reference in New Issue
Block a user