fix(continue-watching): clear the resume position when an item is marked watched

Marking a movie or episode watched left it sitting in Continue Watching with a
checkmark, and the only way to shift it was to play it and skip to the end.

Continue Watching membership on a MediaBrowser server is derived from
UserData.PlaybackPositionTicks alone; Played is never consulted. Marking played
normally zeroes that position as a side effect, so the row usually disappears
and nothing ever checked that it had. When something writes a position back
afterwards the item is left played *and* resumable, which the resume route
happily keeps returning forever. markWatched now reads the UserItemDataDto the
mark already returns and clears the bookmark itself when the server left one
behind, so the postcondition holds however the item got into that state. The
follow-up write costs a request only when the invariant is actually broken.

The writer putting items there is our own offline queue. insertWatchAction
already drops queued progress for an item when the mark is itself queued, but
the online mark writes straight to the server and queues nothing, so a progress
row recorded earlier survived and replayed afterwards — pending actions go out
oldest first — restoring the very position the mark had cleared. The sync
service now listens for watch-state events and discards queued progress for
that item as the mark lands. Progress recorded after a mark is a rewatch and is
queued later, so it is untouched. Plex never showed this because it forwards the
recorded-at timestamp and lets the server discard a stale replay; the
MediaBrowser stop report has nowhere to put one.

Continue Watching also drops the row locally now instead of waiting a round trip
for the refetch to confirm it, matching what removal events already did, and
marking a season or show takes its on-deck episode with it.

Watched items are deliberately still not filtered out of the shelf: Jellyfin
keeps Played set when new progress arrives, so a rewatch in progress is
indistinguishable from a stuck row, and filtering would hide it.

close #1812
This commit is contained in:
edde746
2026-08-06 04:21:24 +02:00
parent 309a107912
commit 094be1fa3e
9 changed files with 386 additions and 6 deletions
+31
View File
@@ -984,6 +984,37 @@ class AppDatabase extends _$AppDatabase {
}); });
} }
/// Drop queued `progress` rows for one item, leaving `watched`/`unwatched`
/// rows alone. Returns how many were removed.
///
/// The mirror of the purge [insertWatchAction] performs: a terminal watch
/// state written straight to the server (the online path, which queues
/// nothing) also supersedes any progress still waiting to replay. Without
/// it, [getPendingWatchActions] hands back the older progress row — it
/// orders by `createdAt` — and replaying it rewrites the resume position the
/// mark just cleared, pinning the item to Continue Watching (#1812).
///
/// Progress queued *after* a mark is a genuine rewatch and is not affected:
/// this only runs at the moment the mark lands.
Future<int> deleteQueuedProgressForItem({
String? profileId,
required ServerId serverId,
String? clientScopeId,
required String ratingKey,
}) {
return _runPendingMutation(() async {
final globalKey = buildGlobalKey(ServerId(serverId), ratingKey);
return (delete(offlineWatchProgress)..where(
(t) =>
t.globalKey.equals(globalKey) &
_nullableTextPredicate(t.profileId, profileId) &
_nullableTextPredicate(t.clientScopeId, clientScopeId) &
t.actionType.equals(OfflineActionType.progress.id),
))
.go();
});
}
/// Delete a specific watch action after successful sync /// Delete a specific watch action after successful sync
Future<void> deleteWatchAction(int id) { Future<void> deleteWatchAction(int id) {
return _runPendingMutation(() async { return _runPendingMutation(() async {
+8
View File
@@ -348,6 +348,14 @@ abstract class MediaServerClient {
/// emission (and tracker fan-out); the offline sync replay calls this /// emission (and tracker fan-out); the offline sync replay calls this
/// directly precisely because the event already fired when the action was /// directly precisely because the event already fired when the action was
/// queued. /// queued.
///
/// Postcondition: once this completes the backend must no longer treat
/// [item] as resumable, so it cannot come back from [fetchContinueWatching].
/// Plex gets this for free (PMS filters watched items out of its on-deck
/// hub). MediaBrowser derives Continue Watching membership purely from
/// `UserData.PlaybackPositionTicks > 0`, so its implementation must ensure
/// the resume position is cleared rather than assume the played flag did it
/// (#1812).
Future<void> markWatched(MediaItem item); Future<void> markWatched(MediaItem item);
/// Mark [item] as unwatched. Transport only — see [markWatched]. /// Mark [item] as unwatched. Transport only — see [markWatched].
+19 -5
View File
@@ -489,15 +489,29 @@ class DiscoverProvider extends ChangeNotifier with DisposableChangeNotifierMixin
} }
if (event.changeType == WatchStateChangeType.removedFromContinueWatching) { if (event.changeType == WatchStateChangeType.removedFromContinueWatching) {
final remaining = _onDeck.where((item) => item.id != event.itemId).toList(); _evictFromOnDeck((item) => item.id == event.itemId);
if (remaining.length != _onDeck.length) { } else if (event.changeType == WatchStateChangeType.watched ||
_onDeck = remaining; (event.changeType == WatchStateChangeType.progressUpdate && event.isNowWatched == true)) {
safeNotifyListeners(); // Finished items have no business in Continue Watching, so drop the row
} // now instead of waiting a round trip for the refetch below to confirm
// it. Marking a season or show watched takes its on-deck episode with
// it, matching the parent-aware filter this subscription uses — the
// series' successor comes back from the refetch (#1812).
_evictFromOnDeck(
(item) => item.id == event.itemId || item.parentId == event.itemId || item.grandparentId == event.itemId,
);
} }
unawaited(refreshContinueWatching()); unawaited(refreshContinueWatching());
} }
void _evictFromOnDeck(bool Function(MediaItem item) matches) {
final remaining = _onDeck.where((item) => !matches(item)).toList();
if (remaining.length == _onDeck.length) return;
_onDeck = remaining;
safeNotifyListeners();
unawaited(_syncSystemShelf(_onDeck));
}
/// Everything on screen: the Continue Watching row plus every hub row. /// Everything on screen: the Continue Watching row plus every hub row.
Iterable<MediaItem> get _visibleItems => _onDeck.followedBy(_hubs.expand((hub) => hub.items)); Iterable<MediaItem> get _visibleItems => _onDeck.followedBy(_hubs.expand((hub) => hub.items));
@@ -1,10 +1,41 @@
part of '../../jellyfin_client.dart'; part of '../../jellyfin_client.dart';
mixin _JellyfinWatchStateMethods on _JellyfinClientInternals { mixin _JellyfinWatchStateMethods on _JellyfinClientInternals {
/// Marking played normally zeroes `UserData.PlaybackPositionTicks` server-side,
/// which is what drops the row from Continue Watching — membership on this API
/// is derived from the position alone, never from `Played` (verified on
/// Jellyfin 10.11.10). Relying on that side effect is not enough: a stale
/// playback report replayed from the offline queue, another client, or a
/// server-side plugin can leave `Played` set *and* a position behind, and the
/// row is then pinned to Continue Watching forever (#1812).
///
/// So assert the postcondition instead of assuming it, using the
/// `UserItemDataDto` the mark already returns. The follow-up write costs a
/// request only when the invariant is actually broken.
///
/// Folders (series/season) report their own position as 0 while the server
/// resets their children recursively, so nothing extra is owed here.
@override @override
Future<void> markWatched(MediaItem item) async { Future<void> markWatched(MediaItem item) async {
final response = await _http.post(paths.playedItem(item.id), queryParameters: {'userId': connection.userId}); final response = await _http.post(paths.playedItem(item.id), queryParameters: {'userId': connection.userId});
throwIfHttpError(response); throwIfHttpError(response);
final data = response.data;
final positionMs = data is Map<String, dynamic> ? jellyfinTicksToMs(data['PlaybackPositionTicks']) : null;
if (positionMs == null || positionMs <= 0) return;
appLogger.d('JellyfinClient: ${item.id} stayed resumable after mark-played; clearing its resume position');
await _clearResumePosition(item.id);
}
/// Drop [itemId]'s resume bookmark without touching its played flag.
Future<void> _clearResumePosition(String itemId) async {
final response = await _http.post(
paths.userItemData(itemId),
queryParameters: {'userId': connection.userId},
body: {'PlaybackPositionTicks': 0},
);
throwIfHttpError(response);
} }
@override @override
+12
View File
@@ -34,6 +34,18 @@ class MediaBrowserPaths {
String playedItem(String itemId) => String playedItem(String itemId) =>
dialect.requiresUserScopedItemRoutes ? '$_user/PlayedItems/${_id(itemId)}' : '/UserPlayedItems/${_id(itemId)}'; dialect.requiresUserScopedItemRoutes ? '$_user/PlayedItems/${_id(itemId)}' : '/UserPlayedItems/${_id(itemId)}';
/// Per-user playback-state write. `POST` with `{"PlaybackPositionTicks": 0}`
/// clears the resume bookmark while leaving `Played` untouched (verified on
/// Jellyfin 10.11.10 for both spellings).
///
/// Continue Watching membership on this API is derived purely from
/// `UserData.PlaybackPositionTicks > 0` — `Played` is not consulted — so this
/// is the only route that can guarantee a finished item stops being
/// resumable. See [MediaServerClient.markWatched].
String userItemData(String itemId) => dialect.requiresUserScopedItemRoutes
? '$_user/Items/${_id(itemId)}/UserData'
: '/UserItems/${_id(itemId)}/UserData';
/// Favourite flag write route (`POST` to add, `DELETE` to remove). /// Favourite flag write route (`POST` to add, `DELETE` to remove).
String favoriteItem(String itemId) => dialect.requiresUserScopedItemRoutes String favoriteItem(String itemId) => dialect.requiresUserScopedItemRoutes
? '$_user/FavoriteItems/${_id(itemId)}' ? '$_user/FavoriteItems/${_id(itemId)}'
+44 -1
View File
@@ -86,7 +86,48 @@ class OfflineWatchSyncService extends ChangeNotifier {
/// silently drops local watch progress. /// silently drops local watch progress.
static const int maxSyncAttempts = 5; static const int maxSyncAttempts = 5;
OfflineWatchSyncService({required this._database, required this._serverManager}); StreamSubscription<WatchStateEvent>? _watchStateSubscription;
OfflineWatchSyncService({required this._database, required this._serverManager}) {
_watchStateSubscription = WatchStateNotifier().stream.listen(_onWatchStateChanged);
}
/// A terminal watch state just landed, so any progress still queued for that
/// item is stale and must not replay.
///
/// [AppDatabase.insertWatchAction] already purges the queue when the mark is
/// itself queued (offline). The online path writes straight to the server and
/// queues nothing, so without this the older progress row survives and
/// [syncPendingItems] later rewrites the resume position the mark cleared —
/// on MediaBrowser that pins the item to Continue Watching for good (#1812).
/// Plex is unaffected in practice (PMS discards a replay whose `updated`
/// timestamp is stale) but the queue entry is meaningless there too.
///
/// Progress recorded *after* the mark is a genuine rewatch: it is queued
/// later, so it is never touched here.
void _onWatchStateChanged(WatchStateEvent event) {
if (_isShutDown) return;
if (event.changeType != WatchStateChangeType.watched && event.changeType != WatchStateChangeType.unwatched) {
return;
}
unawaited(_discardQueuedProgress(ServerId(event.serverId), event.itemId));
}
Future<void> _discardQueuedProgress(ServerId serverId, String itemId) async {
try {
final removed = await _database.deleteQueuedProgressForItem(
profileId: _activeProfileId,
serverId: serverId,
clientScopeId: await _clientScopeIdForItem(serverId, itemId),
ratingKey: itemId,
);
if (removed == 0) return;
appLogger.d('Dropped $removed superseded queued progress action(s) for $serverId:$itemId');
notifyListeners();
} catch (e) {
appLogger.w('Failed to drop superseded queued progress for $serverId:$itemId', error: e);
}
}
/// Whether a sync is currently in progress /// Whether a sync is currently in progress
bool get isSyncing => _isSyncing; bool get isSyncing => _isSyncing;
@@ -834,6 +875,8 @@ class OfflineWatchSyncService extends ChangeNotifier {
@override @override
void dispose() { void dispose() {
_isShutDown = true; _isShutDown = true;
_watchStateSubscription?.cancel();
_watchStateSubscription = null;
if (_offlineModeSource != null && _offlineModeListener != null) { if (_offlineModeSource != null && _offlineModeListener != null) {
_offlineModeSource!.removeListener(_offlineModeListener!); _offlineModeSource!.removeListener(_offlineModeListener!);
} }
@@ -415,6 +415,66 @@ void main() {
expect(aggregation.hubCalls, hubCallsBefore); expect(aggregation.hubCalls, hubCallsBefore);
}); });
test('watched event drops the row immediately, before the refetch answers', () async {
aggregation.onDeckResult = () => [_item('ep-1'), _item('ep-2')];
await provider.load();
// A finished item has no business on the shelf, so the row must go now
// rather than a round trip later — and it must not come back if the
// backend is still returning it (#1812).
var sawImmediateRemoval = false;
provider.addListener(() {
if (provider.onDeck.length == 1 && provider.onDeck.single.id == 'ep-2') {
sawImmediateRemoval = true;
}
});
aggregation.onDeckResult = () => [_item('ep-2')];
WatchStateNotifier().notifyWatched(item: _item('ep-1'));
await pumpEventQueue();
expect(sawImmediateRemoval, isTrue);
expect(provider.onDeck.map((i) => i.id), ['ep-2']);
});
test('marking a show watched drops its on-deck episode', () async {
aggregation.onDeckResult = () => [_item('ep-1', parentId: 'season-1', grandparentId: 'show-1'), _item('ep-2')];
await provider.load();
aggregation.onDeckResult = () => [_item('ep-2')];
WatchStateNotifier().notifyWatched(item: _item('show-1', kind: MediaKind.show));
await pumpEventQueue();
expect(provider.onDeck.map((i) => i.id), ['ep-2']);
});
test('threshold-crossing progress drops the row too', () async {
aggregation.onDeckResult = () => [_item('ep-1'), _item('ep-2')];
await provider.load();
aggregation.onDeckResult = () => [_item('ep-2')];
WatchStateNotifier().notifyProgress(item: _item('ep-1'), viewOffset: 95000, duration: 100000);
await pumpEventQueue();
expect(provider.onDeck.map((i) => i.id), ['ep-2']);
});
test('unwatched event evicts nothing', () async {
aggregation.onDeckResult = () => [_item('ep-1'), _item('ep-2')];
await provider.load();
var sawShorterList = false;
provider.addListener(() {
if (provider.onDeck.length < 2) sawShorterList = true;
});
WatchStateNotifier().notifyWatched(item: _item('ep-1'), isNowWatched: false);
await pumpEventQueue();
expect(sawShorterList, isFalse);
expect(provider.onDeck.map((i) => i.id), ['ep-1', 'ep-2']);
});
test('deletion drops the item from on-deck and hubs, then refreshes continue watching only', () async { test('deletion drops the item from on-deck and hubs, then refreshes continue watching only', () async {
aggregation.onDeckResult = () => [_item('ep-1'), _item('ep-2')]; aggregation.onDeckResult = () => [_item('ep-1'), _item('ep-2')];
aggregation.hubsResult = () => [ aggregation.hubsResult = () => [
@@ -1356,6 +1356,82 @@ void main() {
}); });
}); });
group('MediaBrowser mark-watched clears a surviving resume position', () {
// Continue Watching membership on this API is `PlaybackPositionTicks > 0`
// alone, so a played item that keeps a position is pinned to the shelf
// forever. markWatched must assert that postcondition, not assume it
// (#1812).
http.Response Function(http.Request) respondWithPosition(int ticks) {
return (request) {
if (request.url.path.endsWith('/UserData')) return http.Response('', 204);
return jsonResponse({'ItemId': 'item-1', 'Played': true, 'PlaybackPositionTicks': ticks});
};
}
test('Jellyfin clears it through the unprefixed user-data route', () async {
final requests = _RequestCapture(respondWithPosition(12000000000));
final client = testJellyfinClient(handler: requests.handle);
addTearDown(client.close);
await client.markWatched(_item(MediaBackend.jellyfin));
expect(requests.log, [
'POST /UserPlayedItems/item-1?userId=user-1',
'POST /UserItems/item-1/UserData?userId=user-1',
]);
expect(jsonDecode(requests.requests.last.body), {'PlaybackPositionTicks': 0});
});
test('Emby clears it through the user-scoped user-data route', () async {
final requests = _RequestCapture(respondWithPosition(12000000000));
final client = testEmbyClient(handler: requests.handle);
addTearDown(client.close);
await client.markWatched(_item(MediaBackend.emby));
expect(requests.log, [
'POST /Users/user-1/PlayedItems/item-1?userId=user-1',
'POST /Users/user-1/Items/item-1/UserData?userId=user-1',
]);
expect(jsonDecode(requests.requests.last.body), {'PlaybackPositionTicks': 0});
});
test('a mark that already zeroed the position issues no follow-up write', () async {
// The normal case. A second request on every mark would double the cost
// of the app's most common watch-state write.
final requests = _RequestCapture(respondWithPosition(0));
final client = testJellyfinClient(handler: requests.handle);
addTearDown(client.close);
await client.markWatched(_item(MediaBackend.jellyfin));
expect(requests.log, ['POST /UserPlayedItems/item-1?userId=user-1']);
});
test('a body without UserData is left alone', () async {
// Older servers answer the played toggle with 204 and no DTO; there is
// nothing to assert against, so do not guess.
final requests = _RequestCapture((_) => http.Response('', 204));
final client = testJellyfinClient(handler: requests.handle);
addTearDown(client.close);
await client.markWatched(_item(MediaBackend.jellyfin));
expect(requests.log, ['POST /UserPlayedItems/item-1?userId=user-1']);
});
test('a failed clear surfaces as MediaServerHttpException', () async {
final requests = _RequestCapture((request) {
if (request.url.path.endsWith('/UserData')) return http.Response('nope', 500);
return jsonResponse({'ItemId': 'item-1', 'Played': true, 'PlaybackPositionTicks': 12000000000});
});
final client = testJellyfinClient(handler: requests.handle);
addTearDown(client.close);
await expectLater(client.markWatched(_item(MediaBackend.jellyfin)), throwsA(isA<MediaServerHttpException>()));
});
});
group('MediaBrowser name-pair item fields', () { group('MediaBrowser name-pair item fields', () {
test('Emby item tags and genres survive the browse path via the name-pair arrays', () async { test('Emby item tags and genres survive the browse path via the name-pair arrays', () async {
// Real Emby DTO shape: the plain arrays are absent, the name-pair // Real Emby DTO shape: the plain arrays are absent, the name-pair
@@ -604,6 +604,111 @@ void main() {
}); });
}); });
// ============================================================
// Superseded queued progress (#1812)
// ============================================================
group('queued progress superseded by a watch-state write', () {
MediaItem itemFor(String id) =>
testMediaItem(id: id, backend: MediaBackend.jellyfin, kind: MediaKind.movie, serverId: 'srv');
// The online mark writes straight to the server and queues nothing, so
// nothing purges the queue the way insertWatchAction does for the offline
// mark. Replaying the stale row afterwards rewrites the resume position the
// mark cleared, which pins the item to Continue Watching on MediaBrowser.
test('a watched event drops the queued progress row for that item', () async {
final (svc: svc, db: db, mgr: mgr) = _makeService();
addTearDown(() async {
svc.dispose();
mgr.dispose();
await db.close();
});
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '42', viewOffset: 50000, duration: 100000);
expect(await svc.getPendingSyncCount(), 1);
WatchStateNotifier().notifyWatched(item: itemFor('42'));
await pumpEventQueue();
expect(await svc.getPendingSyncCount(), 0);
expect(await db.getLatestWatchAction('srv:42'), isNull);
});
test('an unwatched event drops it too', () async {
final (svc: svc, db: db, mgr: mgr) = _makeService();
addTearDown(() async {
svc.dispose();
mgr.dispose();
await db.close();
});
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '42', viewOffset: 50000, duration: 100000);
WatchStateNotifier().notifyWatched(item: itemFor('42'), isNowWatched: false);
await pumpEventQueue();
expect(await svc.getPendingSyncCount(), 0);
});
test('other items and queued manual marks are left alone', () async {
final (svc: svc, db: db, mgr: mgr) = _makeService();
addTearDown(() async {
svc.dispose();
mgr.dispose();
await db.close();
});
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '42', viewOffset: 50000, duration: 100000);
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '43', viewOffset: 50000, duration: 100000);
await svc.queueMarkWatched(serverId: ServerId('srv'), itemId: '44');
WatchStateNotifier().notifyWatched(item: itemFor('42'));
await pumpEventQueue();
expect(await db.getLatestWatchAction('srv:42'), isNull);
expect((await db.getLatestWatchAction('srv:43'))?.actionType, 'progress');
expect((await db.getLatestWatchAction('srv:44'))?.actionType, 'watched');
});
test('progress recorded after the mark survives — that is a rewatch', () async {
final (svc: svc, db: db, mgr: mgr) = _makeService();
addTearDown(() async {
svc.dispose();
mgr.dispose();
await db.close();
});
WatchStateNotifier().notifyWatched(item: itemFor('42'));
await pumpEventQueue();
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '42', viewOffset: 5000, duration: 100000);
expect((await db.getLatestWatchAction('srv:42'))?.viewOffset, 5000);
});
test('a superseded row never reaches the server on the next sync', () async {
final (svc: svc, db: db, mgr: mgr) = _makeService();
addTearDown(() async {
svc.dispose();
mgr.dispose();
await db.close();
});
svc.setActiveProfileId('p1');
final client = _RecordingMediaClient(serverId: ServerId('srv'), backend: MediaBackend.jellyfin);
mgr.debugRegisterClientForTesting(client);
await svc.queueProgressUpdate(serverId: ServerId('srv'), itemId: '42', viewOffset: 50000, duration: 100000);
WatchStateNotifier().notifyWatched(item: itemFor('42'));
await pumpEventQueue();
await svc.syncPendingItems();
// The whole point: no position write, so the resume bookmark the mark
// cleared stays cleared.
expect(client.stopped, isEmpty);
expect(client.started, isEmpty);
});
});
// ============================================================ // ============================================================
// getLocalWatchStatus // getLocalWatchStatus
// ============================================================ // ============================================================