mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-10-20 22:41:38 +02:00
fix(playlists): prevent overlapping picker submissions
This commit is contained in:
1 parent
92e3fa94ec
commit
0fdcbc3cbd
2 files changed
+150
-10
No files matched your search
@@ -7,12 +7,15 @@ import 'package:spotiflac_android/l10n/l10n.dart';
|
||||
import 'package:spotiflac_android/models/track.dart';
|
||||
import 'package:spotiflac_android/providers/library_collections_provider.dart';
|
||||
import 'package:spotiflac_android/theme/mornye_theme.dart';
|
||||
import 'package:spotiflac_android/utils/logger.dart';
|
||||
import 'package:spotiflac_android/widgets/app_action_button.dart';
|
||||
import 'package:spotiflac_android/widgets/app_alert_dialog.dart';
|
||||
import 'package:spotiflac_android/widgets/app_bottom_sheet.dart';
|
||||
import 'package:spotiflac_android/widgets/cached_cover_image.dart';
|
||||
import 'package:spotiflac_android/widgets/mornye_chrome.dart';
|
||||
|
||||
final _log = AppLogger('PlaylistPicker');
|
||||
|
||||
Future<void> showAddTrackToPlaylistSheet(
|
||||
BuildContext context,
|
||||
WidgetRef ref,
|
||||
@@ -68,6 +71,7 @@ class _PlaylistPickerSheetContentState
|
||||
late final PlaylistPickerSummaryRequest _summaryRequest;
|
||||
final Set<String> _selectedPlaylistIds = {};
|
||||
final Set<String> _committedPlaylistIds = {};
|
||||
bool _isSubmitting = false;
|
||||
|
||||
@override
|
||||
void initState() {
|
||||
@@ -75,7 +79,9 @@ class _PlaylistPickerSheetContentState
|
||||
_summaryRequest = PlaylistPickerSummaryRequest.fromTracks(widget.tracks);
|
||||
}
|
||||
|
||||
void _handleDone(List<PlaylistPickerSummary> playlists) async {
|
||||
Future<void> _handleDone(List<PlaylistPickerSummary> playlists) async {
|
||||
if (_isSubmitting) return;
|
||||
setState(() => _isSubmitting = true);
|
||||
final notifier = ref.read(libraryCollectionsProvider.notifier);
|
||||
final effectiveDisabledIds = <String>{
|
||||
..._committedPlaylistIds,
|
||||
@@ -88,12 +94,23 @@ class _PlaylistPickerSheetContentState
|
||||
};
|
||||
final addedNames = <String>[];
|
||||
|
||||
for (final playlistId in idsToAdd) {
|
||||
final playlistName = playlistNamesById[playlistId];
|
||||
if (playlistName != null && playlistName.isNotEmpty) {
|
||||
addedNames.add(playlistName);
|
||||
try {
|
||||
for (final playlistId in idsToAdd) {
|
||||
await notifier.addTracksToPlaylist(playlistId, widget.tracks);
|
||||
_committedPlaylistIds.add(playlistId);
|
||||
final playlistName = playlistNamesById[playlistId];
|
||||
if (playlistName != null && playlistName.isNotEmpty) {
|
||||
addedNames.add(playlistName);
|
||||
}
|
||||
}
|
||||
await notifier.addTracksToPlaylist(playlistId, widget.tracks);
|
||||
} catch (error, stack) {
|
||||
_log.e('Failed to add tracks to selected playlists', error, stack);
|
||||
if (!mounted) return;
|
||||
setState(() => _isSubmitting = false);
|
||||
ScaffoldMessenger.of(
|
||||
context,
|
||||
).showSnackBar(SnackBar(content: Text(context.friendlyError(error))));
|
||||
return;
|
||||
}
|
||||
|
||||
if (!mounted) return;
|
||||
@@ -134,9 +151,11 @@ class _PlaylistPickerSheetContentState
|
||||
final hasNewSelections = idsToAdd.isNotEmpty;
|
||||
|
||||
void onDone() {
|
||||
if (_isSubmitting) return;
|
||||
if (hasNewSelections) {
|
||||
_handleDone(resolvedPlaylists);
|
||||
} else {
|
||||
setState(() => _isSubmitting = true);
|
||||
Navigator.of(context).pop();
|
||||
}
|
||||
}
|
||||
@@ -187,7 +206,9 @@ class _PlaylistPickerSheetContentState
|
||||
color: Theme.of(context).colorScheme.primary,
|
||||
),
|
||||
title: Text(context.l10n.collectionCreatePlaylist),
|
||||
enabled: !_isSubmitting,
|
||||
onTap: () async {
|
||||
if (_isSubmitting) return;
|
||||
final name = await _promptPlaylistName(
|
||||
context,
|
||||
widget.playlistNamePrefill,
|
||||
@@ -250,10 +271,11 @@ class _PlaylistPickerSheetContentState
|
||||
playlist.trackCount,
|
||||
),
|
||||
),
|
||||
enabled: !isAlreadyIn,
|
||||
enabled: !isAlreadyIn && !_isSubmitting,
|
||||
selected: isSelected,
|
||||
onTap: !isAlreadyIn
|
||||
onTap: !isAlreadyIn && !_isSubmitting
|
||||
? () {
|
||||
if (_isSubmitting) return;
|
||||
setState(() {
|
||||
if (_selectedPlaylistIds.contains(
|
||||
playlist.id,
|
||||
@@ -295,12 +317,12 @@ class _PlaylistPickerSheetContentState
|
||||
width: double.infinity,
|
||||
child: context.isMornye
|
||||
? AppActionButton(
|
||||
onPressed: onDone,
|
||||
onPressed: _isSubmitting ? null : onDone,
|
||||
icon: const Icon(CupertinoIcons.checkmark),
|
||||
label: Text(context.l10n.dialogDone),
|
||||
)
|
||||
: FilledButton(
|
||||
onPressed: onDone,
|
||||
onPressed: _isSubmitting ? null : onDone,
|
||||
child: Text(context.l10n.dialogDone),
|
||||
),
|
||||
),
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:flutter/cupertino.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
@@ -10,6 +12,7 @@ import 'package:spotiflac_android/models/track.dart';
|
||||
import 'package:spotiflac_android/providers/library_collections_provider.dart';
|
||||
import 'package:spotiflac_android/providers/settings_provider.dart';
|
||||
import 'package:spotiflac_android/theme/mornye_theme.dart';
|
||||
import 'package:spotiflac_android/widgets/app_action_button.dart';
|
||||
import 'package:spotiflac_android/widgets/app_alert_dialog.dart';
|
||||
import 'package:spotiflac_android/widgets/mornye_chrome.dart';
|
||||
import 'package:spotiflac_android/widgets/playlist_picker_sheet.dart';
|
||||
@@ -32,6 +35,7 @@ class _Collections extends LibraryCollectionsNotifier {
|
||||
List<PlaylistPickerSummary> _summaries = [];
|
||||
final additions = <String>[];
|
||||
final createdNames = <String>[];
|
||||
final pendingAdditions = <Completer<void>>[];
|
||||
|
||||
@override
|
||||
LibraryCollectionsState build() => LibraryCollectionsState(isLoaded: true);
|
||||
@@ -50,6 +54,9 @@ class _Collections extends LibraryCollectionsNotifier {
|
||||
Iterable<Track> tracks,
|
||||
) async {
|
||||
additions.add(playlistId);
|
||||
if (pendingAdditions.isNotEmpty) {
|
||||
await pendingAdditions.removeAt(0).future;
|
||||
}
|
||||
return PlaylistAddBatchResult(
|
||||
addedCount: tracks.length,
|
||||
alreadyInPlaylistCount: 0,
|
||||
@@ -57,6 +64,19 @@ class _Collections extends LibraryCollectionsNotifier {
|
||||
}
|
||||
}
|
||||
|
||||
class _PopObserver extends NavigatorObserver {
|
||||
int pops = 0;
|
||||
|
||||
@override
|
||||
void didPop(Route<dynamic> route, Route<dynamic>? previousRoute) {
|
||||
pops++;
|
||||
}
|
||||
}
|
||||
|
||||
VoidCallback? _doneAction(WidgetTester tester, bool mornye) => mornye
|
||||
? tester.widget<AppActionButton>(find.byType(AppActionButton)).onPressed
|
||||
: tester.widget<FilledButton>(find.byType(FilledButton)).onPressed;
|
||||
|
||||
PlaylistPickerSummary _playlist(String name, {bool contains = false}) =>
|
||||
PlaylistPickerSummary(
|
||||
id: name,
|
||||
@@ -73,6 +93,7 @@ Widget _app(
|
||||
required Brightness brightness,
|
||||
bool mornye = true,
|
||||
double textScale = 1,
|
||||
List<NavigatorObserver> navigatorObservers = const [],
|
||||
}) => ProviderScope(
|
||||
overrides: [
|
||||
settingsProvider.overrideWith(_Settings.new),
|
||||
@@ -82,6 +103,7 @@ Widget _app(
|
||||
),
|
||||
],
|
||||
child: MaterialApp(
|
||||
navigatorObservers: navigatorObservers,
|
||||
theme: mornye ? MornyeTheme.build(brightness) : ThemeData(),
|
||||
localizationsDelegates: AppLocalizations.localizationsDelegates,
|
||||
supportedLocales: AppLocalizations.supportedLocales,
|
||||
@@ -157,6 +179,102 @@ void main() {
|
||||
}
|
||||
|
||||
for (final mornye in [true, false]) {
|
||||
testWidgets('picker submits and closes once (Mornye: $mornye)', (
|
||||
tester,
|
||||
) async {
|
||||
final addition = Completer<void>();
|
||||
final collections = _Collections()
|
||||
.._summaries = [_playlist('Road trip')]
|
||||
..pendingAdditions.add(addition);
|
||||
final observer = _PopObserver();
|
||||
await tester.pumpWidget(
|
||||
_app(
|
||||
Consumer(
|
||||
builder: (context, ref, _) => TextButton(
|
||||
onPressed: () =>
|
||||
showAddTracksToPlaylistSheet(context, ref, [_track]),
|
||||
child: const Text('Open'),
|
||||
),
|
||||
),
|
||||
collections,
|
||||
brightness: Brightness.light,
|
||||
mornye: mornye,
|
||||
navigatorObservers: [observer],
|
||||
),
|
||||
);
|
||||
await tester.tap(find.text('Open'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text('Road trip'));
|
||||
await tester.pump();
|
||||
final submit = _doneAction(tester, mornye)!;
|
||||
// A repeated callback can arrive before the disabled button is rebuilt.
|
||||
submit();
|
||||
submit();
|
||||
await tester.pump();
|
||||
expect(collections.additions, ['Road trip']);
|
||||
expect(_doneAction(tester, mornye), isNull);
|
||||
expect(observer.pops, 0);
|
||||
|
||||
addition.complete();
|
||||
await tester.pump();
|
||||
// The route remains mounted while its closing animation runs.
|
||||
submit();
|
||||
await tester.pumpAndSettle();
|
||||
expect(collections.additions, ['Road trip']);
|
||||
expect(observer.pops, 1);
|
||||
expect(find.text('Open'), findsOneWidget);
|
||||
expect(tester.takeException(), isNull);
|
||||
});
|
||||
|
||||
testWidgets('picker retains selections after failure (Mornye: $mornye)', (
|
||||
tester,
|
||||
) async {
|
||||
final firstAddition = Completer<void>()..complete();
|
||||
final secondAddition = Completer<void>();
|
||||
final collections = _Collections()
|
||||
.._summaries = [_playlist('Road trip'), _playlist('Evening music')]
|
||||
..pendingAdditions.addAll([firstAddition, secondAddition]);
|
||||
final observer = _PopObserver();
|
||||
await tester.pumpWidget(
|
||||
_app(
|
||||
Consumer(
|
||||
builder: (context, ref, _) => TextButton(
|
||||
onPressed: () =>
|
||||
showAddTracksToPlaylistSheet(context, ref, [_track]),
|
||||
child: const Text('Open'),
|
||||
),
|
||||
),
|
||||
collections,
|
||||
brightness: Brightness.light,
|
||||
mornye: mornye,
|
||||
navigatorObservers: [observer],
|
||||
),
|
||||
);
|
||||
await tester.tap(find.text('Open'));
|
||||
await tester.pumpAndSettle();
|
||||
await tester.tap(find.text('Road trip'));
|
||||
await tester.tap(find.text('Evening music'));
|
||||
await tester.pump();
|
||||
await tester.tap(find.text('Done'));
|
||||
await tester.pump();
|
||||
expect(collections.additions, ['Road trip', 'Evening music']);
|
||||
secondAddition.completeError(StateError('write failed'));
|
||||
await tester.pump();
|
||||
expect(observer.pops, 0);
|
||||
expect(_doneAction(tester, mornye), isNotNull);
|
||||
|
||||
// Retrying keeps the failed selection and skips the persisted playlist.
|
||||
await tester.tap(find.text('Done'));
|
||||
await tester.pumpAndSettle();
|
||||
expect(collections.additions, [
|
||||
'Road trip',
|
||||
'Evening music',
|
||||
'Evening music',
|
||||
]);
|
||||
expect(observer.pops, 1);
|
||||
expect(tester.takeException(), isNull);
|
||||
});
|
||||
|
||||
testWidgets('picker adds only new selections (Mornye: $mornye)', (
|
||||
tester,
|
||||
) async {
|
||||
|
||||
Reference in new issue
Block a user