Rewrite of save_playlist (extensive)

app/model.py

@@ -13,7 +13,7 @@ from sqlalchemy import (
     String,
     func
 )
-from sqlalchemy.orm.exc import NoResultFound
+from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound
 from sqlalchemy.orm import relationship, sessionmaker
 
 from config import Config
@@ -63,7 +63,7 @@ class Notes(Base):
 
     @staticmethod
     def delete_note(id):
-        DEBUG(f"delete_note(id={id}") 
+        DEBUG(f"delete_note(id={id}")
 
         session.query(Notes).filter(Notes.id == id).delete()
         session.commit()
@@ -222,26 +222,51 @@ class PlaylistTracks(Base):
         return last_row + 1
 
     @staticmethod
-    def remove_track(playlist_id, track_id):
+    def add_track(playlist_id, track_id, row):
-        DEBUG(f"remove_track(playlist_id={playlist_id}, track_id={track_id})")
+        DEBUG(
-        session.query(PlaylistTracks).filter(
+            f"PlaylistTracks.add_track(playlist_id={playlist_id}, "
-            PlaylistTracks.playlist_id == playlist_id,
+            f"track_id={track_id}, row={row})"
-            PlaylistTracks.track_id == track_id
+        )
-        ).delete()
+        plt = PlaylistTracks()
+        plt.playlist_id = playlist_id,
+        plt.track_id = track_id,
+        plt.row = row
+        session.add(plt)
+        session.commit()
 
     @staticmethod
-    def update_track_row(playlist_id, track_id, old_row, new_row):
+    def remove_track(playlist_id, row):
         DEBUG(
-            f"update_track_row(playlist_id={playlist_id}, "
+            f"PlaylistTracks.remove_track(playlist_id={playlist_id}, "
-            f"track_id={track_id}, old_row={old_row}, new_row={new_row})"
+            f"row={row})"
+        )
+        session.query(PlaylistTracks).filter(
+            PlaylistTracks.playlist_id == playlist_id,
+            PlaylistTracks.row == row
+        ).delete()
+        session.commit()
+
+    @staticmethod
+    def update_row_track(playlist_id, row, track_id):
+        DEBUG(
+            f"PlaylistTracks.update_track_row(playlist_id={playlist_id}, "
+            f"row={row}, track_id={track_id})"
         )
 
-        plt = session.query(PlaylistTracks).filter(
+        try:
-            PlaylistTracks.playlist_id == playlist_id,
+            plt = session.query(PlaylistTracks).filter(
-            PlaylistTracks.track_id == track_id,
+                PlaylistTracks.playlist_id == playlist_id,
-            PlaylistTracks.row == old_row
+                PlaylistTracks.row == row
-        ).one()
+            ).one()
-        plt.row = new_row
+        except MultipleResultsFound:
+            ERROR(
+                f"Multiple rows matched in query: "
+                f"PlaylistTracks.playlist_id == {playlist_id}, "
+                f"PlaylistTracks.row == {row}"
+            )
+            return
+
+        plt.track_id = track_id
         session.commit()
 
 

app/playlists.py

@@ -125,6 +125,7 @@ class Playlist(QTableWidget):
             f"Moved row(s) {rows} to become row {drop_row}"
         )
 
+        self._save_playlist()
         self._repaint()
 
     def eventFilter(self, source, event):
@@ -172,12 +173,13 @@ class Playlist(QTableWidget):
         Notes object.
         """
 
+        DEBUG(f"add_to_playlist(data={data}, repaint={repaint}, row={row}")
         if not row:
             if self.selectionModel().hasSelection():
                 row = self.currentRow()
             else:
                 row = self.rowCount()
-        DEBUG(f"add_to_playlist(data={data}): row={row}")
+            DEBUG(f"add_to_playlist: row set to {row}")
 
         self.insertRow(row)
 
@@ -196,8 +198,7 @@ class Playlist(QTableWidget):
             self.setItem(row, self.COL_DURATION, item)
             item = QTableWidgetItem(track.path)
             self.setItem(row, self.COL_PATH, item)
-        else:
+        elif isinstance(data, Notes):
-            # This is a note
             DEBUG(f"add_to_playlist: note.id={data.id}")
             note = data
 
@@ -220,7 +221,7 @@ class Playlist(QTableWidget):
             self.setSpan(row, self.COL_NOTE, self.NOTE_ROW_SPAN,
                          self.NOTE_COL_SPAN)
 
-            # Add start times or empty items, otherwise background
+            # Add start times or empty items as background
             # colour won't be set for columns without items
             self._set_row_time(row, start_time)
             item = QTableWidgetItem()
@@ -228,10 +229,15 @@ class Playlist(QTableWidget):
 
             self._meta_set_note(row)
 
+        else:
+            ERROR(f"Unknown data passed to add_to_playlist({data})")
+            return
+
         # Scroll to new row
         self.scrollToItem(titleitem, QAbstractItemView.PositionAtCenter)
 
         if repaint:
+            self._save_playlist()
             self._repaint(clear_selection=False)
 
     def create_playlist(self, name):
@@ -241,6 +247,11 @@ class Playlist(QTableWidget):
         self.load_playlist(new_id)
 
     def fade(self):
+        "Fade currently playing track"
+
+        if not self.current_track:
+            return
+
         self.previous_track = self.current_track
         self.previous_track_position = self.music.fade()
 
@@ -302,7 +313,9 @@ class Playlist(QTableWidget):
         """
         Load tracks and notes from playlist id.
 
-        Set first track as next track to play.
+        If this is not the first playlist loaded, we may already have
+        a next track set in which case don't change it. Otherwise, set
+        the first non-notes row as next track to play.
         """
 
         DEBUG(f"load_playlist(plid={plid})")
@@ -330,9 +343,7 @@ class Playlist(QTableWidget):
         for item in sorted(data, key=lambda x: x[0]):
             self.add_to_playlist(item[1], repaint=False)
 
-        # If this is not the first playlist loaded, we may already have
+        # Set next track if we don't have one already set
-        # a next track set in which case don't change it. Otherwise, set
-        # the first non-notes row as next track to play.
         if not self.next_track:
             notes_rows = self._meta_get_notes()
             for row in range(self.rowCount()):
@@ -340,6 +351,8 @@ class Playlist(QTableWidget):
                     continue
                 self._cue_next_track(row)
                 break
+        else:
+            self._repaint()
 
         # Scroll to top
         scroll_to = self.item(0, self.COL_INDEX)
@@ -405,9 +418,8 @@ class Playlist(QTableWidget):
         for row in range(start, self.rowCount()):
             if row in notes_rows:
                 continue
-            if self.item(row, self.COL_INDEX):
+            self._cue_next_track(row)
-                self._cue_next_track(row)
+            break
-                break
 
         # Tell database to record it as played
         self.current_track.update_lastplayed()
@@ -468,8 +480,7 @@ class Playlist(QTableWidget):
             DEBUG("_calculate_next_start_time() called with row=None")
             return None
 
-        duration = Tracks.get_duration(
+        duration = Tracks.get_duration(self._get_row_id(row))
-            int(self.item(row, self.COL_INDEX).text()))
         return start + timedelta(milliseconds=duration)
 
     def _can_read_track(self, track):
@@ -481,36 +492,40 @@ class Playlist(QTableWidget):
 
         self.menu.exec_(self.mapToGlobal(pos))
 
-    def _cue_next_track(self, next_row):
+    def _cue_next_track(self, row):
         """
         Set the passed row as the next track to play
         """
 
-        if next_row is not None:
+        track_id = self._get_row_id(row)
-            self._meta_set_next(next_row)
+        if not track_id:
-            track_id = int(self.item(next_row, self.COL_INDEX).text())
+            return
-            if not self.next_track or self.next_track.id != track_id:
+
-                self.next_track = Tracks.get_track(track_id)
+        self._meta_set_next(row)
-                # Check we can read it
+
-                if not self._can_read_track(self.next_track):
+        if not self.next_track or self.next_track.id != track_id:
-                    self.parent().parent().show_warning(
+            self.next_track = Tracks.get_track(track_id)
-                        "Can't read next track",
+            # Check we can read it
-                        self.next_track.path)
+            if not self._can_read_track(self.next_track):
-        else:
+                self.parent().parent().show_warning(
-            self.next_track = None
+                    "Can't read next track",
+                    self.next_track.path)
 
-        # Update display
         self._repaint()
 
     def _delete_row(self, row):
         "Delete row"
 
+        DEBUG(f"playlist._delete_row({row})")
+
         if row == self._meta_get_current():
             # TODO
             DEBUG("playlist._delete_row(): Can't delete playing track")
+            return
         elif row == self._meta_get_next():
             # TODO
             DEBUG("playlist._delete_row(): Can't delete next track")
+            return
 
         else:
             title = self.item(row, self.COL_TITLE).text()
@@ -522,12 +537,11 @@ class Playlist(QTableWidget):
             msg.setDefaultButton(QMessageBox.Cancel)
             msg.setWindowTitle("Delete row")
             if msg.exec() == QMessageBox.Yes:
-                DEBUG(f"playlist._delete_row(): Delete row {row}")
+                id = self._get_row_id(row)
-                id = int(self.item(row, self.COL_INDEX).text())
                 if row in self._meta_get_notes():
                     Notes.delete_note(id)
                 else:
-                    PlaylistTracks.remove_track(self.playlist_id, id)
+                    PlaylistTracks.remove_track(self.playlist_id, row)
                 self.removeRow(row)
 
         self._repaint()
@@ -540,6 +554,21 @@ class Playlist(QTableWidget):
         return (index.row() + 1 if self._is_below(event.pos(), index)
                 else index.row())
 
+    def _get_row_id(self, row):
+        "Return item id as integer from passed row"
+
+        if self.item(row, self.COL_INDEX):
+            try:
+                return int(self.item(row, self.COL_INDEX).text())
+            except TypeError:
+                ERROR(
+                    f"_get_row_id({row}): error retrieving row id "
+                    f"({self.item(row, self.COL_INDEX).text()})"
+                )
+        else:
+            ERROR(f"(_get_row_id({row}): no COL_INDEX data in row")
+            return None
+
     def _get_row_time(self, row):
         try:
             if self.item(row, self.COL_START_TIME):
@@ -669,6 +698,8 @@ class Playlist(QTableWidget):
         be played and return True. Otherwise return False.
         """
 
+        DEBUG(f"_set_next({row})")
+
         if row in self._meta_get_notes():
             return False
 
@@ -683,8 +714,6 @@ class Playlist(QTableWidget):
 
         DEBUG(f"_repaint(clear_selection={clear_selection})")
 
-        self._save_playlist()
-
         if clear_selection:
             self.clearSelection()
         current = self._meta_get_current()
@@ -750,8 +779,7 @@ class Playlist(QTableWidget):
                     colour = QColor(Config.COLOUR_EVEN_PLAYLIST)
                 self._set_row_colour(row, colour)
 
-                if (int(self.item(row, self.COL_INDEX).text()) in
+                if self._get_row_id(row) in self.played_tracks:
-                        self.played_tracks):
                     self._set_row_not_bold(row)
                 else:
                     # Set time only if we haven't played it yet
@@ -767,58 +795,36 @@ class Playlist(QTableWidget):
 
     def _save_playlist(self):
         """
-        Save playlist to database. Add missing notes/tracks; remove any that
+        Save playlist to database. We do this by correcting differences
-        are in database but not playlist. Correct row number in database if
+        between the on-screen (definitive) playlist and that in the
-        necessary.
+        database.
+
+        We treat the notes rows and the tracks rows differently. Notes must
+        appear only once in only one playlist. Tracks can appear multiple
+        times in one playlist and in multiple playlists.
         """
 
-        note_rows = self._meta_get_notes()
         playlist = Playlists.get_playlist_by_id(self.playlist_id)
 
-        # Create dictionaries indexed by track/note id
+        # Notes first
+        # Create dictionaries indexed by note_id
         playlist_notes = {}
-        playlist_tracks = {}
         database_notes = {}
-        database_tracks = {}
+        notes_rows = self._meta_get_notes()
 
         # Playlist
-        for row in range(self.rowCount()):
+        for row in notes_rows:
-            # Get id of item
+            note_id = self._get_row_id(row)
-            if self.item(row, self.COL_INDEX):
+            if not note_id:
-                id = int(self.item(row, self.COL_INDEX).text())
-            else:
                 DEBUG(f"(_save_playlist(): no COL_INDEX data in row {row}")
                 continue
-            if row in note_rows:
+            playlist_notes[note_id] = row
-                playlist_notes[id] = row
-            else:
-                playlist_tracks[id] = row
 
         # Database
         for note in playlist.notes:
             database_notes[note.id] = note.row
-        for track in playlist.tracks:
-            database_tracks[track.track_id] = track.row
 
-        # Notes to remove from playlist in database
+        # Notes to add to database
-        for note_id in set(database_notes.keys()) - set(playlist_notes.keys()):
-            DEBUG(
-                f"_save_playlist(): Delete note.id={id} "
-                f"from playlist {playlist} in database"
-            )
-            Notes.delete_note(note_id)
-
-        # Tracks to remove from playlist in database
-        for track_id in (
-                set(database_tracks.keys()) - set(playlist_tracks.keys())
-        ):
-            DEBUG(
-                f"_save_playlist(): Delete track.id={track_id} "
-                f"from playlist {playlist} in database"
-            )
-            PlaylistTracks.remove_track(playlist.id, track_id)
-
-        # Notes to add to playlist database
         # This should never be needed as notes are added to a specific
         # playlist upon creation
         for note_id in set(playlist_notes.keys()) - set(database_notes.keys()):
@@ -827,10 +833,10 @@ class Playlist(QTableWidget):
                 f"missing from playlist {playlist} in database"
             )
 
-        # Notes to remove from playlist database
+        # Notes to remove from database
         for note_id in set(database_notes.keys()) - set(playlist_notes.keys()):
             DEBUG(
-                f"_save_playlist(): Remove note.id={note_id} "
+                f"_save_playlist(): Delete note note_id={note_id} "
                 f"from playlist {playlist} in database"
             )
             Notes.delete_note(note_id)
@@ -839,28 +845,58 @@ class Playlist(QTableWidget):
         for note_id in set(playlist_notes.keys()) & set(database_notes.keys()):
             if playlist_notes[note_id] != database_notes[note_id]:
                 DEBUG(
-                    f"_save_playlist(): Set database note.id {note_id} "
+                    f"_save_playlist(): Update database note.id {note_id} "
-                    f"row={playlist_notes[note_id]} "
+                    f"from row={database_notes[note_id]} to "
-                    f"in playlist {playlist} in database"
+                    f"row={playlist_notes[note_id]}"
                 )
                 Notes.update_note(note_id, playlist_notes[note_id])
 
-        # Track rows to update in playlist database
+        # Now check tracks
-        for track_id in (
+        # Create dictionaries indexed by row
+        playlist_tracks = {}
+        database_tracks = {}
+
+        # Playlist
+        for row in range(self.rowCount()):
+            if row in notes_rows:
+                continue
+            playlist_tracks[row] = self._get_row_id(row)
+
+        # Database
+        for track in playlist.tracks:
+            database_tracks[track.row] = track.track_id
+
+        # Tracks rows to add to database
+        for row in (
+                set(set(playlist_tracks.keys()) - set(database_tracks.keys()))
+        ):
+            DEBUG(f"_save_playlist(): row {row} missing from database")
+            PlaylistTracks.add_track(self.playlist_id, playlist_tracks[row],
+                                     row)
+
+        # Track rows to remove from database
+        for row in (
+                set(database_tracks.keys()) - set(playlist_tracks.keys())
+        ):
+            track = database_tracks[row]
+            DEBUG(
+                f"_save_playlist(): row {row} in database not playlist "
+                f"(track={track})"
+            )
+            PlaylistTracks.remove_track(playlist.id, row)
+
+        # Track rows to update in database
+        for row in (
                 set(playlist_tracks.keys()) & set(database_tracks.keys())
         ):
-            if playlist_tracks[track_id] != database_tracks[track_id]:
+            if playlist_tracks[row] != database_tracks[row]:
                 DEBUG(
-                    f"_save_playlist(): Set database track.id {track_id} "
+                    "_save_playlist(): Update row={row} in database for "
-                    f"row={playlist_tracks[track_id]} "
+                    f"playlist {playlist} from track={database_tracks[row]} "
-                    f"in playlist {playlist} in database"
+                    f"to track={playlist_tracks[row]}"
-                )
-                PlaylistTracks.update_track_row(
-                    playlist_id=self.playlist_id,
-                    track_id=track_id,
-                    old_row=database_tracks[track_id],
-                    new_row=playlist_tracks[track_id]
                 )
+                PlaylistTracks.update_row_track(
+                    self.playlist_id, row, playlist_tracks[row])
 
     def _set_row_bold(self, row, bold=True):
         boldfont = QFont()