fix(playlists): reorder position collision + dedup remove helper

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.
This commit is contained in:
npeter83 2026-07-11 18:33:16 +02:00
parent 970e725d39
commit ecb2e0b749

View file

@ -74,6 +74,23 @@ def _add_item(db: Session, pl: Playlist, video_id: str, *, mark_dirty: bool) ->
return True 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( def _summary(
db: Session, db: Session,
pl: Playlist, pl: Playlist,
@ -239,14 +256,7 @@ def remove_watch_later(
) )
) )
if pl is not None: if pl is not None:
item = db.scalar( _remove_item(db, pl, video_id, mark_dirty=False)
select(PlaylistItem).where(
PlaylistItem.playlist_id == pl.id, PlaylistItem.video_id == video_id
)
)
if item is not None:
db.delete(item)
db.commit()
return {"saved": False} return {"saved": False}
@ -452,15 +462,7 @@ def remove_item(
db: Session = Depends(get_db), db: Session = Depends(get_db),
) -> dict: ) -> dict:
pl = _own_playlist(db, user, playlist_id) pl = _own_playlist(db, user, playlist_id)
item = db.scalar( _remove_item(db, pl, video_id, mark_dirty=True)
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()
return {"playlist_id": pl.id, "video_id": video_id} return {"playlist_id": pl.id, "video_id": video_id}
@ -480,11 +482,20 @@ def reorder_items(
).scalars() ).scalars()
} }
pos = 0 pos = 0
seen: set[str] = set()
for vid in order: for vid in order:
it = items.get(vid) it = items.get(vid)
if it is not None: if it is not None:
it.position = pos it.position = pos
pos += 1 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) recompute_dirty(db, pl)
db.commit() db.commit()
return {"playlist_id": pl.id, "count": pos} return {"playlist_id": pl.id, "count": pos}