diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..f6293143 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,18 @@ +# koel/player guidelines + +## Self-Explanatory Code +- Code should read on its own. If a piece of code needs a comment to be understood, that's a signal the code is wrong, not that the comment is needed — refactor it: extract a named helper, rename a variable to encode intent, lift a condition into a named flag, pull a block into a small function. Use a comment only when refactoring genuinely can't carry the intent (a hidden invariant, a workaround tied to a specific external bug, behaviour a reader would otherwise misjudge). Never write comments that narrate the next line, summarise the surrounding block, or restate what well-named identifiers already say. +- Don't use single-letter variable names. The only allowed ones are `i` / `j` for loop counters and `h` for the test harness. For everything else (callback params, destructured fields, lambda args, etc.) pick a name that says what it is. + +## Commit and PR Messages +- Same rule applies to commit bodies and PR descriptions: short, focused, no prose that just restates the diff. The "what" is in the diff; only spell out the "why" when it's non-obvious. +- Never mention Claude Code in commits, PR descriptions, or any generated content. No "Generated with Claude Code" footers, no Co-Authored-By lines referencing Claude. +- When the implementation of a PR changes (e.g. during code review), update the PR title and description to reflect the current state. + +## Tests +- Every change must be programmatically tested. Write a new test or update an existing test, then run the affected tests to make sure they pass. +- Don't run `flutter test` / `flutter analyze` on every edit — only before committing or when explicitly asked. +- Prefer behaviour tests that exercise a real round-trip (mocked HTTP client + state assertions) over literal-pin tests that just assert a hardcoded string against another hardcoded string. + +## Comments on Existing Code +- The repo has a memory of past comment overuse. When editing code that has lengthy comments, take the opportunity to refactor them away if you can. diff --git a/lib/providers/album_provider.dart b/lib/providers/album_provider.dart index 4f53b615..dec30364 100644 --- a/lib/providers/album_provider.dart +++ b/lib/providers/album_provider.dart @@ -1,6 +1,9 @@ +import 'dart:async'; + import 'package:app/enums.dart'; import 'package:app/mixins/stream_subscriber.dart'; import 'package:app/models/models.dart'; +import 'package:app/providers/artist_provider.dart'; import 'package:app/providers/auth_provider.dart'; import 'package:app/utils/api_request.dart'; import 'package:flutter/foundation.dart'; @@ -12,6 +15,9 @@ class AlbumProvider with ChangeNotifier, StreamSubscriber { var _sortField = 'name'; var _sortOrder = SortOrder.asc; + static final _renamedController = StreamController.broadcast(); + static final renamedStream = _renamedController.stream; + String get sortField => _sortField; SortOrder get sortOrder => _sortOrder; @@ -36,6 +42,19 @@ class AlbumProvider with ChangeNotifier, StreamSubscriber { notifyListeners(); })); + + subscribe(ArtistProvider.renamedStream.listen(_onArtistRenamed)); + } + + void _onArtistRenamed(Artist artist) { + var changed = false; + for (final album in _vault.values) { + if (album.artistId == artist.id && album.artistName != artist.name) { + album.artistName = artist.name; + changed = true; + } + } + if (changed) notifyListeners(); } List byIds(List ids) { @@ -135,6 +154,8 @@ class AlbumProvider with ChangeNotifier, StreamSubscriber { 'year': year, }); + final renamed = album.name != response['name']; + album ..name = response['name'] ..year = response['year'] == null @@ -142,5 +163,7 @@ class AlbumProvider with ChangeNotifier, StreamSubscriber { : int.parse(response['year'].toString()); notifyListeners(); + + if (renamed) _renamedController.add(album); } } diff --git a/lib/providers/artist_provider.dart b/lib/providers/artist_provider.dart index 5de6ed5c..eab658e7 100644 --- a/lib/providers/artist_provider.dart +++ b/lib/providers/artist_provider.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:app/enums.dart'; import 'package:app/mixins/stream_subscriber.dart'; import 'package:app/models/models.dart'; @@ -12,6 +14,9 @@ class ArtistProvider with ChangeNotifier, StreamSubscriber { var _sortField = 'name'; var _sortOrder = SortOrder.asc; + static final _renamedController = StreamController.broadcast(); + static final renamedStream = _renamedController.stream; + String get sortField => _sortField; SortOrder get sortOrder => _sortOrder; @@ -130,8 +135,12 @@ class ArtistProvider with ChangeNotifier, StreamSubscriber { 'name': name, }); + final renamed = artist.name != response['name']; + artist.name = response['name']; notifyListeners(); + + if (renamed) _renamedController.add(artist); } } diff --git a/lib/providers/playable_provider.dart b/lib/providers/playable_provider.dart index 5dbd0805..d7d2689e 100644 --- a/lib/providers/playable_provider.dart +++ b/lib/providers/playable_provider.dart @@ -17,6 +17,36 @@ class PlayableProvider with ChangeNotifier, StreamSubscriber { _vault.clear(); notifyListeners(); })); + + subscribe(AlbumProvider.renamedStream.listen(_onAlbumRenamed)); + subscribe(ArtistProvider.renamedStream.listen(_onArtistRenamed)); + } + + void _onAlbumRenamed(Album album) { + var changed = false; + for (final song in _vault.values.whereType()) { + if (song.albumId == album.id && song.albumName != album.name) { + song.albumName = album.name; + changed = true; + } + } + if (changed) notifyListeners(); + } + + void _onArtistRenamed(Artist artist) { + var changed = false; + for (final song in _vault.values.whereType()) { + if (song.artistId == artist.id && song.artistName != artist.name) { + song.artistName = artist.name; + changed = true; + } + if (song.albumArtistId == artist.id && + song.albumArtistName != artist.name) { + song.albumArtistName = artist.name; + changed = true; + } + } + if (changed) notifyListeners(); } List syncWithVault(dynamic _playables) { diff --git a/lib/ui/screens/album_details.dart b/lib/ui/screens/album_details.dart index b831ba0a..3417074c 100644 --- a/lib/ui/screens/album_details.dart +++ b/lib/ui/screens/album_details.dart @@ -51,9 +51,10 @@ class _AlbumDetailsScreenState extends State { return Scaffold( body: GradientDecoratedContainer( - child: FutureBuilder( - future: buildRequest(albumId), - builder: (_, AsyncSnapshot> snapshot) { + child: Consumer( + builder: (_, __, ___) => FutureBuilder( + future: buildRequest(albumId), + builder: (_, AsyncSnapshot> snapshot) { if (!snapshot.hasData || snapshot.connectionState == ConnectionState.active) return const PlayableListScreenPlaceholder(); @@ -123,7 +124,8 @@ class _AlbumDetailsScreenState extends State { ], ), ); - }, + }, + ), ), ), ); diff --git a/lib/ui/screens/artist_details.dart b/lib/ui/screens/artist_details.dart index d1b35af5..cd8bc656 100644 --- a/lib/ui/screens/artist_details.dart +++ b/lib/ui/screens/artist_details.dart @@ -43,9 +43,10 @@ class _ArtistDetailsScreenState extends State { return Scaffold( body: GradientDecoratedContainer( - child: FutureBuilder( - future: buildRequest(artistId), - builder: (_, AsyncSnapshot> snapshot) { + child: Consumer( + builder: (_, __, ___) => FutureBuilder( + future: buildRequest(artistId), + builder: (_, AsyncSnapshot> snapshot) { if (!snapshot.hasData || snapshot.connectionState == ConnectionState.active) return const PlayableListScreenPlaceholder(); @@ -131,7 +132,8 @@ class _ArtistDetailsScreenState extends State { ), ), ); - }, + }, + ), ), ), ); diff --git a/lib/ui/widgets/album_card.dart b/lib/ui/widgets/album_card.dart index e368ae9f..e8707b52 100644 --- a/lib/ui/widgets/album_card.dart +++ b/lib/ui/widgets/album_card.dart @@ -1,8 +1,10 @@ import 'package:app/models/models.dart'; +import 'package:app/providers/album_provider.dart'; import 'package:app/router.dart'; import 'package:app/ui/screens/album_action_sheet.dart'; import 'package:app/ui/widgets/widgets.dart'; import 'package:flutter/material.dart'; +import 'package:provider/provider.dart'; class AlbumCard extends StatefulWidget { final Album album; @@ -24,43 +26,45 @@ class _AlbumCardState extends State { @override Widget build(BuildContext context) { - return GestureDetector( - onTapDown: (_) => setState(() => _opacity = 0.4), - onTapUp: (_) => setState(() => _opacity = 1.0), - onTapCancel: () => setState(() => _opacity = 1.0), - onTap: () => widget.router.gotoAlbumDetailsScreen( - context, - albumId: widget.album.id, - ), - onLongPress: () => showAlbumActionSheet(context, album: widget.album), - behavior: HitTestBehavior.opaque, - child: AnimatedOpacity( - duration: const Duration(milliseconds: 100), - opacity: _opacity, - child: Column( - children: [ - AlbumArtistThumbnail.md(entity: widget.album, asHero: true), - const SizedBox(height: 12), - SizedBox( - width: _cardWidth, - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text( - widget.album.name, - style: const TextStyle(fontWeight: FontWeight.bold), - overflow: TextOverflow.ellipsis, - ), - const SizedBox(height: 6), - Text( - widget.album.artistName, - style: const TextStyle(color: Colors.white54), - overflow: TextOverflow.ellipsis, - ), - ], + return Consumer( + builder: (_, __, ___) => GestureDetector( + onTapDown: (_) => setState(() => _opacity = 0.4), + onTapUp: (_) => setState(() => _opacity = 1.0), + onTapCancel: () => setState(() => _opacity = 1.0), + onTap: () => widget.router.gotoAlbumDetailsScreen( + context, + albumId: widget.album.id, + ), + onLongPress: () => showAlbumActionSheet(context, album: widget.album), + behavior: HitTestBehavior.opaque, + child: AnimatedOpacity( + duration: const Duration(milliseconds: 100), + opacity: _opacity, + child: Column( + children: [ + AlbumArtistThumbnail.md(entity: widget.album, asHero: true), + const SizedBox(height: 12), + SizedBox( + width: _cardWidth, + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + widget.album.name, + style: const TextStyle(fontWeight: FontWeight.bold), + overflow: TextOverflow.ellipsis, + ), + const SizedBox(height: 6), + Text( + widget.album.artistName, + style: const TextStyle(color: Colors.white54), + overflow: TextOverflow.ellipsis, + ), + ], + ), ), - ), - ], + ], + ), ), ), ); diff --git a/lib/ui/widgets/artist_card.dart b/lib/ui/widgets/artist_card.dart index 79dc1ff4..63e6448b 100644 --- a/lib/ui/widgets/artist_card.dart +++ b/lib/ui/widgets/artist_card.dart @@ -1,8 +1,10 @@ import 'package:app/models/models.dart'; +import 'package:app/providers/artist_provider.dart'; import 'package:app/router.dart'; import 'package:app/ui/screens/artist_action_sheet.dart'; import 'package:app/ui/widgets/widgets.dart'; import 'package:flutter/material.dart'; +import 'package:provider/provider.dart'; class ArtistCard extends StatefulWidget { final Artist artist; @@ -24,37 +26,40 @@ class _ArtistCardState extends State { @override Widget build(BuildContext context) { - return GestureDetector( - onTapDown: (_) => setState(() => _opacity = 0.4), - onTapUp: (_) => setState(() => _opacity = 1.0), - onTapCancel: () => setState(() => _opacity = 1.0), - onTap: () => widget.router.gotoArtistDetailsScreen( - context, - artistId: widget.artist.id, - ), - onLongPress: () => showArtistActionSheet(context, artist: widget.artist), - behavior: HitTestBehavior.opaque, - child: AnimatedOpacity( - duration: const Duration(milliseconds: 100), - opacity: _opacity, - child: Column( - children: [ - AlbumArtistThumbnail.md(entity: widget.artist, asHero: true), - const SizedBox(height: 12), - SizedBox( - width: _cardWidth, - child: Column( - crossAxisAlignment: CrossAxisAlignment.center, - children: [ - Text( - widget.artist.name, - style: const TextStyle(fontWeight: FontWeight.bold), - overflow: TextOverflow.ellipsis, - ), - ], + return Consumer( + builder: (_, __, ___) => GestureDetector( + onTapDown: (_) => setState(() => _opacity = 0.4), + onTapUp: (_) => setState(() => _opacity = 1.0), + onTapCancel: () => setState(() => _opacity = 1.0), + onTap: () => widget.router.gotoArtistDetailsScreen( + context, + artistId: widget.artist.id, + ), + onLongPress: () => + showArtistActionSheet(context, artist: widget.artist), + behavior: HitTestBehavior.opaque, + child: AnimatedOpacity( + duration: const Duration(milliseconds: 100), + opacity: _opacity, + child: Column( + children: [ + AlbumArtistThumbnail.md(entity: widget.artist, asHero: true), + const SizedBox(height: 12), + SizedBox( + width: _cardWidth, + child: Column( + crossAxisAlignment: CrossAxisAlignment.center, + children: [ + Text( + widget.artist.name, + style: const TextStyle(fontWeight: FontWeight.bold), + overflow: TextOverflow.ellipsis, + ), + ], + ), ), - ), - ], + ], + ), ), ), ); diff --git a/lib/ui/widgets/now_playing/playable_info.dart b/lib/ui/widgets/now_playing/playable_info.dart index eb188276..227bd68b 100644 --- a/lib/ui/widgets/now_playing/playable_info.dart +++ b/lib/ui/widgets/now_playing/playable_info.dart @@ -1,43 +1,43 @@ import 'package:app/models/models.dart'; +import 'package:app/providers/playable_provider.dart'; import 'package:app/ui/widgets/marquee_text.dart'; import 'package:flutter/material.dart'; +import 'package:provider/provider.dart'; class PlayableInfo extends StatelessWidget { final Playable playable; const PlayableInfo({Key? key, required this.playable}) : super(key: key); + String get _subtitle { + if (playable is Song) return (playable as Song).artistName; + if (playable is Episode) return (playable as Episode).podcastTitle; + return ''; + } + @override Widget build(BuildContext context) { - late final String subtitle; - - if (playable is Song) { - subtitle = (playable as Song).artistName; - } else if (playable is Episode) { - subtitle = (playable as Episode).podcastTitle; - } else { - subtitle = ''; - } - - return Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - MarqueeText( - text: playable.title, - style: const TextStyle( - fontSize: 20, - fontWeight: FontWeight.w600, + return Consumer( + builder: (_, __, ___) => Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + MarqueeText( + text: playable.title, + style: const TextStyle( + fontSize: 20, + fontWeight: FontWeight.w600, + ), ), - ), - const SizedBox(height: 4), - MarqueeText( - text: subtitle, - style: const TextStyle( - color: Colors.white70, - fontSize: 18, + const SizedBox(height: 4), + MarqueeText( + text: _subtitle, + style: const TextStyle( + color: Colors.white70, + fontSize: 18, + ), ), - ), - ], + ], + ), ); } } diff --git a/lib/ui/widgets/playable_row.dart b/lib/ui/widgets/playable_row.dart index 046db687..594a9e4b 100644 --- a/lib/ui/widgets/playable_row.dart +++ b/lib/ui/widgets/playable_row.dart @@ -66,67 +66,62 @@ class _PlayableRowState extends State { return 0; } - @override - Widget build(BuildContext context) { - late String subtitle; - + String _subtitle() { switch (widget.listContext) { case PlayableListContext.album: case PlayableListContext.artist: - subtitle = (widget.playable as Song).albumName; - break; + return (widget.playable as Song).albumName; case PlayableListContext.podcast: - var createdAt = (widget.playable as Episode).createdAt; - // To be more user-friendly, we display the human readable format, - // but not for too old (>6 months in the past) dates. - subtitle = - DateTime.now().difference(createdAt) > const Duration(days: 180) - ? DateFormat.yMMMd().format(createdAt) - : timeago.format(createdAt); - break; + final createdAt = (widget.playable as Episode).createdAt; + return DateTime.now().difference(createdAt) > const Duration(days: 180) + ? DateFormat.yMMMd().format(createdAt) + : timeago.format(createdAt); default: - if (widget.playable is Episode) - subtitle = (widget.playable as Episode).podcastTitle; - else if (widget.playable is Song) - subtitle = (widget.playable as Song).artistName; - break; + if (widget.playable is Episode) { + return (widget.playable as Episode).podcastTitle; + } + return (widget.playable as Song).artistName; } + } - return Card( - child: InkWell( - onTap: () async => await audioHandler.maybeQueueAndPlay( - widget.playable, - position: await getPlaybackStartPosition(widget.playable), - ), - onLongPress: () { - HapticFeedback.mediumImpact(); - widget.router.showPlayableActionSheet( - context, - playable: widget.playable, - ); - }, - child: ListTile( - key: UniqueKey(), - shape: widget.bordered - ? Border(bottom: Divider.createBorderSide(context)) - : null, - leading: widget.listContext == PlayableListContext.album - ? PlayableRowTrackNumber(song: widget.playable as Song) - : widget.listContext == PlayableListContext.podcast - ? null - : PlayableRowThumbnail(playable: widget.playable), - minLeadingWidth: - widget.listContext == PlayableListContext.album ? 0 : null, - title: Text(widget.playable.title, overflow: TextOverflow.ellipsis), - subtitle: Text( - subtitle, - overflow: TextOverflow.ellipsis, - style: const TextStyle(color: Colors.white60), + @override + Widget build(BuildContext context) { + return Consumer( + builder: (_, __, ___) => Card( + child: InkWell( + onTap: () async => await audioHandler.maybeQueueAndPlay( + widget.playable, + position: await getPlaybackStartPosition(widget.playable), ), - trailing: PlayableRowTrailingActions( - playable: widget.playable, - listContext: widget.listContext, - index: widget.index, + onLongPress: () { + HapticFeedback.mediumImpact(); + widget.router.showPlayableActionSheet( + context, + playable: widget.playable, + ); + }, + child: ListTile( + shape: widget.bordered + ? Border(bottom: Divider.createBorderSide(context)) + : null, + leading: widget.listContext == PlayableListContext.album + ? PlayableRowTrackNumber(song: widget.playable as Song) + : widget.listContext == PlayableListContext.podcast + ? null + : PlayableRowThumbnail(playable: widget.playable), + minLeadingWidth: + widget.listContext == PlayableListContext.album ? 0 : null, + title: Text(widget.playable.title, overflow: TextOverflow.ellipsis), + subtitle: Text( + _subtitle(), + overflow: TextOverflow.ellipsis, + style: const TextStyle(color: Colors.white60), + ), + trailing: PlayableRowTrailingActions( + playable: widget.playable, + listContext: widget.listContext, + index: widget.index, + ), ), ), ), diff --git a/lib/ui/widgets/song_card.dart b/lib/ui/widgets/song_card.dart index 70abc0aa..181a8005 100644 --- a/lib/ui/widgets/song_card.dart +++ b/lib/ui/widgets/song_card.dart @@ -1,9 +1,11 @@ import 'package:app/main.dart'; import 'package:app/models/models.dart'; +import 'package:app/providers/providers.dart'; import 'package:app/router.dart'; import 'package:app/ui/widgets/widgets.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +import 'package:provider/provider.dart'; class SongCard extends StatefulWidget { final Playable playable; @@ -36,46 +38,48 @@ class _SongCardState extends State { @override Widget build(BuildContext context) { - return GestureDetector( - onTapDown: (_) => setState(() => _opacity = 0.7), - onTapCancel: () => setState(() => _opacity = 1.0), - onTap: () async { - setState(() => _opacity = 1.0); - await audioHandler.queueAndPlay(widget.playable); - }, - onLongPress: () { - HapticFeedback.mediumImpact(); - widget.router - .showPlayableActionSheet(context, playable: widget.playable); - }, - behavior: HitTestBehavior.opaque, - child: AnimatedOpacity( - duration: const Duration(microseconds: 100), - opacity: _opacity, - child: Column( - children: [ - PlayableThumbnail.md(playable: widget.playable), - const SizedBox(height: 12), - SizedBox( - width: _cardWidth, - child: Column( - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - Text( - widget.playable.title, - style: const TextStyle(fontWeight: FontWeight.bold), - overflow: TextOverflow.ellipsis, - ), - const SizedBox(height: 6), - Text( - _subtitle, - style: const TextStyle(color: Colors.white54), - overflow: TextOverflow.ellipsis, - ) - ], + return Consumer( + builder: (_, __, ___) => GestureDetector( + onTapDown: (_) => setState(() => _opacity = 0.7), + onTapCancel: () => setState(() => _opacity = 1.0), + onTap: () async { + setState(() => _opacity = 1.0); + await audioHandler.queueAndPlay(widget.playable); + }, + onLongPress: () { + HapticFeedback.mediumImpact(); + widget.router + .showPlayableActionSheet(context, playable: widget.playable); + }, + behavior: HitTestBehavior.opaque, + child: AnimatedOpacity( + duration: const Duration(microseconds: 100), + opacity: _opacity, + child: Column( + children: [ + PlayableThumbnail.md(playable: widget.playable), + const SizedBox(height: 12), + SizedBox( + width: _cardWidth, + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + widget.playable.title, + style: const TextStyle(fontWeight: FontWeight.bold), + overflow: TextOverflow.ellipsis, + ), + const SizedBox(height: 6), + Text( + _subtitle, + style: const TextStyle(color: Colors.white54), + overflow: TextOverflow.ellipsis, + ), + ], + ), ), - ), - ], + ], + ), ), ), ); diff --git a/test/providers/rename_propagation_test.dart b/test/providers/rename_propagation_test.dart new file mode 100644 index 00000000..12a15edc --- /dev/null +++ b/test/providers/rename_propagation_test.dart @@ -0,0 +1,146 @@ +import 'package:app/models/album.dart'; +import 'package:app/models/artist.dart'; +import 'package:app/models/song.dart'; +import 'package:app/providers/album_provider.dart'; +import 'package:app/providers/artist_provider.dart'; +import 'package:app/providers/playable_provider.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/api_test_setup.dart'; + +PlayableProvider _seedPlayables(List songs) { + final provider = PlayableProvider(); + provider.syncWithVault(songs); + return provider; +} + +Future _flushMicrotasks() => Future.delayed(Duration.zero); + +void main() { + setUpAll(initApiTestEnvironment); + setUp(setUpApiTest); + tearDown(tearDownApiTest); + + group('AlbumProvider rename → PlayableProvider', () { + test( + 'songs sharing the renamed album get their albumName updated', + (() async { + CapturingClient() + ..willReturn(json: {'name': 'New Album', 'year': null}) + ..install(); + + final artist = Artist.fake(); + final album = Album.fake(id: 'a-1', artist: artist)..name = 'Old'; + final matching = Song.fake(album: album); + final unrelated = Song.fake(); + final playables = _seedPlayables([matching, unrelated]); + + final albumProvider = AlbumProvider(); + var notifyCount = 0; + playables.addListener(() => notifyCount++); + + await albumProvider.update(album, name: 'New Album'); + await _flushMicrotasks(); + + expect(matching.albumName, 'New Album'); + expect(unrelated.albumName, isNot('New Album')); + expect(notifyCount, greaterThanOrEqualTo(1)); + }), + ); + + test( + 'no propagation (and no notify) when the name did not change', + (() async { + final album = Album.fake(id: 'a-2')..name = 'Same Name'; + CapturingClient() + ..willReturn(json: {'name': 'Same Name', 'year': null}) + ..install(); + + final song = Song.fake(album: album); + final playables = _seedPlayables([song]); + + var notifyCount = 0; + playables.addListener(() => notifyCount++); + + await AlbumProvider().update(album, name: 'Same Name'); + await _flushMicrotasks(); + + expect(song.albumName, 'Same Name'); + expect(notifyCount, 0); + }), + ); + }); + + group('ArtistProvider rename → PlayableProvider', () { + test( + 'songs whose artist matches get artistName updated', + (() async { + CapturingClient()..willReturn(json: {'name': 'New Artist'})..install(); + + final artist = Artist.fake(id: 'ar-1')..name = 'Old'; + final unrelatedArtist = Artist.fake(); + final song = Song.fake(artist: artist); + final unrelated = Song.fake(artist: unrelatedArtist); + _seedPlayables([song, unrelated]); + + await ArtistProvider().update(artist, name: 'New Artist'); + await _flushMicrotasks(); + + expect(song.artistName, 'New Artist'); + expect(unrelated.artistName, isNot('New Artist')); + expect(song.albumArtistName, 'New Artist'); + }), + ); + + test( + 'songs where albumArtistId matches but artistId does not get ' + 'only albumArtistName updated', + (() async { + CapturingClient()..willReturn(json: {'name': 'Renamed AA'})..install(); + + final albumArtist = Artist.fake(id: 'aa-1')..name = 'Old AA'; + final featuredArtist = Artist.fake(id: 'ft-1')..name = 'Featured'; + final song = Song.fake( + artist: featuredArtist, + albumArtist: albumArtist, + ); + _seedPlayables([song]); + + await ArtistProvider().update(albumArtist, name: 'Renamed AA'); + await _flushMicrotasks(); + + expect(song.albumArtistName, 'Renamed AA'); + expect(song.artistName, 'Featured'); + }), + ); + }); + + group('ArtistProvider rename → AlbumProvider', () { + test( + 'albums sharing the renamed artist get their artistName updated', + (() async { + CapturingClient() + ..willReturn(json: {'name': 'Rebranded'}) + ..install(); + + final artist = Artist.fake(id: 'ar-2')..name = 'Old'; + final unrelatedArtist = Artist.fake(); + final albumProvider = AlbumProvider(); + albumProvider.syncWithVault([ + Album.fake(id: 'al-1', artist: artist), + Album.fake(id: 'al-2', artist: unrelatedArtist), + ]); + + var notifyCount = 0; + albumProvider.addListener(() => notifyCount++); + + await ArtistProvider().update(artist, name: 'Rebranded'); + await _flushMicrotasks(); + + expect(albumProvider.byId('al-1')!.artistName, 'Rebranded'); + expect(albumProvider.byId('al-2')!.artistName, isNot('Rebranded')); + expect(notifyCount, greaterThanOrEqualTo(1)); + }), + ); + }); +} diff --git a/test/ui/widgets/album_card_test.dart b/test/ui/widgets/album_card_test.dart index c7363222..e70dc363 100644 --- a/test/ui/widgets/album_card_test.dart +++ b/test/ui/widgets/album_card_test.dart @@ -1,43 +1,67 @@ import 'package:app/models/album.dart'; import 'package:app/models/artist.dart'; +import 'package:app/providers/album_provider.dart'; import 'package:app/router.dart'; import 'package:app/ui/widgets/album_artist_thumbnail.dart'; import 'package:app/ui/widgets/album_card.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; import '../../extensions/widget_tester_extension.dart'; import 'album_card_test.mocks.dart'; +Future _pump(WidgetTester tester, Widget child, AlbumProvider provider) => + tester.pumpAppWidget( + ChangeNotifierProvider.value(value: provider, child: child), + ); + @GenerateMocks([AppRouter]) void main() { late Album album; + late AlbumProvider provider; - setUpAll(() { + setUp(() { album = Album.fake( name: 'A Whole New Bunch', artist: Artist.fake(name: 'Banana'), ); + provider = AlbumProvider(); + addTearDown(provider.dispose); + provider.syncWithVault([album]); }); - testWidgets('renders', (WidgetTester tester) async { - await tester.pumpAppWidget(AlbumCard(album: album)); + testWidgets('renders', (tester) async { + await _pump(tester, AlbumCard(album: album), provider); expect(find.byType(AlbumArtistThumbnail), findsOneWidget); expect(find.text('Banana'), findsOneWidget); expect(find.text('A Whole New Bunch'), findsOneWidget); }); - testWidgets('goes to Album Details screen', (WidgetTester tester) async { - MockAppRouter router = MockAppRouter(); + testWidgets('goes to Album Details screen', (tester) async { + final router = MockAppRouter(); when( router.gotoAlbumDetailsScreen(any, albumId: album.id), ).thenAnswer((_) async => null); - await tester.pumpAppWidget(AlbumCard(album: album, router: router)); + await _pump(tester, AlbumCard(album: album, router: router), provider); await tester.tap(find.text('A Whole New Bunch')); verify(router.gotoAlbumDetailsScreen(any, albumId: album.id)).called(1); }); + + testWidgets('rebuilds when AlbumProvider notifies', (tester) async { + await _pump(tester, AlbumCard(album: album), provider); + expect(find.text('Banana'), findsOneWidget); + + album.artistName = 'Renamed'; + provider.notifyListeners(); + await tester.pump(); + + expect(find.text('Renamed'), findsOneWidget); + expect(find.text('Banana'), findsNothing); + }); } diff --git a/test/ui/widgets/artist_card_test.dart b/test/ui/widgets/artist_card_test.dart index c2169147..090b8f77 100644 --- a/test/ui/widgets/artist_card_test.dart +++ b/test/ui/widgets/artist_card_test.dart @@ -1,38 +1,62 @@ import 'package:app/models/artist.dart'; +import 'package:app/providers/artist_provider.dart'; import 'package:app/router.dart'; import 'package:app/ui/widgets/album_artist_thumbnail.dart'; import 'package:app/ui/widgets/artist_card.dart'; +import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/annotations.dart'; import 'package:mockito/mockito.dart'; +import 'package:provider/provider.dart'; import '../../extensions/widget_tester_extension.dart'; import 'artist_card_test.mocks.dart'; +Future _pump(WidgetTester tester, Widget child, ArtistProvider provider) => + tester.pumpAppWidget( + ChangeNotifierProvider.value(value: provider, child: child), + ); + @GenerateMocks([AppRouter]) void main() { late Artist artist; + late ArtistProvider provider; - setUpAll(() { + setUp(() { artist = Artist.fake(name: 'Banana'); + provider = ArtistProvider(); + addTearDown(provider.dispose); + provider.syncWithVault([artist]); }); - testWidgets('renders', (WidgetTester tester) async { - await tester.pumpAppWidget(ArtistCard(artist: artist)); + testWidgets('renders', (tester) async { + await _pump(tester, ArtistCard(artist: artist), provider); expect(find.byType(AlbumArtistThumbnail), findsOneWidget); expect(find.text('Banana'), findsOneWidget); }); - testWidgets('goes to Artist Details screen', (WidgetTester tester) async { - MockAppRouter router = MockAppRouter(); + testWidgets('goes to Artist Details screen', (tester) async { + final router = MockAppRouter(); when( router.gotoArtistDetailsScreen(any, artistId: artist.id), ).thenAnswer((_) async => null); - await tester.pumpAppWidget(ArtistCard(artist: artist, router: router)); + await _pump(tester, ArtistCard(artist: artist, router: router), provider); await tester.tap(find.text('Banana')); verify(router.gotoArtistDetailsScreen(any, artistId: artist.id)).called(1); }); + + testWidgets('rebuilds when ArtistProvider notifies', (tester) async { + await _pump(tester, ArtistCard(artist: artist), provider); + expect(find.text('Banana'), findsOneWidget); + + artist.name = 'Renamed'; + provider.notifyListeners(); + await tester.pump(); + + expect(find.text('Renamed'), findsOneWidget); + expect(find.text('Banana'), findsNothing); + }); } diff --git a/test/ui/widgets/song_card_test.dart b/test/ui/widgets/song_card_test.dart index de2c6b17..2e9b76cc 100644 --- a/test/ui/widgets/song_card_test.dart +++ b/test/ui/widgets/song_card_test.dart @@ -1,23 +1,44 @@ import 'package:app/models/models.dart'; +import 'package:app/providers/playable_provider.dart'; import 'package:app/ui/widgets/song_card.dart'; import 'package:app/ui/widgets/playable_thumbnail.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; import '../../extensions/widget_tester_extension.dart'; +class _Harness { + final PlayableProvider provider; + final Future Function() pump; + _Harness(this.provider, this.pump); +} + +_Harness _harness(WidgetTester tester, Playable playable) { + final provider = PlayableProvider(); + addTearDown(provider.dispose); + if (playable is Song) provider.syncWithVault([playable]); + return _Harness( + provider, + () => tester.pumpAppWidget( + ChangeNotifierProvider.value( + value: provider, + child: SongCard(playable: playable), + ), + ), + ); +} + void main() { - testWidgets('renders a Song with artist name', (WidgetTester tester) async { + testWidgets('renders a Song with artist name', (tester) async { final song = Song.fake(title: 'Test Song'); - - await tester.pumpAppWidget(SongCard(playable: song)); + await _harness(tester, song).pump(); expect(find.text('Test Song'), findsOneWidget); expect(find.text(song.artistName), findsOneWidget); expect(find.byType(PlayableThumbnail), findsOneWidget); }); - testWidgets('renders an Episode with podcast title', - (WidgetTester tester) async { + testWidgets('renders an Episode with podcast title', (tester) async { final episode = Episode( id: 'ep-1', length: 300, @@ -30,10 +51,27 @@ void main() { link: null, createdAt: DateTime.now(), ); - - await tester.pumpAppWidget(SongCard(playable: episode)); + await _harness(tester, episode).pump(); expect(find.text('Test Episode'), findsOneWidget); expect(find.text('My Podcast'), findsOneWidget); }); + + testWidgets( + 'rebuilds the artist subtitle when PlayableProvider notifies after a ' + 'rename mutation', + (tester) async { + final song = Song.fake(title: 'Anthem')..artistName = 'Old Artist'; + final harness = _harness(tester, song); + await harness.pump(); + expect(find.text('Old Artist'), findsOneWidget); + + song.artistName = 'Renamed Artist'; + harness.provider.notifyListeners(); + await tester.pump(); + + expect(find.text('Renamed Artist'), findsOneWidget); + expect(find.text('Old Artist'), findsNothing); + }, + ); }