Clean up last played time in update_display

Fixes #106

app/models.py

@@ -153,6 +153,30 @@ class Notes(Base):
         session.query(Notes).filter_by(id=self.id).delete()
         session.flush()
 
+    @staticmethod
+    def max_used_row(session: Session, playlist_id: int) -> Optional[int]:
+        """
+        Return maximum notes row for passed playlist ID or None if not notes
+        """
+
+        last_row = session.query(func.max(Notes.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:
+            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"]:
         """Return note or None"""
@@ -265,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)
 
@@ -318,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:
         """
@@ -387,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, 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,10 +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)
-            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)
@@ -527,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
@@ -1077,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 ##########
 
@@ -314,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"""
@@ -398,21 +399,48 @@ 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.update_display(session)
 
@@ -582,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:
         """
@@ -856,8 +885,7 @@ class PlaylistTab(QTableWidget):
                 # This is a track row other than next or current
                 if row in played:
                     # Played today, so update last played column
-                    last_playedtime = Playdates.last_played(
+                    last_playedtime = track.lastplayed
-                        session, track.id)
                     last_played_str = get_relative_date(last_playedtime)
                     self.item(row, self.COL_LAST_PLAYED).setText(
                         last_played_str)
@@ -1114,6 +1142,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;
@@ -1173,11 +1206,6 @@ class PlaylistTab(QTableWidget):
         except ValueError:
             return None
 
-    def _get_notes_rows(self) -> List[int]:
-        """Return rows marked as notes, or None"""
-
-        return self._meta_search(RowMeta.NOTE, one=False)
-
     def _get_row_duration(self, row: int) -> int:
         """Return duration associated with this row"""