From 199f0e27fafa9a8d9d67a1f3a114e9e3bd98218d Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Fri, 17 Nov 2023 22:17:47 +0000 Subject: [PATCH] WIP V3: fixup row insertion/deletion All row insertions and deletions are now wrapped in beginRemoveRows / endRemoveRows (and similar for insertions). --- app/models.py | 8 ++-- app/musicmuster.py | 5 +- app/playlistmodel.py | 105 ++++++++++++++++++------------------------ app/playlists.py | 40 +++++++++++++++- test_playlistmodel.py | 39 ++++++++++++---- 5 files changed, 121 insertions(+), 76 deletions(-) diff --git a/app/models.py b/app/models.py index f81b26b..918288d 100644 --- a/app/models.py +++ b/app/models.py @@ -489,17 +489,17 @@ class PlaylistRows(Base): session.flush() @staticmethod - def delete_rows( - session: scoped_session, playlist_id: int, row_numbers: List[int] + def delete_row( + 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, ) ) diff --git a/app/musicmuster.py b/app/musicmuster.py index c8ba478..f1eb13b 100755 --- a/app/musicmuster.py +++ b/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( - self.active_tab().get_selected_row_number(), dlg.textValue() + model.insert_row( + proposed_row_number=self.active_tab().get_selected_row_number(), + note=dlg.textValue(), ) def insert_track(self) -> None: diff --git a/app/playlistmodel.py b/app/playlistmodel.py index b6f02e3..0fd7edc 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -147,15 +147,7 @@ class PlaylistModel(QAbstractTableModel): if playlist_id != self.playlist_id: return - # Insert track if we have one - 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 + self.insert_row(proposed_row_number=new_row_number, track_id=track_id, note=note) 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: - """ - Insert a header row. - """ - - 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] + def insert_row( + self, + proposed_row_number: Optional[int], + track_id: Optional[int] = None, + note: Optional[str] = None, ) -> 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) - # Update the PlaylistRows object + super().beginInsertRows(QModelIndex(), new_row_number, new_row_number) + plr = PlaylistRows.insert_row(session, self.playlist_id, new_row_number) + plr.track_id = track_id - if text: - plr.note = text - # Repopulate self.playlist_rows + if note: + plr.note = note + 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 diff --git a/app/playlists.py b/app/playlists.py index cd6e1ac..863deda 100644 --- a/app/playlists.py +++ b/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, diff --git a/test_playlistmodel.py b/test_playlistmodel.py index 0f5a447..f4cc438 100644 --- a/test_playlistmodel.py +++ b/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) - newrow = plr.plr_rownum - plr.note = str(newrow) - model.playlist_rows[newrow] = playlistmodel.PlaylistRowData(plr) + plr = model.insert_row(proposed_row_number=row, note=str(row)) + model.playlist_rows[plr.plr_rownum] = 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_header_row(END_ROW, "-") + model.insert_row(proposed_row_number=START_ROW, note="start+") + 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)