From c2fe14229374efee173b08ab7662faefecbd1755 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 12 Jun 2026 14:09:21 +0200 Subject: [PATCH] refactor(watch): WatchActions owns mark-watched orchestration and events --- lib/media/media_server_client.dart | 26 +++--- lib/screens/media_detail/action_buttons.dart | 34 ++------ lib/screens/media_detail_screen.dart | 2 +- lib/services/jellyfin_client.dart | 1 - .../jellyfin_client/parts/watch_state.dart | 2 - lib/services/offline_watch_sync_service.dart | 16 ++-- lib/services/plex_client.dart | 24 ++---- lib/services/watch_actions.dart | 83 +++++++++++++++++++ lib/widgets/media_context_menu.dart | 53 +++--------- .../playback_progress_tracker_test.dart | 13 +-- 10 files changed, 140 insertions(+), 114 deletions(-) create mode 100644 lib/services/watch_actions.dart diff --git a/lib/media/media_server_client.dart b/lib/media/media_server_client.dart index b143838c..46b4dcf1 100644 --- a/lib/media/media_server_client.dart +++ b/lib/media/media_server_client.dart @@ -270,11 +270,14 @@ abstract class MediaServerClient { /// endpoint. Backends without true hub pagination may return a single page. Future> fetchMoreHubItemsPage(String hubId, {int? start, int? size, AbortController? abort}); - /// Mark [item] as watched. The full item is passed (not just an id) so - /// implementations can fire a [WatchStateEvent] on [WatchStateNotifier] - /// for UI invalidation — episode/season/show parent chain, library - /// section etc. live on the item. + /// Mark [item] as watched. Transport only: no [WatchStateEvent] is emitted + /// here — UI surfaces go through `WatchActions`, which owns the single + /// emission (and tracker fan-out); the offline sync replay calls this + /// directly precisely because the event already fired when the action was + /// queued. Future markWatched(MediaItem item); + + /// Mark [item] as unwatched. Transport only — see [markWatched]. Future markUnwatched(MediaItem item); /// Hide an item from Continue Watching without changing watched status or @@ -589,17 +592,16 @@ extension MediaServerClientScope on MediaServerClient { /// Mark [item] watched because it crossed [watchedThreshold] during playback, /// when a playback-stopped report is/was also sent for the same playback. /// Backends that mark played from the stop report - /// ([marksWatchedOnPlaybackStopped]) only emit the local watch event — - /// issuing [markWatched] too would double-scrobble via the Jellyfin Trakt - /// plugin (#1287). The local event still keeps the UI and Plezy's own Trakt - /// sync (which key on `watched` events, not progress) in sync; the stop - /// report syncs the server. + /// ([marksWatchedOnPlaybackStopped]) skip the server call — issuing + /// [markWatched] too would double-scrobble via the Jellyfin Trakt plugin + /// (#1287). The single local event emitted here keeps the UI and Plezy's + /// own Trakt sync (which key on `watched` events, not progress) in sync; + /// the stop report syncs the server. Future markWatchedFromPlaybackStop(MediaItem item) async { - if (marksWatchedOnPlaybackStopped) { - WatchStateNotifier().notifyWatched(item: item, isNowWatched: true, cacheServerId: cacheServerId); - } else { + if (!marksWatchedOnPlaybackStopped) { await markWatched(item); } + WatchStateNotifier().notifyWatched(item: item, isNowWatched: true, cacheServerId: cacheServerId); } } diff --git a/lib/screens/media_detail/action_buttons.dart b/lib/screens/media_detail/action_buttons.dart index edc03e88..cdcc0b36 100644 --- a/lib/screens/media_detail/action_buttons.dart +++ b/lib/screens/media_detail/action_buttons.dart @@ -283,36 +283,16 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState { Future _handleWatchedTogglePressed(MediaItem metadata) async { try { final isWatched = metadata.isWatched; - if (widget.isOffline) { - // Offline mode: queue action for later sync - final offlineWatch = context.read(); - if (isWatched) { - await offlineWatch.markAsUnwatched(serverId: ServerId(metadata.serverId!), itemId: metadata.id); - } else { - await offlineWatch.markAsWatched(serverId: ServerId(metadata.serverId!), itemId: metadata.id); - } - if (mounted) { + final outcome = await WatchActions.setWatched(context, metadata, watched: !isWatched, offline: widget.isOffline); + if (!mounted) return; + switch (outcome) { + case WatchMarkOutcome.queuedOffline: showAppSnackBar(context, isWatched ? t.messages.markedAsUnwatchedOffline : t.messages.markedAsWatchedOffline); - } - } else { - // Online mode: dispatch via the right backend's neutral method so - // Jellyfin items hit /UserPlayedItems and Plex items hit /:/scrobble. - final serverId = metadata.serverId; - if (serverId == null) return; - final client = context.tryGetMediaClientForServer(ServerId(serverId)); - if (client == null) return; - - if (isWatched) { - await client.markUnwatched(metadata); - unawaited(TrackerCoordinator.instance.markUnwatched(metadata, client)); - } else { - await client.markWatched(metadata); - unawaited(TrackerCoordinator.instance.markWatched(metadata, client)); - } - if (mounted) { + case WatchMarkOutcome.marked: _watchStateChanged = true; showSuccessSnackBar(context, isWatched ? t.messages.markedAsUnwatched : t.messages.markedAsWatched); - } + case WatchMarkOutcome.skipped: + break; } } catch (e) { if (mounted) { diff --git a/lib/screens/media_detail_screen.dart b/lib/screens/media_detail_screen.dart index 294f6c0d..a442eb43 100644 --- a/lib/screens/media_detail_screen.dart +++ b/lib/screens/media_detail_screen.dart @@ -47,7 +47,7 @@ import '../services/download_storage_service.dart'; import '../utils/download_version_utils.dart'; import '../utils/download_utils.dart'; import '../services/settings_service.dart'; -import '../services/trackers/tracker_coordinator.dart'; +import '../services/watch_actions.dart'; import '../widgets/settings_builder.dart'; import '../utils/grid_size_calculator.dart'; import '../utils/layout_constants.dart'; diff --git a/lib/services/jellyfin_client.dart b/lib/services/jellyfin_client.dart index f5c68a45..ae171e7b 100644 --- a/lib/services/jellyfin_client.dart +++ b/lib/services/jellyfin_client.dart @@ -44,7 +44,6 @@ import '../utils/external_ids.dart'; import '../utils/media_server_http_client.dart'; import '../utils/resolution_label.dart'; import '../utils/track_label_builder.dart'; -import '../utils/watch_state_notifier.dart'; import '../exceptions/media_server_exceptions.dart'; import '../i18n/strings.g.dart'; import '../utils/jellyfin_time.dart'; diff --git a/lib/services/jellyfin_client/parts/watch_state.dart b/lib/services/jellyfin_client/parts/watch_state.dart index 5e569ac7..94d5f46d 100644 --- a/lib/services/jellyfin_client/parts/watch_state.dart +++ b/lib/services/jellyfin_client/parts/watch_state.dart @@ -11,7 +11,6 @@ mixin _JellyfinWatchStateMethods on MediaServerCacheMixin { queryParameters: {'userId': connection.userId}, ); throwIfHttpError(response); - WatchStateNotifier().notifyWatched(item: item, isNowWatched: true, cacheServerId: cacheServerId); } @override @@ -21,7 +20,6 @@ mixin _JellyfinWatchStateMethods on MediaServerCacheMixin { queryParameters: {'userId': connection.userId}, ); throwIfHttpError(response); - WatchStateNotifier().notifyWatched(item: item, isNowWatched: false, cacheServerId: cacheServerId); } @override diff --git a/lib/services/offline_watch_sync_service.dart b/lib/services/offline_watch_sync_service.dart index ef932d6f..7673c283 100644 --- a/lib/services/offline_watch_sync_service.dart +++ b/lib/services/offline_watch_sync_service.dart @@ -521,17 +521,19 @@ class OfflineWatchSyncService extends ChangeNotifier { /// `/UserPlayedItems/{id}` and `/Sessions/Playing*` endpoints receive /// the same queued state Plex's `/:/scrobble` and `/:/timeline` do. Future _syncAction(MediaServerClient client, OfflineWatchProgressItem action) async { - // Fetch metadata so the WatchStateNotifier emission inside - // markWatched/markUnwatched carries enough context for downstream - // listeners (UI invalidation, Trakt sync). Best-effort: a missed metadata - // fetch falls back to a minimal MediaItem — the network call still goes - // through, just without a rich event payload. - final emitsEvent = + // Fetch metadata so trackers (and the stop-path watch event) get enough + // context — external ids, parent chain, library section. The plain + // watched/unwatched replays deliberately emit no WatchStateEvent: the + // offline provider already emitted it when the action was queued, and + // client markWatched/markUnwatched are transport-only. Best-effort: a + // missed metadata fetch falls back to a minimal MediaItem — the network + // call still goes through. + final needsRichItem = action.actionType == OfflineActionType.watched.id || action.actionType == OfflineActionType.unwatched.id || (action.actionType == OfflineActionType.progress.id && action.shouldMarkWatched); MediaItem? item; - if (emitsEvent) { + if (needsRichItem) { try { item = await client.fetchItem(action.ratingKey); } catch (_) { diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index 0d65d30e..13c71a02 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -61,7 +61,6 @@ import '../utils/plex_cache_parser.dart'; import '../utils/plex_library_section_utils.dart'; import '../utils/plex_url_helper.dart'; import '../utils/session_identifier.dart' as session_id; -import '../utils/watch_state_notifier.dart'; import '../i18n/strings.g.dart'; import '../mpv/mpv.dart'; import 'api_cache.dart'; @@ -1675,30 +1674,20 @@ class PlexClient } } - /// Mark media as watched - /// - /// If [item] is provided, emits a [WatchStateEvent] for UI updates. - Future markAsWatched(String ratingKey, {MediaItem? item}) async { + /// Mark media as watched (transport only — see [MediaServerClient.markWatched]). + Future markAsWatched(String ratingKey) async { await _getWithFailover( '/:/scrobble', queryParameters: {'key': ratingKey, 'identifier': 'com.plexapp.plugins.library'}, ); - if (item != null) { - WatchStateNotifier().notifyWatched(item: item, isNowWatched: true); - } } - /// Mark media as unwatched - /// - /// If [item] is provided, emits a [WatchStateEvent] for UI updates. - Future markAsUnwatched(String ratingKey, {MediaItem? item}) async { + /// Mark media as unwatched (transport only — see [MediaServerClient.markUnwatched]). + Future markAsUnwatched(String ratingKey) async { await _getWithFailover( '/:/unscrobble', queryParameters: {'key': ratingKey, 'identifier': 'com.plexapp.plugins.library'}, ); - if (item != null) { - WatchStateNotifier().notifyWatched(item: item, isNowWatched: false); - } } /// Update playback progress @@ -3789,15 +3778,14 @@ class PlexClient } @override - Future markWatched(MediaItem item) => markAsWatched(item.id, item: item); + Future markWatched(MediaItem item) => markAsWatched(item.id); @override - Future markUnwatched(MediaItem item) => markAsUnwatched(item.id, item: item); + Future markUnwatched(MediaItem item) => markAsUnwatched(item.id); @override Future removeFromContinueWatching(MediaItem item) async { await removeFromOnDeck(item.id); - WatchStateNotifier().notifyRemovedFromContinueWatching(item: item); } /// Rate a media item (0.0-10.0 scale, where each integer = half a star). diff --git a/lib/services/watch_actions.dart b/lib/services/watch_actions.dart new file mode 100644 index 00000000..dcc38e43 --- /dev/null +++ b/lib/services/watch_actions.dart @@ -0,0 +1,83 @@ +import 'dart:async'; + +import 'package:flutter/widgets.dart'; +import 'package:provider/provider.dart'; + +import '../media/ids.dart'; +import '../media/media_item.dart'; +import '../media/media_server_client.dart'; +import '../providers/offline_mode_provider.dart'; +import '../providers/offline_watch_provider.dart'; +import '../utils/provider_extensions.dart'; +import '../utils/watch_state_notifier.dart'; +import 'trackers/tracker_coordinator.dart'; + +enum WatchMarkOutcome { + /// Queued for later sync (offline). The offline provider emitted the event. + queuedOffline, + + /// Marked on the server; event emitted, trackers fired. + marked, + + /// Nothing to do (no server id / no bound client). + skipped, +} + +/// Orchestrates watched/unwatched marks: routes offline marks to the offline +/// queue, online marks to the backend client, emits the single +/// [WatchStateNotifier] event, and fires trackers. UI surfaces call this +/// instead of hand-rolling the offline/online + tracker dance; snackbars and +/// refresh callbacks stay with the caller. Client `markWatched`/`markUnwatched` +/// are transport-only — they must never be called directly from UI code. +class WatchActions { + WatchActions._(); + + /// Marks [item] watched/unwatched. [offline] overrides the + /// [OfflineModeProvider] read for callers that already know (e.g. screens + /// rendering downloaded content). + static Future setWatched( + BuildContext context, + MediaItem item, { + required bool watched, + bool? offline, + }) async { + final isOffline = offline ?? context.read().isOffline; + final serverId = item.serverId; + + if (isOffline && serverId != null) { + final offlineWatch = context.read(); + if (watched) { + await offlineWatch.markAsWatched(serverId: ServerId(serverId), itemId: item.id); + } else { + await offlineWatch.markAsUnwatched(serverId: ServerId(serverId), itemId: item.id); + } + return WatchMarkOutcome.queuedOffline; + } + + if (serverId == null) return WatchMarkOutcome.skipped; + final client = context.tryGetMediaClientForServer(ServerId(serverId)); + if (client == null) return WatchMarkOutcome.skipped; + + if (watched) { + await client.markWatched(item); + } else { + await client.markUnwatched(item); + } + WatchStateNotifier().notifyWatched(item: item, isNowWatched: watched, cacheServerId: client.cacheServerId); + unawaited( + watched + ? TrackerCoordinator.instance.markWatched(item, client) + : TrackerCoordinator.instance.markUnwatched(item, client), + ); + return WatchMarkOutcome.marked; + } + + /// Removes [item] from Continue Watching without touching watch state. + /// Throws when no client is bound for the item's server (mirrors the + /// pre-existing menu behaviour so callers surface an error snackbar). + static Future removeFromContinueWatching(BuildContext context, MediaItem item) async { + final client = context.getMediaClientForServer(ServerId(item.serverId!)); + await client.removeFromContinueWatching(item); + WatchStateNotifier().notifyRemovedFromContinueWatching(item: item); + } +} diff --git a/lib/widgets/media_context_menu.dart b/lib/widgets/media_context_menu.dart index 3e5eca24..912dbdc9 100644 --- a/lib/widgets/media_context_menu.dart +++ b/lib/widgets/media_context_menu.dart @@ -20,7 +20,7 @@ import '../services/plex_client.dart'; import '../services/media_list_playback_launcher.dart'; import '../services/offline_watch_sync_service.dart'; import '../services/playlist_items_loader.dart'; -import '../services/trackers/tracker_coordinator.dart'; +import '../services/watch_actions.dart'; import '../models/transcode_quality_preset.dart'; import '../utils/download_version_utils.dart'; import '../utils/download_utils.dart'; @@ -30,7 +30,6 @@ import '../utils/global_key_utils.dart'; import '../providers/download_provider.dart'; import '../providers/multi_server_provider.dart'; import '../providers/offline_mode_provider.dart'; -import '../providers/offline_watch_provider.dart'; import '../profiles/active_profile_provider.dart'; import '../profiles/profile.dart'; import '../utils/provider_extensions.dart'; @@ -544,49 +543,22 @@ class MediaContextMenuState extends State { break; case 'watch': - final isOffline = context.read().isOffline; - if (isOffline && mediaItem?.serverId != null) { - // Offline mode: queue action for later sync (emits WatchStateEvent) - final offlineWatch = context.read(); - await offlineWatch.markAsWatched(serverId: ServerId(mediaItem!.serverId!), itemId: mediaItem.id); - if (context.mounted) { - showAppSnackBar(context, t.messages.markedAsWatchedOffline); - _notifyRefresh(mediaItem.id); - } - } else { - // Resolve the right backend client — Plex hits scrobble, Jellyfin - // hits /UserPlayedItems. WatchStateNotifier event is fired in both - // paths so cross-screen UI updates regardless of backend. - await _executeAction(context, () async { - final item = mediaItem; - final client = context.tryGetMediaClientForServer(ServerId(_itemServerId!)); - if (client != null && item != null) { - await client.markWatched(item); - unawaited(TrackerCoordinator.instance.markWatched(item, client)); - } - }, t.messages.markedAsWatched); - } - break; - case 'unwatch': + final watched = selected == 'watch'; + final item = mediaItem; + if (item == null) break; final isOffline = context.read().isOffline; - if (isOffline && mediaItem?.serverId != null) { - // Offline mode: queue action for later sync (emits WatchStateEvent) - final offlineWatch = context.read(); - await offlineWatch.markAsUnwatched(serverId: ServerId(mediaItem!.serverId!), itemId: mediaItem.id); + if (isOffline && item.serverId != null) { + // Queue for later sync — the offline provider emits the WatchStateEvent. + await WatchActions.setWatched(context, item, watched: watched, offline: true); if (context.mounted) { - showAppSnackBar(context, t.messages.markedAsUnwatchedOffline); - _notifyRefresh(mediaItem.id); + showAppSnackBar(context, watched ? t.messages.markedAsWatchedOffline : t.messages.markedAsUnwatchedOffline); + _notifyRefresh(item.id); } } else { await _executeAction(context, () async { - final item = mediaItem; - final client = context.tryGetMediaClientForServer(ServerId(_itemServerId!)); - if (client != null && item != null) { - await client.markUnwatched(item); - unawaited(TrackerCoordinator.instance.markUnwatched(item, client)); - } - }, t.messages.markedAsUnwatched); + await WatchActions.setWatched(context, item, watched: watched, offline: false); + }, watched ? t.messages.markedAsWatched : t.messages.markedAsUnwatched); } break; @@ -595,8 +567,7 @@ class MediaContextMenuState extends State { // This preserves the progression for partially watched items // and doesn't mark unwatched next episodes as watched try { - final client = _getMediaClientForItem(); - await client.removeFromContinueWatching(mediaItem!); + await WatchActions.removeFromContinueWatching(context, mediaItem!); if (context.mounted) { showSuccessSnackBar(context, t.messages.removedFromContinueWatching); if (widget.onRemoveFromContinueWatching != null) { diff --git a/test/services/playback_progress_tracker_test.dart b/test/services/playback_progress_tracker_test.dart index 2e860bf5..2fb979e5 100644 --- a/test/services/playback_progress_tracker_test.dart +++ b/test/services/playback_progress_tracker_test.dart @@ -113,6 +113,11 @@ class _FakePlexClient implements PlexClient { @override int get watchedThresholdPercent => thresholdPercent; + /// markWatchedFromPlaybackStop resolves the event's cacheServerId from + /// [serverId] after the transport call. + @override + ServerId get serverId => ServerId('scrobbler'); + @override double get watchedThreshold => thresholdPercent / 100.0; @@ -213,6 +218,8 @@ class _FakePlexClient implements PlexClient { return updateProgress(itemId, time: position.inMilliseconds, state: 'stopped', duration: duration?.inMilliseconds); } + // Transport-only, like production: the single watch event for the stop + // flow is emitted by markWatchedFromPlaybackStop after this returns. @override Future markWatched(MediaItem item) async { if (throwOnNextCall != null) { @@ -221,20 +228,16 @@ class _FakePlexClient implements PlexClient { throw err; } markWatchedCalls.add(item.id); - WatchStateNotifier().notifyWatched(item: item, isNowWatched: true); } @override - Future markAsWatched(String ratingKey, {MediaItem? item}) async { + Future markAsWatched(String ratingKey) async { if (throwOnNextCall != null) { final err = throwOnNextCall!; throwOnNextCall = null; throw err; } markWatchedCalls.add(ratingKey); - if (item != null) { - WatchStateNotifier().notifyWatched(item: item, isNowWatched: true); - } } @override