From ecb2e0b7495e2fbdefac38214acbea7f8a90e0d4 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 11 Jul 2026 18:33:16 +0200 Subject: [PATCH] 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}