From f57bcc37f6bceb7b7913e9b370094ab0c24bc643 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Fri, 27 Oct 2023 06:58:22 +0100 Subject: [PATCH] WIP V3 model development --- app/playlistmodel.py | 31 +++-- conftest.py | 2 +- test_playlistmodel.py | 261 ++++++++++++++++++++---------------------- 3 files changed, 150 insertions(+), 144 deletions(-) diff --git a/app/playlistmodel.py b/app/playlistmodel.py index 462478e..152d2c4 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -81,6 +81,18 @@ 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 ): @@ -93,7 +105,7 @@ class PlaylistModel(QAbstractTableModel): self.refresh_data() def __repr__(self) -> str: - return f"" + return f"" def background_role(self, row: int, column: int, prd: PlaylistRowData) -> QBrush: """Return background setting""" @@ -106,6 +118,7 @@ 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)) @@ -171,13 +184,8 @@ class PlaylistModel(QAbstractTableModel): Return text for display """ - # Detect whether this is a header row if not prd.path: - header = prd.note - else: - header = "" - - if header: + # No track so this is a header row if column == HEADER_NOTES_COLUMN: self.signals.span_cells_signal.emit( row, HEADER_NOTES_COLUMN, 1, self.columnCount() - 1 @@ -329,6 +337,15 @@ class PlaylistModel(QAbstractTableModel): 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( diff --git a/conftest.py b/conftest.py index 97edbca..b284c31 100644 --- a/conftest.py +++ b/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) diff --git a/test_playlistmodel.py b/test_playlistmodel.py index 76f36bc..81a13d1 100644 --- a/test_playlistmodel.py +++ b/test_playlistmodel.py @@ -20,190 +20,179 @@ def create_model_with_playlist_rows( 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 - 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) - assert model.rowCount() == 1 + playlist = Playlists(session, "test playlist") + # Create a model + model = playlistmodel.PlaylistModel(playlist.id, None) + assert model.rowCount() == 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 - 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) - assert model.rowCount() == 1 + playlist = Playlists(session, "test playlist") + # Create a model + model = playlistmodel.PlaylistModel(playlist.id, None) + assert model.rowCount() == 0 + 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) - with Session() as session: - model = create_model_with_playlist_rows(session, 11) - assert model.rowCount() == 11 - assert max(model.playlist_rows.keys()) == 10 - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row + monkeypatch.setattr(playlistmodel, "Session", session) + model = create_model_with_playlist_rows(session, 11) + assert model.rowCount() == 11 + assert max(model.playlist_rows.keys()) == 10 + for row in range(model.rowCount()): + assert row in model.playlist_rows + 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) - 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 - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row - if row not in [3, 4, 5]: - assert model.playlist_rows[row].note == str(row) - elif row == 3: - assert model.playlist_rows[row].note == str(4) - elif row == 4: - assert model.playlist_rows[row].note == str(5) - elif row == 5: - assert model.playlist_rows[row].note == str(3) + monkeypatch.setattr(playlistmodel, "Session", session) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([3], 5) + # Check we have all rows and plr_rownums are correct + for row in range(model.rowCount()): + assert row in model.playlist_rows + assert model.playlist_rows[row].plr_rownum == row + if row not in [3, 4, 5]: + assert model.playlist_rows[row].note == str(row) + elif row == 3: + assert model.playlist_rows[row].note == str(4) + elif row == 4: + assert model.playlist_rows[row].note == str(5) + elif row == 5: + 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) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([4], 3) - # Check we have all rows and plr_rownums are correct - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row - if row not in [3, 4]: - assert model.playlist_rows[row].note == str(row) - elif row == 3: - assert model.playlist_rows[row].note == str(4) - elif row == 4: - assert model.playlist_rows[row].note == str(3) + # Check we have all rows and plr_rownums are correct + for row in range(model.rowCount()): + assert row in model.playlist_rows + assert model.playlist_rows[row].plr_rownum == row + if row not in [3, 4]: + assert model.playlist_rows[row].note == str(row) + elif row == 3: + assert model.playlist_rows[row].note == str(4) + elif row == 4: + 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) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([4], 2) - # Check we have all rows and plr_rownums are correct - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row - if row not in [2, 3, 4]: - assert model.playlist_rows[row].note == str(row) - elif row == 2: - assert model.playlist_rows[row].note == str(4) - elif row == 3: - assert model.playlist_rows[row].note == str(2) - elif row == 4: - assert model.playlist_rows[row].note == str(3) + # Check we have all rows and plr_rownums are correct + for row in range(model.rowCount()): + assert row in model.playlist_rows + assert model.playlist_rows[row].plr_rownum == row + if row not in [2, 3, 4]: + assert model.playlist_rows[row].note == str(row) + elif row == 2: + assert model.playlist_rows[row].note == str(4) + elif row == 3: + assert model.playlist_rows[row].note == str(2) + elif row == 4: + 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) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([1, 4, 5, 10], 8) - # Check we have all rows and plr_rownums are correct - new_order = [] - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row - new_order.append(int(model.playlist_rows[row].note)) - assert new_order == [0, 2, 3, 6, 7, 8, 9, 1, 4, 5, 10] + # Check we have all rows and plr_rownums are correct + new_order = [] + for row in range(model.rowCount()): + assert row in model.playlist_rows + assert model.playlist_rows[row].plr_rownum == row + new_order.append(int(model.playlist_rows[row].note)) + 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) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([3, 6], 5) - # Check we have all rows and plr_rownums are correct - new_order = [] - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row - new_order.append(int(model.playlist_rows[row].note)) - assert new_order == [0, 1, 2, 4, 5, 3, 6, 7, 8, 9, 10] + # Check we have all rows and plr_rownums are correct + new_order = [] + for row in range(model.rowCount()): + assert row in model.playlist_rows + assert model.playlist_rows[row].plr_rownum == row + new_order.append(int(model.playlist_rows[row].note)) + 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) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([3, 5, 6], 8) - # Check we have all rows and plr_rownums are correct - new_order = [] - for row in range(model.rowCount()): - assert row in model.playlist_rows - assert model.playlist_rows[row].plr_rownum == row - new_order.append(int(model.playlist_rows[row].note)) - assert new_order == [0, 1, 2, 4, 7, 8, 9, 10, 3, 5, 6] + # Check we have all rows and plr_rownums are correct + new_order = [] + for row in range(model.rowCount()): + assert row in model.playlist_rows + assert model.playlist_rows[row].plr_rownum == row + new_order.append(int(model.playlist_rows[row].note)) + 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) + model = create_model_with_playlist_rows(session, 11) + model.move_rows([7, 8, 10], 5) - # Check we have all rows and plr_rownums are correct - new_order = [] - for row in range(model.rowCount()): - assert row in model.playlist_rows - 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] + # Check we have all rows and plr_rownums are correct + new_order = [] + for row in range(model.rowCount()): + assert row in model.playlist_rows + 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(monkeypatch, Session): - # insert header row +def test_insert_header_row_end(monkeypatch, session): + # insert header row at end of playlist - monkeypatch.setattr(playlistmodel, "Session", Session) + monkeypatch.setattr(playlistmodel, "Session", session) note_text = "test text" - with Session() as session: - 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 + 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