Fix moving tracks between playlists

app/models.py

@@ -17,9 +17,10 @@ from sqlalchemy import (
     DateTime,
     Float,
     ForeignKey,
+    func,
     Integer,
     String,
-    func
+    UniqueConstraint,
 )
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import (
@@ -39,7 +40,6 @@ from helpers import (
 )
 from log import DEBUG, ERROR
 
-
 Base: DeclarativeMeta = declarative_base()
 
 
@@ -265,7 +265,7 @@ class Playlists(Base):
         """
 
         if not row:
-            row = PlaylistTracks.next_free_row(session, self)
+            row = PlaylistTracks.next_free_row(session, self.id)
 
         PlaylistTracks(session, self.id, track_id, row)
 
@@ -317,7 +317,8 @@ class Playlists(Base):
         self.last_used = datetime.now()
         session.flush()
 
-    def move_track(self, session: Session, rows: List[int],
+    def move_track(
+            self, session: Session, rows: List[int],
             to_playlist: "Playlists") -> None:
         """Move tracks to another playlist"""
 
@@ -367,6 +368,10 @@ class PlaylistTracks(Base):
             cascade="all, delete-orphan"
         )
     )
+    # Ensure row numbers are unique within each playlist
+    __table_args__ = (UniqueConstraint
+                          ('row', 'playlist_id', name="uniquerow"),
+                      )
 
     def __init__(
             self, session: Session, playlist_id: int, track_id: int,
@@ -380,14 +385,14 @@ class PlaylistTracks(Base):
         session.flush()
 
     @staticmethod
-    def next_free_row(session: Session, playlist: Playlists) -> int:
+    def next_free_row(session: Session, playlist_id: int) -> int:
         """Return next free row number"""
 
         row: int
 
         last_row = session.query(
             func.max(PlaylistTracks.row)
-        ).filter_by(playlist_id=playlist.id).first()
+        ).filter_by(playlist_id=playlist_id).first()
         # if there are no rows, the above returns (None, ) which is True
         if last_row and last_row[0] is not None:
             row = last_row[0] + 1
@@ -396,6 +401,33 @@ class PlaylistTracks(Base):
 
         return row
 
+    @staticmethod
+    def move_rows(
+            session: Session, rows: List[int], from_playlist_id: int,
+            to_playlist_id: int) -> None:
+        """Move rows between playlists"""
+
+        # A constraint deliberately blocks duplicate (playlist_id, row)
+        # entries in database; however, unallocated rows in the database
+        # are fine (ie, we can have rows 1, 4, 6 and no 2, 3, 5).
+        # Unallocated rows will be automatically removed when the
+        # playlist is saved.
+
+        lowest_source_row: int = min(rows)
+        first_destination_free_row = PlaylistTracks.next_free_row(
+            session, to_playlist_id)
+        # Calculate offset that will put the lowest row number being
+        # moved at the first free row in destination playlist
+        offset = first_destination_free_row - lowest_source_row
+
+        session.query(PlaylistTracks).filter(
+            PlaylistTracks.playlist_id == from_playlist_id,
+            PlaylistTracks.row.in_(rows)
+        ).update({'playlist_id': to_playlist_id,
+                  'row': PlaylistTracks.row + offset},
+            False
+        )
+
 
 class Settings(Base):
     __tablename__ = 'settings'
@@ -450,7 +482,8 @@ class Tracks(Base):
         back_populates="tracks",
         lazy="joined")
 
-    def __init__(self,
+    def __init__(
+            self,
             session: Session,
             path: str,
             title: Optional[str] = None,

app/musicmuster.py

@@ -424,7 +424,8 @@ class Window(QMainWindow, Ui_MainWindow):
 
             # Update database for both source and destination playlists
             rows = visible_tab.get_selected_rows()
-            source_playlist.move_track(session, rows, destination_playlist)
+            PlaylistTracks.move_rows(session, rows, source_playlist.id,
+                                     destination_playlist.id)
 
             # Update destination playlist_tab if visible (if not visible, it
             # will be re-populated when it is opened)
@@ -594,10 +595,10 @@ class Window(QMainWindow, Ui_MainWindow):
             dlg = SelectPlaylistDialog(self, playlists=playlists,
                                        session=session)
             dlg.exec()
-            if dlg.plid:
+            playlist = dlg.playlist
-                p = Playlists.get_by_id(session=session, playlist_id=dlg.plid)
+            if playlist:
-                p.mark_open(session)
+                playlist.mark_open(session)
-                self.create_playlist_tab(session, p)
+                self.create_playlist_tab(session, playlist)
 
     def select_next_row(self) -> None:
         """Select next or first row in playlist"""
@@ -971,11 +972,6 @@ class SelectPlaylistDialog(QDialog):
         height = record.f_int or 600
         self.resize(width, height)
 
-        # for (plid, plname) in [(a.id, a.name) for a in playlists]:
-        #    p = QListWidgetItem()
-        #    p.setText(plname)
-        #    p.setData(Qt.UserRole, plid)
-        #    self.ui.lstPlaylists.addItem(p)
         for playlist in playlists:
             p = QListWidgetItem()
             p.setText(playlist.name)