diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 1169204a..5b67a423 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -16,6 +16,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" "git.fabledsword.com/bvandeusen/minstrel/internal/library" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" "git.fabledsword.com/bvandeusen/minstrel/internal/logging" @@ -173,7 +174,18 @@ func run() error { // threading the bus through RunScan + TryStartScan + the Scheduler // + every test caller. Per-process singleton, set once at startup. library.SetEventBus(bus) - lidarrReconciler := lidarrrequests.NewReconciler(pool, lidarrCfg, logger.With("component", "lidarr"), bus) + // Per-tick Lidarr client factory: re-reads config so an admin save + // takes effect without a restart, returning nil while Lidarr is + // disabled/unconfigured. Mirrors server.go's lidarrClientFn; the + // reconciler uses it to (re)send unconfirmed adds. + lidarrClientFn := func() *lidarr.Client { + c, cerr := lidarrCfg.Get(ctx) + if cerr != nil || !c.Enabled || c.BaseURL == "" || c.APIKey == "" { + return nil + } + return lidarr.NewClient(c.BaseURL, c.APIKey) + } + lidarrReconciler := lidarrrequests.NewReconciler(pool, lidarrCfg, lidarrClientFn, logger.With("component", "lidarr"), bus) go lidarrReconciler.Run(ctx) // library_changes compactor (#357 follow-up). Daily tick; deletes diff --git a/flutter_client/android/app/src/main/res/drawable/ic_stat_favorite.xml b/flutter_client/android/app/src/main/res/drawable/ic_stat_favorite.xml new file mode 100644 index 00000000..9c9aca56 --- /dev/null +++ b/flutter_client/android/app/src/main/res/drawable/ic_stat_favorite.xml @@ -0,0 +1,11 @@ + + + + diff --git a/flutter_client/android/app/src/main/res/drawable/ic_stat_favorite_border.xml b/flutter_client/android/app/src/main/res/drawable/ic_stat_favorite_border.xml new file mode 100644 index 00000000..96ecf90f --- /dev/null +++ b/flutter_client/android/app/src/main/res/drawable/ic_stat_favorite_border.xml @@ -0,0 +1,15 @@ + + + + diff --git a/flutter_client/lib/admin/admin_landing_screen.dart b/flutter_client/lib/admin/admin_landing_screen.dart index 3849c77f..79d0a03c 100644 --- a/flutter_client/lib/admin/admin_landing_screen.dart +++ b/flutter_client/lib/admin/admin_landing_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -34,7 +35,7 @@ class AdminLandingScreen extends ConsumerWidget { children: [ AdminSectionCard( key: const Key('admin_card_requests'), - icon: Icons.inbox, + icon: LucideIcons.inbox, title: 'Requests', subtitle: 'Approve or reject Lidarr requests', count: c.requests, @@ -42,7 +43,7 @@ class AdminLandingScreen extends ConsumerWidget { ), AdminSectionCard( key: const Key('admin_card_quarantine'), - icon: Icons.warning_amber, + icon: LucideIcons.triangle_alert, title: 'Quarantine', subtitle: 'Resolve scan failures', count: c.quarantine, @@ -50,7 +51,7 @@ class AdminLandingScreen extends ConsumerWidget { ), AdminSectionCard( key: const Key('admin_card_users'), - icon: Icons.people, + icon: LucideIcons.users, title: 'Users', subtitle: 'Manage accounts and invites', count: c.users, diff --git a/flutter_client/lib/admin/admin_users_screen.dart b/flutter_client/lib/admin/admin_users_screen.dart index d12226e2..122a5891 100644 --- a/flutter_client/lib/admin/admin_users_screen.dart +++ b/flutter_client/lib/admin/admin_users_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter/services.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; @@ -59,7 +60,7 @@ class AdminUsersScreen extends ConsumerWidget { trailing: TextButton.icon( key: const Key('invite_generate_button'), onPressed: () => _showGenerateInvite(context, ref), - icon: Icon(Icons.add, color: fs.parchment), + icon: Icon(LucideIcons.plus, color: fs.parchment), label: Text('Generate', style: TextStyle(color: fs.parchment)), ), diff --git a/flutter_client/lib/admin/widgets/admin_quarantine_row.dart b/flutter_client/lib/admin/widgets/admin_quarantine_row.dart index 6e81e7e7..8221b82b 100644 --- a/flutter_client/lib/admin/widgets/admin_quarantine_row.dart +++ b/flutter_client/lib/admin/widgets/admin_quarantine_row.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import '../../models/admin_quarantine_item.dart'; import '../../theme/theme_extension.dart'; @@ -45,7 +46,7 @@ class AdminQuarantineRow extends StatelessWidget { ), trailing: PopupMenuButton( key: Key('admin_quarantine_menu_${item.trackId}'), - icon: Icon(Icons.more_vert, color: fs.parchment), + icon: Icon(LucideIcons.ellipsis_vertical, color: fs.parchment), onSelected: (action) async { switch (action) { case 'resolve': diff --git a/flutter_client/lib/admin/widgets/admin_user_row.dart b/flutter_client/lib/admin/widgets/admin_user_row.dart index b029f748..e4deee49 100644 --- a/flutter_client/lib/admin/widgets/admin_user_row.dart +++ b/flutter_client/lib/admin/widgets/admin_user_row.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import '../../models/admin_user.dart'; import '../../theme/theme_extension.dart'; @@ -29,7 +30,7 @@ class AdminUserRow extends StatelessWidget { _Badge(label: 'auto-approve', color: fs.moss), ], ), - trailing: Icon(Icons.chevron_right, color: fs.ash), + trailing: Icon(LucideIcons.chevron_right, color: fs.ash), onTap: onTap, ); } diff --git a/flutter_client/lib/admin/widgets/invite_row.dart b/flutter_client/lib/admin/widgets/invite_row.dart index 6b74a6a5..5a9987dd 100644 --- a/flutter_client/lib/admin/widgets/invite_row.dart +++ b/flutter_client/lib/admin/widgets/invite_row.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter/services.dart'; import '../../models/invite.dart'; @@ -31,7 +32,7 @@ class InviteRow extends StatelessWidget { ), ), IconButton( - icon: Icon(Icons.copy, color: fs.ash, size: 18), + icon: Icon(LucideIcons.copy, color: fs.ash, size: 18), tooltip: 'Copy', onPressed: () => Clipboard.setData(ClipboardData(text: invite.token)), @@ -63,7 +64,7 @@ class InviteRow extends StatelessWidget { ], ), trailing: IconButton( - icon: Icon(Icons.delete_outline, color: fs.oxblood), + icon: Icon(LucideIcons.trash_2, color: fs.oxblood), tooltip: 'Revoke', onPressed: onRevoke, ), diff --git a/flutter_client/lib/app.dart b/flutter_client/lib/app.dart index 93a6684c..2c9a5160 100644 --- a/flutter_client/lib/app.dart +++ b/flutter_client/lib/app.dart @@ -6,8 +6,10 @@ import 'cache/metadata_prefetcher.dart'; import 'cache/offline_provider.dart'; import 'cache/mutation_queue.dart'; import 'cache/prefetcher.dart'; +import 'cache/resume_controller.dart'; import 'cache/sync_controller.dart'; import 'player/play_events_reporter.dart'; +import 'player/playback_error_reporter.dart'; import 'shared/live_events_dispatcher.dart'; import 'shared/routing.dart'; import 'theme/theme_data.dart'; @@ -62,6 +64,15 @@ class _MinstrelAppState extends ConsumerState { // scoring, scrobbles, and system-playlist rotation, and is the // path that carries the `source` tag for #415. ref.read(playEventsReporterProvider); + // Resume-on-launch (#54): restores the last persisted session + // (paused) and persists queue/index/position on track change, + // pause, and app teardown. Pairs with the #52 idle teardown so a + // torn-down session is recoverable instead of lost. + ref.read(resumeControllerProvider); + // Playback-error reporter (#58): turns the handler's silent + // dead-track skips into a debounced/coalesced SnackBar so a + // vanished track isn't mysterious (and aids server/cache debug). + ref.read(playbackErrorReporterProvider); // Offline marker (#427 S1): periodic /healthz reachability // probe → offlineProvider. Read here to start the poller; S4 // gates system-playlist play + Shuffle-all on it. @@ -75,6 +86,7 @@ class _MinstrelAppState extends ConsumerState { final mode = ref.watch(themeModeProvider).value ?? AppThemeMode.system; return MaterialApp.router( title: 'Minstrel', + scaffoldMessengerKey: scaffoldMessengerKey, theme: buildLightTheme(), darkTheme: buildDarkTheme(), themeMode: mode.materialMode, diff --git a/flutter_client/lib/cache/db.dart b/flutter_client/lib/cache/db.dart index 824adcb9..c46aaacd 100644 --- a/flutter_client/lib/cache/db.dart +++ b/flutter_client/lib/cache/db.dart @@ -230,6 +230,20 @@ class CachedQuarantineMine extends Table { Set get primaryKey => {trackId}; } +/// Single-row snapshot of the last playback session — queue (TrackRef +/// JSON), current index, position, and #415 source. Lets a torn-down +/// session (the #52 idle/dismissed teardown) resume on next launch; +/// without it the headset / lock-screen play button has nothing to +/// resume. Mirrors the CachedHomeSnapshot single-row JSON pattern. +/// Schema 10+. +class CachedResumeState extends Table { + IntColumn get id => integer().withDefault(const Constant(1))(); + TextColumn get json => text()(); + DateTimeColumn get updatedAt => dateTime().withDefault(currentDateAndTime)(); + @override + Set get primaryKey => {id}; +} + enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental } @DriftDatabase(tables: [ @@ -247,12 +261,13 @@ enum CacheSource { manual, autoLiked, autoPlaylist, autoPrefetch, incidental } CachedHomeIndex, CachedSystemPlaylistsStatus, CachedMutations, + CachedResumeState, ]) class AppDb extends _$AppDb { AppDb([QueryExecutor? e]) : super(e ?? driftDatabase(name: 'minstrel_cache')); @override - int get schemaVersion => 9; + int get schemaVersion => 10; @override MigrationStrategy get migration => MigrationStrategy( @@ -316,6 +331,12 @@ class AppDb extends _$AppDb { 'UPDATE audio_cache_index SET last_played_at = cached_at', ); } + if (from < 10) { + // Schema 10 (#54): cached_resume_state — single-row last- + // session snapshot for resume-on-launch. Empty on upgrade; + // first persist populates it. + await m.createTable(cachedResumeState); + } }, ); } diff --git a/flutter_client/lib/cache/resume_controller.dart b/flutter_client/lib/cache/resume_controller.dart new file mode 100644 index 00000000..80878dd5 --- /dev/null +++ b/flutter_client/lib/cache/resume_controller.dart @@ -0,0 +1,183 @@ +// Resume-on-launch (#54). The #52 teardown tears the audio_service +// session down (and clears mediaItem) when idle/dismissed, so the +// headset / lock-screen play button otherwise has nothing to resume. +// This controller persists the live queue + index + position + #415 +// source to a single-row drift snapshot on track change / pause / app +// teardown, and on construction restores the last snapshot into the +// handler PAUSED (the user sees their last track and continues with +// play / a media button — we never auto-blast on launch). +// +// Persist guard: when the handler clears (teardown → mediaItem null / +// empty queue) we deliberately do NOT write, so the last good snapshot +// survives for the next launch — that survival is the whole point. + +import 'dart:async'; +import 'dart:convert'; + +import 'package:drift/drift.dart' as drift; +import 'package:flutter/widgets.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import '../auth/auth_provider.dart'; +import '../models/track.dart'; +import '../player/player_provider.dart'; +import 'audio_cache_manager.dart' show appDbProvider; +import 'db.dart'; + +class ResumeController with WidgetsBindingObserver { + ResumeController(this._ref); + final Ref _ref; + + final _subs = >[]; + Timer? _debounce; + bool _disposed = false; + + Future start() async { + try { + // audioHandlerProvider throws until main() overrides it (the real + // app always does). In tests / no-audio environments there's + // nothing to resume — stay inert. + _ref.read(audioHandlerProvider); + } catch (_) { + return; + } + if (_disposed) return; + await _restore(); + if (_disposed) return; + final h = _ref.read(audioHandlerProvider); + // Media-button-when-torn-down hook (#448): play() invokes this when + // mediaItem is null so a headset/watch press resumes the snapshot. + h.setResumeHook(resumeFromMediaButton); + _subs.add(h.mediaItem.listen((_) => _schedulePersist())); + _subs.add(h.playbackState.listen((_) => _schedulePersist())); + WidgetsBinding.instance.addObserver(this); + } + + void _schedulePersist() { + if (_disposed) return; + _debounce?.cancel(); + _debounce = Timer(const Duration(seconds: 3), () { + unawaited(_persist()); + }); + } + + Future _persist() async { + if (_disposed) return; + try { + final h = _ref.read(audioHandlerProvider); + final tracks = h.queuedTracks; + final mi = h.mediaItem.value; + // Cleared by teardown — keep the last good snapshot for next + // launch rather than wiping it with an empty queue. + if (tracks.isEmpty || mi == null) return; + var idx = tracks.indexWhere((t) => t.id == mi.id); + if (idx < 0) idx = 0; + final blob = jsonEncode({ + 'v': 1, + 'source': h.queueSource, + 'index': idx, + 'position_ms': h.position.inMilliseconds, + 'tracks': tracks.map((t) => t.toJson()).toList(), + }); + final db = _ref.read(appDbProvider); + await db.into(db.cachedResumeState).insertOnConflictUpdate( + CachedResumeStateCompanion.insert( + json: blob, + updatedAt: drift.Value(DateTime.now()), + ), + ); + } catch (e, st) { + debugPrint('resume_controller: persist failed: $e\n$st'); + } + } + + /// Launch path: restore the last session PAUSED (no auto-blast). + Future _restore() async { + try { + await _loadAndRestore(); + } catch (e, st) { + debugPrint('resume_controller: restore failed: $e\n$st'); + } + } + + /// Media-button path (#448): the user pressed play on the headset / + /// watch / lock screen while the session was fully torn down (#52) and + /// nothing is loaded. Restore the snapshot, then START playback (they + /// asked to play). Registered as the handler's resume hook in start(). + /// No-op if there's nothing to resume. + Future resumeFromMediaButton() async { + try { + if (await _loadAndRestore()) { + await _ref.read(audioHandlerProvider).play(); + } + } catch (e, st) { + debugPrint('resume_controller: media-button resume failed: $e\n$st'); + } + } + + /// Shared loader. Restores the last persisted session PAUSED and + /// returns whether it actually restored a queue. Guards: an already- + /// active session (don't stomp), missing auth, no/empty snapshot. + Future _loadAndRestore() async { + final h = _ref.read(audioHandlerProvider); + if (h.mediaItem.value != null) return false; + final url = await _ref.read(serverUrlProvider.future); + final token = + await _ref.read(secureStorageProvider).read(key: 'session_token'); + if (url == null || url.isEmpty || token == null || token.isEmpty) { + return false; + } + final db = _ref.read(appDbProvider); + final row = await db.select(db.cachedResumeState).getSingleOrNull(); + if (row == null) return false; + final m = jsonDecode(row.json) as Map; + final rawTracks = (m['tracks'] as List?) ?? const []; + final tracks = rawTracks + .map((e) => TrackRef.fromJson(e as Map)) + .toList(); + if (tracks.isEmpty) return false; + final idx = (m['index'] as num?)?.toInt() ?? 0; + final posMs = (m['position_ms'] as num?)?.toInt() ?? 0; + final source = m['source'] as String?; + await _ref.read(playerActionsProvider).restoreQueue( + tracks, + initialIndex: idx, + position: Duration(milliseconds: posMs), + source: source, + ); + return true; + } + + @override + void didChangeAppLifecycleState(AppLifecycleState state) { + // App backgrounded / killed: persist durably right now (the debounce + // may not fire before teardown). + if (state == AppLifecycleState.paused || + state == AppLifecycleState.detached) { + _debounce?.cancel(); + unawaited(_persist()); + } + } + + void dispose() { + _disposed = true; + _debounce?.cancel(); + WidgetsBinding.instance.removeObserver(this); + for (final s in _subs) { + s.cancel(); + } + _subs.clear(); + } +} + +/// Read once at app start (app.dart postFrame). On construction it +/// restores the last persisted session (paused) then persists +/// queue/index/position on track change, pause, and app teardown. +/// Disposed via ref.onDispose when the scope tears down. +final resumeControllerProvider = Provider((ref) { + final c = ResumeController(ref); + ref.onDispose(c.dispose); + // ignore: unawaited_futures + c.start(); + return c; +}); diff --git a/flutter_client/lib/discover/discover_screen.dart b/flutter_client/lib/discover/discover_screen.dart index 9519d109..bee54a48 100644 --- a/flutter_client/lib/discover/discover_screen.dart +++ b/flutter_client/lib/discover/discover_screen.dart @@ -1,6 +1,7 @@ import 'package:cached_network_image/cached_network_image.dart'; import 'package:dio/dio.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -167,7 +168,7 @@ class _DiscoverScreenState extends ConsumerState { backgroundColor: fs.obsidian, elevation: 0, leading: IconButton( - icon: Icon(Icons.arrow_back, color: fs.parchment), + icon: Icon(LucideIcons.arrow_left, color: fs.parchment), onPressed: () => context.pop(), ), title: Text('Discover', style: TextStyle(color: fs.parchment)), @@ -193,7 +194,7 @@ class _DiscoverScreenState extends ConsumerState { borderSide: BorderSide(color: fs.accent), ), suffixIcon: IconButton( - icon: Icon(Icons.search, color: fs.ash), + icon: Icon(LucideIcons.search, color: fs.ash), onPressed: _runSearch, ), ), @@ -336,14 +337,14 @@ class _ResultTile extends StatelessWidget { height: 56, color: fs.slate, child: row.imageUrl.isEmpty - ? Icon(Icons.album, color: fs.ash) + ? Icon(LucideIcons.disc_3, color: fs.ash) : CachedNetworkImage( imageUrl: row.imageUrl, fit: BoxFit.cover, fadeInDuration: const Duration(milliseconds: 120), fadeOutDuration: Duration.zero, errorWidget: (_, __, ___) => - Icon(Icons.album, color: fs.ash), + Icon(LucideIcons.disc_3, color: fs.ash), ), ), ), @@ -419,14 +420,14 @@ class _SuggestionTile extends StatelessWidget { height: 56, color: fs.slate, child: s.imageUrl.isEmpty - ? Icon(Icons.person, color: fs.ash) + ? Icon(LucideIcons.user, color: fs.ash) : CachedNetworkImage( imageUrl: s.imageUrl, fit: BoxFit.cover, fadeInDuration: const Duration(milliseconds: 120), fadeOutDuration: Duration.zero, errorWidget: (_, __, ___) => - Icon(Icons.person, color: fs.ash), + Icon(LucideIcons.user, color: fs.ash), ), ), ), diff --git a/flutter_client/lib/library/album_detail_screen.dart b/flutter_client/lib/library/album_detail_screen.dart index 9b628906..621a8fcd 100644 --- a/flutter_client/lib/library/album_detail_screen.dart +++ b/flutter_client/lib/library/album_detail_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/likes.dart'; @@ -115,7 +116,7 @@ class AlbumDetailScreen extends ConsumerWidget { width: 48, height: 48, decoration: BoxDecoration(color: fs.accent, shape: BoxShape.circle), child: IconButton( - icon: Icon(Icons.play_arrow, color: fs.parchment), + icon: Icon(LucideIcons.play, color: fs.parchment), onPressed: () => ref.read(playerActionsProvider).playTracks(r.tracks), ), ), diff --git a/flutter_client/lib/library/artist_detail_screen.dart b/flutter_client/lib/library/artist_detail_screen.dart index 9f83ec38..f893c4fc 100644 --- a/flutter_client/lib/library/artist_detail_screen.dart +++ b/flutter_client/lib/library/artist_detail_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -90,7 +91,7 @@ class ArtistDetailScreen extends ConsumerWidget { width: 48, height: 48, decoration: BoxDecoration(color: fs.accent, shape: BoxShape.circle), child: IconButton( - icon: Icon(Icons.play_arrow, color: fs.parchment), + icon: Icon(LucideIcons.play, color: fs.parchment), onPressed: () async { try { final tracks = diff --git a/flutter_client/lib/library/home_screen.dart b/flutter_client/lib/library/home_screen.dart index a3eff3dc..df488102 100644 --- a/flutter_client/lib/library/home_screen.dart +++ b/flutter_client/lib/library/home_screen.dart @@ -1,5 +1,6 @@ import 'package:dio/dio.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -245,12 +246,12 @@ class _PlaylistsSection extends StatelessWidget { if (offline) ...const [ _OfflinePoolCard( label: 'Recently played', - icon: Icons.history, + icon: LucideIcons.history, kind: _OfflinePoolKind.recentlyPlayed, ), _OfflinePoolCard( label: 'Liked', - icon: Icons.favorite, + icon: LucideIcons.heart, kind: _OfflinePoolKind.liked, ), ], diff --git a/flutter_client/lib/library/library_screen.dart b/flutter_client/lib/library/library_screen.dart index d6cb3a93..4695bccb 100644 --- a/flutter_client/lib/library/library_screen.dart +++ b/flutter_client/lib/library/library_screen.dart @@ -2,6 +2,7 @@ import 'dart:convert'; import 'package:drift/drift.dart' as drift; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -382,7 +383,7 @@ class _LibraryScreenState extends ConsumerState IconButton( key: const Key('shuffle_all_button'), tooltip: 'Shuffle all', - icon: Icon(Icons.shuffle, color: fs.parchment), + icon: Icon(LucideIcons.shuffle, color: fs.parchment), onPressed: () async { final messenger = ScaffoldMessenger.of(context); final refs = await ref.read(shuffleSourceProvider).tracks(); @@ -866,7 +867,7 @@ class _QuarantineTile extends ConsumerWidget { ), IconButton( tooltip: 'Unhide', - icon: Icon(Icons.restore, color: fs.ash, size: 20), + icon: Icon(LucideIcons.archive_restore, color: fs.ash, size: 20), onPressed: () async { try { await ref.read(myQuarantineProvider.notifier).unflag(row.trackId); diff --git a/flutter_client/lib/library/widgets/cached_indicator.dart b/flutter_client/lib/library/widgets/cached_indicator.dart index 8a3c0d84..685aabc5 100644 --- a/flutter_client/lib/library/widgets/cached_indicator.dart +++ b/flutter_client/lib/library/widgets/cached_indicator.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../cache/audio_cache_manager.dart'; @@ -21,7 +22,7 @@ class CachedIndicator extends ConsumerWidget { if (snap.data != true) return const SizedBox.shrink(); return Padding( padding: const EdgeInsets.only(left: 4), - child: Icon(Icons.download_done, size: 14, color: fs.accent), + child: Icon(LucideIcons.circle_check_big, size: 14, color: fs.accent), ); }, ); diff --git a/flutter_client/lib/library/widgets/play_circle_button.dart b/flutter_client/lib/library/widgets/play_circle_button.dart index 6c76dac7..acfb0562 100644 --- a/flutter_client/lib/library/widgets/play_circle_button.dart +++ b/flutter_client/lib/library/widgets/play_circle_button.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import '../../theme/theme_extension.dart'; @@ -85,7 +86,7 @@ class _PlayCircleButtonState extends State { ), ) : Icon( - Icons.play_arrow, + LucideIcons.play, color: fs.parchment, size: iconSize, ), diff --git a/flutter_client/lib/likes/like_button.dart b/flutter_client/lib/likes/like_button.dart index 8a43af3c..729d15da 100644 --- a/flutter_client/lib/likes/like_button.dart +++ b/flutter_client/lib/likes/like_button.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../api/endpoints/likes.dart'; +import '../shared/widgets/lucide_heart.dart'; import '../theme/theme_extension.dart'; import 'likes_provider.dart'; @@ -25,8 +26,8 @@ class LikeButton extends ConsumerWidget { orElse: () => false, ); return IconButton( - icon: Icon( - liked ? Icons.favorite : Icons.favorite_border, + icon: LucideHeart( + filled: liked, color: liked ? fs.accent : fs.ash, size: size, ), diff --git a/flutter_client/lib/main.dart b/flutter_client/lib/main.dart index dfeaa143..88d5cec9 100644 --- a/flutter_client/lib/main.dart +++ b/flutter_client/lib/main.dart @@ -14,6 +14,13 @@ Future main() async { androidNotificationChannelId: 'com.fabledsword.minstrel.audio', androidNotificationChannelName: 'Minstrel playback', androidNotificationOngoing: true, + // 8a: hand external surfaces (notification / lock screen / Wear) + // a downscaled cover instead of full-res album art — smaller + // payload, faster paint, lower memory. 300px is ample for those + // targets. preloadArtwork warms it so the first paint isn't blank. + artDownscaleWidth: 300, + artDownscaleHeight: 300, + preloadArtwork: true, ), ); runApp(ProviderScope( diff --git a/flutter_client/lib/models/track.dart b/flutter_client/lib/models/track.dart index 8c270776..1e5adc58 100644 --- a/flutter_client/lib/models/track.dart +++ b/flutter_client/lib/models/track.dart @@ -40,4 +40,19 @@ class TrackRef { durationSec: (j['duration_sec'] as num?)?.toInt() ?? 0, streamUrl: j['stream_url'] as String? ?? '', ); + + /// Round-trips through [TrackRef.fromJson] (same server snake_case + /// keys). Used to persist the playback queue for resume-on-launch. + Map toJson() => { + 'id': id, + 'title': title, + 'album_id': albumId, + 'album_title': albumTitle, + 'artist_id': artistId, + 'artist_name': artistName, + if (trackNumber != null) 'track_number': trackNumber, + if (discNumber != null) 'disc_number': discNumber, + 'duration_sec': durationSec, + 'stream_url': streamUrl, + }; } diff --git a/flutter_client/lib/player/audio_handler.dart b/flutter_client/lib/player/audio_handler.dart index b8156f08..1b801160 100644 --- a/flutter_client/lib/player/audio_handler.dart +++ b/flutter_client/lib/player/audio_handler.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:io'; import 'package:audio_service/audio_service.dart'; +import 'package:audio_session/audio_session.dart'; import 'package:flutter/foundation.dart'; import 'package:just_audio/just_audio.dart'; import 'package:path_provider/path_provider.dart'; @@ -38,6 +39,7 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // the index and the eviction loop can't reclaim them. _player.bufferedPositionStream .listen((_) => unawaited(_maybeRegisterStreamCache())); + unawaited(_configureAudioSession()); } final AudioPlayer _player = AudioPlayer(); @@ -57,6 +59,32 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// time the buffered-position stream emits (~200ms cadence). String? _cacheDirPath; + /// How long the session may sit not-actively-playing (paused, or a + /// finished queue) before we tear it down so the Wear tile / lock + /// screen / notification don't linger on a stale track. The + /// notification is configured ongoing (main.dart), so nothing else + /// ever drives the MediaSession to a terminal state. + static const _idleStopTimeout = Duration(minutes: 5); + + /// Single-shot cleanup timer, armed while not actively playing and + /// cancelled the moment playback resumes or a new queue is set. + Timer? _idleStopTimer; + + /// Periodic PlaybackState re-broadcast while actively playing so + /// external surfaces (lock screen, Wear, Android Auto) interpolate the + /// scrubber smoothly. The in-app bar uses positionStream and doesn't + /// need this. Active only while playing; cancelled otherwise. + Timer? _positionBroadcastTimer; + + /// Player volume captured when an OS duck interruption begins, + /// restored when it ends. Null when not currently ducked. + double? _volumeBeforeDuck; + + /// True when an interruption (call / other media) paused playback that + /// was actively going, so a transient (pause-type) interruption end + /// auto-resumes. Cleared on resume or a non-resumable end. + bool _interruptedWhilePlaying = false; + /// True while _fillRemainingSources is doing backward-fill inserts /// at index 0..initialIndex-1. Each insert shifts the player's /// currentIndex (it tracks the actively-playing source through @@ -74,6 +102,20 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// new queue, leaving the player "locked" to a corrupted state. int _queueGeneration = 0; + /// Logical-queue index that just_audio player-index 0 currently maps + /// to. setQueueFromTracks fast-starts with a SINGLE source at player + /// index 0 while queue.value already holds the full list, so the + /// playing track's logical index is clampedInitial, not 0. The + /// transient currentIndexStream→0 emission that arrives just after + /// setAudioSources resolves (and after _suppressIndexUpdates is back + /// to false) would otherwise make _onCurrentIndexChanged broadcast + /// queue.value[0] — the FIRST track — over the correct item, pinning + /// the mini bar / playlist marker to the wrong track until a later + /// index event. Decremented in lockstep with the backward fill's + /// front inserts so player-index → logical-index stays correct and + /// lands at 0 once the player list fully matches queue.value. + int _logicalIndexBase = 0; + /// Tracks the most recent setQueueFromTracks() input so /// skipToQueueItem can reconstruct the source list. just_audio /// requires every source to be built before it can be a skip @@ -91,6 +133,11 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl String? _queueSource; String? get queueSource => _queueSource; + /// The full TrackRef list backing the current queue (even when only + /// part is built as just_audio sources). Empty when nothing is + /// queued. Exposed for resume-state persistence (#54). + List get queuedTracks => _lastTracks; + /// Volume stream for UI subscribers. Mirrors the just_audio player's /// volume directly; set via setVolume(double). Stream get volumeStream => _player.volumeStream; @@ -104,6 +151,14 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl Stream get positionStream => _player.positionStream; Duration get position => _player.position; + /// Broadcasts the title of a track that just failed to play (404, + /// decoder failure, premature EOS, network drop) and was auto-skipped + /// or paused by _handlePlaybackError. The app listens and surfaces a + /// debounced/coalesced SnackBar so a silent skip isn't mysterious. + /// App-lifetime singleton handler — intentionally never closed. + final _playbackErrors = StreamController.broadcast(); + Stream get playbackErrorStream => _playbackErrors.stream; + void configure({ required String baseUrl, required String? token, @@ -118,12 +173,24 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl if (audioCacheManager != null) _audioCacheManager = audioCacheManager; } + /// Invoked by play() when nothing is loaded (mediaItem == null) so a + /// media-button press after the #52 teardown resumes the last + /// persisted session (#448). Registered by ResumeController.start(); + /// null until then (then it's a no-op fall-through to _player.play()). + Future Function()? _resumeHook; + void setResumeHook(Future Function() hook) => _resumeHook = hook; + Future setQueueFromTracks( List tracks, { int initialIndex = 0, String? source, }) async { if (tracks.isEmpty) return; + // New playback supersedes any pending idle cleanup. play() below + // re-broadcasts and _reconcileIdleTimer would cancel anyway; doing + // it up front avoids a race during the pause→swap window. + _idleStopTimer?.cancel(); + _idleStopTimer = null; _queueSource = source; final clampedInitial = initialIndex.clamp(0, tracks.length - 1); @@ -178,6 +245,11 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl _suppressIndexUpdates = true; try { await _player.setAudioSources([initial], initialIndex: 0); + // Player-index 0 holds the single fast-start source, which is + // logical-queue index clampedInitial. Record the offset before + // broadcasting so the post-resolve currentIndexStream→0 emission + // maps back to the correct item instead of queue.value[0]. + _logicalIndexBase = clampedInitial; // Broadcast in this order: queue first (so any consumer that // reacts to mediaItem and reads queue.value sees the consistent // pair), then mediaItem. @@ -242,6 +314,11 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl final src = await _buildAudioSource(tracks[i]); if (gen != _queueGeneration) return; await _player.insertAudioSource(i, src); + // Each front insert shifts the playing source's player index + // up by one; drop the base in lockstep so player-index → + // logical-index stays correct (and reaches 0 once the player + // list fully matches queue.value). + _logicalIndexBase -= 1; } } catch (e) { debugPrint('audio_handler: backward fill failed: $e'); @@ -416,6 +493,12 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl /// If we're at the last track, seekToNext is a no-op; the state /// drops to idle and _broadcastState reflects that. Future _handlePlaybackError() async { + // mediaItem is the correctly-mapped current track (post #49 logical + // index), so it's the one that just failed — no player-index math. + final failed = mediaItem.value?.title; + if (failed != null && failed.isNotEmpty) { + _playbackErrors.add(failed); + } final currentIdx = _player.currentIndex; final queueLen = queue.value.length; if (currentIdx == null || currentIdx + 1 >= queueLen) { @@ -447,8 +530,15 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl // track was passed via setQueueFromTracks(initialIndex:) regardless // of skip/auto-advance. final items = queue.value; - if (idx >= 0 && idx < items.length) { - mediaItem.add(items[idx]); + // Map the just_audio player index back to the logical queue index. + // During the fast-start/fill window the player list is a moving + // window offset from queue.value by _logicalIndexBase; mapping the + // raw player index straight in here is what previously broadcast + // queue.value[0] (the first track) over the correct item on every + // fast-start with initialIndex > 0. + final logical = idx + _logicalIndexBase; + if (logical >= 0 && logical < items.length) { + mediaItem.add(items[logical]); } unawaited(_loadArtForCurrentItem()); } @@ -472,8 +562,61 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl mediaItem.add(current.copyWith(artUri: Uri.file(path))); } + /// Drives the session to a terminal state and tears down the + /// foreground service so external surfaces (Wear tile, lock screen, + /// in-app mini bar) drop it instead of showing a stale paused track. + /// Called on the idle timeout and from onTaskRemoved when not playing; + /// no other path stops the session (the notification is ongoing). @override - Future play() => _player.play(); + Future stop() async { + _idleStopTimer?.cancel(); + _idleStopTimer = null; + _positionBroadcastTimer?.cancel(); + _positionBroadcastTimer = null; + try { + await _player.stop(); + } catch (_) {} + // Explicit terminal broadcast before super.stop() tears down the + // isolate/notification, so subscribers see idle even if the + // player's own event lags the service teardown. queue/mediaItem + // cleared so the in-app mini bar collapses in lockstep with the + // watch tile rather than lingering on the last track. + playbackState.add(playbackState.value.copyWith( + playing: false, + processingState: AudioProcessingState.idle, + )); + mediaItem.add(null); + queue.add([]); + await super.stop(); + } + + /// App swiped away from recents. Keep playing if audio is active + /// (standard media behaviour — music shouldn't die because the app + /// left recents); otherwise stop so the watch tile / notification + /// don't linger on a stale paused track. + @override + Future onTaskRemoved() async { + if (_player.playing && + _player.processingState != ProcessingState.completed) { + await super.onTaskRemoved(); + return; + } + await stop(); + } + + @override + Future play() async { + // Nothing loaded (fresh, or torn down by the #52 idle/dismiss + // teardown): a media-button press resumes the last persisted + // session instead of no-op'ing (#448). The hook restores the queue + // and starts playback itself, so we return without calling + // _player.play() on an empty player. + if (mediaItem.value == null && _resumeHook != null) { + await _resumeHook!(); + return; + } + await _player.play(); + } @override Future pause() => _player.pause(); @@ -526,6 +669,12 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl mediaItem.add(media.copyWith(rating: Rating.newHeartRating(liked))); } + /// Re-broadcasts PlaybackState so the notification favorite control's + /// icon/label reflects a like toggled from elsewhere (TrackRow, kebab, + /// another device via SSE). Sibling to refreshCurrentRating, which + /// updates the Wear/lock heart via MediaItem.rating. + void refreshFavoriteControl() => _broadcastState(null); + @override Future setShuffleMode(AudioServiceShuffleMode shuffleMode) async { await _player @@ -550,16 +699,56 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl await _player.setVolume(v.clamp(0.0, 1.0)); } + /// Handles the notification favorite control (and any future custom + /// actions). Toggles the current track's like via the LikeBridge, + /// then re-broadcasts so the heart icon/label flips. Signature + /// matches `AudioHandler.customAction` (`Future`). + @override + Future customAction(String name, + [Map? extras]) async { + if (name == 'minstrel.favorite') { + final media = mediaItem.value; + final bridge = _likeBridge; + if (media == null || bridge == null) return null; + try { + await bridge.toggleTrackLike(media.id); + } catch (_) {} + _broadcastState(null); + return null; + } + return super.customAction(name, extras); + } + // _broadcastState accepts a nullable event because the shuffle/repeat // listeners don't have one — we just want to re-emit PlaybackState // with up-to-date shuffleMode/repeatMode fields. void _broadcastState(PlaybackEvent? event) { final playing = _player.playing; + // Favorite control: only when a track is playing and the LikeBridge + // is wired. Icon/label toggle by current like state; tapping it + // routes to customAction('minstrel.favorite'). Implemented as a + // MediaControl.custom (NOT setRating — that path is broken upstream, + // audio_service #376, and regressed the Pixel Watch). Kept out of + // androidCompactActionIndices so the compact/lock view is unchanged. + final favTrackId = mediaItem.value?.id; + final favBridge = _likeBridge; + final showFav = favTrackId != null && favBridge != null; + final favLiked = favTrackId != null && + favBridge != null && + favBridge.isTrackLiked(favTrackId); playbackState.add(PlaybackState( controls: [ MediaControl.skipToPrevious, if (playing) MediaControl.pause else MediaControl.play, MediaControl.skipToNext, + if (showFav) + MediaControl.custom( + androidIcon: favLiked + ? 'drawable/ic_stat_favorite' + : 'drawable/ic_stat_favorite_border', + label: favLiked ? 'Unfavorite' : 'Favorite', + name: 'minstrel.favorite', + ), ], // androidCompactActionIndices tells the system which controls // appear in the collapsed/lock-screen view. Without this, some @@ -608,6 +797,121 @@ class MinstrelAudioHandler extends BaseAudioHandler with QueueHandler, SeekHandl LoopMode.all => AudioServiceRepeatMode.all, }, )); + _reconcileIdleTimer(); + _reconcilePositionBroadcast(); + } + + /// Arms (or cancels) the idle-cleanup timer based on the current + /// player state. Driven from _broadcastState, which fires on every + /// play/pause/buffer/complete transition, so paused AND finished-queue + /// both arm it. Idle (already stopped) never re-arms — otherwise stop() + /// would loop every _idleStopTimeout. + void _reconcileIdleTimer() { + final ps = _player.processingState; + if (ps == ProcessingState.idle) { + _idleStopTimer?.cancel(); + _idleStopTimer = null; + return; + } + final activelyPlaying = + _player.playing && ps != ProcessingState.completed; + if (activelyPlaying) { + _idleStopTimer?.cancel(); + _idleStopTimer = null; + return; + } + _idleStopTimer ??= Timer(_idleStopTimeout, _onIdleTimeout); + } + + void _onIdleTimeout() { + _idleStopTimer = null; + final ps = _player.processingState; + if (ps == ProcessingState.idle) return; // already torn down + if (_player.playing && ps != ProcessingState.completed) { + return; // resumed between arm and fire + } + unawaited(stop()); + } + + /// While actively playing, keeps a 1s periodic PlaybackState re- + /// broadcast running so updateTime/updatePosition stay fresh and + /// external surfaces interpolate the scrubber smoothly. Idempotent and + /// driven from _broadcastState — which the periodic tick itself calls, + /// so the "already running" guard prevents pile-up. Cancels the moment + /// playback is no longer active. + void _reconcilePositionBroadcast() { + final ps = _player.processingState; + final active = _player.playing && + ps != ProcessingState.idle && + ps != ProcessingState.completed; + if (!active) { + _positionBroadcastTimer?.cancel(); + _positionBroadcastTimer = null; + return; + } + _positionBroadcastTimer ??= Timer.periodic( + const Duration(seconds: 1), + (_) => _broadcastState(null), + ); + } + + /// Configures the OS audio session for music playback and wires + /// interruption + becoming-noisy handling. just_audio auto-activates / + /// deactivates the session around playback once it's configured, but + /// does NOT handle interruptions or the headphones-unplugged event + /// itself — that's done here. Best effort: a failure must not break + /// playback. + Future _configureAudioSession() async { + try { + final session = await AudioSession.instance; + await session.configure(const AudioSessionConfiguration.music()); + session.interruptionEventStream.listen(_onInterruption); + session.becomingNoisyEventStream.listen((_) { + // Headphones unplugged / Bluetooth disconnected — never blast the + // phone speaker; pause like every other media app. + unawaited(_player.pause()); + }); + } catch (e, st) { + debugPrint('audio_handler: audio session configure failed: $e\n$st'); + } + } + + void _onInterruption(AudioInterruptionEvent event) { + if (event.begin) { + switch (event.type) { + case AudioInterruptionType.duck: + // Transient: another app wants the foreground briefly. Lower + // our volume rather than stopping (OS may also auto-duck). + _volumeBeforeDuck = _player.volume; + unawaited(_player.setVolume(0.3)); + case AudioInterruptionType.pause: + case AudioInterruptionType.unknown: + // Call / other media took focus. Remember whether we were + // actively playing so a transient end can resume us. + _interruptedWhilePlaying = _player.playing && + _player.processingState != ProcessingState.completed; + unawaited(_player.pause()); + } + } else { + switch (event.type) { + case AudioInterruptionType.duck: + unawaited(_player.setVolume(_volumeBeforeDuck ?? 1.0)); + _volumeBeforeDuck = null; + case AudioInterruptionType.pause: + // Transient interruption ended — resume only if WE paused it + // and the session is still alive (a long call may have let the + // #52 idle timer tear it down; re-init is resume-last-session + // territory, out of scope here). + if (_interruptedWhilePlaying && + _player.processingState != ProcessingState.idle) { + unawaited(_player.play()); + } + _interruptedWhilePlaying = false; + case AudioInterruptionType.unknown: + // Unknown end — do not auto-resume (could surprise the user). + _interruptedWhilePlaying = false; + } + } } } diff --git a/flutter_client/lib/player/now_playing_screen.dart b/flutter_client/lib/player/now_playing_screen.dart index 5f529aea..95817b8f 100644 --- a/flutter_client/lib/player/now_playing_screen.dart +++ b/flutter_client/lib/player/now_playing_screen.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:audio_service/audio_service.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -376,13 +377,13 @@ class _TopBar extends StatelessWidget { padding: const EdgeInsets.symmetric(horizontal: 4), child: Row(children: [ IconButton( - icon: Icon(Icons.expand_more, color: fs.parchment), + icon: Icon(LucideIcons.chevron_down, color: fs.parchment), tooltip: 'Close', onPressed: () => Navigator.of(context).maybePop(), ), const Spacer(), IconButton( - icon: Icon(Icons.queue_music, color: fs.parchment), + icon: Icon(LucideIcons.list_music, color: fs.parchment), tooltip: 'Queue', onPressed: () => GoRouter.of(context).push('/queue'), ), @@ -535,13 +536,13 @@ class _PrimaryControls extends StatelessWidget { children: [ IconButton( iconSize: 36, - icon: Icon(Icons.skip_previous, color: fs.parchment), + icon: Icon(LucideIcons.skip_back, color: fs.parchment), onPressed: () => ref.read(audioHandlerProvider).skipToPrevious(), ), IconButton( iconSize: 72, icon: Icon( - isPlaying ? Icons.pause_circle_filled : Icons.play_circle_filled, + isPlaying ? LucideIcons.circle_pause : LucideIcons.circle_play, color: fs.accent, ), onPressed: () { @@ -555,7 +556,7 @@ class _PrimaryControls extends StatelessWidget { ), IconButton( iconSize: 36, - icon: Icon(Icons.skip_next, color: fs.parchment), + icon: Icon(LucideIcons.skip_forward, color: fs.parchment), onPressed: () => ref.read(audioHandlerProvider).skipToNext(), ), ], @@ -589,7 +590,7 @@ class _SecondaryControls extends StatelessWidget { IconButton( tooltip: shuffleOn ? 'Shuffle on' : 'Shuffle off', icon: Icon( - Icons.shuffle, + LucideIcons.shuffle, color: shuffleOn ? fs.accent : fs.ash, ), onPressed: actions.toggleShuffle, @@ -602,8 +603,8 @@ class _SecondaryControls extends StatelessWidget { }, icon: Icon( repeatMode == AudioServiceRepeatMode.one - ? Icons.repeat_one - : Icons.repeat, + ? LucideIcons.repeat_1 + : LucideIcons.repeat, color: repeatMode == AudioServiceRepeatMode.none ? fs.ash : fs.accent, @@ -612,7 +613,7 @@ class _SecondaryControls extends StatelessWidget { ), IconButton( tooltip: 'Queue', - icon: Icon(Icons.queue_music, color: fs.ash), + icon: Icon(LucideIcons.list_music, color: fs.ash), onPressed: () => GoRouter.of(context).push('/queue'), ), LikeButton(kind: LikeKind.track, id: media.id, size: 22), diff --git a/flutter_client/lib/player/playback_error_reporter.dart b/flutter_client/lib/player/playback_error_reporter.dart new file mode 100644 index 00000000..b4f8b67d --- /dev/null +++ b/flutter_client/lib/player/playback_error_reporter.dart @@ -0,0 +1,78 @@ +// Surfaces playback errors (#58). _handlePlaybackError in the audio +// handler silently skips a dead track (404 / decoder failure / premature +// EOS / network drop) with only a debugPrint — which hides exactly the +// signal that distinguishes "this track is broken" from "the app is +// flaky" (server file moved, auth expired, cache miss, transcode fail). +// +// This listens to the handler's playbackErrorStream and shows a +// transient SnackBar via a global ScaffoldMessenger key. Bursts are +// coalesced: a debounce window collects errors and emits one message +// ("Skipped N unplayable tracks") instead of stacking N toasts. + +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; + +import 'player_provider.dart'; + +/// Set on MaterialApp.router so SnackBars can be shown from outside any +/// widget's BuildContext (the handler's error stream is isolate-side). +final scaffoldMessengerKey = GlobalKey(); + +class PlaybackErrorReporter { + PlaybackErrorReporter(this._ref); + final Ref _ref; + + StreamSubscription? _sub; + Timer? _debounce; + final _buffer = []; + bool _disposed = false; + + void start() { + try { + // audioHandlerProvider throws until main() overrides it (the real + // app always does). In tests / no-audio environments there's + // nothing to report — stay inert. + final h = _ref.read(audioHandlerProvider); + _sub = h.playbackErrorStream.listen(_onError); + } catch (_) { + return; + } + } + + void _onError(String title) { + if (_disposed) return; + _buffer.add(title); + _debounce?.cancel(); + _debounce = Timer(const Duration(seconds: 2), _flush); + } + + void _flush() { + if (_disposed || _buffer.isEmpty) return; + final n = _buffer.length; + final first = _buffer.first; + _buffer.clear(); + final msg = n == 1 + ? 'Couldn’t play “$first” — skipping' + : 'Skipped $n unplayable tracks'; + scaffoldMessengerKey.currentState?.showSnackBar( + SnackBar(content: Text(msg)), + ); + } + + void dispose() { + _disposed = true; + _debounce?.cancel(); + _sub?.cancel(); + } +} + +/// Read once at app start (app.dart postFrame). Disposed via +/// ref.onDispose when the scope tears down. +final playbackErrorReporterProvider = Provider((ref) { + final r = PlaybackErrorReporter(ref); + ref.onDispose(r.dispose); + r.start(); + return r; +}); diff --git a/flutter_client/lib/player/player_bar.dart b/flutter_client/lib/player/player_bar.dart index e2d559ea..3c8be42d 100644 --- a/flutter_client/lib/player/player_bar.dart +++ b/flutter_client/lib/player/player_bar.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:audio_service/audio_service.dart'; import 'package:cached_network_image/cached_network_image.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -229,7 +230,7 @@ class _PlayControls extends StatelessWidget { child: IconButton( padding: EdgeInsets.zero, iconSize: 22, - icon: Icon(Icons.skip_previous, color: fs.parchment), + icon: Icon(LucideIcons.skip_back, color: fs.parchment), onPressed: () => ref.read(audioHandlerProvider).skipToPrevious(), ), ), @@ -240,7 +241,7 @@ class _PlayControls extends StatelessWidget { padding: EdgeInsets.zero, iconSize: 32, icon: Icon( - isPlaying ? Icons.pause_circle_filled : Icons.play_circle_filled, + isPlaying ? LucideIcons.circle_pause : LucideIcons.circle_play, color: fs.accent, ), onPressed: () { @@ -259,7 +260,7 @@ class _PlayControls extends StatelessWidget { child: IconButton( padding: EdgeInsets.zero, iconSize: 22, - icon: Icon(Icons.skip_next, color: fs.parchment), + icon: Icon(LucideIcons.skip_forward, color: fs.parchment), onPressed: () => ref.read(audioHandlerProvider).skipToNext(), ), ), diff --git a/flutter_client/lib/player/player_provider.dart b/flutter_client/lib/player/player_provider.dart index c1134aba..8a8be4db 100644 --- a/flutter_client/lib/player/player_provider.dart +++ b/flutter_client/lib/player/player_provider.dart @@ -62,7 +62,9 @@ class PlayerActions { // kebab menu, or another logged-in device propagating via SSE. _ref.listen>(likedIdsProvider, (_, next) { if (next.value == null) return; - _ref.read(audioHandlerProvider).refreshCurrentRating(); + _ref.read(audioHandlerProvider) + ..refreshCurrentRating() + ..refreshFavoriteControl(); }); } final Ref _ref; @@ -101,6 +103,36 @@ class PlayerActions { await h.play(); } + /// Rebuilds a previously-persisted queue WITHOUT auto-playing, then + /// seeks to [position]. The resume-on-launch path (#54): the user + /// sees their last track in the mini bar, paused, and continues with + /// play / a media button. Mirrors playTracks' configure step so + /// streaming works after restore; intentionally no h.play(). + Future restoreQueue( + List tracks, { + int initialIndex = 0, + Duration position = Duration.zero, + String? source, + }) async { + if (tracks.isEmpty) return; + final url = await _ref.read(serverUrlProvider.future); + final token = await _ref.read(secureStorageProvider).read(key: 'session_token'); + final cache = _ref.read(albumCoverCacheProvider); + final audioCache = _ref.read(audioCacheManagerProvider); + final h = _ref.read(audioHandlerProvider) + ..configure( + baseUrl: url ?? '', + token: token, + coverCache: cache, + audioCacheManager: audioCache, + likeBridge: _buildLikeBridge(), + ); + await h.setQueueFromTracks(tracks, initialIndex: initialIndex, source: source); + if (position > Duration.zero) { + await h.seek(position); + } + } + /// Builds the adapter the audio handler uses to read + flip the /// current track's like state in response to external media- /// controller events (Wear's heart button, lock-screen favorite). diff --git a/flutter_client/lib/player/queue_screen.dart b/flutter_client/lib/player/queue_screen.dart index f09c45c1..87eff48f 100644 --- a/flutter_client/lib/player/queue_screen.dart +++ b/flutter_client/lib/player/queue_screen.dart @@ -1,5 +1,6 @@ import 'package:audio_service/audio_service.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -20,7 +21,7 @@ class QueueScreen extends ConsumerWidget { backgroundColor: fs.obsidian, elevation: 0, leading: IconButton( - icon: Icon(Icons.arrow_back, color: fs.parchment), + icon: Icon(LucideIcons.arrow_left, color: fs.parchment), onPressed: () => context.pop(), ), title: Text('Queue', style: TextStyle(color: fs.parchment)), @@ -84,7 +85,7 @@ class _QueueRow extends StatelessWidget { if (isCurrent) Padding( padding: const EdgeInsets.only(right: 8), - child: Icon(Icons.graphic_eq, color: fs.accent, size: 16), + child: Icon(LucideIcons.audio_lines, color: fs.accent, size: 16), ), Expanded( child: Column( diff --git a/flutter_client/lib/playlists/playlist_detail_screen.dart b/flutter_client/lib/playlists/playlist_detail_screen.dart index ddb19d77..c91e733e 100644 --- a/flutter_client/lib/playlists/playlist_detail_screen.dart +++ b/flutter_client/lib/playlists/playlist_detail_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -54,7 +55,7 @@ class PlaylistDetailScreen extends ConsumerWidget { backgroundColor: fs.obsidian, elevation: 0, leading: IconButton( - icon: Icon(Icons.arrow_back, color: fs.parchment), + icon: Icon(LucideIcons.arrow_left, color: fs.parchment), onPressed: () => context.pop(), ), title: headerName.isEmpty @@ -268,7 +269,7 @@ class _Header extends ConsumerWidget { OutlinedButton.icon( key: const Key('regenerate_playlist_button'), onPressed: () => _regenerate(context, ref), - icon: const Icon(Icons.refresh, size: 16), + icon: const Icon(LucideIcons.refresh_cw, size: 16), label: const Text('Regenerate'), ), const SizedBox(width: 8), @@ -281,7 +282,7 @@ class _Header extends ConsumerWidget { source: p.refreshable ? p.systemVariant : null, ); }, - icon: const Icon(Icons.play_arrow), + icon: const Icon(LucideIcons.play), label: const Text('Play'), style: FilledButton.styleFrom( backgroundColor: fs.accent, diff --git a/flutter_client/lib/playlists/playlists_list_screen.dart b/flutter_client/lib/playlists/playlists_list_screen.dart index bfddbf9a..23410842 100644 --- a/flutter_client/lib/playlists/playlists_list_screen.dart +++ b/flutter_client/lib/playlists/playlists_list_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -82,11 +83,11 @@ class _PlaylistTile extends StatelessWidget { height: 56, color: fs.slate, child: playlist.coverUrl.isEmpty - ? Icon(Icons.queue_music, color: fs.ash) + ? Icon(LucideIcons.list_music, color: fs.ash) : ServerImage( url: playlist.coverUrl, fit: BoxFit.cover, - fallback: Icon(Icons.queue_music, color: fs.ash), + fallback: Icon(LucideIcons.list_music, color: fs.ash), ), ), ), @@ -128,7 +129,7 @@ class _PlaylistTile extends StatelessWidget { ], ), ), - Icon(Icons.chevron_right, color: fs.ash), + Icon(LucideIcons.chevron_right, color: fs.ash), ]), ), ); diff --git a/flutter_client/lib/playlists/widgets/playlist_card.dart b/flutter_client/lib/playlists/widgets/playlist_card.dart index c7a0a2c5..7e16cc8f 100644 --- a/flutter_client/lib/playlists/widgets/playlist_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_card.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -55,12 +56,12 @@ class PlaylistCard extends ConsumerWidget { // errorBuilder shows the queue_music icon over the // slate background. child: playlist.coverUrl.isEmpty - ? Icon(Icons.queue_music, color: fs.ash, size: 56) + ? Icon(LucideIcons.list_music, color: fs.ash, size: 56) : ServerImage( url: playlist.coverUrl, fit: BoxFit.cover, fallback: - Icon(Icons.queue_music, color: fs.ash, size: 56), + Icon(LucideIcons.list_music, color: fs.ash, size: 56), ), ), ), @@ -80,7 +81,7 @@ class PlaylistCard extends ConsumerWidget { top: 2, right: 2, child: PopupMenuButton( - icon: Icon(Icons.more_vert, color: fs.parchment, size: 18), + icon: Icon(LucideIcons.ellipsis_vertical, color: fs.parchment, size: 18), tooltip: 'Playlist actions', color: fs.iron, onSelected: (_) => _refresh(context, ref), @@ -88,7 +89,7 @@ class PlaylistCard extends ConsumerWidget { PopupMenuItem( value: 'refresh', child: Row(children: [ - Icon(Icons.refresh, color: fs.parchment, size: 18), + Icon(LucideIcons.refresh_cw, color: fs.parchment, size: 18), const SizedBox(width: 8), Text(_refreshLabel, style: TextStyle(color: fs.parchment)), diff --git a/flutter_client/lib/playlists/widgets/playlist_placeholder_card.dart b/flutter_client/lib/playlists/widgets/playlist_placeholder_card.dart index 59db7b92..d2192c26 100644 --- a/flutter_client/lib/playlists/widgets/playlist_placeholder_card.dart +++ b/flutter_client/lib/playlists/widgets/playlist_placeholder_card.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import '../../theme/theme_extension.dart'; @@ -65,9 +66,9 @@ class PlaylistPlaceholderCard extends StatelessWidget { child: CircularProgressIndicator(strokeWidth: 2, color: fs.accent), ); case 'failed': - return Icon(Icons.warning_amber, color: fs.error, size: 32); + return Icon(LucideIcons.triangle_alert, color: fs.error, size: 32); default: - return Icon(Icons.queue_music, color: fs.ash, size: 40); + return Icon(LucideIcons.list_music, color: fs.ash, size: 40); } } diff --git a/flutter_client/lib/requests/requests_screen.dart b/flutter_client/lib/requests/requests_screen.dart index 2cf98fdd..d4a15d38 100644 --- a/flutter_client/lib/requests/requests_screen.dart +++ b/flutter_client/lib/requests/requests_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -32,7 +33,7 @@ class RequestsScreen extends ConsumerWidget { backgroundColor: fs.obsidian, elevation: 0, leading: IconButton( - icon: Icon(Icons.arrow_back, color: fs.parchment), + icon: Icon(LucideIcons.arrow_left, color: fs.parchment), onPressed: () => context.pop(), ), title: Text('Your requests', style: TextStyle(color: fs.parchment)), @@ -130,9 +131,9 @@ class _RequestRow extends StatelessWidget { Widget _kindAvatar(FabledSwordTheme fs) { final icon = switch (request.kind) { - 'artist' => Icons.album, - 'album' => Icons.library_music, - _ => Icons.music_note, + 'artist' => LucideIcons.disc_3, + 'album' => LucideIcons.library_big, + _ => LucideIcons.music, }; return CircleAvatar( backgroundColor: fs.iron, @@ -164,7 +165,7 @@ class _RequestRow extends StatelessWidget { ); if (ok == true) onCancel(); }, - icon: const Icon(Icons.close, size: 16), + icon: const Icon(LucideIcons.x, size: 16), label: const Text('Cancel'), style: TextButton.styleFrom(foregroundColor: fs.ash), ); @@ -172,7 +173,7 @@ class _RequestRow extends StatelessWidget { if (request.status == 'completed' && href != null) { return TextButton.icon( onPressed: () => context.push(href), - icon: const Icon(Icons.play_arrow, size: 16), + icon: const Icon(LucideIcons.play, size: 16), label: const Text('Listen'), style: TextButton.styleFrom(foregroundColor: fs.accent), ); diff --git a/flutter_client/lib/search/search_screen.dart b/flutter_client/lib/search/search_screen.dart index 6b4728b0..06315eb6 100644 --- a/flutter_client/lib/search/search_screen.dart +++ b/flutter_client/lib/search/search_screen.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -47,7 +48,7 @@ class _SearchScreenState extends ConsumerState { backgroundColor: fs.obsidian, elevation: 0, leading: IconButton( - icon: Icon(Icons.arrow_back, color: fs.parchment), + icon: Icon(LucideIcons.arrow_left, color: fs.parchment), onPressed: () => context.pop(), ), title: TextField( @@ -67,7 +68,7 @@ class _SearchScreenState extends ConsumerState { actions: [ if (_controller.text.isNotEmpty) IconButton( - icon: Icon(Icons.clear, color: fs.ash), + icon: Icon(LucideIcons.x, color: fs.ash), onPressed: () { _controller.clear(); ref.read(searchQueryProvider.notifier).set(''); diff --git a/flutter_client/lib/settings/about_section.dart b/flutter_client/lib/settings/about_section.dart index efd5ada1..9a6131be 100644 --- a/flutter_client/lib/settings/about_section.dart +++ b/flutter_client/lib/settings/about_section.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:package_info_plus/package_info_plus.dart'; @@ -88,7 +89,7 @@ class _AboutSectionState extends ConsumerState { padding: const EdgeInsets.fromLTRB(16, 0, 16, 4), child: Row( children: [ - Icon(Icons.info_outline, color: fs.parchment, size: 18), + Icon(LucideIcons.info, color: fs.parchment, size: 18), const SizedBox(width: 8), Text('Installed version', style: TextStyle(color: fs.parchment, fontSize: 14)), @@ -105,7 +106,7 @@ class _AboutSectionState extends ConsumerState { padding: const EdgeInsets.fromLTRB(16, 4, 16, 4), child: Row( children: [ - Icon(Icons.cloud_outlined, color: fs.parchment, size: 18), + Icon(LucideIcons.cloud, color: fs.parchment, size: 18), const SizedBox(width: 8), Text('Latest version', style: TextStyle(color: fs.parchment, fontSize: 14)), @@ -148,7 +149,7 @@ class _AboutSectionState extends ConsumerState { valueColor: AlwaysStoppedAnimation(fs.parchment), ), ) - : const Icon(Icons.refresh, size: 18), + : const Icon(LucideIcons.refresh_cw, size: 18), label: Text(_checking ? 'Checking…' : 'Check for updates'), ), if (hasUpdate) ...[ @@ -172,7 +173,7 @@ class _AboutSectionState extends ConsumerState { valueColor: AlwaysStoppedAnimation(fs.parchment), ), ) - : const Icon(Icons.system_update, size: 18), + : const Icon(LucideIcons.download, size: 18), label: Text( _installStage == _InstallStage.downloading ? 'Downloading…' diff --git a/flutter_client/lib/settings/settings_screen.dart b/flutter_client/lib/settings/settings_screen.dart index e8810062..ba611366 100644 --- a/flutter_client/lib/settings/settings_screen.dart +++ b/flutter_client/lib/settings/settings_screen.dart @@ -1,5 +1,6 @@ import 'package:dio/dio.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -37,7 +38,7 @@ class SettingsScreen extends ConsumerWidget { backgroundColor: fs.obsidian, elevation: 0, leading: IconButton( - icon: Icon(Icons.arrow_back, color: fs.parchment), + icon: Icon(LucideIcons.arrow_left, color: fs.parchment), onPressed: () => context.pop(), ), title: Text('Settings', style: TextStyle(color: fs.parchment)), @@ -107,7 +108,7 @@ class _RequestsSection extends StatelessWidget { final fs = Theme.of(context).extension()!; return ListTile( key: const Key('settings_requests_card'), - leading: Icon(Icons.queue_music, color: fs.parchment), + leading: Icon(LucideIcons.list_music, color: fs.parchment), title: Text( 'My requests', style: TextStyle( @@ -120,7 +121,7 @@ class _RequestsSection extends StatelessWidget { "Track what you've asked Minstrel to add", style: TextStyle(color: fs.ash), ), - trailing: Icon(Icons.chevron_right, color: fs.ash), + trailing: Icon(LucideIcons.chevron_right, color: fs.ash), onTap: () => context.push('/requests'), ); } @@ -143,7 +144,7 @@ class _AdminSection extends ConsumerWidget { const _Divider(), ListTile( key: const Key('settings_admin_card'), - leading: Icon(Icons.shield, color: fs.parchment), + leading: Icon(LucideIcons.shield, color: fs.parchment), title: Text( 'Admin', style: TextStyle( @@ -156,7 +157,7 @@ class _AdminSection extends ConsumerWidget { 'Manage requests, quarantine, and users', style: TextStyle(color: fs.ash), ), - trailing: Icon(Icons.chevron_right, color: fs.ash), + trailing: Icon(LucideIcons.chevron_right, color: fs.ash), onTap: () => context.push('/admin'), ), ], diff --git a/flutter_client/lib/settings/storage_section.dart b/flutter_client/lib/settings/storage_section.dart index 68e6b84c..8bee9d7a 100644 --- a/flutter_client/lib/settings/storage_section.dart +++ b/flutter_client/lib/settings/storage_section.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../cache/audio_cache_manager.dart'; @@ -140,7 +141,7 @@ class _StorageSectionState extends ConsumerState { height: 14, child: CircularProgressIndicator(strokeWidth: 2), ) - : const Icon(Icons.sync, size: 16), + : const Icon(LucideIcons.refresh_cw, size: 16), label: const Text('Sync now'), ), ]), diff --git a/flutter_client/lib/shared/widgets/lucide_heart.dart b/flutter_client/lib/shared/widgets/lucide_heart.dart new file mode 100644 index 00000000..9e103435 --- /dev/null +++ b/flutter_client/lib/shared/widgets/lucide_heart.dart @@ -0,0 +1,43 @@ +import 'package:flutter/widgets.dart'; +import 'package:flutter_svg/flutter_svg.dart'; + +/// The Lucide "heart" silhouette rendered as either an outline (stroke) +/// or a solid fill. Lucide ships only an outline heart, so the liked +/// state fills the same authoritative Lucide path (verified verbatim +/// from lucide-icons/lucide) — keeping both states visually Lucide +/// rather than introducing a Material filled heart. Used by LikeButton +/// (and, re-derived to a VectorDrawable, by the media notification). +class LucideHeart extends StatelessWidget { + const LucideHeart({ + required this.filled, + required this.color, + this.size = 22, + super.key, + }); + + final bool filled; + final Color color; + final double size; + + static const _path = + 'M2 9.5a5.5 5.5 0 0 1 9.591-3.676.56.56 0 0 0 .818 0A5.49 5.49 0 0 1 ' + '22 9.5c0 2.29-1.5 4-3 5.5l-5.492 5.313a2 2 0 0 1-3 .019L5 15c-1.5-1.5' + '-3-3.2-3-5.5'; + + @override + Widget build(BuildContext context) { + final svg = filled + ? '' + '' + : '' + ''; + return SvgPicture.string( + svg, + width: size, + height: size, + colorFilter: ColorFilter.mode(color, BlendMode.srcIn), + ); + } +} diff --git a/flutter_client/lib/shared/widgets/main_app_bar_actions.dart b/flutter_client/lib/shared/widgets/main_app_bar_actions.dart index e0eab4a8..741cd4db 100644 --- a/flutter_client/lib/shared/widgets/main_app_bar_actions.dart +++ b/flutter_client/lib/shared/widgets/main_app_bar_actions.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -26,27 +27,27 @@ class MainAppBarActions extends ConsumerWidget { if (currentRoute != '/home') IconButton( key: const Key('app_bar_home'), - icon: Icon(Icons.home, color: fs.parchment), + icon: Icon(LucideIcons.house, color: fs.parchment), tooltip: 'Home', onPressed: () => context.go('/home'), ), if (currentRoute != '/library') IconButton( key: const Key('app_bar_library'), - icon: Icon(Icons.library_music, color: fs.parchment), + icon: Icon(LucideIcons.library_big, color: fs.parchment), tooltip: 'Library', onPressed: () => context.push('/library'), ), if (currentRoute != '/search') IconButton( key: const Key('app_bar_search'), - icon: Icon(Icons.search, color: fs.parchment), + icon: Icon(LucideIcons.search, color: fs.parchment), tooltip: 'Search', onPressed: () => context.push('/search'), ), PopupMenuButton( key: const Key('app_bar_overflow'), - icon: Icon(Icons.more_vert, color: fs.parchment), + icon: Icon(LucideIcons.ellipsis_vertical, color: fs.parchment), tooltip: 'More', onSelected: (route) => context.push(route), itemBuilder: (_) => [ diff --git a/flutter_client/lib/shared/widgets/track_actions/add_to_playlist_sheet.dart b/flutter_client/lib/shared/widgets/track_actions/add_to_playlist_sheet.dart index 28814fb4..b3103420 100644 --- a/flutter_client/lib/shared/widgets/track_actions/add_to_playlist_sheet.dart +++ b/flutter_client/lib/shared/widgets/track_actions/add_to_playlist_sheet.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../../playlists/playlists_provider.dart'; @@ -70,7 +71,7 @@ class AddToPlaylistSheet extends ConsumerWidget { final p = owned[i]; return ListTile( key: Key('add_to_playlist_${p.id}'), - leading: Icon(Icons.queue_music, color: fs.parchment), + leading: Icon(LucideIcons.list_music, color: fs.parchment), title: Text( p.name, style: TextStyle(color: fs.parchment), diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart index 6937e9a9..e5c58772 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_button.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import '../../../models/track.dart'; import '../../../theme/theme_extension.dart'; @@ -29,7 +30,7 @@ class TrackActionsButton extends StatelessWidget { Widget build(BuildContext context) { final fs = Theme.of(context).extension()!; return IconButton( - icon: Icon(Icons.more_vert, color: fs.ash, size: 18), + icon: Icon(LucideIcons.ellipsis_vertical, color: fs.ash, size: 18), tooltip: 'Track actions', onPressed: () => TrackActionsSheet.show( context, diff --git a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart index 68e2391d..4847784b 100644 --- a/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart +++ b/flutter_client/lib/shared/widgets/track_actions/track_actions_sheet.dart @@ -1,5 +1,6 @@ import 'package:drift/drift.dart' as drift; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:go_router/go_router.dart'; @@ -73,7 +74,7 @@ class TrackActionsSheet extends ConsumerWidget { if (!hideQueueActions) ...[ _MenuItem( key: const Key('track_actions_play_next'), - icon: Icons.playlist_play, + icon: LucideIcons.list_video, label: 'Play next', onTap: () async { Navigator.pop(context); @@ -87,7 +88,7 @@ class TrackActionsSheet extends ConsumerWidget { ), _MenuItem( key: const Key('track_actions_enqueue'), - icon: Icons.queue_music, + icon: LucideIcons.list_music, label: 'Add to queue', onTap: () async { Navigator.pop(context); @@ -103,7 +104,7 @@ class TrackActionsSheet extends ConsumerWidget { ], _MenuItem( key: const Key('track_actions_like'), - icon: liked ? Icons.favorite : Icons.favorite_border, + icon: LucideIcons.heart, label: liked ? 'Unlike' : 'Like', onTap: () async { Navigator.pop(context); @@ -112,7 +113,7 @@ class TrackActionsSheet extends ConsumerWidget { ), _MenuItem( key: const Key('track_actions_add_to_playlist'), - icon: Icons.playlist_add, + icon: LucideIcons.list_plus, label: 'Add to playlist…', onTap: () async { Navigator.pop(context); @@ -136,7 +137,7 @@ class TrackActionsSheet extends ConsumerWidget { ), _MenuItem( key: const Key('track_actions_start_radio'), - icon: Icons.radio, + icon: LucideIcons.radio, label: 'Start radio', onTap: () async { Navigator.pop(context); @@ -154,7 +155,7 @@ class TrackActionsSheet extends ConsumerWidget { const _Divider(), _MenuItem( key: const Key('track_actions_go_to_album'), - icon: Icons.album, + icon: LucideIcons.disc_3, label: 'Go to album', onTap: () { Navigator.pop(context); @@ -168,7 +169,7 @@ class TrackActionsSheet extends ConsumerWidget { ), _MenuItem( key: const Key('track_actions_go_to_artist'), - icon: Icons.person, + icon: LucideIcons.user, label: 'Go to artist', onTap: () { Navigator.pop(context); @@ -183,7 +184,7 @@ class TrackActionsSheet extends ConsumerWidget { const _Divider(), _MenuItem( key: const Key('track_actions_hide'), - icon: hidden ? Icons.visibility : Icons.visibility_off, + icon: hidden ? LucideIcons.eye : LucideIcons.eye_off, label: hidden ? 'Unhide' : 'Hide', onTap: () async { Navigator.pop(context); diff --git a/flutter_client/lib/shared/widgets/version_gate.dart b/flutter_client/lib/shared/widgets/version_gate.dart index 2d001910..ab57acc6 100644 --- a/flutter_client/lib/shared/widgets/version_gate.dart +++ b/flutter_client/lib/shared/widgets/version_gate.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'package:dio/dio.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:package_info_plus/package_info_plus.dart'; import 'package:pub_semver/pub_semver.dart'; @@ -238,7 +239,7 @@ class VersionTooOldBanner extends ConsumerWidget { color: fs.error.withValues(alpha: 0.15), padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 10), child: Row(children: [ - Icon(Icons.warning_amber_rounded, color: fs.error, size: 18), + Icon(LucideIcons.triangle_alert, color: fs.error, size: 18), const SizedBox(width: 8), Expanded( child: Text( diff --git a/flutter_client/lib/update/update_banner.dart b/flutter_client/lib/update/update_banner.dart index 6605dc28..559777a9 100644 --- a/flutter_client/lib/update/update_banner.dart +++ b/flutter_client/lib/update/update_banner.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../theme/theme_extension.dart'; @@ -66,7 +67,7 @@ class _UpdateBannerState extends ConsumerState { child: Row( crossAxisAlignment: CrossAxisAlignment.center, children: [ - Icon(Icons.system_update, color: fs.parchment, size: 16), + Icon(LucideIcons.download, color: fs.parchment, size: 16), const SizedBox(width: 8), Expanded( child: Column( @@ -132,7 +133,7 @@ class _UpdateBannerState extends ConsumerState { tooltip: 'Dismiss', padding: EdgeInsets.zero, iconSize: 16, - icon: Icon(Icons.close, color: fs.ash), + icon: Icon(LucideIcons.x, color: fs.ash), onPressed: () => _onDismiss(info), ), ), diff --git a/flutter_client/pubspec.yaml b/flutter_client/pubspec.yaml index 0d9aab7a..0c425aeb 100644 --- a/flutter_client/pubspec.yaml +++ b/flutter_client/pubspec.yaml @@ -1,7 +1,7 @@ name: minstrel description: Minstrel mobile client publish_to: 'none' -version: 2026.5.18+9 +version: 2026.5.19+10 environment: sdk: '>=3.5.0 <4.0.0' @@ -14,9 +14,17 @@ dependencies: dio: ^5.7.0 just_audio: ^0.10.5 audio_service: ^0.18.15 + # Audio focus + interruptions + becoming-noisy. just_audio auto-manages + # session activation once configured; the app must still handle + # interruption/becoming-noisy itself (just_audio does not). Same author + # as just_audio/audio_service so versions track together. + audio_session: ^0.2.3 flutter_secure_storage: ^10.1.0 go_router: ^17.2.3 flutter_svg: ^2.0.16 + # Lucide icon set (design system mandates Lucide, not Material). + # Icons exposed as LucideIcons. IconData usable in Icon(). + flutter_lucide: ^1.11.0 google_fonts: ^8.1.0 # 10.x conflicts with flutter_secure_storage 10.x on win32. Hold at 8.3.1 # until either lib bumps win32 to 6.x. diff --git a/flutter_client/test/playlists/widgets/playlist_card_test.dart b/flutter_client/test/playlists/widgets/playlist_card_test.dart index 69086907..186041d1 100644 --- a/flutter_client/test/playlists/widgets/playlist_card_test.dart +++ b/flutter_client/test/playlists/widgets/playlist_card_test.dart @@ -1,4 +1,5 @@ import 'package:flutter/material.dart'; +import 'package:flutter_lucide/flutter_lucide.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -67,7 +68,7 @@ void main() { body: PlaylistCard(playlist: _forYou), ), ))); - expect(find.byIcon(Icons.more_vert), findsOneWidget); + expect(find.byIcon(LucideIcons.ellipsis_vertical), findsOneWidget); }); testWidgets('no refresh kebab on user playlists', (tester) async { @@ -78,6 +79,6 @@ void main() { body: PlaylistCard(playlist: _userPlaylist), ), ))); - expect(find.byIcon(Icons.more_vert), findsNothing); + expect(find.byIcon(LucideIcons.ellipsis_vertical), findsNothing); }); } diff --git a/internal/api/admin_requests_test.go b/internal/api/admin_requests_test.go index 741a7d6a..2106b5ed 100644 --- a/internal/api/admin_requests_test.go +++ b/internal/api/admin_requests_test.go @@ -287,9 +287,12 @@ func TestHandleApproveRequest_OverrideUsed(t *testing.T) { } } -// TestHandleApproveRequest_LidarrUnreachable_503 points config at an unreachable -// port and verifies POST /approve → 503 lidarr_unreachable, row stays pending. -func TestHandleApproveRequest_LidarrUnreachable_503(t *testing.T) { +// TestHandleApproveRequest_LidarrUnreachable_ApprovesDeferred points config +// at an unreachable port and verifies the durable-approve contract: POST +// /approve still succeeds (200) and the request becomes approved with the +// Lidarr add deferred (lidarr_add_confirmed_at NULL) for the reconciler to +// retry — the admin's decision is never lost to a transient Lidarr outage. +func TestHandleApproveRequest_LidarrUnreachable_ApprovesDeferred(t *testing.T) { h, _ := testHandlersWithClientFn(t) resetLidarrState(t, h) @@ -301,31 +304,39 @@ func TestHandleApproveRequest_LidarrUnreachable_503(t *testing.T) { rv := seedPendingArtistRequest(t, h, alice, "artist-mbid-unreachable", "Unreachable") w := doAdminRequestReq(t, h, http.MethodPost, "/api/admin/requests/"+uuidToString(rv.ID)+"/approve", nil, admin) - if w.Code != http.StatusServiceUnavailable { - t.Fatalf("status = %d, want 503; body = %s", w.Code, w.Body.String()) - } - var resp map[string]string - if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v; body = %s", err, w.Body.String()) - } - if resp["error"] != "lidarr_unreachable" { - t.Errorf("error = %q, want lidarr_unreachable", resp["error"]) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (durable approve despite Lidarr down); body = %s", w.Code, w.Body.String()) } - // Row must still be pending. - rows, err := h.lidarrRequests.ListByStatus(context.Background(), "pending", 10) + // Must no longer be pending. + pending, err := h.lidarrRequests.ListByStatus(context.Background(), "pending", 10) if err != nil { t.Fatalf("list pending: %v", err) } - found := false - for _, r := range rows { + for _, r := range pending { if r.ID == rv.ID { - found = true + t.Error("row still pending after durable approve") + } + } + + // Must be approved with the add deferred (confirmed_at NULL) so the + // reconciler retries it — not failed, not lost. + approved, err := h.lidarrRequests.ListByStatus(context.Background(), "approved", 10) + if err != nil { + t.Fatalf("list approved: %v", err) + } + var got *dbq.LidarrRequest + for i := range approved { + if approved[i].ID == rv.ID { + got = &approved[i] break } } - if !found { - t.Error("row is no longer pending after failed approve") + if got == nil { + t.Fatalf("row %s not in approved after approve with Lidarr down", uuidToString(rv.ID)) + } + if got.LidarrAddConfirmedAt.Valid { + t.Error("lidarr_add_confirmed_at set despite Lidarr unreachable; want NULL (deferred to reconciler)") } } diff --git a/internal/db/dbq/lidarr_requests.sql.go b/internal/db/dbq/lidarr_requests.sql.go index ac9d796e..73f2c716 100644 --- a/internal/db/dbq/lidarr_requests.sql.go +++ b/internal/db/dbq/lidarr_requests.sql.go @@ -20,7 +20,7 @@ UPDATE lidarr_requests decided_by = $4, updated_at = now() WHERE id = $1 AND status = 'pending' - RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at + RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at ` type ApproveLidarrRequestParams struct { @@ -60,6 +60,7 @@ func (q *Queries) ApproveLidarrRequest(ctx context.Context, arg ApproveLidarrReq &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ) return i, err } @@ -72,7 +73,7 @@ UPDATE lidarr_requests decided_by = $2, updated_at = now() WHERE id = $1 AND user_id = $2 AND status = 'pending' - RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at + RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at ` type CancelLidarrRequestParams struct { @@ -110,6 +111,7 @@ func (q *Queries) CancelLidarrRequest(ctx context.Context, arg CancelLidarrReque &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ) return i, err } @@ -123,7 +125,7 @@ UPDATE lidarr_requests completed_at = now(), updated_at = now() WHERE id = $1 AND status = 'approved' - RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at + RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at ` type CompleteLidarrRequestParams struct { @@ -169,6 +171,7 @@ func (q *Queries) CompleteLidarrRequest(ctx context.Context, arg CompleteLidarrR &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ) return i, err } @@ -179,7 +182,7 @@ INSERT INTO lidarr_requests ( lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) -RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at +RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at ` type CreateLidarrRequestParams struct { @@ -227,12 +230,13 @@ func (q *Queries) CreateLidarrRequest(ctx context.Context, arg CreateLidarrReque &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ) return i, err } const getLidarrRequestByID = `-- name: GetLidarrRequestByID :one -SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests WHERE id = $1 +SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests WHERE id = $1 ` func (q *Queries) GetLidarrRequestByID(ctx context.Context, id pgtype.UUID) (LidarrRequest, error) { @@ -260,6 +264,53 @@ func (q *Queries) GetLidarrRequestByID(ctx context.Context, id pgtype.UUID) (Lid &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, + ) + return i, err +} + +const getNonTerminalRequestForMBID = `-- name: GetNonTerminalRequestForMBID :one +SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests + WHERE status IN ('pending', 'approved', 'completed') + AND ((kind = 'artist' AND lidarr_artist_mbid = $1) + OR (kind = 'album' AND lidarr_album_mbid = $1) + OR (kind = 'track' AND lidarr_track_mbid = $1)) + ORDER BY requested_at ASC + LIMIT 1 +` + +// Returns the oldest non-terminal (pending/approved/completed) request +// whose own-kind MBID matches @mbid, or pgx.ErrNoRows when none. Create() +// uses this to dedupe: a re-request for something already in flight +// returns the existing row instead of inserting a duplicate. MBIDs are +// globally unique per entity type, so the cross-column OR cannot +// false-match (an artist MBID never collides with an album/track MBID). +func (q *Queries) GetNonTerminalRequestForMBID(ctx context.Context, mbid string) (LidarrRequest, error) { + row := q.db.QueryRow(ctx, getNonTerminalRequestForMBID, mbid) + var i LidarrRequest + err := row.Scan( + &i.ID, + &i.UserID, + &i.Status, + &i.Kind, + &i.LidarrArtistMbid, + &i.LidarrAlbumMbid, + &i.LidarrTrackMbid, + &i.ArtistName, + &i.AlbumTitle, + &i.TrackTitle, + &i.QualityProfileID, + &i.RootFolderPath, + &i.DecidedAt, + &i.DecidedBy, + &i.Notes, + &i.CompletedAt, + &i.MatchedTrackID, + &i.MatchedAlbumID, + &i.MatchedArtistID, + &i.RequestedAt, + &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ) return i, err } @@ -287,7 +338,7 @@ func (q *Queries) HasNonTerminalRequestForMBID(ctx context.Context, mbid string) } const listApprovedLidarrRequestsForReconcile = `-- name: ListApprovedLidarrRequestsForReconcile :many -SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests +SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests WHERE status = 'approved' ORDER BY decided_at ASC LIMIT $1 @@ -324,6 +375,7 @@ func (q *Queries) ListApprovedLidarrRequestsForReconcile(ctx context.Context, li &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ); err != nil { return nil, err } @@ -336,7 +388,7 @@ func (q *Queries) ListApprovedLidarrRequestsForReconcile(ctx context.Context, li } const listLidarrRequestsByStatus = `-- name: ListLidarrRequestsByStatus :many -SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests +SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests WHERE status = $1 ORDER BY requested_at DESC LIMIT $2 @@ -378,6 +430,7 @@ func (q *Queries) ListLidarrRequestsByStatus(ctx context.Context, arg ListLidarr &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ); err != nil { return nil, err } @@ -390,7 +443,7 @@ func (q *Queries) ListLidarrRequestsByStatus(ctx context.Context, arg ListLidarr } const listLidarrRequestsForUser = `-- name: ListLidarrRequestsForUser :many -SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at FROM lidarr_requests +SELECT id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at FROM lidarr_requests WHERE user_id = $1 ORDER BY requested_at DESC LIMIT $2 @@ -432,6 +485,7 @@ func (q *Queries) ListLidarrRequestsForUser(ctx context.Context, arg ListLidarrR &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ); err != nil { return nil, err } @@ -443,6 +497,23 @@ func (q *Queries) ListLidarrRequestsForUser(ctx context.Context, arg ListLidarrR return items, nil } +const markLidarrRequestAddConfirmed = `-- name: MarkLidarrRequestAddConfirmed :exec +UPDATE lidarr_requests + SET lidarr_add_confirmed_at = now(), + updated_at = now() + WHERE id = $1 + AND status = 'approved' + AND lidarr_add_confirmed_at IS NULL +` + +// Idempotently records that Lidarr accepted the add for an approved +// request. The lidarr_add_confirmed_at IS NULL guard makes a redundant +// reconciler retry a no-op rather than bumping updated_at every tick. +func (q *Queries) MarkLidarrRequestAddConfirmed(ctx context.Context, id pgtype.UUID) error { + _, err := q.db.Exec(ctx, markLidarrRequestAddConfirmed, id) + return err +} + const rejectLidarrRequest = `-- name: RejectLidarrRequest :one UPDATE lidarr_requests SET status = 'rejected', @@ -451,7 +522,7 @@ UPDATE lidarr_requests decided_by = $3, updated_at = now() WHERE id = $1 AND status = 'pending' - RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at + RETURNING id, user_id, status, kind, lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, artist_name, album_title, track_title, quality_profile_id, root_folder_path, decided_at, decided_by, notes, completed_at, matched_track_id, matched_album_id, matched_artist_id, requested_at, updated_at, lidarr_add_confirmed_at ` type RejectLidarrRequestParams struct { @@ -485,6 +556,7 @@ func (q *Queries) RejectLidarrRequest(ctx context.Context, arg RejectLidarrReque &i.MatchedArtistID, &i.RequestedAt, &i.UpdatedAt, + &i.LidarrAddConfirmedAt, ) return i, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 3b772075..230daadc 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -325,27 +325,28 @@ type LidarrQuarantineAction struct { } type LidarrRequest struct { - ID pgtype.UUID - UserID pgtype.UUID - Status LidarrRequestStatus - Kind LidarrRequestKind - LidarrArtistMbid string - LidarrAlbumMbid *string - LidarrTrackMbid *string - ArtistName string - AlbumTitle *string - TrackTitle *string - QualityProfileID *int32 - RootFolderPath *string - DecidedAt pgtype.Timestamptz - DecidedBy pgtype.UUID - Notes *string - CompletedAt pgtype.Timestamptz - MatchedTrackID pgtype.UUID - MatchedAlbumID pgtype.UUID - MatchedArtistID pgtype.UUID - RequestedAt pgtype.Timestamptz - UpdatedAt pgtype.Timestamptz + ID pgtype.UUID + UserID pgtype.UUID + Status LidarrRequestStatus + Kind LidarrRequestKind + LidarrArtistMbid string + LidarrAlbumMbid *string + LidarrTrackMbid *string + ArtistName string + AlbumTitle *string + TrackTitle *string + QualityProfileID *int32 + RootFolderPath *string + DecidedAt pgtype.Timestamptz + DecidedBy pgtype.UUID + Notes *string + CompletedAt pgtype.Timestamptz + MatchedTrackID pgtype.UUID + MatchedAlbumID pgtype.UUID + MatchedArtistID pgtype.UUID + RequestedAt pgtype.Timestamptz + UpdatedAt pgtype.Timestamptz + LidarrAddConfirmedAt pgtype.Timestamptz } type PasswordReset struct { diff --git a/internal/db/migrations/0031_lidarr_add_confirmed_at.down.sql b/internal/db/migrations/0031_lidarr_add_confirmed_at.down.sql new file mode 100644 index 00000000..40964d14 --- /dev/null +++ b/internal/db/migrations/0031_lidarr_add_confirmed_at.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE lidarr_requests + DROP COLUMN lidarr_add_confirmed_at; diff --git a/internal/db/migrations/0031_lidarr_add_confirmed_at.up.sql b/internal/db/migrations/0031_lidarr_add_confirmed_at.up.sql new file mode 100644 index 00000000..37b05a72 --- /dev/null +++ b/internal/db/migrations/0031_lidarr_add_confirmed_at.up.sql @@ -0,0 +1,8 @@ +-- Additive: records when Lidarr accepted the add (POST artist/album) +-- for an approved request. NULL = not yet sent/confirmed; the +-- reconciler idempotently (re)sends the add until this is set, then +-- watches for the import. Decouples the durable admin approval from the +-- best-effort Lidarr call so a transient Lidarr outage no longer forces +-- a manual re-approve. +ALTER TABLE lidarr_requests + ADD COLUMN lidarr_add_confirmed_at timestamptz; diff --git a/internal/db/queries/lidarr_requests.sql b/internal/db/queries/lidarr_requests.sql index d075a07a..7561acb0 100644 --- a/internal/db/queries/lidarr_requests.sql +++ b/internal/db/queries/lidarr_requests.sql @@ -93,3 +93,29 @@ SELECT EXISTS ( OR (kind = 'album' AND lidarr_album_mbid = @mbid) OR (kind = 'track' AND lidarr_track_mbid = @mbid)) ); + +-- name: GetNonTerminalRequestForMBID :one +-- Returns the oldest non-terminal (pending/approved/completed) request +-- whose own-kind MBID matches @mbid, or pgx.ErrNoRows when none. Create() +-- uses this to dedupe: a re-request for something already in flight +-- returns the existing row instead of inserting a duplicate. MBIDs are +-- globally unique per entity type, so the cross-column OR cannot +-- false-match (an artist MBID never collides with an album/track MBID). +SELECT * FROM lidarr_requests + WHERE status IN ('pending', 'approved', 'completed') + AND ((kind = 'artist' AND lidarr_artist_mbid = @mbid) + OR (kind = 'album' AND lidarr_album_mbid = @mbid) + OR (kind = 'track' AND lidarr_track_mbid = @mbid)) + ORDER BY requested_at ASC + LIMIT 1; + +-- name: MarkLidarrRequestAddConfirmed :exec +-- Idempotently records that Lidarr accepted the add for an approved +-- request. The lidarr_add_confirmed_at IS NULL guard makes a redundant +-- reconciler retry a no-op rather than bumping updated_at every tick. +UPDATE lidarr_requests + SET lidarr_add_confirmed_at = now(), + updated_at = now() + WHERE id = $1 + AND status = 'approved' + AND lidarr_add_confirmed_at IS NULL; diff --git a/internal/lidarr/client.go b/internal/lidarr/client.go index 83f2f10c..dcbbe0c0 100644 --- a/internal/lidarr/client.go +++ b/internal/lidarr/client.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -300,12 +301,29 @@ func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { } resp, err := c.post(ctx, "/api/v1/artist", body) if err != nil { + if isAlreadyExists(err) { + return ErrAlreadyExists + } return err } _ = resp.Body.Close() return nil } +// isAlreadyExists detects Lidarr's 400 "already in your library" rejection +// from an add endpoint. The post() helper buckets it as ErrLookupFailed +// but embeds Lidarr's errorMessage body snippet; matching the phrase lets +// the idempotent retry path treat a re-add as success instead of error. +func isAlreadyExists(err error) bool { + if !errors.Is(err, ErrLookupFailed) { + return false + } + m := strings.ToLower(err.Error()) + return strings.Contains(m, "already exists") || + strings.Contains(m, "already been added") || + strings.Contains(m, "already in your library") +} + // AddAlbum posts to POST /api/v1/album. Returns nil on 2xx; typed error // otherwise. func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error { @@ -324,6 +342,9 @@ func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error { } resp, err := c.post(ctx, "/api/v1/album", body) if err != nil { + if isAlreadyExists(err) { + return ErrAlreadyExists + } return err } _ = resp.Body.Close() diff --git a/internal/lidarr/errors.go b/internal/lidarr/errors.go index 0351f12c..fc27d359 100644 --- a/internal/lidarr/errors.go +++ b/internal/lidarr/errors.go @@ -14,6 +14,12 @@ var ( ErrLookupFailed = errors.New("lidarr: lookup failed") // 4xx other than 401/403 ErrServerError = errors.New("lidarr: server error") // 5xx ErrInvalidPayload = errors.New("lidarr: invalid payload") + // ErrAlreadyExists is returned by AddArtist/AddAlbum when Lidarr + // rejects the add with a 400 because the artist/album is already in + // its library. This is the desired end state, not a failure — the + // idempotent retry path (reconciler re-sending an add after a partial + // success) treats it as confirmed rather than spinning forever. + ErrAlreadyExists = errors.New("lidarr: already exists") // ErrNotFound is returned by LookupArtistByMBID and LookupAlbumByMBID // when Lidarr returns 200 with an empty array — i.e., the MBID isn't in // Lidarr's monitored set. Distinguished from network/auth errors so admin diff --git a/internal/lidarrrequests/reconciler.go b/internal/lidarrrequests/reconciler.go index 3546dc1b..22b3c5cb 100644 --- a/internal/lidarrrequests/reconciler.go +++ b/internal/lidarrrequests/reconciler.go @@ -2,6 +2,7 @@ package lidarrrequests import ( "context" + "fmt" "log/slog" "time" @@ -11,16 +12,20 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" ) // Reconciler is a background worker that periodically scans approved -// lidarr_requests and transitions them to completed when their target -// track/album/artist has appeared in the local library (matched by MBID). -// Errors per-row are logged at WARN and do not abort the tick. +// lidarr_requests. For rows whose Lidarr add hasn't been confirmed it +// (re)sends the add idempotently until it sticks; for confirmed rows it +// transitions them to completed when their target track/album/artist has +// appeared in the local library (matched by MBID). Errors per-row are +// logged at WARN and do not abort the tick. type Reconciler struct { pool *pgxpool.Pool lidarrCfg *lidarrconfig.Service + clientFn func() *lidarr.Client logger *slog.Logger bus *eventbus.Bus tick time.Duration @@ -28,12 +33,19 @@ type Reconciler struct { } // NewReconciler constructs a Reconciler with production defaults: -// 5-minute tick, batch size 50. Pass nil for bus when no SSE publishing -// is desired (tests, or environments without the event stream enabled). -func NewReconciler(pool *pgxpool.Pool, cfg *lidarrconfig.Service, logger *slog.Logger, bus *eventbus.Bus) *Reconciler { +// 5-minute tick, batch size 50. clientFn is the per-tick Lidarr client +// factory (same pattern as Service) so config changes apply without a +// restart; pass nil to disable the add-retry path. Pass nil for bus when +// no SSE publishing is desired (tests, or environments without the event +// stream enabled). +func NewReconciler(pool *pgxpool.Pool, cfg *lidarrconfig.Service, clientFn func() *lidarr.Client, logger *slog.Logger, bus *eventbus.Bus) *Reconciler { + if clientFn == nil { + clientFn = func() *lidarr.Client { return nil } + } return &Reconciler{ pool: pool, lidarrCfg: cfg, + clientFn: clientFn, logger: logger, bus: bus, tick: 5 * time.Minute, @@ -118,7 +130,7 @@ func (r *Reconciler) tickOnce(ctx context.Context) error { } for _, row := range rows { - if err := r.reconcileRow(ctx, q, row); err != nil { + if err := r.reconcileRow(ctx, q, cfg, row); err != nil { r.logger.Warn("lidarrrequests: reconciler row failed", "request_id", row.ID, "kind", row.Kind, @@ -129,10 +141,23 @@ func (r *Reconciler) tickOnce(ctx context.Context) error { return nil } -// reconcileRow checks the library for a match and, if found, calls -// CompleteLidarrRequest. It returns an error only for unexpected DB -// failures; a no-match is not an error. -func (r *Reconciler) reconcileRow(ctx context.Context, q *dbq.Queries, row dbq.LidarrRequest) error { +// reconcileRow drives a request forward by one step. If Lidarr hasn't +// confirmed the add yet it (re)sends it and returns — the import can't +// land before Lidarr has the add, so there's nothing to match this tick. +// Once confirmed, it checks the library for a match and, if found, calls +// CompleteLidarrRequest. Returns an error only for unexpected failures +// (the add retry's transient error included, so tickOnce logs WARN and +// it's retried next tick — "Lidarr just keeps trying"); a no-match is +// not an error. +func (r *Reconciler) reconcileRow(ctx context.Context, q *dbq.Queries, cfg lidarrconfig.Config, row dbq.LidarrRequest) error { + if !row.LidarrAddConfirmedAt.Valid { + if err := r.ensureLidarrAdd(ctx, q, cfg, row); err != nil { + return err + } + // ensureLidarrAdd either confirmed the add or no-op'd (Lidarr + // client unavailable). Fall through to import-matching so an + // item already present in the library still completes this tick. + } switch row.Kind { case dbq.LidarrRequestKindArtist: return r.reconcileArtist(ctx, q, row) @@ -146,6 +171,26 @@ func (r *Reconciler) reconcileRow(ctx context.Context, q *dbq.Queries, row dbq.L } } +// ensureLidarrAdd (re)sends the Lidarr add for an approved request whose +// add hasn't been confirmed. On success (including ErrAlreadyExists, +// handled inside sendLidarrAdd) it stamps lidarr_add_confirmed_at so this +// stops re-sending and the next tick moves to import-matching. A failure +// is returned so tickOnce logs WARN and the row is retried next tick; +// there is intentionally no failed-state or attempt cap — per product +// decision Lidarr just keeps trying and the operator monitors Lidarr. +func (r *Reconciler) ensureLidarrAdd(ctx context.Context, q *dbq.Queries, cfg lidarrconfig.Config, row dbq.LidarrRequest) error { + client := r.clientFn() + if client == nil { + // Lidarr disabled mid-flight; tickOnce already gates on + // cfg.Enabled, so this is just defensive — nothing to do. + return nil + } + if err := sendLidarrAdd(ctx, client, cfg, row); err != nil { + return fmt.Errorf("lidarr add retry: %w", err) + } + return q.MarkLidarrRequestAddConfirmed(ctx, row.ID) +} + func (r *Reconciler) reconcileArtist(ctx context.Context, q *dbq.Queries, row dbq.LidarrRequest) error { var artistID pgtype.UUID err := r.pool.QueryRow(ctx, diff --git a/internal/lidarrrequests/reconciler_integration_test.go b/internal/lidarrrequests/reconciler_integration_test.go index 28bbd50a..959b6067 100644 --- a/internal/lidarrrequests/reconciler_integration_test.go +++ b/internal/lidarrrequests/reconciler_integration_test.go @@ -121,7 +121,7 @@ func TestReconciler_MatchesArtistByMBID(t *testing.T) { Kind: "artist", LidarrArtistMBID: artistMBID, ArtistName: "Boards of Canada", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -158,7 +158,7 @@ func TestReconciler_MatchesAlbumByMBID(t *testing.T) { LidarrAlbumMBID: albumMBID, AlbumTitle: "Test Album", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -200,7 +200,7 @@ func TestReconciler_MatchesTrackViaAlbumMBID(t *testing.T) { LidarrTrackMBID: trackMBID, TrackTitle: "Track One", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -234,7 +234,7 @@ func TestReconciler_NoMatchLeavesPending(t *testing.T) { Kind: "artist", LidarrArtistMBID: "nonexistent-mbid-xyz", ArtistName: "Ghost Artist", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -278,7 +278,7 @@ func TestReconciler_AlreadyCompletedRowNotReprocessed(t *testing.T) { t.Fatalf("CompleteLidarrRequest setup: %v", err) } - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -313,7 +313,7 @@ func TestReconciler_DisabledIsNoOp(t *testing.T) { Kind: "artist", LidarrArtistMBID: artistMBID, ArtistName: "No-Op Artist", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -345,7 +345,7 @@ func TestReconciler_RunRunsAtLeastOneTick(t *testing.T) { Kind: "artist", LidarrArtistMBID: mbid, ArtistName: "Run Artist", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) rec.tick = 25 * time.Millisecond done := make(chan struct{}) @@ -399,7 +399,7 @@ func TestReconciler_TrackKindNoTracksInAlbumYet(t *testing.T) { LidarrTrackMBID: "t-mbid-not-yet", TrackTitle: "Some Track", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } @@ -428,7 +428,7 @@ func TestReconciler_AlbumKindNoMatchInAlbumsTable(t *testing.T) { LidarrAlbumMBID: "al-not-in-library", AlbumTitle: "Nope Album", }) - rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger(), nil) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) if err := rec.tickOnce(ctx); err != nil { t.Fatalf("tickOnce: %v", err) } diff --git a/internal/lidarrrequests/service.go b/internal/lidarrrequests/service.go index 7552b3b9..c5721732 100644 --- a/internal/lidarrrequests/service.go +++ b/internal/lidarrrequests/service.go @@ -84,6 +84,23 @@ func (s *Service) Create(ctx context.Context, userID pgtype.UUID, p CreateParams return dbq.LidarrRequest{}, err } q := dbq.New(s.pool) + // Dedup: if a non-terminal request for this MBID already exists, + // return it instead of inserting a duplicate. The Discover/search UI + // already hides requested candidates; this backstops races and direct + // API callers so we don't accrue duplicate request rows. Match on the + // new request's own-kind MBID (mirrors HasNonTerminalRequestForMBID). + dedupMBID := p.LidarrArtistMBID + switch p.Kind { + case "album": + dedupMBID = p.LidarrAlbumMBID + case "track": + dedupMBID = p.LidarrTrackMBID + } + if existing, derr := q.GetNonTerminalRequestForMBID(ctx, dedupMBID); derr == nil { + return existing, nil + } else if !errors.Is(derr, pgx.ErrNoRows) { + return dbq.LidarrRequest{}, fmt.Errorf("create: dedup check: %w", derr) + } row, err := q.CreateLidarrRequest(ctx, dbq.CreateLidarrRequestParams{ UserID: userID, Kind: dbq.LidarrRequestKind(p.Kind), @@ -145,11 +162,14 @@ func (s *Service) ListForUser(ctx context.Context, userID pgtype.UUID, limit int }) } -// Approve transitions a pending request to approved, snapshotting the -// chosen quality profile + root folder, then calls Lidarr to actually -// add the artist/album, then triggers a library scan. If Lidarr returns -// an error, the request stays pending — the admin sees the error and -// can retry without losing the request. +// Approve records the admin's approval DURABLY first (snapshotting the +// chosen quality profile + root folder), then makes a best-effort Lidarr +// add and triggers a library scan. The add is no longer a gate: if Lidarr +// is unreachable the request still becomes approved and the reconciler +// idempotently (re)sends the add on each tick until it sticks (see +// sendLidarrAdd + Reconciler.ensureLidarrAdd) — "Lidarr just keeps +// trying" without the admin re-clicking. Pre-conditions (lidarr disabled, +// not pending, defaults incomplete) still fail fast before any approval. func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pgtype.UUID, ov ApproveOverrides) (dbq.LidarrRequest, error) { cfg, err := s.lidarrCfg.Get(ctx) if err != nil { @@ -182,23 +202,62 @@ func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pg return dbq.LidarrRequest{}, ErrDefaultsIncomplete } - // Lidarr requires a metadata profile id on POST /api/v1/artist (and - // on /api/v1/album when it has to create the parent artist). Prefer - // the operator's pinned choice from cfg; fall back to fetching the - // list and using the first when nothing's been saved yet (operators - // upgrading from before metadata-profile support landed). + // Record the approval durably FIRST so a Lidarr outage can't lose the + // admin's decision. The snapshotted qp/rf also feed the reconciler's + // retry, so a deferred add uses exactly what the admin approved with. + approved, err := dbq.New(s.pool).ApproveLidarrRequest(ctx, dbq.ApproveLidarrRequestParams{ + ID: requestID, + QualityProfileID: int32Ptr(qp), + RootFolderPath: strPtr(rf), + DecidedBy: adminID, + }) + if err != nil { + return dbq.LidarrRequest{}, fmt.Errorf("approve: persist: %w", err) + } + + // Best-effort immediate add for the common (Lidarr-up) path. Any + // failure is deliberately swallowed: the request is already durably + // approved and the reconciler retries the add on its next tick, + // leaving lidarr_add_confirmed_at NULL until it succeeds. + if addErr := sendLidarrAdd(ctx, client, cfg, approved); addErr == nil { + _ = dbq.New(s.pool).MarkLidarrRequestAddConfirmed(ctx, requestID) + } + s.scanFn() + return approved, nil +} + +// sendLidarrAdd performs the idempotent Lidarr add for an approved +// request. Used by Service.Approve (best-effort, common path) and by the +// Reconciler (retry path). qp/rf come from the request's snapshot (set at +// Approve time) with a config-default fallback; the metadata profile is +// resolved from config, falling back to Lidarr's first profile for +// operators who upgraded from before metadata-profile support. Treats +// lidarr.ErrAlreadyExists as success — the artist/album is already in +// Lidarr, which is exactly the desired end state. +func sendLidarrAdd(ctx context.Context, client *lidarr.Client, cfg lidarrconfig.Config, row dbq.LidarrRequest) error { + qp := cfg.DefaultQualityProfileID + if row.QualityProfileID != nil && *row.QualityProfileID != 0 { + qp = int(*row.QualityProfileID) + } + rf := cfg.DefaultRootFolderPath + if row.RootFolderPath != nil && *row.RootFolderPath != "" { + rf = *row.RootFolderPath + } + if qp == 0 || rf == "" { + return ErrDefaultsIncomplete + } mdProfileID := cfg.DefaultMetadataProfileID if mdProfileID == 0 { mdProfiles, mdErr := client.ListMetadataProfiles(ctx) if mdErr != nil { - return dbq.LidarrRequest{}, fmt.Errorf("approve: list metadata profiles: %w", mdErr) + return fmt.Errorf("list metadata profiles: %w", mdErr) } if len(mdProfiles) == 0 { - return dbq.LidarrRequest{}, fmt.Errorf("approve: no metadata profiles configured in lidarr") + return fmt.Errorf("no metadata profiles configured in lidarr") } mdProfileID = mdProfiles[0].ID } - + var err error switch row.Kind { case dbq.LidarrRequestKindArtist: err = client.AddArtist(ctx, lidarr.AddArtistParams{ @@ -224,22 +283,10 @@ func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pg RootFolderPath: rf, }) } - if err != nil { - return dbq.LidarrRequest{}, fmt.Errorf("approve: lidarr add: %w", err) + if err != nil && !errors.Is(err, lidarr.ErrAlreadyExists) { + return err } - - approved, err := dbq.New(s.pool).ApproveLidarrRequest(ctx, dbq.ApproveLidarrRequestParams{ - ID: requestID, - QualityProfileID: int32Ptr(qp), - RootFolderPath: strPtr(rf), - DecidedBy: adminID, - }) - if err != nil { - // Lidarr accepted but our DB update failed; admin should retry. - return dbq.LidarrRequest{}, fmt.Errorf("approve: persist: %w", err) - } - s.scanFn() - return approved, nil + return nil } // Reject transitions a pending request to rejected, recording notes and