Move notes with tracks

Fixes #106

app/models.py

@@ -154,16 +154,28 @@ class Notes(Base):
         session.flush()
 
     @staticmethod
-    def move_rows(session: Session, rows: List[int],
+    def max_used_row(session: Session, playlist_id: int) -> Optional[int]:
-                  from_playlist_id: int, to_playlist_id: int):
         """
-        Move note(s) to another playlist
+        Return maximum notes row for passed playlist ID or None if not notes
         """
 
-        session.query(Notes).filter(
+        last_row = session.query(func.max(Notes.row)).filter_by(
-            Notes.playlist_id == from_playlist_id,
+            playlist_id=playlist_id).first()
-            Notes.row.in_(rows)
+        # if there are no rows, the above returns (None, ) which is True
-        ).update({'playlist_id': to_playlist_id}, False)
+        if last_row and last_row[0] is not None:
+            return last_row[0]
+        else:
+            return None
+
+    def move_row(self, session: Session, row: int, to_playlist_id: int) \
+            -> None:
+        """
+        Move note to another playlist
+        """
+
+        self.row = row
+        self.playlist_id = to_playlist_id
+        session.commit()
 
     @classmethod
     def get_by_id(cls, session: Session, note_id: int) -> Optional["Notes"]:
@@ -277,8 +289,8 @@ class Playlists(Base):
         If row=None, add to end of playlist
         """
 
-        if not row:
+        if row is None:
-            row = PlaylistTracks.next_free_row(session, self.id)
+            row = self.next_free_row(session, self.id)
 
         PlaylistTracks(session, self.id, track_id, row)
 
@@ -330,17 +342,23 @@ class Playlists(Base):
         self.last_used = datetime.now()
         session.flush()
 
-    def move_track(
+    @staticmethod
-            self, session: Session, rows: List[int],
+    def next_free_row(session: Session, playlist_id: int) -> int:
-            to_playlist: "Playlists") -> None:
+        """Return next free row for this playlist"""
-        """Move tracks to another playlist"""
 
-        for row in rows:
+        max_notes_row = Notes.max_used_row(session, playlist_id)
-            track = self.tracks[row]
+        max_tracks_row = PlaylistTracks.max_used_row(session, playlist_id)
-            to_playlist.add_track(session, track.id)
-            del self.tracks[row]
 
-        session.flush()
+        if max_notes_row is not None and max_tracks_row is not None:
+            return max(max_notes_row, max_tracks_row) + 1
+
+        if max_notes_row is None and max_tracks_row is None:
+            return 0
+
+        if max_notes_row is None:
+            return max_tracks_row + 1
+        else:
+            return max_notes_row + 1
 
     def remove_all_tracks(self, session: Session) -> None:
         """
@@ -399,48 +417,30 @@ class PlaylistTracks(Base):
         session.flush()
 
     @staticmethod
-    def next_free_row(session: Session, playlist_id: int) -> int:
+    def max_used_row(session: Session, playlist_id: int) -> Optional[int]:
-        """Return next free row number"""
+        """
-
+        Return highest track row number used or None if there are no
-        row: int
+        tracks
+        """
 
         last_row = session.query(
             func.max(PlaylistTracks.row)
         ).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
+            return last_row[0]
         else:
-            row = 0
+            return None
-
-        return row
 
     @staticmethod
-    def move_rows(
+    def move_row(session: Session, from_row: int, from_playlist_id: int,
-            session: Session, rows: List[int], from_playlist_id: int,
+                 to_row: int, to_playlist_id: int) -> None:
-            to_playlist_id: int) -> None:
+        """Move row to another playlist"""
-        """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)
+            PlaylistTracks.row == from_row).update(
-        ).update({'playlist_id': to_playlist_id,
+                {'playlist_id': to_playlist_id, 'row': to_row}, False)
-                  'row': PlaylistTracks.row + offset},
-                 False
-                 )
 
 
 class Settings(Base):

app/musicmuster.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python
 
 import argparse
-import os.path
 import psutil
 import sys
 import threading
@@ -34,8 +33,7 @@ import helpers
 import music
 
 from config import Config
-from models import (Base, Notes, Playdates, Playlists, PlaylistTracks,
+from models import (Base, Playdates, Playlists, Settings, Tracks)
-                    Settings, Tracks)
 from playlists import PlaylistTab
 from sqlalchemy.orm.exc import DetachedInstanceError
 from ui.dlg_search_database_ui import Ui_Dialog
@@ -504,17 +502,8 @@ class Window(QMainWindow, Ui_MainWindow):
                 return
             destination_playlist = dlg.playlist
 
-            # Update database for both source and destination playlists
+            self.visible_playlist_tab().move_selected_to_playlist(
-            rows = visible_tab.get_selected_rows()
+                session, destination_playlist.id)
-            notes_rows = visible_tab.get_notes_rows()
-            track_rows_to_move = [a for a in rows if a not in notes_rows]
-            note_rows_to_move = [a for a in rows if a in notes_rows]
-            PlaylistTracks.move_rows(
-                session, track_rows_to_move, source_playlist.id,
-                destination_playlist.id)
-            Notes.move_rows(
-                session, note_rows_to_move, source_playlist.id,
-                destination_playlist.id)
 
             # Update destination playlist_tab if visible (if not visible, it
             # will be re-populated when it is opened)
@@ -534,9 +523,6 @@ class Window(QMainWindow, Ui_MainWindow):
                 destination_visible_playlist_tab.populate(
                     session, dlg.playlist.id)
 
-        # Update source playlist
-        self.visible_playlist_tab().remove_rows(rows)
-
     def open_info_tabs(self) -> None:
         """
         Ensure we have info tabs for next and current track titles
@@ -1084,6 +1070,7 @@ class SelectPlaylistDialog(QDialog):
         self.ui.buttonBox.accepted.connect(self.open)
         self.ui.buttonBox.rejected.connect(self.close)
         self.session = session
+        self.playlist = None
         self.plid = None
 
         record = Settings.get_int_settings(

app/playlists.py

@@ -30,6 +30,7 @@ from models import (
     Notes,
     Playdates,
     Playlists,
+    PlaylistTracks,
     Settings,
     Tracks,
     NoteColours
@@ -150,7 +151,7 @@ class PlaylistTab(QTableWidget):
         self.populate(session, self.playlist_id)
 
     def __repr__(self) -> str:
-        return (f"<PlaylistTab(id={self.playlist_id}")
+        return f"<PlaylistTab(id={self.playlist_id}"
 
     # ########## Events ##########
 
@@ -182,7 +183,7 @@ class PlaylistTab(QTableWidget):
         # rows. Check and fix:
         row = 0  # So row is defined even if there are no rows in range
         for row in range(drop_row, drop_row + len(rows_to_move)):
-            if row in self.get_notes_rows():
+            if row in self._get_notes_rows():
                 self.setSpan(
                     row, self.COL_NOTE, self.NOTE_ROW_SPAN, self.NOTE_COL_SPAN)
 
@@ -305,11 +306,6 @@ class PlaylistTab(QTableWidget):
                     session, self.playlist_id, row, dlg.textValue())
                 self._insert_note(session, note, row, True)         # checked
 
-    def get_notes_rows(self) -> List[int]:
-        """Return rows marked as notes, or None"""
-
-        return self._meta_search(RowMeta.NOTE, one=False)
-
     def get_selected_row(self) -> Optional[int]:
         """Return row number of first selected row, or None if none selected"""
 
@@ -319,10 +315,10 @@ class PlaylistTab(QTableWidget):
             return self.selectionModel().selectedRows()[0].row()
 
     def get_selected_rows(self) -> List[int]:
-        """Return a list of selected row numbers"""
+        """Return a sorted list of selected row numbers"""
 
         rows = self.selectionModel().selectedRows()
-        return [row.row() for row in rows]
+        return sorted([row.row() for row in rows])
 
     def get_selected_title(self) -> Optional[str]:
         """Return title of selected row or None"""
@@ -403,23 +399,50 @@ class PlaylistTab(QTableWidget):
             self.save_playlist(session)
             self.update_display(session, clear_selection=False)
 
-    def remove_rows(self, rows) -> None:
+    def move_selected_to_playlist(self, session: Session, playlist_id: int) \
-        """Remove rows passed in rows list"""
+            -> None:
+        """
+        Move selected rows and any immediately preceding notes to
+        other playlist
+        """
 
-        # Row number will change as we delete rows so remove them in
+        notes_rows = self._get_notes_rows()
-        # reverse order.
+        destination_row = Playlists.next_free_row(session, playlist_id)
+        rows_to_remove = []
+
+        for row in self.get_selected_rows():
+            if row in notes_rows:
+                note_obj = self._get_row_notes_object(row, session)
+                note_obj.move_row(session, destination_row, playlist_id)
+            else:
+                # For tracks, check for a preceding notes row and move
+                # that as well if it exists
+                if row - 1 in notes_rows:
+                    note_obj = self._get_row_notes_object(row - 1, session)
+                    note_obj.move_row(session, destination_row, playlist_id)
+                    destination_row += 1
+                    rows_to_remove.append(row - 1)
+                # Move track
+                PlaylistTracks.move_row(
+                    session, row, self.playlist_id,
+                    destination_row, playlist_id
+                )
+            destination_row += 1
+            rows_to_remove.append(row)
+
+        # Remove rows. Row number will change as we delete rows so
+        # remove them in reverse order.
 
         try:
             self.selecting_in_progress = True
-            for row in sorted(rows, reverse=True):
+            for row in sorted(rows_to_remove, reverse=True):
                 self.removeRow(row)
         finally:
             self.selecting_in_progress = False
             self._select_event()
 
-        with Session() as session:
+        self.save_playlist(session)
-            self.save_playlist(session)
+        self.update_display(session)
-            self.update_display(session)
 
     def play_started(self, session: Session) -> None:
         """
@@ -539,7 +562,7 @@ class PlaylistTab(QTableWidget):
         # Create dictionaries indexed by note_id
         playlist_notes: Dict[int, Notes] = {}
         database_notes: Dict[int, Notes] = {}
-        notes_rows: List[int] = self.get_notes_rows()
+        notes_rows: List[int] = self._get_notes_rows()
 
         # PlaylistTab
         for row in notes_rows:
@@ -587,6 +610,7 @@ class PlaylistTab(QTableWidget):
             track_id: int = self.item(
                 row, self.COL_USERDATA).data(self.CONTENT_OBJECT)
             playlist.add_track(session, track_id, row)
+            session.commit()
 
     def select_next_row(self) -> None:
         """
@@ -611,7 +635,7 @@ class PlaylistTab(QTableWidget):
 
         # Don't select notes
         wrapped: bool = False
-        while row in self.get_notes_rows():
+        while row in self._get_notes_rows():
             row += 1
             if row >= self.rowCount():
                 if wrapped:
@@ -657,7 +681,7 @@ class PlaylistTab(QTableWidget):
 
         # Don't select notes
         wrapped: bool = False
-        while row in self.get_notes_rows():
+        while row in self._get_notes_rows():
             row -= 1
             if row < 0:
                 if wrapped:
@@ -714,7 +738,7 @@ class PlaylistTab(QTableWidget):
 
         current_row: Optional[int] = self._get_current_track_row()
         next_row: Optional[int] = self._get_next_track_row()
-        notes: List[int] = self.get_notes_rows()
+        notes: List[int] = self._get_notes_rows()
         played: Optional[List[int]] = self._get_played_track_rows()
         unreadable: List[int] = self._get_unreadable_track_rows()
 
@@ -836,8 +860,8 @@ class PlaylistTab(QTableWidget):
             if row == next_row:
                 # if there's a track playing, set start time from that
                 if current_row is not None:
-                        start_time = self._calculate_row_end_time(
+                    start_time = self._calculate_row_end_time(
-                            current_row, self.current_track_start_time)
+                        current_row, self.current_track_start_time)
                 else:
                     # No current track to base from, but don't change
                     # time if it's already set
@@ -901,7 +925,7 @@ class PlaylistTab(QTableWidget):
 
         DEBUG(f"_audacity({row})")
 
-        if row in self.get_notes_rows():
+        if row in self._get_notes_rows():
             return None
 
         with Session() as session:
@@ -931,7 +955,7 @@ class PlaylistTab(QTableWidget):
 
         DEBUG(f"_copy_path({row})")
 
-        if row in self.get_notes_rows():
+        if row in self._get_notes_rows():
             return None
 
         with Session() as session:
@@ -953,7 +977,7 @@ class PlaylistTab(QTableWidget):
         DEBUG(f"_cell_changed({row=}, {column=}, {new_text=}")
 
         with Session() as session:
-            if row in self.get_notes_rows():
+            if row in self._get_notes_rows():
                 # Save change to database
                 note: Notes = self._get_row_notes_object(row, session)
                 note.update(session, row, new_text)
@@ -1051,7 +1075,7 @@ class PlaylistTab(QTableWidget):
             set(item.row() for item in self.selectedItems())
         )
         rows_to_delete: List[int] = []
-        note_rows: Optional[List[int]] = self.get_notes_rows()
+        note_rows: Optional[List[int]] = self._get_notes_rows()
         row: int
         row_object: Union[Tracks, Notes]
 
@@ -1119,6 +1143,11 @@ class PlaylistTab(QTableWidget):
 
         return False
 
+    def _get_notes_rows(self) -> List[int]:
+        """Return rows marked as notes, or None"""
+
+        return self._meta_search(RowMeta.NOTE, one=False)
+
     def _find_next_track_row(self, starting_row: int = None) -> Optional[int]:
         """
         Find next track to play. If a starting row is given, start there;
@@ -1136,7 +1165,7 @@ class PlaylistTab(QTableWidget):
                 starting_row = current_row + 1
             else:
                 starting_row = 0
-        notes_rows = self.get_notes_rows()
+        notes_rows = self._get_notes_rows()
         played_rows = self._get_played_track_rows()
         for row in range(starting_row, self.rowCount()):
             if row in notes_rows or row in played_rows:
@@ -1219,7 +1248,7 @@ class PlaylistTab(QTableWidget):
         """Return rows marked as unplayed, or None"""
 
         unplayed_rows: Set[int] = set(self._meta_notset(RowMeta.PLAYED))
-        notes_rows: Set[int] = set(self.get_notes_rows())
+        notes_rows: Set[int] = set(self._get_notes_rows())
 
         return list(unplayed_rows - notes_rows)
 
@@ -1259,7 +1288,7 @@ class PlaylistTab(QTableWidget):
         txt: str
 
         with Session() as session:
-            if row in self.get_notes_rows():
+            if row in self._get_notes_rows():
                 note: Notes = self._get_row_notes_object(row, session)
                 txt = note.note
             else:
@@ -1507,7 +1536,7 @@ class PlaylistTab(QTableWidget):
             self.musicmuster.lblSumPlaytime.setText("")
             return
 
-        notes_rows: Set[int] = set(self.get_notes_rows())
+        notes_rows: Set[int] = set(self._get_notes_rows())
         ms: int = 0
         with Session() as session:
             for row in (sel_rows - notes_rows):
@@ -1567,7 +1596,7 @@ class PlaylistTab(QTableWidget):
         DEBUG(f"_set_next({row=})")
 
         # Check row is a track row
-        if row in self.get_notes_rows():
+        if row in self._get_notes_rows():
             return None
         track: Tracks = self._get_row_track_object(row, session)
         if not track:

test_playlists.py

@@ -101,7 +101,7 @@ def test_meta_all_clear(qtbot, session):
 
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == []
+    assert playlist_tab._get_notes_rows() == []
     assert playlist_tab._get_played_track_rows() == []
     assert len(playlist_tab._get_unreadable_track_rows()) == 3
 
@@ -131,13 +131,13 @@ def test_meta(qtbot, session):
     assert playlist_tab._get_played_track_rows() == []
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == []
+    assert playlist_tab._get_notes_rows() == []
 
     playlist_tab._set_played_row(0)
     assert playlist_tab._get_played_track_rows() == [0]
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == []
+    assert playlist_tab._get_notes_rows() == []
 
     # Add a note
     note_text = "my note"
@@ -148,44 +148,44 @@ def test_meta(qtbot, session):
     assert playlist_tab._get_played_track_rows() == [0]
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
     playlist_tab._set_next_track_row(1)
     assert playlist_tab._get_played_track_rows() == [0]
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() == 1
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
     playlist_tab._set_current_track_row(2)
     assert playlist_tab._get_played_track_rows() == [0]
     assert playlist_tab._get_current_track_row() == 2
     assert playlist_tab._get_next_track_row() == 1
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
     playlist_tab._clear_played_row_status(0)
     assert playlist_tab._get_played_track_rows() == []
     assert playlist_tab._get_current_track_row() == 2
     assert playlist_tab._get_next_track_row() == 1
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
     playlist_tab._meta_clear_next()
     assert playlist_tab._get_played_track_rows() == []
     assert playlist_tab._get_current_track_row() == 2
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
     playlist_tab._clear_current_track_row()
     assert playlist_tab._get_played_track_rows() == []
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
     # Test clearing again has no effect
     playlist_tab._clear_current_track_row()
     assert playlist_tab._get_played_track_rows() == []
     assert playlist_tab._get_current_track_row() is None
     assert playlist_tab._get_next_track_row() is None
-    assert playlist_tab.get_notes_rows() == [3]
+    assert playlist_tab._get_notes_rows() == [3]
 
 
 def test_clear_next(qtbot, session):