app/models.py

@@ -123,7 +123,7 @@ class NoteColours(Base):
         Return all records
         """
 
-        return session.scalars(select(cls)).all()
+        return session.execute(select(cls)).scalars().all()
 
     @staticmethod
     def get_colour(session: scoped_session, text: str) -> Optional[str]:
@@ -135,11 +135,12 @@ class NoteColours(Base):
             return None
 
         for rec in (
-            session.scalars(
+            session.execute(
                 select(NoteColours)
                 .filter(NoteColours.enabled.is_(True))
                 .order_by(NoteColours.order)
             )
+            .scalars()
             .all()
         ):
             if rec.is_regex:
@@ -203,11 +204,12 @@ class Playdates(Base):
         """Return a list of Playdates objects since passed time"""
 
         return (
-            session.scalars(
+            session.execute(
                 select(Playdates)
                 .where(Playdates.lastplayed >= since)
                 .order_by(Playdates.lastplayed)
             )
+            .scalars()
             .all()
         )
 
@@ -286,11 +288,12 @@ class Playlists(Base):
         """Returns a list of all playlists ordered by last use"""
 
         return (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .filter(cls.is_template.is_(False))
                 .order_by(cls.tab.desc(), cls.last_used.desc())
             )
+            .scalars()
             .all()
         )
 
@@ -299,9 +302,10 @@ class Playlists(Base):
         """Returns a list of all templates ordered by name"""
 
         return (
-            session.scalars(
+            session.execute(
                 select(cls).filter(cls.is_template.is_(True)).order_by(cls.name)
             )
+            .scalars()
             .all()
         )
 
@@ -310,7 +314,7 @@ class Playlists(Base):
         """Returns a list of all closed playlists ordered by last use"""
 
         return (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .filter(
                     cls.tab.is_(None),
@@ -319,6 +323,7 @@ class Playlists(Base):
                 )
                 .order_by(cls.last_used.desc())
             )
+            .scalars()
             .all()
         )
 
@@ -329,7 +334,8 @@ class Playlists(Base):
         """
 
         return (
-            session.scalars(select(cls).where(cls.tab.is_not(None)).order_by(cls.tab))
+            session.execute(select(cls).where(cls.tab.is_not(None)).order_by(cls.tab))
+            .scalars()
             .all()
         )
 
@@ -434,9 +440,10 @@ class PlaylistRows(Base):
         """Copy playlist entries"""
 
         src_rows = (
-            session.scalars(
+            session.execute(
                 select(PlaylistRows).filter(PlaylistRows.playlist_id == src_id)
             )
+            .scalars()
             .all()
         )
 
@@ -513,11 +520,12 @@ class PlaylistRows(Base):
         """
 
         plrs = (
-            session.scalars(
+            session.execute(
                 select(PlaylistRows)
                 .where(PlaylistRows.playlist_id == playlist_id)
                 .order_by(PlaylistRows.plr_rownum)
             )
+            .scalars()
             .all()
         )
 
@@ -537,11 +545,12 @@ class PlaylistRows(Base):
         """
 
         plrs = (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .where(cls.playlist_id == playlist_id, cls.id.in_(plr_ids))
                 .order_by(cls.plr_rownum)
             )
+            .scalars()
             .all()
         )
 
@@ -582,11 +591,12 @@ class PlaylistRows(Base):
         """
 
         plrs = (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .where(cls.playlist_id == playlist_id, cls.played.is_(True))
                 .order_by(cls.plr_rownum)
             )
+            .scalars()
             .all()
         )
 
@@ -613,7 +623,7 @@ class PlaylistRows(Base):
         if to_row is not None:
             query = query.where(cls.plr_rownum <= to_row)
 
-        plrs = session.scalars((query).order_by(cls.plr_rownum)).all()
+        plrs = session.execute((query).order_by(cls.plr_rownum)).scalars().all()
 
         return plrs
 
@@ -627,7 +637,7 @@ class PlaylistRows(Base):
         """
 
         plrs = (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .where(
                     cls.playlist_id == playlist_id,
@@ -636,6 +646,7 @@ class PlaylistRows(Base):
                 )
                 .order_by(cls.plr_rownum)
             )
+            .scalars()
             .all()
         )
 
@@ -691,7 +702,7 @@ class Settings(Base):
 
         result = {}
 
-        settings = session.scalars(select(cls)).all()
+        settings = session.execute(select(cls)).scalars().all()
         for setting in settings:
             result[setting.name] = setting
 
@@ -778,7 +789,7 @@ class Tracks(Base):
     def get_all(cls, session) -> List["Tracks"]:
         """Return a list of all tracks"""
 
-        return session.scalars(select(cls)).unique().all()
+        return session.execute(select(cls)).scalars().unique().all()
 
     @classmethod
     def get_by_path(cls, session: scoped_session, path: str) -> Optional["Tracks"]:
@@ -806,12 +817,13 @@ class Tracks(Base):
         """
 
         return (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .options(joinedload(Tracks.playdates))
                 .where(cls.artist.ilike(f"%{text}%"))
                 .order_by(cls.title)
             )
+            .scalars()
             .unique()
             .all()
         )
@@ -826,12 +838,13 @@ class Tracks(Base):
         https://docs.sqlalchemy.org/en/20/orm/queryguide/relationships.html#joined-eager-loading
         """
         return (
-            session.scalars(
+            session.execute(
                 select(cls)
                 .options(joinedload(Tracks.playdates))
                 .where(cls.title.like(f"{text}%"))
                 .order_by(cls.title)
             )
+            .scalars()
             .unique()
             .all()
         )

app/musicmuster.py

@@ -22,8 +22,6 @@ from typing import (
     Sequence,
 )
 
-from playlistmodel import PlaylistModel
-
 from sqlalchemy import text
 
 from PyQt6.QtCore import (
@@ -1040,11 +1038,9 @@ class Window(QMainWindow, Ui_MainWindow):
         """Show dialog box to enter header text and add to playlist"""
 
         try:
-            model = cast(PlaylistModel, self.visible_playlist_tab().model())
-            if model is None:
-                return
+            playlist_tab = self.visible_playlist_tab()
         except AttributeError:
-            # Just return if there's no visible playlist tab model
+            # Just return if there's no visible playlist tab
             return
 
         # Get header text
@@ -1054,10 +1050,9 @@ class Window(QMainWindow, Ui_MainWindow):
         dlg.resize(500, 100)
         ok = dlg.exec()
         if ok:
-            model.insert_header_row(
-                self.visible_playlist_tab().get_selected_row_number(),
-                dlg.textValue()
-            )
+            with Session() as session:
+                playlist_tab.insert_header(session, dlg.textValue())
+                playlist_tab.save_playlist(session)
 
     def insert_track(self) -> None:
         """Show dialog box to select and add track from database"""
@@ -1518,24 +1513,20 @@ class Window(QMainWindow, Ui_MainWindow):
 
         visible_playlist_id = self.visible_playlist_tab().playlist_id
         # Get row number of duplicate rows
-        sql = text(
-            f"""
+        sql = text(f"""
             SELECT max(plr_rownum)
             FROM playlist_rows
             WHERE playlist_id = {visible_playlist_id}
                 AND track_id != 0
             GROUP BY track_id
             HAVING count(id) > 1
-            """
-        )
+            """)
 
         with Session() as session:
             row_numbers = [int(a) for a in session.execute(sql).scalars().all()]
             if row_numbers:
                 self.visible_playlist_tab().select_rows(row_numbers)
-            self.statusbar.showMessage(
-                f"{len(row_numbers)} duplicate rows selected", 10000
-            )
+            self.statusbar.showMessage(f"{len(row_numbers)} duplicate rows selected", 10000)
 
     def select_next_row(self) -> None:
         """Select next or first row in playlist"""

app/playlistmodel.py

@@ -81,18 +81,6 @@ class PlaylistRowData:
 
 
 class PlaylistModel(QAbstractTableModel):
-    """
-    The Playlist Model
-
-    Update strategy: update the database and then refresh the cached copy (self.playlist_rows).
-    We do not try to edit playlist_rows directly. It would be too easy for a bug to get us
-    out of sync with the database, and if that wasn't immediately apparent then debugging it
-    would be hard.
-
-    refresh_row() will populate one row of playlist_rows from the database
-    refresh_data() will repopulate all of playlist_rows from the database
-    """
-
     def __init__(
         self, playlist_id: int, signals: "MusicMusterSignals", *args, **kwargs
     ):
@@ -105,7 +93,7 @@ class PlaylistModel(QAbstractTableModel):
         self.refresh_data()
 
     def __repr__(self) -> str:
-        return f"<PlaylistModel: playlist_id={self.playlist_id}, {self.rowCount() rows>"
+        return f"<PlaylistModel: playlist_id={self.playlist_id}>"
 
     def background_role(self, row: int, column: int, prd: PlaylistRowData) -> QBrush:
         """Return background setting"""
@@ -118,7 +106,6 @@ class PlaylistModel(QAbstractTableModel):
         if file_is_unreadable(prd.path):
             return QBrush(QColor(Config.COLOUR_UNREADABLE))
 
-        # Individual cell colouring
         if column == Col.START_GAP.value:
             if prd.start_gap and prd.start_gap >= Config.START_GAP_WARNING_THRESHOLD:
                 return QBrush(QColor(Config.COLOUR_LONG_START))
@@ -184,8 +171,13 @@ class PlaylistModel(QAbstractTableModel):
         Return text for display
         """
 
+        # Detect whether this is a header row
         if not prd.path:
-            # No track so this is a header row
+            header = prd.note
+        else:
+            header = ""
+
+        if header:
             if column == HEADER_NOTES_COLUMN:
                 self.signals.span_cells_signal.emit(
                     row, HEADER_NOTES_COLUMN, 1, self.columnCount() - 1
@@ -290,89 +282,53 @@ class PlaylistModel(QAbstractTableModel):
 
         return QVariant()
 
-    def insert_header_row(self, row_number: Optional[int], text: str) -> Optional[int]:
-        """
-        Insert a header row. Return row number or None if insertion failed.
-        """
-
-        with Session() as session:
-            prd = self._insert_row(session, row_number)
-            # Update playlist_rows
-            prd.note = text
-            # Get row from db and update
-            plr = session.get(PlaylistRows, prd.plrid)
-            if plr:
-                plr.note = text
-                self.refresh_row(session, plr.plr_rownum)
-                self.invalidate_row(plr.plr_rownum)
-                return plr.plr_rownum
-
-        return None
-
-    def _insert_row(
-        self, session: scoped_session, row_number: Optional[int]
-    ) -> PlaylistRowData:
+    def insert_row(self, session: scoped_session, row_number: int) -> int:
         """
         Make space for a row at row_number. If row_number is greater
         than length of list plus 1, or if row number is -1, put row at
         end of list.
 
-        Return the new PlaylistData structure
+        Return new row number that has an empty, valid entry in self.playlist_rows.
         """
 
-        modified_rows: List[int] = []
-
-        if row_number is None or row_number > len(self.playlist_rows):
-            new_row_number = len(self.playlist_rows)
+        if row_number > len(self.playlist_rows) or row_number == -1:
+            new_row_number = len(self.playlist_rows) + 1
         elif row_number < 0:
             raise ValueError(
                 f"playlistmodel.insert_row, invalid row number ({row_number})"
             )
         else:
             new_row_number = row_number
-        modified_rows.append(new_row_number)
 
         # Move rows below new row down
+        modified_rows: List[int] = []
         for i in reversed(range(new_row_number, len(self.playlist_rows))):
             self.playlist_rows[i + 1] = self.playlist_rows[i]
             self.playlist_rows[i + 1].plr_rownum += 1
             modified_rows.append(i + 1)
-        # If we are not adding to the end of the list, we need to clear
-        # out the existing recored at new_row_number (which we have
-        # already copied to its new location)
-        if new_row_number in self.playlist_rows:
-            del self.playlist_rows[new_row_number]
 
-        *** Problem here is that we haven't yet updated the database so when we insert a new row
-        with the PlaylistRows.__init__ call below, we'll get a duplicate. How best to keep
-        playlist_rows in step with database?
-
-        # Insert new row, possibly replace old row
-        plr = PlaylistRows(
+        # Replace old row
+        self.playlist_rows[new_row_number] = PlaylistRowData(
+            PlaylistRows(
                 session=session, playlist_id=self.playlist_id, row_number=new_row_number
             )
-        prd = PlaylistRowData(plr)
-        # Add row to playlist_rows
-        self.playlist_rows[new_row_number] = prd
-
-        return prd
-
-    def invalidate_row(self, modified_row: int) -> None:
-        """
-        Signal to view to refresh invlidated row
-        """
-
-        self.dataChanged.emit(
-            self.index(modified_row, 0), self.index(modified_row, self.columnCount())
         )
 
+        # Refresh rows
+        self.invalidate_rows(modified_rows)
+
+        return new_row_number
+
     def invalidate_rows(self, modified_rows: List[int]) -> None:
         """
         Signal to view to refresh invlidated rows
         """
 
         for modified_row in modified_rows:
-            self.invalidate_row(modified_row)
+            self.dataChanged.emit(
+                self.index(modified_row, 0),
+                self.index(modified_row, self.columnCount()),
+            )
 
     def move_rows(self, from_rows: List[int], to_row: int) -> None:
         """
@@ -423,13 +379,12 @@ class PlaylistModel(QAbstractTableModel):
                 # Fix in self.playlist_rows
                 self.playlist_rows[idx].plr_rownum = idx
                 # Update display
-                self.invalidate_row(idx)
+                self.invalidate_rows([idx])
 
     def refresh_data(self):
         """Populate dicts for data calls"""
 
         # Populate self.playlist_rows with playlist data
-        self.playlist_rows.clear()
         with Session() as session:
             for p in PlaylistRows.deep_rows(session, self.playlist_id):
                 self.playlist_rows[p.plr_rownum] = PlaylistRowData(p)
@@ -438,6 +393,7 @@ class PlaylistModel(QAbstractTableModel):
         """Populate dict for one row for data calls"""
 
         p = PlaylistRows.deep_row(session, self.playlist_id, row_number)
+        self.playlist_rows.clear()
         self.playlist_rows[p.plr_rownum] = PlaylistRowData(p)
 
     def rowCount(self, index: QModelIndex = QModelIndex()) -> int:

app/playlists.py

@@ -522,18 +522,6 @@ class PlaylistTab(QTableView):
         self.clearSelection()
         # self.setDragEnabled(False)
 
-    def get_selected_row_number(self) -> Optional[int]:
-        """
-        Return the selected row number or None if none selected.
-        """
-
-        sm = self.selectionModel()
-        if sm and sm.hasSelection():
-            index = sm.currentIndex()
-            if index.isValid():
-                return index.row()
-        return None
-
     # def get_new_row_number(self) -> int:
     #     """
     #     Return the selected row or the row count if no row selected

conftest.py

@@ -20,7 +20,7 @@ def db_engine():
 
 
 @pytest.fixture(scope="function")
-def session(db_engine):
+def Session(db_engine):
     connection = db_engine.connect()
     transaction = connection.begin()
     sm = sessionmaker(bind=connection)

test_playlistmodel.py

@@ -12,39 +12,41 @@ def create_model_with_playlist_rows(
     # Create a model
     model = playlistmodel.PlaylistModel(playlist.id, None)
     for row in range(rows):
-        plr = model._insert_row(session, row)
-        newrow = plr.plr_rownum
+        newrow = model.insert_row(session, row)
         model.playlist_rows[newrow].note = str(newrow)
 
     session.commit()
     return model
 
 
-def test_insert_row(monkeypatch, session):
-    monkeypatch.setattr(playlistmodel, "Session", session)
+def test_insert_row(monkeypatch, Session):
+    monkeypatch.setattr(playlistmodel, "Session", Session)
     # Create a playlist
-    playlist = Playlists(session, "test playlist")
+    with Session() as session:
+        playlist = Playlists(Session, "test playlist")
         # Create a model
         model = playlistmodel.PlaylistModel(playlist.id, None)
         assert model.rowCount() == 0
-    model._insert_row(session, 0)
+        model.insert_row(session, 0)
         assert model.rowCount() == 1
 
 
-def test_insert_high_row(monkeypatch, session):
-    monkeypatch.setattr(playlistmodel, "Session", session)
+def test_insert_high_row(monkeypatch, Session):
+    monkeypatch.setattr(playlistmodel, "Session", Session)
     # Create a playlist
-    playlist = Playlists(session, "test playlist")
+    with Session() as session:
+        playlist = Playlists(Session, "test playlist")
         # Create a model
         model = playlistmodel.PlaylistModel(playlist.id, None)
         assert model.rowCount() == 0
-    model._insert_row(session, 5)
+        model.insert_row(session, 5)
         assert model.rowCount() == 1
 
 
-def test_11_row_playlist(monkeypatch, session):
+def test_11_row_playlist(monkeypatch, Session):
     # Create multirow playlist
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         assert model.rowCount() == 11
         assert max(model.playlist_rows.keys()) == 10
@@ -53,9 +55,10 @@ def test_11_row_playlist(monkeypatch, session):
             assert model.playlist_rows[row].plr_rownum == row
 
 
-def test_move_rows_test2(monkeypatch, session):
+def test_move_rows_test2(monkeypatch, Session):
     # move row 3 to row 5
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([3], 5)
         # Check we have all rows and plr_rownums are correct
@@ -72,11 +75,12 @@ def test_move_rows_test2(monkeypatch, session):
                 assert model.playlist_rows[row].note == str(3)
 
 
-def test_move_rows_test3(monkeypatch, session):
+def test_move_rows_test3(monkeypatch, Session):
     # move row 4 to row 3
 
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
 
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([4], 3)
 
@@ -92,11 +96,12 @@ def test_move_rows_test3(monkeypatch, session):
                 assert model.playlist_rows[row].note == str(3)
 
 
-def test_move_rows_test4(monkeypatch, session):
+def test_move_rows_test4(monkeypatch, Session):
     # move row 4 to row 2
 
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
 
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([4], 2)
 
@@ -114,11 +119,12 @@ def test_move_rows_test4(monkeypatch, session):
                 assert model.playlist_rows[row].note == str(3)
 
 
-def test_move_rows_test5(monkeypatch, session):
+def test_move_rows_test5(monkeypatch, Session):
     # move rows [1, 4, 5, 10] → 8
 
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
 
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([1, 4, 5, 10], 8)
 
@@ -131,11 +137,12 @@ def test_move_rows_test5(monkeypatch, session):
         assert new_order == [0, 2, 3, 6, 7, 8, 9, 1, 4, 5, 10]
 
 
-def test_move_rows_test6(monkeypatch, session):
+def test_move_rows_test6(monkeypatch, Session):
     # move rows [3, 6] → 5
 
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
 
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([3, 6], 5)
 
@@ -148,11 +155,12 @@ def test_move_rows_test6(monkeypatch, session):
         assert new_order == [0, 1, 2, 4, 5, 3, 6, 7, 8, 9, 10]
 
 
-def test_move_rows_test7(monkeypatch, session):
+def test_move_rows_test7(monkeypatch, Session):
     # move rows [3, 5, 6] → 8
 
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
 
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([3, 5, 6], 8)
 
@@ -165,11 +173,12 @@ def test_move_rows_test7(monkeypatch, session):
         assert new_order == [0, 1, 2, 4, 7, 8, 9, 10, 3, 5, 6]
 
 
-def test_move_rows_test8(monkeypatch, session):
+def test_move_rows_test8(monkeypatch, Session):
     # move rows [7, 8, 10] → 5
 
-    monkeypatch.setattr(playlistmodel, "Session", session)
+    monkeypatch.setattr(playlistmodel, "Session", Session)
 
+    with Session() as session:
         model = create_model_with_playlist_rows(session, 11)
         model.move_rows([7, 8, 10], 5)
 
@@ -180,19 +189,3 @@ def test_move_rows_test8(monkeypatch, session):
             assert model.playlist_rows[row].plr_rownum == row
             new_order.append(int(model.playlist_rows[row].note))
         assert new_order == [0, 1, 2, 3, 4, 7, 8, 10, 5, 6, 9]
-
-
-def test_insert_header_row_end(monkeypatch, session):
-    # insert header row at end of playlist
-
-    monkeypatch.setattr(playlistmodel, "Session", session)
-    note_text = "test text"
-
-    model = create_model_with_playlist_rows(session, 11)
-    row_number = model.insert_header_row(None, note_text)
-    session.flush()
-    assert model.rowCount() == row_number + 1
-    prd = model.playlist_rows[row_number]
-    # Test against edit_role because display_role for headers is
-    # handled differently (sets up row span)
-    assert model.edit_role(row_number, playlistmodel.Col.NOTE.value, prd) == note_text