Skip to content

feat: add album download button #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rshackleton wants to merge 1 commit into
base: master
Choose a base branch
from
+56 −0
Open

feat: add album download button #115

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions lib/screens/album_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import '../theme/app_theme.dart';
import '../widgets/widgets.dart';
import 'artist_screen.dart';
import '../l10n/app_localizations.dart';
import '../services/offline_service.dart';

class AlbumScreen extends StatefulWidget {
final String albumId;
Expand All @@ -23,6 +24,7 @@ class _AlbumScreenState extends State<AlbumScreen> {
Album? _album;
List<Song> _songs = [];
bool _isLoading = true;
bool _isDownloading = false;

@override
void initState() {
Expand Down Expand Up @@ -81,6 +83,42 @@ class _AlbumScreenState extends State<AlbumScreen> {
playerProvider.playSong(playlist.first, playlist: playlist, startIndex: 0);
}

Future<void> _downloadAlbum() async {
if (_songs.isEmpty) return;

final subsonicService = Provider.of<SubsonicService>(
context,
listen: false,
);
final offlineService = OfflineService();
await offlineService.initialize();

setState(() => _isDownloading = true);

offlineService.startBackgroundDownload(_songs, subsonicService).then((_) {
if (mounted) {
setState(() => _isDownloading = false);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
'Downloaded ${_songs.length} songs from ${_album!.name}',
),
duration: const Duration(seconds: 3),
),
);
Comment on lines +98 to +108
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid unconditional success messaging after completion.

At Line 104, the snackbar claims all songs were downloaded, but OfflineService.startBackgroundDownload() absorbs failures and still completes normally. This can report success on partial/failed downloads.

Suggested adjustment in this file (safe interim) 
-            content: Text(
-              'Downloaded ${_songs.length} songs from ${_album!.name}',
-            ),
+            content: Text(
+              'Album download finished. Some songs may have failed.',
+            ),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
offlineService.startBackgroundDownload(_songs, subsonicService).then((_) {
if (mounted) {
setState(() => _isDownloading = false);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
'Downloaded ${_songs.length} songs from ${_album!.name}',
),
duration: const Duration(seconds: 3),
),
);
offlineService.startBackgroundDownload(_songs, subsonicService).then((_) {
if (mounted) {
setState(() => _isDownloading = false);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
'Album download finished. Some songs may have failed.',
),
duration: const Duration(seconds: 3),
),
);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/screens/album_screen.dart` around lines 98 - 108, The success snackbar is
unconditional because offlineService.startBackgroundDownload(_songs,
subsonicService) currently completes even when some downloads fail; change
startBackgroundDownload to return a result (e.g., numberDownloaded, list of
failed items, or a success flag) or throw on fatal error, then await that result
in album_screen and use it to set _isDownloading and craft the SnackBar message:
reference offlineService.startBackgroundDownload, _songs and _album to compute
the actual downloaded count and/or report failures, and only show the
"Downloaded X songs" message when the returned result confirms X > 0 (otherwise
show a partial/failure message).

}
});

if (mounted) {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text('Downloading ${_songs.length} songs in background…'),
Comment on lines +104 to +115
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Localize newly added UI strings.

At Lines 104, 115, and 253, strings are hardcoded in English. This bypasses AppLocalizations used elsewhere in this screen.

Also applies to: 253-253

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/screens/album_screen.dart` around lines 104 - 115, Replace the hardcoded
English SnackBar and Text strings with localized strings from AppLocalizations:
use AppLocalizations.of(context)! to build the messages that include dynamic
values (_songs.length and _album!.name) and update the SnackBar content in the
ScaffoldMessenger/ SnackBar code path (the block that constructs 'Downloaded
${_songs.length} songs from ${_album!.name}' and the 'Downloading
${_songs.length} songs in background…' message). Ensure you add or use
appropriate keys in AppLocalizations (e.g., downloadedSongs and
downloadingInBackground) and format them with the dynamic values so both
SnackBars and any other hardcoded string at the referenced location (line ~253)
are localized.

duration: const Duration(seconds: 2),
),
);
}
}

@override
Widget build(BuildContext context) {
final theme = Theme.of(context);
Expand Down Expand Up @@ -160,6 +198,10 @@ class _AlbumScreenState extends State<AlbumScreen> {
final hours = totalDuration ~/ 3600;
final minutes = (totalDuration % 3600) ~/ 60;

final isOffline =
Provider.of<AuthProvider>(context, listen: false).state ==
AuthState.offlineMode;
Comment on lines +201 to +203
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "album_screen.dart" | head -5

Repository: dddevid/Musly

Length of output: 88

🏁 Script executed:

find . -type f -name "auth_provider.dart" | head -5

Repository: dddevid/Musly

Length of output: 91

🏁 Script executed:

cat -n lib/screens/album_screen.dart | sed -n '195,260p'

Repository: dddevid/Musly

Length of output: 2968

🏁 Script executed:

cat -n lib/providers/auth_provider.dart

Repository: dddevid/Musly

Length of output: 11420

Make auth gating reactive and restrict download to authenticated state.

At line 202, using listen: false in build makes this value stale if auth state changes. Also, hiding only for offlineMode is too permissive—the download button currently shows in unsafe states like unauthenticated, error, and serverUnreachable. Only authenticated users should be able to download.

Replace with reactive pattern using context.watch<AuthProvider>() and gate the button to AuthState.authenticated only.

Also applies to: 251

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@lib/screens/album_screen.dart` around lines 201 - 203, The current build uses
Provider.of<AuthProvider>(context, listen: false) to set isOffline which
prevents UI updates when auth changes and only checks for AuthState.offlineMode;
change both usages (the isOffline declaration and the other check around the
download button) to use context.watch<AuthProvider>() so the widget rebuilds
reactively and replace the gating logic so the download UI/action is allowed
only when watch<AuthProvider>().state == AuthState.authenticated, denying it for
unauthenticated/error/serverUnreachable/offline states.


return Scaffold(
body: Stack(
children: [
Expand Down Expand Up @@ -205,6 +247,20 @@ class _AlbumScreenState extends State<AlbumScreen> {
],
),
),
actions: [
if (!isOffline)
IconButton(
tooltip: 'Download album',
onPressed: _isDownloading ? null : _downloadAlbum,
icon: _isDownloading
? const SizedBox(
width: 20,
height: 20,
child: CircularProgressIndicator(strokeWidth: 2),
)
: const Icon(CupertinoIcons.cloud_download),
),
],
),
SliverToBoxAdapter(
child: Padding(
Expand Down