diff --git a/app/ds.py b/app/ds.py index 21bc389..3dbd422 100644 --- a/app/ds.py +++ b/app/ds.py @@ -472,6 +472,8 @@ def _playlist_check_playlist( else raise ApplicationError. """ + fixed = False + playlist_rows = ( session.execute( select(PlaylistRows) @@ -492,9 +494,13 @@ def _playlist_check_playlist( if fix: log.debug(msg) plr.row_number = idx + fixed = True else: raise ApplicationError(msg) + if fixed: + session.commit() + # @log_call def _playlist_shift_rows( @@ -696,88 +702,145 @@ def playlist_move_rows( to_playlist_id: int | None = None, ) -> None: """ - Move rows with or between playlists. + Call helper function depending upon whether we are moving rows within + a playlist or between playlists. + """ + + # If to_playlist_id isn't specified, we're moving within the one + # playlist. + if to_playlist_id is None or to_playlist_id == from_playlist_id: + _playlist_move_rows_within_playlist(from_rows, from_playlist_id, to_row) + else: + _playlist_move_rows_between_playlists( + from_rows, from_playlist_id, to_row, to_playlist_id + ) + + +def _playlist_move_rows_between_playlists( + from_rows: list[int], + from_playlist_id: int, + to_row: int, + to_playlist_id: int, +) -> None: + """ + Move rows between playlists. Algorithm: - Sanity check row numbers - - Check there are no playlist rows with playlist_id == PENDING_MOVE - - Put rows to be moved into PENDING_MOVE playlist - Resequence remaining row numbers - Make space for moved rows - Move the PENDING_MOVE rows back and fixup row numbers - Sanity check row numbers """ - # If to_playlist_id isn't specified, we're moving within the one - # playlist. - if to_playlist_id is None: - to_playlist_id = from_playlist_id + # Sanity check destination not being moved + if to_row in from_rows: + log.error( + f"ds._playlist_move_rows_within_playlist: {to_row=} in {from_rows=}" + ) + return with db.Session() as session: # Sanity check row numbers _playlist_check_playlist(session, from_playlist_id, fix=False) - if from_playlist_id != to_playlist_id: - _playlist_check_playlist(session, to_playlist_id, fix=False) + _playlist_check_playlist(session, to_playlist_id, fix=False) - # Check there are no playlist rows with playlist_id == PENDING_MOVE - pending_move_rows = playlistrows_by_playlist(Config.PLAYLIST_PENDING_MOVE) - if pending_move_rows: - raise ApplicationError(f"move_rows_to_playlist: {pending_move_rows=}") + # Make space in destination playlist + _playlist_shift_rows(session, to_playlist_id, to_row, len(from_rows)) - # We need playlist length if we're moving within a playlist. Get - # that now before we remove rows. - from_playlist_length = len(playlistrows_by_playlist(from_playlist_id)) - # Put rows to be moved into PENDING_MOVE playlist - session.execute( - update(PlaylistRows) - .where( - PlaylistRows.playlist_id == from_playlist_id, - PlaylistRows.row_number.in_(from_rows), - ) - .values(playlist_id=Config.PLAYLIST_PENDING_MOVE) - ) - - # Resequence remaining row numbers - _playlist_check_playlist(session, from_playlist_id, fix=True) - session.commit() - - # Make space for moved rows. If moving within one playlist, - # determning where to make the space is non-trivial. For example, - # if the playlist has ten entries and we're moving four of them - # to row 8, after we've moved the rows to the - # PLAYLIST_PENDING_MOVE there will only be six entries left. - # Clearly we can't make space at row 8... - space_row = to_row - if to_playlist_id == from_playlist_id: - overflow = max(to_row + len(from_rows) - from_playlist_length, 0) - if overflow != 0: - space_row = ( - to_row - overflow - len([a for a in from_rows if a > to_row]) - ) - - _playlist_shift_rows(session, to_playlist_id, space_row, len(from_rows)) - - # Move the PENDING_MOVE rows back and fixup row numbers + # Update database + # Build a dictionary of changes to make update_list: list[dict[str, int]] = [] - next_row = space_row - # PLAYLIST_PENDING_MOVE may have gaps so don't check it - for row_to_move in playlistrows_by_playlist( - Config.PLAYLIST_PENDING_MOVE, check_playlist_itegrity=False - ): + old_row_to_id = _playlist_rows_to_id(from_playlist_id) + next_row = to_row + + for from_row in from_rows: + plrid = old_row_to_id[from_row] update_list.append( - {"id": row_to_move.playlistrow_id, "row_number": next_row} + {"id": plrid, "row_number": next_row} ) update_list.append( - {"id": row_to_move.playlistrow_id, "playlist_id": to_playlist_id} + {"id": plrid, "playlist_id": to_playlist_id} ) next_row += 1 + + session.execute(update(PlaylistRows), update_list) + session.commit() + + # Resequence row numbers in source + _playlist_check_playlist(session, from_playlist_id, fix=True) + # Sanity check destionation + _playlist_check_playlist(session, from_playlist_id, fix=False) + + +def _playlist_rows_to_id(playlist_id: int) -> dict[int, int]: + """ + Return a dict of {row_number: playlistrow_id} for passed playlist + """ + + row_to_id = { + p.row_number: p.playlistrow_id + for p in playlistrows_by_playlist(playlist_id) + } + + return row_to_id + + +# @log_call +def _playlist_move_rows_within_playlist( + from_rows: list[int], + from_playlist_id: int, + to_row: int, +) -> None: + """ + Move rows within playlists. + + Algorithm: + - Sanity checks + - Create a list of row numbers in the new order + - Update the database with the new order + - Sanity check row numbers + """ + + # Sanity check destination not being moved + if to_row in from_rows: + log.error( + f"ds._playlist_move_rows_within_playlist: {to_row=} in {from_rows=}" + ) + return + + with db.Session() as session: + # Sanity check row numbers + _playlist_check_playlist(session, from_playlist_id, fix=False) + + # Create a list showing the new order of rows in playlist + # Start with a list of rows excluding those to be moved + from_playlist_length = len(playlistrows_by_playlist(from_playlist_id)) + new_row_order = [a for a in range(from_playlist_length) if a not in from_rows] + # Insert the moved row numbers + try: + idx = new_row_order.index(to_row) + except ValueError: + raise ApplicationError(f"Can't find {to_row=} in {new_row_order=}") + new_row_order[idx:idx] = from_rows + + # Update database + # Build a dictionary of {old_row_number: new_row_number} where + # they differ + row_changes = {old: new for new, old in enumerate(new_row_order) if old != new} + # Build a dictionary of changes to make + update_list: list[dict[str, int]] = [] + old_row_to_id = _playlist_rows_to_id(from_playlist_id) + for old_row, new_row in row_changes.items(): + plrid = old_row_to_id[old_row] + update_list.append({"id": plrid, "row_number": new_row}) + + # Updte database session.execute(update(PlaylistRows), update_list) session.commit() # Sanity check row numbers _playlist_check_playlist(session, from_playlist_id, fix=False) - if from_playlist_id != to_playlist_id: - _playlist_check_playlist(session, to_playlist_id, fix=False) def playlists_open() -> list[PlaylistDTO]: @@ -905,7 +968,6 @@ def playlist_remove_rows(playlist_id: int, row_numbers: list[int]) -> None: ) # Fixup row number to remove gaps _playlist_check_playlist(session, playlist_id, fix=True) - session.commit() # @log_call diff --git a/tests/test_ds.py b/tests/test_ds.py index 7330e40..629a772 100644 --- a/tests/test_ds.py +++ b/tests/test_ds.py @@ -165,7 +165,7 @@ class MyTestCase(unittest.TestCase): new_order = [] for row in ds.playlistrows_by_playlist(playlist.playlist_id): new_order.append(int(row.note)) - assert new_order == [0, 1, 2, 4, 5, 3, 6, 7, 8, 9] + assert new_order == [0, 1, 2, 4, 3, 5, 6, 7, 8, 9] def test_move_rows_test2(self): # move row 4 to row 3 @@ -207,7 +207,7 @@ class MyTestCase(unittest.TestCase): new_order = [] for row in ds.playlistrows_by_playlist(playlist.playlist_id): new_order.append(int(row.note)) - assert new_order == [0, 2, 3, 6, 7, 8, 1, 4, 5, 10, 9] + assert new_order == [0, 2, 3, 6, 7, 1, 4, 5, 10, 8, 9] def test_move_rows_test5(self): # move rows [3, 6] → 5 @@ -221,7 +221,7 @@ class MyTestCase(unittest.TestCase): new_order = [] for row in ds.playlistrows_by_playlist(playlist.playlist_id): new_order.append(int(row.note)) - assert new_order == [0, 1, 2, 4, 5, 3, 6, 7, 8, 9, 10] + assert new_order == [0, 1, 2, 4, 3, 6, 5, 7, 8, 9, 10] def test_move_rows_test6(self): # move rows [3, 5, 6] → 8 @@ -235,7 +235,7 @@ class MyTestCase(unittest.TestCase): new_order = [] for row in ds.playlistrows_by_playlist(playlist.playlist_id): new_order.append(int(row.note)) - assert new_order == [0, 1, 2, 4, 7, 8, 9, 10, 3, 5, 6] + assert new_order == [0, 1, 2, 4, 7, 3, 5, 6, 8, 9, 10] def test_move_rows_test7(self): # move rows [7, 8, 10] → 5 @@ -258,13 +258,13 @@ class MyTestCase(unittest.TestCase): number_of_rows = 11 (playlist, model) = self.create_rows("test_move_rows_test8", number_of_rows) - ds.playlist_move_rows([0, 1, 2, 3], playlist.playlist_id, 0) + ds.playlist_move_rows([1, 2, 3], playlist.playlist_id, 0) # Check we have all rows and plr_rownums are correct new_order = [] for row in ds.playlistrows_by_playlist(playlist.playlist_id): new_order.append(int(row.note)) - assert new_order == [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + assert new_order == [1, 2, 3, 0, 4, 5, 6, 7, 8, 9, 10] def test_move_rows_to_playlist(self): number_of_rows = 11