feat: Implement playlist reordering with position swapping and reload player on current playlist changes
Some checks failed
Backend CI / lint (push) Failing after 5m7s
Backend CI / test (push) Successful in 5m14s

This commit is contained in:
JSC
2025-08-01 17:49:29 +02:00
parent 0575d12b0e
commit d926779fe4
4 changed files with 161 additions and 8 deletions

View File

@@ -165,6 +165,20 @@ class PlaylistRepository(BaseRepository[Playlist]):
"""
try:
# Phase 1: Set all positions to temporary negative values to avoid conflicts
temp_offset = -10000 # Use large negative number to avoid conflicts
for i, (sound_id, _) in enumerate(sound_positions):
statement = select(PlaylistSound).where(
PlaylistSound.playlist_id == playlist_id,
PlaylistSound.sound_id == sound_id,
)
result = await self.session.exec(statement)
playlist_sound = result.first()
if playlist_sound:
playlist_sound.position = temp_offset + i
# Phase 2: Set the final positions
for sound_id, new_position in sound_positions:
statement = select(PlaylistSound).where(
PlaylistSound.playlist_id == playlist_id,

View File

@@ -14,6 +14,33 @@ from app.repositories.sound import SoundRepository
logger = get_logger(__name__)
async def _reload_player_playlist() -> None:
"""Reload the player playlist after current playlist changes."""
try:
# Import here to avoid circular import issues
from app.services.player import get_player_service # noqa: PLC0415
player = get_player_service()
await player.reload_playlist()
logger.debug("Player playlist reloaded after current playlist change")
except Exception: # noqa: BLE001
# Don't fail the playlist operation if player reload fails
logger.warning("Failed to reload player playlist", exc_info=True)
async def _is_current_playlist(session: AsyncSession, playlist_id: int) -> bool:
"""Check if the given playlist is the current playlist."""
try:
from app.repositories.playlist import PlaylistRepository # noqa: PLC0415
playlist_repo = PlaylistRepository(session)
current_playlist = await playlist_repo.get_current_playlist()
return current_playlist is not None and current_playlist.id == playlist_id
except Exception: # noqa: BLE001
logger.warning("Failed to check if playlist is current", exc_info=True)
return False
class PlaylistService:
"""Service for playlist operations."""
@@ -99,6 +126,11 @@ class PlaylistService:
playlist = await self.playlist_repo.create(playlist_data)
logger.info("Created playlist '%s' for user %s", name, user_id)
# If this was set as current, reload player playlist
if is_current:
await _reload_player_playlist()
return playlist
async def update_playlist( # noqa: PLR0913
@@ -145,6 +177,10 @@ class PlaylistService:
playlist = await self.playlist_repo.update(playlist, update_data)
logger.info("Updated playlist %s for user %s", playlist_id, user_id)
# If is_current was changed, reload player playlist
if "is_current" in update_data:
await _reload_player_playlist()
return playlist
async def delete_playlist(self, playlist_id: int, user_id: int) -> None:
@@ -157,11 +193,15 @@ class PlaylistService:
detail="This playlist cannot be deleted",
)
# Check if this was the current playlist before deleting
was_current = playlist.is_current
await self.playlist_repo.delete(playlist)
logger.info("Deleted playlist %s for user %s", playlist_id, user_id)
# Note: If the deleted playlist was current, main playlist becomes fallback
# No action needed as get_current_playlist() handles the fallback automatically
# If the deleted playlist was current, reload player to use main playlist fallback
if was_current:
await _reload_player_playlist()
async def search_playlists(self, query: str, user_id: int) -> list[Playlist]:
"""Search user's playlists by name."""
@@ -210,6 +250,10 @@ class PlaylistService:
user_id,
)
# If this is the current playlist, reload player
if await _is_current_playlist(self.session, playlist_id):
await _reload_player_playlist()
async def remove_sound_from_playlist(
self,
playlist_id: int,
@@ -235,6 +279,10 @@ class PlaylistService:
user_id,
)
# If this is the current playlist, reload player
if await _is_current_playlist(self.session, playlist_id):
await _reload_player_playlist()
async def reorder_playlist_sounds(
self,
playlist_id: int,
@@ -256,6 +304,9 @@ class PlaylistService:
await self.playlist_repo.reorder_playlist_sounds(playlist_id, sound_positions)
logger.info("Reordered sounds in playlist %s for user %s", playlist_id, user_id)
# If this is the current playlist, reload player
if await _is_current_playlist(self.session, playlist_id):
await _reload_player_playlist()
async def get_playlist_stats(self, playlist_id: int) -> dict[str, Any]:
"""Get statistics for a playlist."""
@@ -281,18 +332,25 @@ class PlaylistService:
msg = "Main playlist has no ID, cannot add sound"
raise ValueError(msg)
# Extract ID before async operations to avoid session issues
main_playlist_id = main_playlist.id
# Check if sound is already in main playlist
if not await self.playlist_repo.is_sound_in_playlist(
main_playlist.id,
main_playlist_id,
sound_id,
):
await self.playlist_repo.add_sound_to_playlist(main_playlist.id, sound_id)
await self.playlist_repo.add_sound_to_playlist(main_playlist_id, sound_id)
logger.info(
"Added sound %s to main playlist for user %s",
sound_id,
user_id,
)
# If main playlist is current, reload player
if await _is_current_playlist(self.session, main_playlist_id):
await _reload_player_playlist()
# Current playlist methods (global by default)
async def set_current_playlist(self, playlist_id: int) -> Playlist:
"""Set a playlist as the current playlist (app-wide)."""
@@ -304,6 +362,10 @@ class PlaylistService:
# Set new current playlist
playlist = await self.playlist_repo.update(playlist, {"is_current": True})
logger.info("Set playlist %s as current playlist", playlist_id)
# Reload player playlist to reflect the change
await _reload_player_playlist()
return playlist
async def unset_current_playlist(self) -> None:
@@ -311,9 +373,11 @@ class PlaylistService:
await self._unset_current_playlist()
logger.info("Unset current playlist, main playlist is now fallback")
# Reload player playlist to reflect the change (will fallback to main)
await _reload_player_playlist()
async def _unset_current_playlist(self) -> None:
"""Unset any current playlist globally."""
current_playlist = await self.playlist_repo.get_current_playlist()
if current_playlist:
await self.playlist_repo.update(current_playlist, {"is_current": False})

View File

@@ -811,3 +811,78 @@ class TestPlaylistRepository:
assert len(sounds) == TWO_SOUNDS
assert sounds[0].id == sound2_id # sound2 now at position 5
assert sounds[1].id == sound1_id # sound1 now at position 10
@pytest.mark.asyncio
async def test_reorder_playlist_sounds_position_swap(
self,
playlist_repository: PlaylistRepository,
test_session: AsyncSession,
ensure_plans: Any,
) -> None:
"""Test reordering sounds with position swapping (regression test)."""
# Create objects within this test
user = User(
email="test@example.com",
name="Test User",
password_hash=PasswordUtils.hash_password("password123"),
role="user",
is_active=True,
plan_id=ensure_plans[0].id,
credits=100,
)
test_session.add(user)
await test_session.commit()
await test_session.refresh(user)
user_id = user.id
playlist = Playlist(
user_id=user_id,
name="Test Playlist",
description="A test playlist",
genre="test",
is_main=False,
is_current=False,
is_deletable=True,
)
test_session.add(playlist)
# Create multiple sounds
sound1 = Sound(name="Sound 1", filename="sound1.mp3", type="SDB", hash="hash1")
sound2 = Sound(name="Sound 2", filename="sound2.mp3", type="SDB", hash="hash2")
test_session.add_all([playlist, sound1, sound2])
await test_session.commit()
await test_session.refresh(playlist)
await test_session.refresh(sound1)
await test_session.refresh(sound2)
# Extract IDs before async calls
playlist_id = playlist.id
sound1_id = sound1.id
sound2_id = sound2.id
# Add sounds to playlist at positions 0 and 1
await playlist_repository.add_sound_to_playlist(
playlist_id, sound1_id, position=0,
)
await playlist_repository.add_sound_to_playlist(
playlist_id, sound2_id, position=1,
)
# Verify initial order
sounds = await playlist_repository.get_playlist_sounds(playlist_id)
assert len(sounds) == TWO_SOUNDS
assert sounds[0].id == sound1_id # sound1 at position 0
assert sounds[1].id == sound2_id # sound2 at position 1
# Swap positions - this used to cause unique constraint violation
sound_positions = [(sound1_id, 1), (sound2_id, 0)]
await playlist_repository.reorder_playlist_sounds(
playlist_id, sound_positions,
)
# Verify swapped order
sounds = await playlist_repository.get_playlist_sounds(playlist_id)
assert len(sounds) == TWO_SOUNDS
assert sounds[0].id == sound2_id # sound2 now at position 0
assert sounds[1].id == sound1_id # sound1 now at position 1

View File

@@ -364,7 +364,7 @@ class TestPlayerService:
mock_playlist = Mock()
mock_playlist.id = 1
mock_playlist.name = "Test Playlist"
mock_repo.get_main_playlist.return_value = mock_playlist
mock_repo.get_current_playlist.return_value = mock_playlist # Return current playlist directly
# Mock sounds
sound1 = Sound(id=1, name="Song 1", filename="song1.mp3", duration=30000)
@@ -562,7 +562,7 @@ class TestPlayerService:
mock_playlist = Mock()
mock_playlist.id = 2 # Different ID
mock_playlist.name = "New Playlist"
mock_repo.get_main_playlist.return_value = mock_playlist
mock_repo.get_current_playlist.return_value = mock_playlist # Return current playlist directly
sound1 = Sound(id=1, name="Song 1", filename="song1.mp3", duration=30000)
mock_sounds = [sound1]
@@ -597,7 +597,7 @@ class TestPlayerService:
mock_playlist = Mock()
mock_playlist.id = 1
mock_playlist.name = "Same Playlist"
mock_repo.get_main_playlist.return_value = mock_playlist
mock_repo.get_current_playlist.return_value = mock_playlist # Return current playlist directly
# Track 2 moved to index 0
sound1 = Sound(id=2, name="Song 2", filename="song2.mp3", duration=45000)