refactor(downloads): one auto-remove-watched core, trimmed offline-watch provider, shared focus-restore mixin

The auto-remove-watched-downloads rule existed twice (DownloadProvider's sweep and OfflineWatchProvider's single-item copy) — the kind/completed/delete/title core now lives once on DownloadProvider, with the watched judgment staying per caller because the offline path fires before metadata can reflect a local mark. OfflineWatchProvider drops five members with no production callers (isSyncing, getPendingSyncCount, isWatched, getViewOffset, getEpisodesWithWatchStatus) plus the sync-service listener that only served them. The downloads screen's three verbatim 'suppressAutoFocus flipped — focus the first item' didUpdateWidget blocks are now one mixin.
This commit is contained in:
edde746
2026-08-17 19:02:20 +02:00
parent 55382deaf9
commit 2e1a7130fc
6 changed files with 86 additions and 243 deletions
+34
View File
@@ -0,0 +1,34 @@
import 'package:flutter/widgets.dart';
/// Focuses [firstItemFocusNode] when the owning widget's `suppressAutoFocus`
/// flag flips from true to false — the downloads screen's tab-switch handoff
/// to whichever content widget just became active.
///
/// Consumers implement [firstItemFocusDebugLabel] and [suppressAutoFocusOf],
/// then hand [firstItemFocusNode] to their first focusable item.
mixin UnsuppressFocusFirstMixin<T extends StatefulWidget> on State<T> {
late final FocusNode firstItemFocusNode = FocusNode(debugLabel: firstItemFocusDebugLabel);
String get firstItemFocusDebugLabel;
/// Reads the owning widget's `suppressAutoFocus` flag.
bool suppressAutoFocusOf(T widget);
@override
void didUpdateWidget(T oldWidget) {
super.didUpdateWidget(oldWidget);
if (suppressAutoFocusOf(oldWidget) && !suppressAutoFocusOf(widget)) {
WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted && firstItemFocusNode.canRequestFocus) {
firstItemFocusNode.requestFocus();
}
});
}
}
@override
void dispose() {
firstItemFocusNode.dispose();
super.dispose();
}
}
+21 -4
View File
@@ -1696,6 +1696,25 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin
}
}
/// Deletes [globalKey] if it is a completed episode/movie download; returns
/// the deleted item's display title, or null when it is not an auto-remove
/// candidate.
///
/// Shared core of the auto-remove-watched rule. The watched judgment stays
/// with the caller: the sweep in [autoDeleteWatchedDownloads] trusts server
/// metadata, while [OfflineWatchProvider] fires right after a local
/// mark-watched that metadata cannot reflect yet.
Future<String?> deleteWatchedDownloadCandidate(String globalKey, {required String logContext}) async {
final meta = _resolvedMetadata(globalKey);
if (meta == null) return null;
if (!meta.isEpisode && !meta.isMovie) return null;
if (_downloads[globalKey]?.status != DownloadStatus.completed) return null;
appLogger.i('Auto-deleting $logContext download: ${meta.title} ($globalKey)');
await deleteDownload(globalKey);
return meta.title ?? t.common.unknown;
}
/// Auto-delete downloaded episodes/movies that are now marked as watched.
///
/// Only deletes individual episodes and movies, never show/season containers.
@@ -1711,16 +1730,14 @@ class DownloadProvider extends ChangeNotifier with DisposableChangeNotifierMixin
for (final globalKey in completedKeys) {
final meta = _resolvedMetadata(globalKey);
if (meta == null) continue;
if (!meta.isEpisode && !meta.isMovie) continue;
if (!meta.isWatched) continue;
// Don't delete the episode that's currently playing
if (activeGlobalKey != null && meta.globalKey == activeGlobalKey) continue;
try {
appLogger.i('Auto-deleting watched download: ${meta.title} ($globalKey)');
await deleteDownload(globalKey);
deletedTitles.add(meta.title ?? t.common.unknown);
final title = await deleteWatchedDownloadCandidate(globalKey, logContext: 'watched');
if (title != null) deletedTitles.add(title);
} catch (e) {
appLogger.w('Failed to auto-delete watched download $globalKey: $e');
}
+11 -98
View File
@@ -1,12 +1,12 @@
import 'dart:async';
import 'package:flutter/foundation.dart';
import '../media/ids.dart';
import '../i18n/strings.g.dart';
import '../media/episode_collection.dart';
import '../media/media_item.dart';
import '../media/media_item_types.dart';
import '../mixins/disposable_change_notifier_mixin.dart';
import '../models/download_models.dart';
import '../services/offline_watch_sync_service.dart';
import '../services/settings_service.dart';
import '../utils/app_logger.dart';
@@ -18,69 +18,13 @@ import '../utils/global_key_utils.dart';
/// Provider for offline watch status UI state.
///
/// Provides:
/// - Effective watch status (local changes + cached server data)
/// - Offline "OnDeck" calculation for shows
/// - Manual mark watched/unwatched while offline
class OfflineWatchProvider extends ChangeNotifier with DisposableChangeNotifierMixin {
final OfflineWatchSyncService _syncService;
final DownloadProvider _downloadProvider;
OfflineWatchProvider({required this._syncService, required this._downloadProvider}) {
_syncService.addListener(_onSyncServiceChanged);
}
void _onSyncServiceChanged() {
safeNotifyListeners();
}
/// Whether a sync is in progress
bool get isSyncing => _syncService.isSyncing;
/// Get count of pending sync items
Future<int> getPendingSyncCount() => _syncService.getPendingSyncCount();
/// Get the effective watch status for a media item.
///
/// Priority:
/// 1. Local offline action (if exists)
/// 2. Cached server data from API cache
/// 3. Metadata from download provider
///
/// Returns true if watched, false otherwise.
Future<bool> isWatched(String globalKey) async {
final localStatus = await _syncService.getLocalWatchStatus(globalKey);
if (localStatus != null) {
return localStatus;
}
final metadata = _downloadProvider.getMetadata(globalKey);
if (metadata != null) {
return metadata.isWatched;
}
return false;
}
/// Get the effective view offset (resume position) for a media item.
///
/// Priority:
/// 1. Local offline progress (if exists)
/// 2. Metadata from download provider
///
/// Returns null if no position is available.
@visibleForTesting
Future<int?> getViewOffset(String globalKey) async {
final localOffset = await _syncService.getLocalViewOffset(globalKey);
if (localOffset != null) {
return localOffset;
}
final localStatus = await _syncService.getLocalWatchStatus(globalKey);
if (localStatus == true) return null;
final metadata = _downloadProvider.getMetadata(globalKey);
return metadata?.viewOffsetMs;
}
OfflineWatchProvider({required this._syncService, required this._downloadProvider});
/// Get episodes for a show in the shared [sortEpisodesByWatchOrder] order,
/// so the offline watch order matches what "download next N" selects
@@ -181,33 +125,21 @@ class OfflineWatchProvider extends ChangeNotifier with DisposableChangeNotifierM
patchId: WatchPatchId.offlineAction(profileId: queued.profileId, rowId: queued.rowId, revision: queued.revision),
);
safeNotifyListeners();
_autoDeleteIfWatched(serverId, itemId);
unawaited(_autoDeleteIfWatched(serverId, itemId));
}
/// Auto-delete a download if the auto-remove setting is enabled.
void _autoDeleteIfWatched(ServerId serverId, String itemId) {
Future<void> _autoDeleteIfWatched(ServerId serverId, String itemId) async {
final settings = SettingsService.instanceOrNull;
if (settings == null || !settings.read(SettingsService.autoRemoveWatchedDownloads)) return;
final globalKey = buildGlobalKey(ServerId(serverId), itemId);
final meta = _downloadProvider.getMetadata(globalKey);
if (meta == null) return;
if (!meta.isEpisode && !meta.isMovie) return;
final progress = _downloadProvider.downloads[globalKey];
if (progress?.status != DownloadStatus.completed) return;
appLogger.i('Auto-deleting locally-watched download: ${meta.title} ($globalKey)');
_downloadProvider
.deleteDownload(globalKey)
.then(
(_) {
showMainSnackBar(t.messages.autoRemovedWatchedDownload(title: meta.title ?? t.common.unknown));
},
onError: (e) {
appLogger.w('Failed to auto-delete locally-watched download $globalKey: $e');
},
);
try {
final title = await _downloadProvider.deleteWatchedDownloadCandidate(globalKey, logContext: 'locally-watched');
if (title != null) showMainSnackBar(t.messages.autoRemovedWatchedDownload(title: title));
} catch (e) {
appLogger.w('Failed to auto-delete locally-watched download $globalKey: $e');
}
}
/// Mark an item as unwatched while offline.
@@ -225,23 +157,4 @@ class OfflineWatchProvider extends ChangeNotifier with DisposableChangeNotifierM
);
safeNotifyListeners();
}
/// Get downloaded episodes for a show with their watch status.
///
/// Returns a list of (episode, isWatched) pairs.
/// Uses batched database query for efficiency.
Future<List<(MediaItem episode, bool isWatched)>> getEpisodesWithWatchStatus(String showId) async {
final episodes = _downloadProvider.getDownloadedEpisodesForShow(showId);
if (episodes.isEmpty) return [];
final watchStatuses = await _resolveEpisodeWatchStatuses(episodes);
return [for (final episode in episodes) (episode, watchStatuses[episode.globalKey]!)];
}
@override
void dispose() {
_syncService.removeListener(_onSyncServiceChanged);
super.dispose();
}
}
+13 -39
View File
@@ -21,6 +21,7 @@ import '../../services/settings_service.dart';
import '../../widgets/settings_builder.dart';
import '../../utils/global_key_utils.dart';
import '../../mixins/tab_navigation_mixin.dart';
import '../../mixins/unsuppress_focus_mixin.dart';
import '../../mixins/refreshable.dart';
import '../../utils/media_image_helper.dart';
import '../../utils/platform_detector.dart';
@@ -268,27 +269,13 @@ class _DownloadsGridContent extends StatefulWidget {
State<_DownloadsGridContent> createState() => _DownloadsGridContentState();
}
class _DownloadsGridContentState extends State<_DownloadsGridContent> {
final FocusNode _firstItemFocusNode = FocusNode(debugLabel: 'DownloadsGrid_firstItem');
class _DownloadsGridContentState extends State<_DownloadsGridContent>
with UnsuppressFocusFirstMixin<_DownloadsGridContent> {
@override
String get firstItemFocusDebugLabel => 'DownloadsGrid_firstItem';
@override
void dispose() {
_firstItemFocusNode.dispose();
super.dispose();
}
@override
void didUpdateWidget(_DownloadsGridContent oldWidget) {
super.didUpdateWidget(oldWidget);
// When suppressAutoFocus changes from true to false, focus the first item
if (oldWidget.suppressAutoFocus && !widget.suppressAutoFocus) {
WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted && _firstItemFocusNode.canRequestFocus) {
_firstItemFocusNode.requestFocus();
}
});
}
}
bool suppressAutoFocusOf(_DownloadsGridContent widget) => widget.suppressAutoFocus;
/// Navigate focus to the sidebar
void _navigateToSidebar() {
@@ -332,7 +319,7 @@ class _DownloadsGridContentState extends State<_DownloadsGridContent> {
final item = items[position.index];
return FocusableMediaCard(
item: item,
focusNode: position.index == 0 ? _firstItemFocusNode : null,
focusNode: position.index == 0 ? firstItemFocusNode : null,
disableScale: position.disableScale,
onBack: widget.onBack,
isOffline: true, // Downloaded content works without server
@@ -392,26 +379,13 @@ class _DownloadedMusicContent extends StatefulWidget {
State<_DownloadedMusicContent> createState() => _DownloadedMusicContentState();
}
class _DownloadedMusicContentState extends State<_DownloadedMusicContent> {
final FocusNode _firstItemFocusNode = FocusNode(debugLabel: 'DownloadsMusic_firstItem');
class _DownloadedMusicContentState extends State<_DownloadedMusicContent>
with UnsuppressFocusFirstMixin<_DownloadedMusicContent> {
@override
String get firstItemFocusDebugLabel => 'DownloadsMusic_firstItem';
@override
void dispose() {
_firstItemFocusNode.dispose();
super.dispose();
}
@override
void didUpdateWidget(_DownloadedMusicContent oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.suppressAutoFocus && !widget.suppressAutoFocus) {
WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted && _firstItemFocusNode.canRequestFocus) {
_firstItemFocusNode.requestFocus();
}
});
}
}
bool suppressAutoFocusOf(_DownloadedMusicContent widget) => widget.suppressAutoFocus;
Future<void> _playAlbumFrom(List<MediaItem> albumTracks, MediaItem track) async {
final album = track.parentId;
@@ -541,7 +515,7 @@ class _DownloadedMusicContentState extends State<_DownloadedMusicContent> {
isFirst: row.isFirst,
isLast: row.isLast,
showArtist: true,
focusNode: isFirstTrackRow ? _firstItemFocusNode : null,
focusNode: isFirstTrackRow ? firstItemFocusNode : null,
onBack: widget.onBack,
onTap: () => _playAlbumFrom(row.albumTracks, item),
),
+5 -17
View File
@@ -10,6 +10,7 @@ import '../media/media_kind.dart';
import '../models/download_models.dart';
import '../utils/dialogs.dart';
import '../utils/global_key_utils.dart';
import '../mixins/unsuppress_focus_mixin.dart';
import 'download_status_icon.dart';
/// Represents a node in the download tree
@@ -79,27 +80,14 @@ class DownloadTreeView extends StatefulWidget {
State<DownloadTreeView> createState() => _DownloadTreeViewState();
}
class _DownloadTreeViewState extends State<DownloadTreeView> {
class _DownloadTreeViewState extends State<DownloadTreeView> with UnsuppressFocusFirstMixin<DownloadTreeView> {
final Set<String> _expandedNodes = {};
final FocusNode _firstItemFocusNode = FocusNode(debugLabel: 'DownloadTreeView_firstItem');
@override
void dispose() {
_firstItemFocusNode.dispose();
super.dispose();
}
String get firstItemFocusDebugLabel => 'DownloadTreeView_firstItem';
@override
void didUpdateWidget(DownloadTreeView oldWidget) {
super.didUpdateWidget(oldWidget);
if (oldWidget.suppressAutoFocus && !widget.suppressAutoFocus) {
WidgetsBinding.instance.addPostFrameCallback((_) {
if (mounted && _firstItemFocusNode.canRequestFocus) {
_firstItemFocusNode.requestFocus();
}
});
}
}
bool suppressAutoFocusOf(DownloadTreeView widget) => widget.suppressAutoFocus;
@override
Widget build(BuildContext context) {
@@ -402,7 +390,7 @@ class _DownloadTreeViewState extends State<DownloadTreeView> {
onDelete: widget.onDelete,
onNavigateLeft: widget.onNavigateLeft,
onBack: widget.onBack,
rowFocusNode: isFirst ? _firstItemFocusNode : null,
rowFocusNode: isFirst ? firstItemFocusNode : null,
autofocus: isFirst && !widget.suppressAutoFocus,
pauseAllChildren: _pauseAllChildren,
resumeAllChildren: _resumeAllChildren,
@@ -44,73 +44,12 @@ void main() {
});
group('OfflineWatchProvider', () {
test('initial isSyncing reflects sync service state', () {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
expect(p.isSyncing, isFalse);
expect(syncService.isSyncing, isFalse);
p.dispose();
});
test('getPendingSyncCount delegates to sync service (initially 0)', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
expect(await p.getPendingSyncCount(), 0);
p.dispose();
});
test('isWatched returns false when no local action and no metadata', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
expect(await p.isWatched('srv:absent'), isFalse);
p.dispose();
});
test('getViewOffset returns null when no local progress and no metadata', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
expect(await p.getViewOffset('srv:absent'), isNull);
p.dispose();
});
test('getViewOffset returns null for local progress that crossed watched threshold', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
await syncService.queueProgressUpdate(
serverId: ServerId('srv'),
itemId: '42',
viewOffset: 95000,
duration: 100000,
);
expect(await p.isWatched('srv:42'), isTrue);
expect(await p.getViewOffset('srv:42'), isNull);
p.dispose();
});
test('getNextUnwatchedEpisode returns null for show with no downloads', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
expect(await p.getNextUnwatchedEpisode('show-123'), isNull);
p.dispose();
});
test('getEpisodesWithWatchStatus returns empty list for show with no downloads', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
expect(await p.getEpisodesWithWatchStatus('show-123'), isEmpty);
p.dispose();
});
test('forwards listener notifications from sync service', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
var notified = 0;
p.addListener(() => notified++);
// queueMarkWatched on the sync service notifies its listeners; the
// provider's internal listener forwards via safeNotifyListeners.
await syncService.queueMarkWatched(serverId: ServerId('srv'), itemId: '42');
expect(notified, greaterThanOrEqualTo(1));
p.dispose();
});
test('markAsWatched queues an offline action and notifies', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
@@ -120,9 +59,7 @@ void main() {
await p.markAsWatched(serverId: ServerId('srv'), itemId: '50');
// The local watch status now reads as true via the sync service.
expect(await p.isWatched('srv:50'), isTrue);
// At least one notification: from sync service forwarding + provider's
// explicit safeNotifyListeners after queueing.
expect(await syncService.getLocalWatchStatus('srv:50'), isTrue);
expect(notified, greaterThanOrEqualTo(1));
p.dispose();
@@ -132,29 +69,9 @@ void main() {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
await p.markAsUnwatched(serverId: ServerId('srv'), itemId: '60');
expect(await p.isWatched('srv:60'), isFalse);
expect(await syncService.getLocalWatchStatus('srv:60'), isFalse);
p.dispose();
});
test('dispose removes the sync service listener', () async {
final p = OfflineWatchProvider(syncService: syncService, downloadProvider: downloadProvider);
var notified = 0;
p.addListener(() => notified++);
// Sanity: listener is registered
await syncService.queueMarkWatched(serverId: ServerId('srv'), itemId: '70');
final preDisposeNotifies = notified;
expect(preDisposeNotifies, greaterThanOrEqualTo(1));
p.dispose();
// After dispose, sync service notifications should not call our
// listener (provider unsubscribed). Mutating the sync service post-
// dispose must not throw on the provider side.
await syncService.queueMarkUnwatched(serverId: ServerId('srv'), itemId: '70');
expect(notified, preDisposeNotifies);
});
});
}