WIP V3: tests for moving rows between playlists pass

app/classes.py

@@ -81,7 +81,9 @@ class MusicMusterSignals(QObject):
 
     add_track_to_header_signal = pyqtSignal(int, int, int)
     add_track_to_playlist_signal = pyqtSignal(int, int, int, str)
+    begin_reset_model_signal = pyqtSignal(int)
     enable_escape_signal = pyqtSignal(bool)
+    end_reset_model_signal = pyqtSignal(int)
     next_track_changed_signal = pyqtSignal()
     search_songfacts_signal = pyqtSignal(str)
     search_wikipedia_signal = pyqtSignal(str)

app/models.py

@@ -6,6 +6,7 @@ from config import Config
 from dbconfig import scoped_session
 
 from datetime import datetime
+from pprint import pprint
 from typing import List, Optional, Sequence
 
 from sqlalchemy.ext.associationproxy import association_proxy

app/musicmuster.py

@@ -610,10 +610,9 @@ class Window(QMainWindow, Ui_MainWindow):
         Cut rows ready for pasting.
         """
 
-        with Session() as session:
         # Save the selected PlaylistRows items ready for a later
         # paste
-            self.selected_plrs = self.active_tab().get_selected_playlistrows(session)
+        self.selected_rows = self.active_tab().get_selected_rows()
 
     def debug(self):
         """Invoke debugger"""

app/playlistmodel.py

@@ -2,6 +2,7 @@ from dataclasses import dataclass
 from datetime import datetime, timedelta
 from enum import auto, Enum
 from operator import attrgetter
+from pprint import pprint
 from typing import List, Optional
 
 from PyQt6.QtCore import (
@@ -124,6 +125,8 @@ class PlaylistModel(QAbstractTableModel):
 
         self.signals.add_track_to_header_signal.connect(self.add_track_to_header)
         self.signals.add_track_to_playlist_signal.connect(self.add_track)
+        self.signals.begin_reset_model_signal.connect(self.begin_reset_model)
+        self.signals.end_reset_model_signal.connect(self.end_reset_model)
 
         with Session() as session:
             # Ensure row numbers in playlist are contiguous
@@ -230,6 +233,15 @@ class PlaylistModel(QAbstractTableModel):
 
         return QBrush()
 
+    def begin_reset_model(self, playlist_id: int) -> None:
+        """
+        Reset model if playlist_id is ours
+        """
+
+        if playlist_id != self.playlist_id:
+            return
+        super().beginResetModel()
+
     def columnCount(self, parent: QModelIndex = QModelIndex()) -> int:
         """Standard function for view"""
 
@@ -403,6 +415,15 @@ class PlaylistModel(QAbstractTableModel):
 
         return QVariant()
 
+    def end_reset_model(self, playlist_id: int) -> None:
+        """
+        End model reset if this is our playlist
+        """
+
+        if playlist_id != self.playlist_id:
+            return
+        super().endResetModel()
+
     def get_duplicate_rows(self) -> List[int]:
         """
         Return a list of duplicate rows. If track appears in rows 2, 3 and 4, return [3, 4]
@@ -776,6 +797,53 @@ class PlaylistModel(QAbstractTableModel):
         self.update_track_times()
         self.invalidate_rows(list(row_map.keys()))
 
+    def move_rows_between_playlists(
+        self, from_rows: List[int], to_row_number: int, to_playlist_id: int
+    ) -> None:
+        """
+        Move the playlist rows given to to_row and below of to_playlist.
+        """
+
+        # Row removal must be wrapped in beginRemoveRows ..
+        # endRemoveRows and the row range must be contiguous. Process
+        # the highest rows first so the lower row numbers are unchanged
+        row_groups = self._reversed_contiguous_row_groups(from_rows)
+        next_to_row = to_row_number
+        with Session() as session:
+            # Make room in destination playlist
+            max_destination_row_number = PlaylistRows.get_last_used_row(
+                session, to_playlist_id
+            )
+            if (
+                max_destination_row_number
+                and to_row_number <= max_destination_row_number
+            ):
+                PlaylistRows.move_rows_down(
+                    session, to_playlist_id, to_row_number, len(from_rows)
+                )
+
+            # Prepare destination playlist for a reset
+            self.signals.begin_reset_model_signal.emit(to_playlist_id)
+            for row_group in row_groups:
+                super().beginRemoveRows(QModelIndex(), min(row_group), max(row_group))
+                for plr in PlaylistRows.plrids_to_plrs(
+                    session,
+                    self.playlist_id,
+                    [self.playlist_rows[a].plrid for a in row_group],
+                ):
+                    plr.playlist_id = to_playlist_id
+                    plr.plr_rownum = next_to_row
+                    next_to_row += 1
+                self.refresh_data(session)
+                super().endRemoveRows()
+            self.signals.end_reset_model_signal.emit(to_playlist_id)
+            # We need to remove gaps in row numbers after tracks have
+            # moved.
+            PlaylistRows.fixup_rownumbers(session, self.playlist_id)
+            self.refresh_data(session)
+
+        self.update_track_times()
+
     def open_in_audacity(self, row_number: int) -> None:
         """
         Open track at passed row number in Audacity
@@ -851,6 +919,34 @@ class PlaylistModel(QAbstractTableModel):
                 self.refresh_row(session, row_number)
                 self.invalidate_row(row_number)
 
+    def _reversed_contiguous_row_groups(
+        self, row_numbers: List[int]
+    ) -> List[List[int]]:
+        """
+        Take the list of row numbers and split into groups of contiguous rows. Return as a list
+        of lists with the highest row numbers first.
+
+        Example:
+            input [2, 3, 4, 5, 7, 9, 10, 13, 17, 20, 21]
+            return: [[20, 21], [17], [13], [9, 10], [7], [2, 3, 4, 5]]
+        """
+
+        result: List[List[int]] = []
+        temp: List[int] = []
+        last_value = row_numbers[0] - 1
+
+        for idx in range(len(row_numbers)):
+            if row_numbers[idx] != last_value + 1:
+                result.append(temp)
+                temp = []
+            last_value = row_numbers[idx]
+            temp.append(last_value)
+        if temp:
+            result.append(temp)
+        result.reverse()
+
+        return result
+
     def rowCount(self, index: QModelIndex = QModelIndex()) -> int:
         """Standard function for view"""
 

app/playlists.py

@@ -7,6 +7,7 @@ import threading
 import obsws_python as obs  # type: ignore
 
 from datetime import datetime, timedelta
+from pprint import pprint
 from typing import Any, Callable, cast, List, Optional, Tuple, TYPE_CHECKING
 
 from PyQt6.QtCore import (
@@ -541,7 +542,7 @@ class PlaylistTab(QTableView):
         # Mark unplayed
         if track_row and model.is_unplayed_row(row_number):
             self._add_context_menu(
-                "Mark unplayed", lambda: self._mark_as_unplayed(self._get_selected_rows())
+                "Mark unplayed", lambda: self._mark_as_unplayed(self.get_selected_rows())
             )
 
         # Unmark as next
@@ -557,22 +558,22 @@ class PlaylistTab(QTableView):
         sort_menu = self.menu.addMenu("Sort")
         self._add_context_menu(
             "by title",
-            lambda: model.sort_by_title(self._get_selected_rows()),
+            lambda: model.sort_by_title(self.get_selected_rows()),
             parent_menu=sort_menu,
         )
         self._add_context_menu(
             "by artist",
-            lambda: model.sort_by_artist(self._get_selected_rows()),
+            lambda: model.sort_by_artist(self.get_selected_rows()),
             parent_menu=sort_menu,
         )
         self._add_context_menu(
             "by duration",
-            lambda: model.sort_by_duration(self._get_selected_rows()),
+            lambda: model.sort_by_duration(self.get_selected_rows()),
             parent_menu=sort_menu,
         )
         self._add_context_menu(
             "by last played",
-            lambda: model.sort_by_lastplayed(self._get_selected_rows()),
+            lambda: model.sort_by_lastplayed(self.get_selected_rows()),
             parent_menu=sort_menu,
         )
 
@@ -652,7 +653,7 @@ class PlaylistTab(QTableView):
         - Pass to model to do the deed
         """
 
-        rows_to_delete = self._get_selected_rows()
+        rows_to_delete = self.get_selected_rows()
         row_count = len(rows_to_delete)
         if row_count < 1:
             return
@@ -663,9 +664,9 @@ class PlaylistTab(QTableView):
             return
 
         model = cast(PlaylistModel, self.model())
-        model.delete_rows(self._get_selected_rows())
+        model.delete_rows(self.get_selected_rows())
 
-    def _get_selected_rows(self) -> List[int]:
+    def get_selected_rows(self) -> List[int]:
         """Return a list of selected row numbers sorted by row"""
 
         # Use a set to deduplicate result (a selected row will have all
@@ -939,13 +940,13 @@ class PlaylistTab(QTableView):
         Toggle drag behaviour according to whether rows are selected
         """
 
-        selected_rows = self._get_selected_rows()
+        selected_rows = self.get_selected_rows()
         # If no rows are selected, we have nothing to do
         if len(selected_rows) == 0:
             self.musicmuster.lblSumPlaytime.setText("")
         else:
             model = cast(PlaylistModel, self.model())
-            selected_duration = model.get_rows_duration(self._get_selected_rows())
+            selected_duration = model.get_rows_duration(self.get_selected_rows())
             if selected_duration > 0:
                 self.musicmuster.lblSumPlaytime.setText(
                     f"Selected duration: {ms_to_mmss(selected_duration)}"

test_models.py

@@ -131,12 +131,12 @@ def test_playlist_open_and_close(session):
     assert len(Playlists.get_open(session)) == 0
     assert len(Playlists.get_closed(session)) == 1
 
-    playlist.mark_open(session, tab_index=0)
+    playlist.mark_open()
 
     assert len(Playlists.get_open(session)) == 1
     assert len(Playlists.get_closed(session)) == 0
 
-    playlist.close(session)
+    playlist.close()
 
     assert len(Playlists.get_open(session)) == 0
     assert len(Playlists.get_closed(session)) == 1

test_playlistmodel.py

@@ -1,8 +1,11 @@
+from pprint import pprint
+from typing import Optional
+
 from app.models import (
     Playlists,
     Tracks,
 )
-from PyQt6.QtCore import Qt
+from PyQt6.QtCore import Qt, QModelIndex
 
 from app.helpers import get_file_metadata
 from app import playlistmodel
@@ -19,8 +22,8 @@ test_tracks = [
 ]
 
 
-def create_model_with_tracks(session: scoped_session) -> "playlistmodel.PlaylistModel":
+def create_model_with_tracks(session: scoped_session, name: Optional[str] = None) -> "playlistmodel.PlaylistModel":
-    playlist = Playlists(session, "test playlist")
+    playlist = Playlists(session, name or "test playlist")
     model = playlistmodel.PlaylistModel(playlist.id)
 
     for row in range(len(test_tracks)):
@@ -34,9 +37,9 @@ def create_model_with_tracks(session: scoped_session) -> "playlistmodel.Playlist
 
 
 def create_model_with_playlist_rows(
-    session: scoped_session, rows: int
+    session: scoped_session, rows: int, name: Optional[str] = None
 ) -> "playlistmodel.PlaylistModel":
-    playlist = Playlists(session, "test playlist")
+    playlist = Playlists(session, name or "test playlist")
     # Create a model
     model = playlistmodel.PlaylistModel(playlist.id)
     for row in range(rows):
@@ -268,6 +271,97 @@ def test_insert_track_new_playlist(monkeypatch, session):
     )
 
 
+def test_reverse_row_groups_one_row(monkeypatch, session):
+    monkeypatch.setattr(playlistmodel, "Session", session)
+
+    rows_to_move = [3]
+
+    model_src = create_model_with_playlist_rows(session, 5, name="source")
+    result = model_src._reversed_contiguous_row_groups(rows_to_move)
+
+    assert len(result) == 1
+    assert result[0] == [3]
+
+
+def test_reverse_row_groups_multiple_row(monkeypatch, session):
+    monkeypatch.setattr(playlistmodel, "Session", session)
+
+    rows_to_move = [2, 3, 4, 5, 7, 9, 10, 13, 17, 20, 21]
+
+    model_src = create_model_with_playlist_rows(session, 5, name="source")
+    result = model_src._reversed_contiguous_row_groups(rows_to_move)
+
+    assert result == [[20, 21], [17], [13], [9, 10], [7], [2, 3, 4, 5]]
+
+
+def test_move_one_row_between_playlists_to_end(monkeypatch, session):
+    monkeypatch.setattr(playlistmodel, "Session", session)
+
+    create_rowcount = 5
+    from_rows = [3]
+    to_row = create_rowcount
+
+    model_src = create_model_with_playlist_rows(session, create_rowcount, name="source")
+    model_dst = create_model_with_playlist_rows(session, create_rowcount, name="destination")
+
+    model_src.move_rows_between_playlists(from_rows, to_row, model_dst.playlist_id)
+    model_dst.refresh_data(session)
+
+    assert len(model_src.playlist_rows) == create_rowcount - len(from_rows)
+    assert len(model_dst.playlist_rows) == create_rowcount + len(from_rows)
+    assert sorted([a.plr_rownum for a in model_src.playlist_rows.values()]) == list(
+        range(len(model_src.playlist_rows))
+    )
+
+
+def test_move_one_row_between_playlists_to_middle(monkeypatch, session):
+    monkeypatch.setattr(playlistmodel, "Session", session)
+
+    create_rowcount = 5
+    from_rows = [3]
+    to_row = 2
+
+    model_src = create_model_with_playlist_rows(session, create_rowcount, name="source")
+    model_dst = create_model_with_playlist_rows(session, create_rowcount, name="destination")
+
+    model_src.move_rows_between_playlists(from_rows, to_row, model_dst.playlist_id)
+    model_dst.refresh_data(session)
+
+    # Check the rows of the destination model
+    row_notes = []
+    for row_number in range(model_dst.rowCount()):
+        index = model_dst.index(row_number, playlistmodel.Col.TITLE.value, QModelIndex())
+        row_notes.append(model_dst.data(index, Qt.ItemDataRole.EditRole).value())
+
+    assert len(model_src.playlist_rows) == create_rowcount - len(from_rows)
+    assert len(model_dst.playlist_rows) == create_rowcount + len(from_rows)
+    assert [int(a) for a in row_notes] == [0, 1, 3, 2, 3, 4]
+
+
+def test_move_multiple_rows_between_playlists_to_end(monkeypatch, session):
+    monkeypatch.setattr(playlistmodel, "Session", session)
+
+    create_rowcount = 5
+    from_rows = [1, 3, 4]
+    to_row = 2
+
+    model_src = create_model_with_playlist_rows(session, create_rowcount, name="source")
+    model_dst = create_model_with_playlist_rows(session, create_rowcount, name="destination")
+
+    model_src.move_rows_between_playlists(from_rows, to_row, model_dst.playlist_id)
+    model_dst.refresh_data(session)
+
+    # Check the rows of the destination model
+    row_notes = []
+    for row_number in range(model_dst.rowCount()):
+        index = model_dst.index(row_number, playlistmodel.Col.TITLE.value, QModelIndex())
+        row_notes.append(model_dst.data(index, Qt.ItemDataRole.EditRole).value())
+
+    assert len(model_src.playlist_rows) == create_rowcount - len(from_rows)
+    assert len(model_dst.playlist_rows) == create_rowcount + len(from_rows)
+    assert [int(a) for a in row_notes] == [0, 1, 3, 4, 1, 2, 3, 4]
+
+
 # def test_edit_header(monkeypatch, session): # edit header row in middle of playlist
 
 #     monkeypatch.setattr(playlistmodel, "Session", session)