fix(explore): list every library copy of a title, not one per server
`MediaServerClient.findByExternalIds` returned `MediaItem?`, so the Explore "In these libraries" chooser could never show more than one copy per server. A movie held by both a 4K library and an HD library on one Plex server therefore resolved to whichever copy came back first, with no way to reach the other. Return every id-verified match instead. `/library/all` is already server-wide and each `Metadata` entry carries its own `librarySectionID`, so both copies come back labelled with no extra request; Plex was simply taking `Metadata[0]` and the title ladder was returning on its first hit. An exact-guid hit no longer short-circuits the title search either — a library still on a legacy agent has a different primary guid and is invisible to the `guid=` filter. Copies are deduped by global key and ordered best-first, and each row now states its resolution, since library names need not mention it. Resolution passes merge rather than replace: the cross-server fan-out logs and skips per-server failures, so a later pass can come back short a server that answered an earlier one, and a failed pass no longer claims the title left the library. Duplicate keys fold field by field, because Jellyfin's library stamp is a best-effort ancestors lookup that returns the item bare when it fails and an unstamped row is indistinguishable from its sibling. Focus nodes are keyed by copy and reclaimed after a merge re-sorts the rows, so a dpad user is not thrown to a different copy. close #1754
This commit is contained in:
@@ -10,6 +10,7 @@ import 'package:plezy/i18n/strings.g.dart';
|
||||
import 'package:plezy/media/media_kind.dart';
|
||||
import 'package:plezy/media/media_rating.dart';
|
||||
import 'package:plezy/media/media_item.dart';
|
||||
import 'package:plezy/media/media_version.dart';
|
||||
import 'package:plezy/models/catalog/catalog_cast_member.dart';
|
||||
import 'package:plezy/models/catalog/catalog_item.dart';
|
||||
import 'package:plezy/models/catalog/catalog_metadata.dart';
|
||||
@@ -24,6 +25,7 @@ import 'package:plezy/theme/mono_theme.dart';
|
||||
import 'package:plezy/utils/platform_detector.dart';
|
||||
import 'package:plezy/widgets/overlay_sheet.dart';
|
||||
import 'package:plezy/widgets/hub_section.dart';
|
||||
import 'package:plezy/widgets/focusable_list_tile.dart';
|
||||
import 'package:plezy/widgets/media_card.dart';
|
||||
import 'package:plezy/widgets/optimized_media_image.dart';
|
||||
import 'package:provider/provider.dart';
|
||||
@@ -146,6 +148,53 @@ class _ExternalIdGatedMatcher extends CatalogLibraryMatcher {
|
||||
}
|
||||
}
|
||||
|
||||
/// Serves one scripted result per `match` call, so a test can model the bare
|
||||
/// row lookup and the detail-enriched lookup independently.
|
||||
class _ScriptedMatcher extends CatalogLibraryMatcher {
|
||||
_ScriptedMatcher(super.multiServer, this.passes);
|
||||
|
||||
final List<List<MediaItem> Function()> passes;
|
||||
int calls = 0;
|
||||
|
||||
@override
|
||||
Future<List<MediaItem>> match(CatalogItem item) async {
|
||||
final pass = passes[calls < passes.length ? calls : passes.length - 1];
|
||||
calls++;
|
||||
return pass();
|
||||
}
|
||||
}
|
||||
|
||||
/// A Plex Discover row whose bare form carries only its own id; the detail
|
||||
/// body adds the external ids (#1715), which is what triggers a second pass.
|
||||
const _bareRow = CatalogItem(
|
||||
source: CatalogSourceId.trakt,
|
||||
kind: MediaKind.movie,
|
||||
title: 'Row-only Movie',
|
||||
ids: CatalogItemIds(trakt: 5),
|
||||
);
|
||||
const _enrichedRow = CatalogItem(
|
||||
source: CatalogSourceId.trakt,
|
||||
kind: MediaKind.movie,
|
||||
title: 'Row-only Movie',
|
||||
ids: CatalogItemIds(trakt: 5, tmdb: 99),
|
||||
);
|
||||
|
||||
MediaItem _libraryCopy({
|
||||
required String id,
|
||||
String? libraryTitle,
|
||||
String? videoResolution,
|
||||
String? serverName = 'Living Room',
|
||||
}) => testMediaItem(
|
||||
id: id,
|
||||
serverId: 'server-1',
|
||||
serverName: serverName,
|
||||
libraryId: libraryTitle == null ? null : id,
|
||||
libraryTitle: libraryTitle,
|
||||
mediaVersions: videoResolution == null
|
||||
? null
|
||||
: [MediaVersion(id: '$id-v', videoResolution: videoResolution, videoCodec: 'hevc', container: 'mkv')],
|
||||
);
|
||||
|
||||
const _item = CatalogItem(
|
||||
source: CatalogSourceId.trakt,
|
||||
kind: MediaKind.movie,
|
||||
@@ -284,6 +333,159 @@ void main() {
|
||||
expect(find.text('Movies'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('lists every library copy of one title, best quality first', (tester) async {
|
||||
// #1754: one movie held by both a 4K library and an HD library on the same
|
||||
// server. Library names are user-chosen, so each row also states the
|
||||
// resolution the user is actually choosing between.
|
||||
final source = _FakeCatalogSource();
|
||||
|
||||
await _pumpDetail(
|
||||
tester,
|
||||
source,
|
||||
matches: [
|
||||
_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080'),
|
||||
_libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k'),
|
||||
],
|
||||
);
|
||||
|
||||
expect(find.text(t.explore.inTheseLibraries), findsOneWidget);
|
||||
expect(find.text('Movies'), findsOneWidget);
|
||||
expect(find.text('4K Movies'), findsOneWidget);
|
||||
|
||||
final subtitles = tester.widgetList<Text>(find.textContaining('Living Room')).map((text) => text.data!).toList();
|
||||
expect(subtitles, hasLength(2));
|
||||
expect(subtitles.first, contains('4K'), reason: 'the 4K copy sorts above the HD one');
|
||||
expect(subtitles.last, contains('1080p'));
|
||||
});
|
||||
|
||||
testWidgets('a re-resolve that comes back short keeps the copies already found', (tester) async {
|
||||
// The cross-server fan-out logs and skips per-server failures, so a later
|
||||
// pass can answer without a server that replied to the first one. Those
|
||||
// rows are still valid and must not be wiped.
|
||||
late _ScriptedMatcher matcher;
|
||||
final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow));
|
||||
|
||||
await _pumpDetail(
|
||||
tester,
|
||||
source,
|
||||
item: _bareRow,
|
||||
matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [
|
||||
() => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies')],
|
||||
() => const [],
|
||||
]),
|
||||
);
|
||||
|
||||
expect(matcher.calls, 2);
|
||||
expect(find.text(t.explore.inTheseLibraries), findsOneWidget);
|
||||
expect(find.text('Movies'), findsOneWidget);
|
||||
expect(find.text(t.explore.notInLibrary), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('a failed re-resolve does not claim the title left the library', (tester) async {
|
||||
late _ScriptedMatcher matcher;
|
||||
final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow));
|
||||
|
||||
await _pumpDetail(
|
||||
tester,
|
||||
source,
|
||||
item: _bareRow,
|
||||
matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [
|
||||
() => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies')],
|
||||
() => throw StateError('server unreachable'),
|
||||
]),
|
||||
);
|
||||
|
||||
expect(matcher.calls, 2);
|
||||
expect(find.text('Movies'), findsOneWidget);
|
||||
expect(find.text(t.explore.notInLibrary), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('a re-resolve that lost its library stamp keeps the one already shown', (tester) async {
|
||||
// Jellyfin stamps a copy's library with a best-effort ancestors lookup
|
||||
// that returns the item bare when it fails. A row that fell back to the
|
||||
// server name would be indistinguishable from its sibling in the same
|
||||
// server's other library.
|
||||
late _ScriptedMatcher matcher;
|
||||
final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow));
|
||||
|
||||
await _pumpDetail(
|
||||
tester,
|
||||
source,
|
||||
item: _bareRow,
|
||||
matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [
|
||||
() => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080')],
|
||||
() => [_libraryCopy(id: 'hd-copy', serverName: null)],
|
||||
]),
|
||||
);
|
||||
|
||||
expect(matcher.calls, 2);
|
||||
expect(find.text('Movies'), findsOneWidget);
|
||||
final subtitles = tester.widgetList<Text>(find.textContaining('Living Room')).map((text) => text.data!);
|
||||
expect(subtitles.single, contains('1080p'), reason: 'the quality hint survives an unstamped re-resolve too');
|
||||
});
|
||||
|
||||
testWidgets('a re-resolve that finds another library adds it to the list', (tester) async {
|
||||
// The exact `plex://` guid only sees libraries on the modern agent; a
|
||||
// legacy-agent sibling arrives with the enriched imdb/tmdb pass (#1754).
|
||||
// The first pass being non-empty must not suppress the second.
|
||||
late _ScriptedMatcher matcher;
|
||||
final source = _FakeCatalogSource(detail: const CatalogDetail(item: _enrichedRow));
|
||||
|
||||
await _pumpDetail(
|
||||
tester,
|
||||
source,
|
||||
item: _bareRow,
|
||||
matcherBuilder: (multiServer) => matcher = _ScriptedMatcher(multiServer, [
|
||||
() => [_libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k')],
|
||||
() => [
|
||||
_libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k'),
|
||||
_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080'),
|
||||
],
|
||||
]),
|
||||
);
|
||||
|
||||
expect(matcher.calls, 2);
|
||||
expect(find.text('4K Movies'), findsOneWidget, reason: 'the copy both passes agree on is not doubled');
|
||||
expect(find.text('Movies'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('focus stays on a library copy when a later pass adds one above it', (tester) async {
|
||||
// Merging re-sorts, so the rows can move. Focus nodes are keyed by the
|
||||
// copy, not by row index, or a dpad user would be thrown to another copy.
|
||||
final detailCompleter = Completer<CatalogDetail>();
|
||||
final source = _FakeCatalogSource(detailCompleter: detailCompleter);
|
||||
|
||||
await _pumpDetail(
|
||||
tester,
|
||||
source,
|
||||
item: _bareRow,
|
||||
matcherBuilder: (multiServer) => _ScriptedMatcher(multiServer, [
|
||||
() => [_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080')],
|
||||
() => [
|
||||
_libraryCopy(id: 'hd-copy', libraryTitle: 'Movies', videoResolution: '1080'),
|
||||
_libraryCopy(id: 'uhd-copy', libraryTitle: '4K Movies', videoResolution: '4k'),
|
||||
],
|
||||
]),
|
||||
);
|
||||
|
||||
final tile = tester.widget<FocusableListTile>(
|
||||
find.ancestor(of: find.text('Movies'), matching: find.byType(FocusableListTile)),
|
||||
);
|
||||
tile.focusNode!.requestFocus();
|
||||
await tester.pump();
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_server-1:hd-copy');
|
||||
|
||||
detailCompleter.complete(const CatalogDetail(item: _enrichedRow));
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text('4K Movies'), findsOneWidget);
|
||||
expect(
|
||||
FocusManager.instance.primaryFocus?.debugLabel,
|
||||
'catalog_library_match_server-1:hd-copy',
|
||||
reason: 'the 4K copy sorted above the focused HD copy without stealing focus',
|
||||
);
|
||||
});
|
||||
|
||||
testWidgets('fetchDetail failure leaves the opening item rendered', (tester) async {
|
||||
final source = _FakeCatalogSource(detailError: StateError('detail unavailable'));
|
||||
|
||||
@@ -832,19 +1034,31 @@ void main() {
|
||||
addTearDown(tester.view.resetDevicePixelRatio);
|
||||
addTearDown(tester.view.resetPhysicalSize);
|
||||
|
||||
// Copies render best-first, so the 4K one leads whatever order the
|
||||
// matcher returned them in.
|
||||
final matches = [
|
||||
testMediaItem(id: 'match_1', libraryTitle: 'Movies', serverName: 'Living Room'),
|
||||
testMediaItem(id: 'match_2', libraryTitle: 'Favorites', serverName: 'Bedroom'),
|
||||
testMediaItem(
|
||||
id: 'match_1',
|
||||
libraryTitle: 'Movies',
|
||||
serverName: 'Living Room',
|
||||
mediaVersions: const [MediaVersion(id: 'v1', videoResolution: '4k')],
|
||||
),
|
||||
testMediaItem(
|
||||
id: 'match_2',
|
||||
libraryTitle: 'Favorites',
|
||||
serverName: 'Bedroom',
|
||||
mediaVersions: const [MediaVersion(id: 'v2', videoResolution: '1080')],
|
||||
),
|
||||
];
|
||||
await _pumpDetail(tester, _FakeCatalogSource(), matches: matches);
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
|
||||
await tester.pumpAndSettle();
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_0');
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_1');
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
|
||||
await tester.pumpAndSettle();
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_1');
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_2');
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowDown);
|
||||
await tester.pumpAndSettle();
|
||||
@@ -852,7 +1066,7 @@ void main() {
|
||||
|
||||
await tester.sendKeyEvent(LogicalKeyboardKey.arrowUp);
|
||||
await tester.pumpAndSettle();
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_1');
|
||||
expect(FocusManager.instance.primaryFocus?.debugLabel, 'catalog_library_match_match_2');
|
||||
});
|
||||
|
||||
testWidgets('pending watchlist action keeps initial focus and its press retries the snapshot', (tester) async {
|
||||
|
||||
Reference in New Issue
Block a user