WIP V3: fixup row insertion/deletion

All row insertions and deletions are now wrapped in beginRemoveRows /
endRemoveRows (and similar for insertions).

app/models.py

@@ -489,17 +489,17 @@ class PlaylistRows(Base):
         session.flush()
 
     @staticmethod
-    def delete_rows(
+    def delete_row(
-        session: scoped_session, playlist_id: int, row_numbers: List[int]
+        session: scoped_session, playlist_id: int, row_number: int
     ) -> None:
         """
-        Delete passed rows in given playlist.
+        Delete passed row in given playlist.
         """
 
         session.execute(
             delete(PlaylistRows).where(
                 PlaylistRows.playlist_id == playlist_id,
-                PlaylistRows.plr_rownum.in_(row_numbers),
+                PlaylistRows.plr_rownum == row_number,
             )
         )
 

app/musicmuster.py

@@ -847,8 +847,9 @@ class Window(QMainWindow, Ui_MainWindow):
         dlg.resize(500, 100)
         ok = dlg.exec()
         if ok:
-            model.insert_header_row(
+            model.insert_row(
-                self.active_tab().get_selected_row_number(), dlg.textValue()
+                proposed_row_number=self.active_tab().get_selected_row_number(),
+                note=dlg.textValue(),
             )
 
     def insert_track(self) -> None:

app/playlistmodel.py

@@ -147,15 +147,7 @@ class PlaylistModel(QAbstractTableModel):
         if playlist_id != self.playlist_id:
             return
 
-        # Insert track if we have one
+        self.insert_row(proposed_row_number=new_row_number, track_id=track_id, note=note)
-        if track_id:
-            self.insert_track_row(new_row_number, track_id, note)
-        # If we only have a note, add as a header row
-        elif note:
-            self.insert_header_row(new_row_number, note)
-        else:
-            # No track, no note, no point
-            return
 
     def add_track_to_header(
         self,
@@ -345,10 +337,17 @@ class PlaylistModel(QAbstractTableModel):
     def delete_rows(self, row_numbers: List[int]) -> None:
         """
         Delete passed rows from model
+
+        Need to delete them in contiguous groups wrapped in beginRemoveRows / endRemoveRows
+        calls. To keep it simple, if inefficient, delete rows one by one.
         """
 
         with Session() as session:
-            PlaylistRows.delete_rows(session, self.playlist_id, row_numbers)
+            for row_number in row_numbers:
+                super().beginRemoveRows(QModelIndex(), row_number, row_number)
+                PlaylistRows.delete_row(session, self.playlist_id, row_number)
+                super().endRemoveRows()
+
             PlaylistRows.fixup_rownumbers(session, self.playlist_id)
             self.refresh_data(session)
         self.update_track_times()
@@ -448,6 +447,25 @@ class PlaylistModel(QAbstractTableModel):
 
         return QVariant(boldfont)
 
+    def _get_new_row_number(self, proposed_row_number: Optional[int]) -> int:
+        """
+        Sanitises proposed new row number.
+
+        If proposed_row_number given, ensure it is valid.
+        If not given, return row number to add to end of model.
+        """
+
+        if proposed_row_number is None or proposed_row_number > len(self.playlist_rows):
+            # We are adding to the end of the list
+            new_row_number = len(self.playlist_rows)
+        elif proposed_row_number < 0:
+            # Add to start of list
+            new_row_number = 0
+        else:
+            new_row_number = proposed_row_number
+
+        return new_row_number
+
     def get_row_track_path(self, row_number: int) -> str:
         """
         Return path of track associated with row or empty string if no track associated
@@ -600,66 +618,33 @@ class PlaylistModel(QAbstractTableModel):
 
         return self.playlist_rows[row_number].played
 
-    def insert_header_row(self, row_number: Optional[int], text: str) -> None:
+    def insert_row(
-        """
+        self,
-        Insert a header row.
+        proposed_row_number: Optional[int],
-        """
+        track_id: Optional[int] = None,
-
+        note: Optional[str] = None,
-        with Session() as session:
-            plr = self._insert_row(session, row_number)
-            # Update the PlaylistRows object
-            plr.note = text
-            # Repopulate self.playlist_rows
-            self.refresh_data(session)
-            # Update the display from the new row onwards
-            self.invalidate_rows(list(range(plr.plr_rownum, len(self.playlist_rows))))
-
-    def _insert_row(
-        self, session: scoped_session, row_number: Optional[int]
     ) -> PlaylistRows:
-        """
-        Insert a row in the database.
-
-        If row_number is greater than length of list plus 1, or if row
-        number is None, put row at end of list.
-
-        Move existing rows to make space if ncessary.
-
-        Return the new PlaylistRows object.
-        """
-
-        if row_number is None or row_number > len(self.playlist_rows):
-            # We are adding to the end of the list so we can optimise
-            new_row_number = len(self.playlist_rows)
-            return PlaylistRows(session, self.playlist_id, new_row_number)
-        elif row_number < 0:
-            raise ValueError(
-                f"playlistmodel._insert_row, invalid row number ({row_number})"
-            )
-        else:
-            new_row_number = row_number
-
-        # Insert the new row and return it
-        return PlaylistRows.insert_row(session, self.playlist_id, new_row_number)
-
-    def insert_track_row(
-        self, row_number: Optional[int], track_id: int, text: Optional[str]
-    ) -> None:
         """
         Insert a track row.
         """
 
+        new_row_number = self._get_new_row_number(proposed_row_number)
+
         with Session() as session:
-            plr = self._insert_row(session, row_number)
+            super().beginInsertRows(QModelIndex(), new_row_number, new_row_number)
-            # Update the PlaylistRows object
+            plr = PlaylistRows.insert_row(session, self.playlist_id, new_row_number)
+
             plr.track_id = track_id
-            if text:
+            if note:
-                plr.note = text
+                plr.note = note
-            # Repopulate self.playlist_rows
+
             self.refresh_data(session)
-            # Update the display from the new row onwards
+            super().endInsertRows()
+
             self.invalidate_rows(list(range(plr.plr_rownum, len(self.playlist_rows))))
 
+        return plr
+
     def invalidate_row(self, modified_row: int) -> None:
         """
         Signal to view to refresh invlidated row

app/playlists.py

@@ -205,7 +205,26 @@ class PlaylistTab(QTableView):
         self.setModel(PlaylistModel(playlist_id))
         self._set_column_widths()
 
-    # ########## Events other than cell editing ##########
+    def closeEditor(
+        self, editor: QWidget | None, hint: QAbstractItemDelegate.EndEditHint
+    ) -> None:
+        """
+        Override closeEditor to enable play controls and update display.
+        """
+
+        self.musicmuster.enable_play_next_controls()
+        self.musicmuster.actionSetNext.setEnabled(True)
+        self.musicmuster.action_Clear_selection.setEnabled(True)
+
+        super(PlaylistTab, self).closeEditor(editor, hint)
+
+        # Optimise row heights after increasing row height for editing
+        self.resizeRowsToContents()
+
+        # Update start times in case a start time in a note has been
+        # edited
+        model = cast(PlaylistModel, self.model())
+        model.update_track_times()
 
     def dropEvent(self, event):
         if event.source() is not self or (
@@ -230,6 +249,25 @@ class PlaylistTab(QTableView):
 
             event.accept()
 
+    def edit(
+        self,
+        index: QModelIndex,
+        trigger: QAbstractItemView.EditTrigger,
+        event: Optional[QEvent],
+    ) -> bool:
+        """
+        Override PySide2.QAbstractItemView.edit to catch when editing starts
+
+        Editing only ever starts with a double click on a cell
+        """
+
+        # 'result' will only be true on double-click
+        result = super().edit(index, trigger, event)
+        if result:
+            self.musicmuster.disable_play_next_controls()
+
+        return result
+
     def _add_context_menu(
         self,
         text: str,

test_playlistmodel.py

@@ -2,6 +2,8 @@ from app.models import (
     Playlists,
     Tracks,
 )
+from PyQt6.QtCore import Qt
+
 from app.helpers import get_file_metadata
 from app import playlistmodel
 from dbconfig import scoped_session
@@ -25,7 +27,7 @@ def create_model_with_tracks(session: scoped_session) -> "playlistmodel.Playlist
         track_path = test_tracks[row % len(test_tracks)]
         metadata = get_file_metadata(track_path)
         track = Tracks(session, **metadata)
-        model.insert_track_row(row, track.id, f"{row=}")
+        model.insert_row(proposed_row_number=row, track_id=track.id, note=f"{row=}")
 
     session.commit()
     return model
@@ -38,10 +40,8 @@ def create_model_with_playlist_rows(
     # Create a model
     model = playlistmodel.PlaylistModel(playlist.id)
     for row in range(rows):
-        plr = model._insert_row(session, row)
+        plr = model.insert_row(proposed_row_number=row, note=str(row))
-        newrow = plr.plr_rownum
+        model.playlist_rows[plr.plr_rownum] = playlistmodel.PlaylistRowData(plr)
-        plr.note = str(newrow)
-        model.playlist_rows[newrow] = playlistmodel.PlaylistRowData(plr)
 
     session.commit()
     return model
@@ -195,7 +195,7 @@ def test_insert_header_row_end(monkeypatch, session):
     initial_row_count = 11
 
     model = create_model_with_playlist_rows(session, initial_row_count)
-    model.insert_header_row(None, note_text)
+    model.insert_row(proposed_row_number=None, note=note_text)
     assert model.rowCount() == initial_row_count + 1
     prd = model.playlist_rows[model.rowCount() - 1]
     # Test against edit_role because display_role for headers is
@@ -215,7 +215,7 @@ def test_insert_header_row_middle(monkeypatch, session):
     insert_row = 6
 
     model = create_model_with_playlist_rows(session, initial_row_count)
-    model.insert_header_row(insert_row, note_text)
+    model.insert_row(proposed_row_number=insert_row, note=note_text)
     assert model.rowCount() == initial_row_count + 1
     prd = model.playlist_rows[insert_row]
     # Test against edit_role because display_role for headers is
@@ -239,14 +239,35 @@ def test_timing_one_track(monkeypatch, session):
     monkeypatch.setattr(playlistmodel, "Session", session)
     model = create_model_with_tracks(session)
 
-    model.insert_header_row(START_ROW, "start+")
+    model.insert_row(proposed_row_number=START_ROW, note="start+")
-    model.insert_header_row(END_ROW, "-")
+    model.insert_row(proposed_row_number=END_ROW, note="-")
 
     prd = model.playlist_rows[START_ROW]
     qv_value = model.display_role(START_ROW, playlistmodel.HEADER_NOTES_COLUMN, prd)
     assert qv_value.value() == "start [1 tracks, 4:23 unplayed]"
 
 
+def test_insert_track_new_playlist(monkeypatch, session):
+    # insert a track into a new playlist
+
+    monkeypatch.setattr(playlistmodel, "Session", session)
+
+    playlist = Playlists(session, "test playlist")
+    # Create a model
+    model = playlistmodel.PlaylistModel(playlist.id)
+
+    track_path = test_tracks[0]
+    metadata = get_file_metadata(track_path)
+    track = Tracks(session, **metadata)
+    model.insert_row(proposed_row_number=0, track_id=track.id)
+
+    prd = model.playlist_rows[model.rowCount() - 1]
+    assert (
+        model.edit_role(model.rowCount() - 1, playlistmodel.Col.TITLE.value, prd)
+        == metadata["title"]
+    )
+
+
 # def test_edit_header(monkeypatch, session): # edit header row in middle of playlist
 
 #     monkeypatch.setattr(playlistmodel, "Session", session)