fix(screens): claim the mandatory profile picker before awaiting and skip the full-screen loader after playback
The mandatory profile picker checked its re-entry flag before several awaits, so an initialization notification could stack two requireSelection routes; the flag is now claimed before the first await and the late-profiles edge only fires on a real transition. Returning from playback also reran the full metadata loader, replacing the whole detail screen with a spinner and resetting the selected season; playback returns now use the non-loading watch-state refresh.
This commit is contained in:
@@ -284,6 +284,68 @@ ProfileInvalidationAction profileInvalidationAction({
|
||||
return ProfileInvalidationAction.none;
|
||||
}
|
||||
|
||||
/// The initial-profile prompt flow behind [_MainScreenState]'s post-frame
|
||||
/// prompt and its late-profiles re-arm, extracted so the mid-await re-entry
|
||||
/// race is testable without the full MainScreen tree.
|
||||
///
|
||||
/// [claimPrompt] must synchronously claim the "picker is up" guard and return
|
||||
/// false when it is already claimed. The claim is taken before the first
|
||||
/// await: the provider notifies mid-initialize (e.g. a fresh sign-in's slow
|
||||
/// home-user fetch), and the listener's re-entrant call would otherwise stack
|
||||
/// a second requireSelection picker on the root navigator. [releasePrompt]
|
||||
/// clears the claim on every exit; on the push path that re-clear is
|
||||
/// idempotent with [pushProfileSelection]'s own post-pop clear.
|
||||
@visibleForTesting
|
||||
Future<void> runInitialProfilePrompt({
|
||||
required ActiveProfileProvider activeProfile,
|
||||
required bool Function() claimPrompt,
|
||||
required void Function() releasePrompt,
|
||||
required bool Function() isMounted,
|
||||
required bool isOfflineMode,
|
||||
required Future<bool> Function() hasConnections,
|
||||
required Future<void> Function() settleSession,
|
||||
required Future<void> Function() pushProfileSelection,
|
||||
Future<SettingsService> Function() settings = SettingsService.getInstance,
|
||||
}) async {
|
||||
if (!claimPrompt()) return;
|
||||
try {
|
||||
// The provider's initialize() is fire-and-forget from MultiProvider —
|
||||
// wait for it to settle so `active` and `profiles` reflect storage
|
||||
// before we decide whether to prompt.
|
||||
await activeProfile.initialize();
|
||||
if (!isMounted()) return;
|
||||
|
||||
final settingsService = await settings();
|
||||
if (!isMounted()) return;
|
||||
|
||||
// Connections but ZERO resolvable profiles (e.g. the home-user fetch
|
||||
// failed at sign-in): a session with nothing to select and no picker is
|
||||
// a dead end. Mirror the boot guard — prune orphans and route to auth
|
||||
// when nothing selectable remains.
|
||||
if (activeProfile.active == null && activeProfile.profiles.isEmpty) {
|
||||
// Offline, "unresolvable" may just be an unreachable plex.tv — don't
|
||||
// kick the user to auth over it.
|
||||
if (!isOfflineMode && await hasConnections() && isMounted()) {
|
||||
appLogger.w('MainScreen: connections exist but no profiles resolved — settling session');
|
||||
await settleSession();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// Always prompt when there's no active profile but profiles exist
|
||||
// (fresh sign-in with multiple Plex Home users): otherwise the binder
|
||||
// has no profile to bind, and the user lands on an empty screen with
|
||||
// no way back to the picker.
|
||||
final hasNoActive = activeProfile.active == null && activeProfile.profiles.isNotEmpty;
|
||||
|
||||
if (!hasNoActive && !activeProfile.requiresSelectionOnOpen(settingsService)) return;
|
||||
|
||||
await pushProfileSelection();
|
||||
} finally {
|
||||
releasePrompt();
|
||||
}
|
||||
}
|
||||
|
||||
class MainScreen extends StatefulWidget {
|
||||
final bool isOfflineMode;
|
||||
|
||||
@@ -483,6 +545,11 @@ class _MainScreenState extends State<MainScreen>
|
||||
final activeProfile = context.read<ActiveProfileProvider>();
|
||||
_activeProfileForListener = activeProfile;
|
||||
_lastSeenProfileId = activeProfile.activeId;
|
||||
// Prime the late-profiles edge alongside _lastSeenProfileId so the
|
||||
// "!_hadProfiles && hasProfilesNow" check in _onActiveProfileChanged
|
||||
// fires only on a genuine empty -> non-empty transition, not on the
|
||||
// first notification of a session that started with profiles.
|
||||
_hadProfiles = activeProfile.profiles.isNotEmpty;
|
||||
activeProfile.addListener(_onActiveProfileChanged);
|
||||
_plexHomeService = context.read<PlexHomeService>();
|
||||
unawaited(_plexHomeService!.start());
|
||||
@@ -680,38 +747,20 @@ class _MainScreenState extends State<MainScreen>
|
||||
|
||||
final activeProfile = context.read<ActiveProfileProvider>();
|
||||
final connections = context.read<ConnectionRegistry>();
|
||||
// The provider's initialize() is fire-and-forget from MultiProvider —
|
||||
// wait for it to settle so `active` and `profiles` reflect storage
|
||||
// before we decide whether to prompt.
|
||||
await activeProfile.initialize();
|
||||
if (!mounted) return;
|
||||
|
||||
final settingsService = await SettingsService.getInstance();
|
||||
if (!mounted) return;
|
||||
|
||||
// Connections but ZERO resolvable profiles (e.g. the home-user fetch
|
||||
// failed at sign-in): a session with nothing to select and no picker is
|
||||
// a dead end. Mirror the boot guard — prune orphans and route to auth
|
||||
// when nothing selectable remains.
|
||||
if (activeProfile.active == null && activeProfile.profiles.isEmpty) {
|
||||
// Offline, "unresolvable" may just be an unreachable plex.tv — don't
|
||||
// kick the user to auth over it.
|
||||
if (!widget.isOfflineMode && (await connections.list()).isNotEmpty && mounted) {
|
||||
appLogger.w('MainScreen: connections exist but no profiles resolved — settling session');
|
||||
await settleSessionAfterRemoval(SessionTeardownScope.of(context));
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// Always prompt when there's no active profile but profiles exist
|
||||
// (fresh sign-in with multiple Plex Home users): otherwise the binder
|
||||
// has no profile to bind, and the user lands on an empty screen with
|
||||
// no way back to the picker.
|
||||
final hasNoActive = activeProfile.active == null && activeProfile.profiles.isNotEmpty;
|
||||
|
||||
if (!hasNoActive && !activeProfile.requiresSelectionOnOpen(settingsService)) return;
|
||||
|
||||
await _pushProfileSelection();
|
||||
await runInitialProfilePrompt(
|
||||
activeProfile: activeProfile,
|
||||
claimPrompt: () {
|
||||
if (_isShowingProfileSelection) return false;
|
||||
_isShowingProfileSelection = true;
|
||||
return true;
|
||||
},
|
||||
releasePrompt: () => _isShowingProfileSelection = false,
|
||||
isMounted: () => mounted,
|
||||
isOfflineMode: widget.isOfflineMode,
|
||||
hasConnections: () async => (await connections.list()).isNotEmpty,
|
||||
settleSession: () => settleSessionAfterRemoval(SessionTeardownScope.of(context)),
|
||||
pushProfileSelection: _pushProfileSelection,
|
||||
);
|
||||
}
|
||||
|
||||
/// Push the picker in "must choose" mode, suppressing the tvOS menu-button
|
||||
|
||||
@@ -51,7 +51,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState {
|
||||
context,
|
||||
metadata: _onDeckEpisode!,
|
||||
isOffline: widget.isOffline,
|
||||
onRefresh: _loadFullMetadata,
|
||||
onRefresh: _refreshWatchState,
|
||||
);
|
||||
} else {
|
||||
// No on deck episode, fetch first episode of first season
|
||||
@@ -64,7 +64,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState {
|
||||
context,
|
||||
metadata: _episodes.first,
|
||||
isOffline: widget.isOffline,
|
||||
onRefresh: _loadFullMetadata,
|
||||
onRefresh: _refreshWatchState,
|
||||
);
|
||||
} else {
|
||||
await _playFirstEpisode();
|
||||
@@ -76,7 +76,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState {
|
||||
context,
|
||||
metadata: metadata,
|
||||
isOffline: widget.isOffline,
|
||||
onRefresh: _loadFullMetadata,
|
||||
onRefresh: _refreshWatchState,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -85,7 +85,7 @@ extension _MediaDetailActionButtons on _MediaDetailScreenState {
|
||||
final didNavigate = await promptAndPlayVersion(context, metadata);
|
||||
// Same post-playback refresh plain Play gets from
|
||||
// navigateToVideoPlayerWithRefresh; the split segment is online-only.
|
||||
if (didNavigate && mounted) unawaited(_loadFullMetadata());
|
||||
if (didNavigate && mounted) unawaited(_refreshWatchState());
|
||||
}
|
||||
|
||||
final primaryTrailer = _getPrimaryTrailer();
|
||||
|
||||
@@ -2937,7 +2937,7 @@ class _MediaDetailScreenState extends State<MediaDetailScreen>
|
||||
context,
|
||||
metadata: episodeWithServerId,
|
||||
isOffline: widget.isOffline,
|
||||
onRefresh: _loadFullMetadata,
|
||||
onRefresh: _refreshWatchState,
|
||||
);
|
||||
}
|
||||
} catch (e) {
|
||||
@@ -2963,7 +2963,7 @@ class _MediaDetailScreenState extends State<MediaDetailScreen>
|
||||
final launcher = MediaListPlaybackLauncher.forItem(context, metadata);
|
||||
final result = await launcher.launchShuffledShow(metadata: metadata);
|
||||
if (result is PlayQueueSuccess && mounted) {
|
||||
unawaited(_loadFullMetadata());
|
||||
unawaited(_refreshWatchState());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,9 +1,22 @@
|
||||
import 'dart:async';
|
||||
|
||||
import 'package:drift/native.dart';
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:plezy/connection/connection_registry.dart';
|
||||
import 'package:plezy/database/app_database.dart';
|
||||
import 'package:plezy/profiles/active_profile_provider.dart';
|
||||
import 'package:plezy/profiles/plex_home_service.dart';
|
||||
import 'package:plezy/profiles/profile.dart';
|
||||
import 'package:plezy/profiles/profile_connection_registry.dart';
|
||||
import 'package:plezy/profiles/profile_registry.dart';
|
||||
import 'package:plezy/screens/main_screen.dart';
|
||||
import 'package:plezy/services/storage_service.dart';
|
||||
import 'package:plezy/widgets/side_navigation_rail.dart';
|
||||
|
||||
import '../test_helpers/prefs.dart';
|
||||
|
||||
void main() {
|
||||
test('side navigation pushes stable foreground off-screen while temporarily expanded', () {
|
||||
const viewportWidth = 1280.0;
|
||||
@@ -297,4 +310,160 @@ void main() {
|
||||
await tester.pumpAndSettle();
|
||||
expect(left(), closeTo(-SideNavigationRailState.expandedWidth, 0.001));
|
||||
});
|
||||
|
||||
group('runInitialProfilePrompt', () {
|
||||
Future<_PromptFixture> makeFixture() async {
|
||||
resetSharedPreferencesForTest();
|
||||
final db = AppDatabase.forTesting(NativeDatabase.memory());
|
||||
final connections = ConnectionRegistry(db);
|
||||
final profileConnections = ProfileConnectionRegistry(db);
|
||||
final profiles = ProfileRegistry(db);
|
||||
final storage = await StorageService.getInstance();
|
||||
final plexHome = PlexHomeService(
|
||||
connections: connections,
|
||||
profileConnections: profileConnections,
|
||||
storage: storage,
|
||||
plexHomeUserFetcher: (_) async => const [],
|
||||
);
|
||||
final active = _GatedInitializeActiveProfileProvider(
|
||||
registry: profiles,
|
||||
plexHome: plexHome,
|
||||
connections: connections,
|
||||
profileConnections: profileConnections,
|
||||
storage: storage,
|
||||
);
|
||||
addTearDown(() async {
|
||||
await active.resetForTesting();
|
||||
active.dispose();
|
||||
await plexHome.dispose();
|
||||
await db.close();
|
||||
});
|
||||
return _PromptFixture(profiles: profiles, storage: storage, active: active);
|
||||
}
|
||||
|
||||
test('mid-initialize synchronous notify pushes exactly one requireSelection picker', () async {
|
||||
final fixture = await makeFixture();
|
||||
// Two selectable profiles, none active: the prompt must push the picker.
|
||||
await fixture.profiles.upsert(Profile.local(id: 'p1', displayName: 'One', createdAt: DateTime(2026, 1, 1)));
|
||||
await fixture.profiles.upsert(Profile.local(id: 'p2', displayName: 'Two', createdAt: DateTime(2026, 1, 2)));
|
||||
|
||||
var claimed = false;
|
||||
var pushes = 0;
|
||||
final pickerPopped = Completer<void>();
|
||||
final runs = <Future<void>>[];
|
||||
|
||||
Future<void> run() => runInitialProfilePrompt(
|
||||
activeProfile: fixture.active,
|
||||
claimPrompt: () {
|
||||
if (claimed) return false;
|
||||
claimed = true;
|
||||
return true;
|
||||
},
|
||||
releasePrompt: () => claimed = false,
|
||||
isMounted: () => true,
|
||||
isOfflineMode: false,
|
||||
hasConnections: () async => fail('zero-profiles settle branch must not run'),
|
||||
settleSession: () async => fail('zero-profiles settle branch must not run'),
|
||||
pushProfileSelection: () async {
|
||||
pushes++;
|
||||
// The real picker route stays up until the user selects a profile.
|
||||
await pickerPopped.future;
|
||||
},
|
||||
);
|
||||
|
||||
// The late-profiles re-arm: any provider notification landing while the
|
||||
// first prompt is parked on initialize() re-enters the prompt, exactly
|
||||
// like _onActiveProfileChanged does.
|
||||
fixture.active.addListener(() => runs.add(run()));
|
||||
|
||||
// Parks on the gated initialize after its synchronous notify re-entered.
|
||||
runs.add(run());
|
||||
expect(runs, hasLength(2), reason: 'the gated initialize notified synchronously');
|
||||
|
||||
fixture.active.initializeGate.complete();
|
||||
await pumpEventQueue();
|
||||
expect(pushes, 1);
|
||||
|
||||
// A notification landing while the picker route is up (e.g. the
|
||||
// connection watcher) must not stack a second requireSelection picker.
|
||||
fixture.active.emitExternalNotify();
|
||||
await pumpEventQueue();
|
||||
expect(pushes, 1);
|
||||
|
||||
pickerPopped.complete();
|
||||
await Future.wait(runs);
|
||||
expect(pushes, 1);
|
||||
expect(claimed, isFalse, reason: 'claim released after the picker resolves');
|
||||
});
|
||||
|
||||
test('non-push exit releases the double-push claim', () async {
|
||||
final fixture = await makeFixture();
|
||||
// One profile, already active, "require selection on open" off: the
|
||||
// prompt must decline to push and release its claim so a later genuine
|
||||
// prompt is still possible.
|
||||
await fixture.profiles.upsert(Profile.local(id: 'p1', displayName: 'One', createdAt: DateTime(2026, 1, 1)));
|
||||
await fixture.storage.setActiveProfileId('p1');
|
||||
fixture.active.initializeGate.complete();
|
||||
|
||||
var claimed = false;
|
||||
var pushes = 0;
|
||||
|
||||
Future<void> run() => runInitialProfilePrompt(
|
||||
activeProfile: fixture.active,
|
||||
claimPrompt: () {
|
||||
if (claimed) return false;
|
||||
claimed = true;
|
||||
return true;
|
||||
},
|
||||
releasePrompt: () => claimed = false,
|
||||
isMounted: () => true,
|
||||
isOfflineMode: false,
|
||||
hasConnections: () async => fail('zero-profiles settle branch must not run'),
|
||||
settleSession: () async => fail('zero-profiles settle branch must not run'),
|
||||
pushProfileSelection: () async => pushes++,
|
||||
);
|
||||
|
||||
await run();
|
||||
expect(pushes, 0);
|
||||
expect(claimed, isFalse, reason: 'a non-push exit must release the claim');
|
||||
|
||||
// The released claim keeps future prompts reachable.
|
||||
await run();
|
||||
expect(pushes, 0);
|
||||
expect(claimed, isFalse);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
class _PromptFixture {
|
||||
_PromptFixture({required this.profiles, required this.storage, required this.active});
|
||||
|
||||
final ProfileRegistry profiles;
|
||||
final StorageService storage;
|
||||
final _GatedInitializeActiveProfileProvider active;
|
||||
}
|
||||
|
||||
/// [ActiveProfileProvider] whose [initialize] parks on [initializeGate] after
|
||||
/// notifying — the fresh sign-in shape where the provider's connection watcher
|
||||
/// notifies while MainScreen's prompt is still awaiting initialize().
|
||||
class _GatedInitializeActiveProfileProvider extends ActiveProfileProvider {
|
||||
_GatedInitializeActiveProfileProvider({
|
||||
required super.registry,
|
||||
required super.plexHome,
|
||||
required super.connections,
|
||||
required super.profileConnections,
|
||||
required super.storage,
|
||||
});
|
||||
|
||||
final Completer<void> initializeGate = Completer<void>();
|
||||
|
||||
@override
|
||||
Future<void> initialize() async {
|
||||
notifyListeners();
|
||||
await initializeGate.future;
|
||||
await super.initialize();
|
||||
}
|
||||
|
||||
/// A notification from outside the prompt (e.g. a connection table write).
|
||||
void emitExternalNotify() => notifyListeners();
|
||||
}
|
||||
|
||||
@@ -1113,6 +1113,65 @@ void main() {
|
||||
expect(find.text('S1E1'), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('returning from playback refreshes watch state without the full-screen loader', (tester) async {
|
||||
// The post-playback refresh used to re-run _loadFullMetadata, which
|
||||
// raises _isLoadingMetadata — build then swaps the whole detail subtree
|
||||
// for a spinner — and refetches seasons, episodes and extras.
|
||||
final show = buildShow();
|
||||
final season1 = buildSeason(show, 1);
|
||||
final episode1 = buildEpisode(show, season1, 1);
|
||||
final episode2 = buildEpisode(show, season1, 2);
|
||||
|
||||
final client = _FakeMediaServerClient(
|
||||
show: show,
|
||||
childrenByParent: {
|
||||
show.id: [season1],
|
||||
season1.id: [episode1, episode2],
|
||||
},
|
||||
)..onDeckEpisode = episode1;
|
||||
final observer = _RecordingNavigatorObserver(popVideoPlayerImmediately: true);
|
||||
|
||||
await pumpPhoneDetail(tester, client, show, observer: observer);
|
||||
|
||||
expect(find.text('S1E1'), findsOneWidget, reason: 'play button targets the on-deck episode');
|
||||
expect(find.text('Episode S1E1'), findsOneWidget);
|
||||
final childrenCallsBeforePlayback = client.childrenPageCalls.length;
|
||||
observer.pushedRouteNames.clear();
|
||||
|
||||
// The next fetchItemWithOnDeck — the post-playback refresh — reports
|
||||
// the following episode as on deck.
|
||||
client.onDeckEpisode = episode2;
|
||||
|
||||
await tester.tap(
|
||||
find.descendant(of: find.byType(FocusableActionBar), matching: find.byIcon(Symbols.play_arrow_rounded)),
|
||||
);
|
||||
// Pump the push, the immediate pop, and the refresh round-trip one
|
||||
// frame at a time: the old full-reload path swapped in a loading
|
||||
// scaffold here and unmounted the episode list.
|
||||
for (var i = 0; i < 8; i++) {
|
||||
await tester.pump();
|
||||
expect(
|
||||
find.byType(CircularProgressIndicator),
|
||||
findsNothing,
|
||||
reason: 'playback return must not raise the full-screen loader',
|
||||
);
|
||||
expect(
|
||||
find.text('Episode S1E1'),
|
||||
findsOneWidget,
|
||||
reason: 'loaded episode rows must survive the playback return',
|
||||
);
|
||||
}
|
||||
await tester.pump(const Duration(milliseconds: 300));
|
||||
|
||||
expect(observer.pushedRouteNames, contains(kVideoPlayerRouteName));
|
||||
// Watch state did refresh: the play button now targets the next episode.
|
||||
expect(find.text('S1E2'), findsOneWidget);
|
||||
// The lightweight refresh fetches the item + on-deck only — no season
|
||||
// or episode page refetch, no early-paint (both are full-loader work).
|
||||
expect(client.childrenPageCalls.length, childrenCallsBeforePlayback);
|
||||
expect(client.earlyPaints, hasLength(1));
|
||||
});
|
||||
|
||||
testWidgets('shows directors when they are the only additional info', (tester) async {
|
||||
final movie = testMediaItem(
|
||||
id: 'director_only',
|
||||
|
||||
Reference in New Issue
Block a user