From ecb2e0b7495e2fbdefac38214acbea7f8a90e0d4 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 11 Jul 2026 18:33:16 +0200 Subject: [PATCH 1/2] fix(playlists): reorder position collision + dedup remove helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG (PB1): reorder_items only repositioned the items present in the payload, leaving any omitted item (e.g. one added concurrently in another tab) at its old position — which then collides with the freshly assigned 0..N-1 range, so get_playlist's order-by-position returns a nondeterministic/duplicated order. Now the omitted items get fresh trailing positions (keeping their relative order), guaranteeing unique contiguous positions. CLEANUP (PC4): extract _remove_item(db, pl, video_id, mark_dirty=) mirroring the existing _add_item — remove_watch_later and remove_item duplicated the same find-by-(playlist,video)/delete/commit block. Behavior-neutral for the normal full-payload reorder. ruff clean, localdev boots. --- backend/app/routes/playlists.py | 45 ++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index a3ebf8a..e81a2c9 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -74,6 +74,23 @@ def _add_item(db: Session, pl: Playlist, video_id: str, *, mark_dirty: bool) -> return True +def _remove_item(db: Session, pl: Playlist, video_id: str, *, mark_dirty: bool) -> bool: + """Remove a video from a playlist if present (idempotent). Returns whether it was removed. + `mark_dirty` recomputes the YouTube-sync dirty flag (skip for Watch later).""" + item = db.scalar( + select(PlaylistItem).where( + PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id + ) + ) + if item is None: + return False + db.delete(item) + if mark_dirty: + recompute_dirty(db, pl) + db.commit() + return True + + def _summary( db: Session, pl: Playlist, @@ -239,14 +256,7 @@ def remove_watch_later( ) ) if pl is not None: - item = db.scalar( - select(PlaylistItem).where( - PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id - ) - ) - if item is not None: - db.delete(item) - db.commit() + _remove_item(db, pl, video_id, mark_dirty=False) return {"saved": False} @@ -452,15 +462,7 @@ def remove_item( db: Session = Depends(get_db), ) -> dict: pl = _own_playlist(db, user, playlist_id) - item = db.scalar( - select(PlaylistItem).where( - PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id - ) - ) - if item is not None: - db.delete(item) - recompute_dirty(db, pl) - db.commit() + _remove_item(db, pl, video_id, mark_dirty=True) return {"playlist_id": pl.id, "video_id": video_id} @@ -480,11 +482,20 @@ def reorder_items( ).scalars() } pos = 0 + seen: set[str] = set() for vid in order: it = items.get(vid) if it is not None: it.position = pos pos += 1 + seen.add(vid) + # Any items the payload omitted (e.g. added concurrently in another tab) keep their relative + # order but get fresh trailing positions, so no two items collide on the same position. + for it in sorted( + (it for vid, it in items.items() if vid not in seen), key=lambda i: i.position + ): + it.position = pos + pos += 1 recompute_dirty(db, pl) db.commit() return {"playlist_id": pl.id, "count": pos} From c0e941bc0cfffa4a3a0ca36cdeaa5f35c270593f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 11 Jul 2026 18:33:16 +0200 Subject: [PATCH 2/2] fix(playlists-ui): surface YouTube 403s + rollback remove; drop dup name/import BUGS: - push/sync/revert (PF1/PF2/PF4) swallowed every failure into a generic warning, so a missing-write-scope 403 gave no hint and no "Connect your YouTube account" affordance (unlike Channels). Route all three through notifyYouTubeActionError (no wizard handle on this screen, so message-only for the 403). - removeItem (PF5) reset the order optimistically then awaited the delete with no catch: a failed call left the row gone locally (and an unhandled rejection). Now resync via refreshAll on either path (api.req surfaces the error dialog). CLEANUP: - Drop the dead `Check` lucide import (PC6; only AddToPlaylist uses it). - Use the module plName(detail) helper instead of the inline watch_later-vs-name ternary that shadowed it in push/revert (PC7). tsc green, localdev boots healthy. --- frontend/src/components/Playlists.tsx | 32 +++++++++++++++------------ 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/frontend/src/components/Playlists.tsx b/frontend/src/components/Playlists.tsx index cbf846f..c157186 100644 --- a/frontend/src/components/Playlists.tsx +++ b/frontend/src/components/Playlists.tsx @@ -19,7 +19,6 @@ import { CSS } from "@dnd-kit/utilities"; import { ArrowDown, ArrowUp, - Check, ExternalLink, GripVertical, ListPlus, @@ -37,6 +36,7 @@ import { import { api, type Playlist, type Video } from "../lib/api"; import { formatDuration } from "../lib/format"; import { notify } from "../lib/notifications"; +import { notifyYouTubeActionError } from "../lib/youtubeErrors"; import { getAccountRaw, LS, readAccountMerged, setAccountRaw, writeAccount } from "../lib/storage"; import { useUndoable } from "../lib/useUndoable"; const PlayerModal = lazy(() => import("./PlayerModal")); @@ -347,14 +347,13 @@ export default function Playlists({ canWrite }: { canWrite: boolean }) { danger: true, }); if (!ok) return; - const plName = detail.kind === "watch_later" ? t("playlists.watchLater") : detail.name; setReverting(true); try { await api.revertPlaylist(selectedId); refreshAll(); - notify({ level: "success", message: t("playlists.revertDone", { name: plName }) }); - } catch { - notify({ level: "warning", message: t("playlists.revertFailed") }); + notify({ level: "success", message: t("playlists.revertDone", { name: plName(detail) }) }); + } catch (e) { + notifyYouTubeActionError(e, t("playlists.revertFailed")); } finally { setReverting(false); } @@ -362,12 +361,11 @@ export default function Playlists({ canWrite }: { canWrite: boolean }) { async function pushToYoutube() { if (selectedId == null || !detail || pushing) return; - const plName = detail.kind === "watch_later" ? t("playlists.watchLater") : detail.name; setPushing(true); try { const plan = await api.playlistPushPlan(selectedId); if (plan.action === "update" && !plan.to_insert && !plan.to_delete && !plan.to_reorder) { - notify({ level: "info", message: t("playlists.pushUpToDate", { name: plName }) }); + notify({ level: "info", message: t("playlists.pushUpToDate", { name: plName(detail) }) }); return; } if (!plan.affordable) { @@ -406,10 +404,10 @@ export default function Playlists({ canWrite }: { canWrite: boolean }) { if (r.failures.length) { notify({ level: "warning", message: t("playlists.pushPartial", { count: r.failures.length }) }); } else { - notify({ level: "success", message: t("playlists.pushDone", { name: plName }) }); + notify({ level: "success", message: t("playlists.pushDone", { name: plName(detail) }) }); } - } catch { - notify({ level: "warning", message: t("playlists.pushFailed") }); + } catch (e) { + notifyYouTubeActionError(e, t("playlists.pushFailed")); } finally { setPushing(false); } @@ -423,8 +421,8 @@ export default function Playlists({ canWrite }: { canWrite: boolean }) { qc.invalidateQueries({ queryKey: ["playlists"] }); qc.invalidateQueries({ queryKey: ["playlist"] }); notify({ level: "success", message: t("playlists.syncedToast", { count: r.synced }) }); - } catch { - notify({ level: "warning", message: t("playlists.syncFailed") }); + } catch (e) { + notifyYouTubeActionError(e, t("playlists.syncFailed")); } finally { setSyncing(false); } @@ -463,8 +461,14 @@ export default function Playlists({ canWrite }: { canWrite: boolean }) { const next = items.filter((v) => v.id !== videoId); order.reset(next); lastSetRef.current = idsKey(next); - await api.removeFromPlaylist(selectedId, videoId); - refreshAll(); + try { + await api.removeFromPlaylist(selectedId, videoId); + refreshAll(); + } catch { + // The optimistic removal above didn't stick — resync from the server to restore the row. + // api.req already surfaced the error to the user via the global error dialog. + refreshAll(); + } } async function saveRename() {