feat: offline download management — playlist/album download, cancel, remove, and offline art#190
Closed
tbrackbill wants to merge 12 commits into
Closed
Conversation
…pletion messages - isSongDownloaded() now rejects files < 64 KB as corrupt/partial so interrupted downloads don't permanently block a track from retrying - downloadSong() deletes the partial file on failure for the same reason - settings_storage_tab: fire-and-forget startBackgroundDownload so the "Library download started" snackbar appears immediately, not after all downloads complete - playlist/album screens: guard against concurrent downloads with an explicit isBackgroundDownloadActive check and user-facing message; use whenComplete instead of .then so no false "Downloaded N songs" message fires when the guard rejects the request Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- hasNext/hasPrevious now return true when repeat-all or shuffle is on, so the next/prev buttons are never greyed out in those modes - skipNext wraps to index 0 on repeat-all, or picks a random track on shuffle, when already at the last song - skipPrevious wraps to last track on repeat-all, or picks a random track on shuffle, when already at the first song - toggleShuffle/toggleRepeat now persist their state via StorageService - _initializePlayer loads persisted shuffle/repeat on startup - playSong shuffles the new queue in-place when shuffle is already enabled Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hrough pre-shuffled queue skipNext now checks shuffle first and picks a random non-current track regardless of queue position, so tapping next always jumps to a random song when shuffle is on. Removed the re-shuffle-on-playSong logic that was causing every skip to restart the sequence from the beginning. _onSongComplete unified through skipNext so auto-advance respects shuffle the same way as manual skips. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
skipNext() on a single-song queue calls playSong() with the same song, which hits the 'same song = togglePlayPause' guard and pauses rather than replaying. Treat repeat-all + queue.length==1 the same as repeat-one: seek to zero and play directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…es, UPnP upstream) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
OfflineService: - Add downloadedSongIds ValueNotifier (Set<String>) seeded from SharedPrefs on initialize(), updated on download success/delete so UI reacts without polling - Add downloadLog ValueNotifier (List<DownloadLogEntry>) with per-song queued/downloading/done/failed status, reset each batch - Extend DownloadState with currentSong and failedSongs for active-download panel - Add getPlaylistDownloadStatus() helper returning (downloaded, total) - Add DownloadStatus enum and DownloadLogEntry model SongTile: - _buildTrailing() wrapped in ValueListenableBuilder on downloadedSongIds; shows 14px green check_circle badge when song is locally downloaded PlaylistScreen / AlbumScreen: - Artwork wrapped in ValueListenableBuilder on downloadedSongIds; shows green check_circle badge when every song in the collection is downloaded Settings (Offline Downloads section): - Active Downloads row: shows current song artist-title and X/Y progress while a batch runs, otherwise "No downloads in progress" - Playlist Downloads row: shows total downloaded-song count from reactive set; navigation to per-playlist breakdown wired in feature/download-detail-screens Build: - android/gradle.properties: add android.newDsl=false (AGP 8.11 compat) - Dockerfile.release + build-release-apk.sh: retained for reference but native build via /rw/flutter is the preferred approach going forward Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add green check badge to _PlaylistTile when all songs downloaded - Create DownloadPlaylistStatusScreen showing downloaded/total per playlist - Wire Settings > Playlist Downloads row to navigate to new screen Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Switch downloadSong() from /stream to /download so files are always the original, non-transcoded version and match song.size exactly - Persist expected file sizes to SharedPrefs before each download so reconciliation works even if the app is killed mid-batch - initialize() now scans the offline dir and recovers any valid files that landed on disk but weren't indexed (handles interrupted downloads) - isSongDownloaded() uses stored expected size when available, 64 KB floor as fallback for songs where size is unknown - One automatic retry pass for failed songs at end of each batch - deleteSong() and deleteAllDownloads() clean up the expected sizes map Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- queuePlaylistDownload() replaces direct startBackgroundDownload calls; multiple playlists/albums stack in a sequential download queue processed one batch at a time - queuedPlaylistIds ValueNotifier drives an outline check_circle_outline badge in playlists list and Library view when a playlist is queued but not yet fully downloaded; fills to check_circle when complete - Queued state (including full song list JSON) is persisted to SharedPrefs so the badge survives app restarts - initialize() loads queued playlist data and unmarks any that are now fully on disk after the reconciliation scan - PlayerProvider resumes incomplete queued downloads automatically on startup via resumeIncompleteDownloads() - deleteAllDownloads() clears the queue and queued playlist data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Playlist/album download spinner now driven by queuedPlaylistIds
ValueNotifier instead of local _isDownloading state, so the spinner
stays visible for the full duration of the download — including when
additional playlists are queued while one is already in progress
- Also save cover art indexed by song.coverArt ID (art_{id}.jpg) so
AlbumArtwork can resolve it offline for album/playlist grid views;
previously art was only stored by song ID and never found by the widget
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace fragile nested ValueListenableBuilder with explicit listeners: - _allDownloaded and _isQueued driven by addListener on OfflineService notifiers; state is updated via _updateDownloadState() called on load and on every notifier change, ensuring correct initial state - Button has 3 clean states: cloud_download (idle) → spinner (queued/ downloading, tap cancels) → cloud_done green (complete, tap removes) - Cancel: removes playlist from queue and stops active download - Remove: confirmation dialog then deletes all songs in playlist/album - Added cancelPlaylistDownload() and deletePlaylistDownloads() to OfflineService Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- cancelPlaylistDownload: only kill the active download when the target
playlist is currently running; cancelling a queued-but-not-started
entry no longer stops an unrelated active download
- _updateDownloadState: disk-check fallback now guarded by _isQueued
(previous state) so it only fires on the queued→not-queued transition,
not on every song-download event for every open screen
- deletePlaylistDownloads: accepts List<Song> and deletes art_{id}.jpg
files alongside song files so stale offline art doesn't persist after
removing downloads
- AlbumArtwork: skip File.existsSync() when downloadedSongIds is empty,
avoiding synchronous disk stats on every build for non-downloading users
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR implements a comprehensive offline download management system with reactive state binding, file validation, and persistence. It reworks player navigation to support shuffle and repeat-all wraparound, adds release APK build infrastructure via Docker, and integrates offline status indicators throughout the UI. ChangesOffline Download System and Player Enhancements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
Closing to rebase on current upstream before re-opening. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
Full offline download workflow for playlists and albums:
cloud_done(complete, tap to remove)coverArtID (not just song ID) soAlbumArtworkcan resolve it in list/grid views when offline/downloadendpoint (no transcoding) and checkssong.sizeto detect partial files; 64 KB floor fallback when size is unknownWhat I kept out
Known limitation
If any song permanently fails to download, the spinner stays on that playlist until the app restarts (when
resumeIncompleteDownloadsre-queues only the missing songs). Acceptable tradeoff vs adding a distinct error state.Summary by CodeRabbit
New Features
Bug Fixes