diff --git a/mobile/lib/features/onboarding/hosted_community_selection.dart b/mobile/lib/features/onboarding/hosted_community_selection.dart index 45f3025402..094c038921 100644 --- a/mobile/lib/features/onboarding/hosted_community_selection.dart +++ b/mobile/lib/features/onboarding/hosted_community_selection.dart @@ -28,15 +28,19 @@ class HostedCommunitySelection extends HookConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { useListenable(flow); - Future addCommunity() async { + void addCommunity() { + if (flow.stage != HostedFlowStage.chooseCommunities) return; + final choices = flow.captureCommunityChoices(); + if (choices == null) return; final navigator = Navigator.of(context, rootNavigator: true); - if (!await flow.prepareToAddCommunity() || !navigator.mounted) return; late final MaterialPageRoute route; var exiting = false; Future exit() async { if (exiting) return; exiting = true; - await flow.signInAccount(); + if (!flow.restoreCommunityChoices(choices)) { + await flow.signInAccount(); + } if (flow.stage == HostedFlowStage.signIn || flow.stage == HostedFlowStage.idle) { exiting = false; @@ -50,6 +54,13 @@ class HostedCommunitySelection extends HookConsumerWidget { addingCommunity: true, restoreAccount: false, onExit: exit, + onComplete: () { + if (flow.restoreCommunityChoices(choices)) { + flow.addSelectedCommunities(returnToAddedCommunity: true); + } else { + flow.signInAccount(); + } + }, ), ); navigator.push(route); diff --git a/mobile/lib/onboarding_app_entry.dart b/mobile/lib/onboarding_app_entry.dart index 8051ac835d..a62af38b55 100644 --- a/mobile/lib/onboarding_app_entry.dart +++ b/mobile/lib/onboarding_app_entry.dart @@ -20,11 +20,15 @@ class OnboardingAppEntry extends HookConsumerWidget { this.addingCommunity = false, this.restoreAccount = true, this.onExit, + this.onComplete, }); final bool addingCommunity; final bool restoreAccount; final VoidCallback? onExit; + /// Continues picker restoration before the completed Add route is removed. + final VoidCallback? onComplete; + @override Widget build(BuildContext context, WidgetRef ref) { final existingUser = useState(false); @@ -45,8 +49,11 @@ class OnboardingAppEntry extends HookConsumerWidget { unawaited(restore()); return null; }, [flow]); + final finishing = useRef(false); void finishEntry() { - if (!addingCommunity) return; + if (!addingCommunity || finishing.value) return; + finishing.value = true; + onComplete?.call(); WidgetsBinding.instance.addPostFrameCallback((_) { if (!context.mounted) return; final route = ModalRoute.of(context); diff --git a/mobile/lib/shared/hosted/hosted_flow.dart b/mobile/lib/shared/hosted/hosted_flow.dart index dc183d5ea9..bd484454e8 100644 --- a/mobile/lib/shared/hosted/hosted_flow.dart +++ b/mobile/lib/shared/hosted/hosted_flow.dart @@ -1,4 +1,5 @@ import '../storage/serial_queue.dart'; + import 'dart:async'; import 'package:flutter/foundation.dart'; @@ -69,6 +70,27 @@ typedef HostedCommunityAdmission = bool Function() isCurrent, ); +/// Reopens the completed Add destination after restoring older memberships. +typedef HostedCommunityReturn = + Future Function(bool Function() isCurrent); + +/// Account-scoped picker state retained while the Add route is open. +class HostedCommunityChoices { + HostedCommunityChoices._( + this._subject, + this._communities, + this._selectedRelays, + this._landingRelay, + this._addedRelays, + ); + + final String _subject; + final List _communities; + final Set _selectedRelays; + final String? _landingRelay; + final Set _addedRelays; +} + /// App-scoped owner: callback subscriptions must outlive any individual page. /// Only public progress reaches widgets; pending auth secrets remain in BrowserAuth. class HostedFlow extends ChangeNotifier { @@ -79,6 +101,7 @@ class HostedFlow extends ChangeNotifier { required this.identity, required this.enterCommunity, this.openLandingCommunity, + this.captureAddedCommunity, Future Function(Uri)? openWeb, }) : _openWeb = openWeb ?? @@ -89,6 +112,14 @@ class HostedFlow extends ChangeNotifier { final HostedIdentityService identity; final HostedCommunityAdmission enterCommunity; final HostedCommunityAdmission? openLandingCommunity; + + /// Captures the persisted Add destination before restoring older memberships. + final Future Function()? captureAddedCommunity; + bool _returnToAddedCommunity = false; + HostedCommunityReturn? _openAddedCommunity; + + /// Restoring older memberships must not replace the newly added destination. + bool get restoringAfterAdd => _returnToAddedCommunity; bool get canRetry => problem == null || problem == HostedFlowProblem.retry || @@ -109,7 +140,6 @@ class HostedFlow extends ChangeNotifier { String? landingRelay; final Set _addedRelays = {}; bool _addingCommunities = false; - Completer? _pendingAdd; String? _batchSubject; String? _conflictingRelay; String? _failedRelay; @@ -673,34 +703,56 @@ class HostedFlow extends ChangeNotifier { !communities.any((item) => item.relayUrl == relay)) { return; } + _returnToAddedCommunity = false; + _openAddedCommunity = null; selectedRelays = Set.unmodifiable({...selectedRelays, relay}); landingRelay = relay; notifyListeners(); } - /// Restore existing memberships before presenting a separate Add route. - Future prepareToAddCommunity() { - if (_pendingAdd != null || stage != HostedFlowStage.chooseCommunities) { - return Future.value(false); + /// Capture only a current account's discovered community choices. + HostedCommunityChoices? captureCommunityChoices() { + if (stage != HostedFlowStage.chooseCommunities || + _batchSubject == null || + account.account?.subject != _batchSubject) { + return null; } - if (selectedRelays.isEmpty) return Future.value(true); - final pending = _pendingAdd = Completer(); - unawaited(addSelectedCommunities()); - return pending.future; + return HostedCommunityChoices._( + _batchSubject!, + List.unmodifiable(communities), + Set.unmodifiable(selectedRelays), + landingRelay, + Set.unmodifiable(_addedRelays), + ); } - void _finishPreparingAdd(bool ready) { - final pending = _pendingAdd; - _pendingAdd = null; - pending?.complete(ready); + /// Return from Add without repeating account discovery or relay admission. + /// A changed or expired account must go through sign-in again. + bool restoreCommunityChoices(HostedCommunityChoices choices) { + if (_disposed || account.account?.subject != choices._subject) return false; + _nextGeneration(); + _target = null; + _pairedPubkey = null; + pairedSensitiveActionPolicy = null; + reconnecting = false; + accountOnly = true; + _clearCommunityBatch(); + _batchSubject = choices._subject; + communities = choices._communities; + selectedRelays = choices._selectedRelays; + landingRelay = choices._landingRelay; + _addedRelays.addAll(choices._addedRelays); + _set(HostedFlowStage.chooseCommunities); + return true; } - Future addSelectedCommunities() { + Future addSelectedCommunities({bool returnToAddedCommunity = false}) { if (!accountOnly || stage != HostedFlowStage.chooseCommunities || selectedRelays.isEmpty) { return Future.value(); } + _returnToAddedCommunity = returnToAddedCommunity || _returnToAddedCommunity; _addingCommunities = true; final generation = _nextGeneration(); _set(HostedFlowStage.working); @@ -710,7 +762,6 @@ class HostedFlow extends ChangeNotifier { /// Keep completed additions while allowing a different landing choice. void backToCommunityChoices() { if (!canChangeCommunities) return; - _finishPreparingAdd(false); _nextGeneration(); _conflictingRelay = null; addingCommunityName = null; @@ -767,6 +818,13 @@ class HostedFlow extends ChangeNotifier { _set(HostedFlowStage.chooseCommunities); return; } + if (_returnToAddedCommunity && _openAddedCommunity == null) { + final capture = captureAddedCommunity; + if (capture == null) throw AccountFailure.unavailable; + final open = await capture(); + if (!current()) return; + _openAddedCommunity = open; + } _failedRelay = null; final ordered = [ ...communities.where((item) => item.relayUrl == landingRelay), @@ -796,9 +854,11 @@ class HostedFlow extends ChangeNotifier { } if (!current()) return; addingCommunityName = null; - if (_pendingAdd != null) { - _set(HostedFlowStage.chooseCommunities); - _finishPreparingAdd(true); + if (_openAddedCommunity != null) { + await _openAddedCommunity!(current); + if (!current()) return; + await browser.clearIntent(); + if (current()) _set(HostedFlowStage.complete); return; } if (_addedRelays.isEmpty) { @@ -922,7 +982,8 @@ class HostedFlow extends ChangeNotifier { bool noCommunitiesToAdd = false; void _clearCommunityBatch() { - _finishPreparingAdd(false); + _returnToAddedCommunity = false; + _openAddedCommunity = null; noCommunitiesToAdd = false; communities = const []; selectedRelays = const {}; @@ -999,7 +1060,6 @@ class HostedFlow extends ChangeNotifier { @override void dispose() { _disposed = true; - _finishPreparingAdd(false); _cancelPolicy(); _cancelProfile(); _nextGeneration(); diff --git a/mobile/lib/shared/hosted/hosted_flow_provider.dart b/mobile/lib/shared/hosted/hosted_flow_provider.dart index d5c5e1994d..d91f622325 100644 --- a/mobile/lib/shared/hosted/hosted_flow_provider.dart +++ b/mobile/lib/shared/hosted/hosted_flow_provider.dart @@ -68,6 +68,36 @@ HostedFlow createHostedFlow( browser: browser, store: store, identity: HostedIdentityService(account: account, store: store), + captureAddedCommunity: () async { + final storage = ref.read(communityStorageProvider); + final activeId = await storage.loadActiveId(); + final saved = await storage.loadAll(); + final destination = saved + .where((item) => item.id == activeId) + .firstOrNull; + if (destination == null) throw AccountFailure.unavailable; + return (isCurrent) async { + if (!isCurrent()) return; + final active = await storage.loadActiveId(); + if (!isCurrent() || active == destination.id) return; + final saved = await storage.loadAll(); + final currentDestination = saved + .where( + (item) => + item.id == destination.id && + item.pubkey == destination.pubkey, + ) + .firstOrNull; + if (!isCurrent()) return; + if (currentDestination == null) throw AccountFailure.unavailable; + await ref + .read(authProvider.notifier) + .authenticateWithCommunity( + currentDestination, + shouldContinue: isCurrent, + ); + }; + }, openLandingCommunity: (link, subject, pubkey, isCurrent) async { bool current() => isCurrent() && account.account?.subject == subject; final storage = ref.read(communityStorageProvider); @@ -106,12 +136,14 @@ HostedFlow createHostedFlow( shouldContinue: current, activate: community.starterSetupIncomplete || - flow.addedCommunityCount == 0, + (flow.addedCommunityCount == 0 && !flow.restoringAfterAdd), ), recoverStarterChannels: (community, current) => finishHostedStarterSetup(ref, community, current), ); - if (flow.addedCommunityCount == 0 && isCurrent()) { + if (flow.addedCommunityCount == 0 && + !flow.restoringAfterAdd && + isCurrent()) { await offerHostedProfile( ref, flow, diff --git a/mobile/test/features/onboarding/fullscreen_onboarding_test.dart b/mobile/test/features/onboarding/fullscreen_onboarding_test.dart index d014fc7cd5..43a98f67eb 100644 --- a/mobile/test/features/onboarding/fullscreen_onboarding_test.dart +++ b/mobile/test/features/onboarding/fullscreen_onboarding_test.dart @@ -1,6 +1,5 @@ import '../../helpers/two_finger_double_tap.dart'; -import 'package:buzz/shared/hosted/builderlab_account.dart'; -import 'package:buzz/shared/hosted/hosted_community_connection.dart'; + import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:buzz/shared/theme/theme.dart'; import 'package:buzz/features/pairing/add_community_page.dart'; @@ -8,7 +7,9 @@ import 'package:flutter/cupertino.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:buzz/shared/community/community_grid_option.dart'; + import 'dart:async'; + import 'package:buzz/onboarding_app_entry.dart'; import 'package:buzz/hosted_app_gate.dart'; @@ -156,64 +157,39 @@ void main() { expect(find.text('Select a community'), findsOneWidget); expect(find.byType(BottomSheet), findsNothing); expect(find.byTooltip('Back'), findsOneWidget); - // Cancelling restore must not open Add after the pending request returns. - final cancelledRestore = Completer(); - fixture.beforeAdmission = (_) => cancelledRestore.future; + // Opening Add must not restore memberships or show the joining loader, + // even when admission would fail. Back still returns to the picker. + var admissionAttempts = 0; + fixture.beforeAdmission = (_) async { + admissionAttempts++; + throw StateError('Opening Add must not admit existing communities'); + }; await tester.ensureVisible( find.byKey(const Key('community-switcher-add')), ); await tester.tap(find.byKey(const Key('community-switcher-add'))); await tester.pump(); - await tester.pump(const Duration(milliseconds: 300)); - expect(find.byTooltip('Back').hitTestable(), findsNothing); - expect(find.byType(BeeLoadingMark), findsOneWidget); - await tester.tap(find.text('Cancel')); - await tester.pump(); - expect(fixture.flow.stage, HostedFlowStage.chooseCommunities); - cancelledRestore.complete(); + expect(find.text('Joining your community'), findsNothing); + expect(find.byType(BeeLoadingMark), findsNothing); await tester.pumpAndSettle(); - expect(find.byType(OnboardingAppEntry), findsNothing); - expect(find.text('Select a community'), findsOneWidget); - for (final conflict in [false, true]) { - fixture.beforeAdmission = (name) async { - if (name == 'two.communities.buzz.xyz') { - if (conflict) throw const SavedCommunityIdentityConflict(); - throw AccountFailure.unavailable; - } - }; - await tester.tap(find.byKey(const Key('community-switcher-add'))); - await tester.pumpAndSettle(); - expect(find.byType(OnboardingAppEntry), findsNothing); - await tester.tap( - find.text( - conflict ? 'Keep saved identity and skip' : 'Skip this community', - ), - ); - await tester.pumpAndSettle(); - expect(find.byType(OnboardingAppEntry), findsOneWidget); - expect(fixture.openedCommunity, isNull); - await tester.pageBack(); - await tester.pumpAndSettle(); - expect(find.text('Select a community'), findsOneWidget); - fixture.admitted.clear(); - } - fixture.beforeAdmission = null; - // Add restores existing memberships, stays opaque, and returns to the - // picker on Back without the disposed Add route cancelling discovery. - await tester.ensureVisible( - find.byKey(const Key('community-switcher-add')), - ); - await tester.tap(find.byKey(const Key('community-switcher-add'))); - await tester.pumpAndSettle(); - expect(fixture.admitted, hasLength(2)); + expect(fixture.admitted, isEmpty); expect(find.byType(OnboardingAppEntry), findsOneWidget); + expect(find.text('Add a community'), findsOneWidget); + expect(admissionAttempts, 0); expect(find.text('Home content').hitTestable(), findsNothing); + expect(fixture.openedCommunity, isNull); + final requestsBeforeBack = List.of(fixture.requests); await tester.pageBack(); + await tester.pump(); + expect(find.text('Getting ready…'), findsNothing); + expect(find.text('Joining your community'), findsNothing); + expect(find.byType(BeeLoadingMark), findsNothing); await tester.pumpAndSettle(); + expect(fixture.requests, requestsBeforeBack); expect(find.byType(OnboardingAppEntry), findsNothing); expect(find.text('Select a community'), findsOneWidget); expect(fixture.flow.stage, HostedFlowStage.chooseCommunities); - fixture.admitted.clear(); + expect(fixture.admitted, isEmpty); final admission = Completer(); fixture.beforeAdmission = (_) => admission.future; diff --git a/mobile/test/features/onboarding/hosted_journeys_test.dart b/mobile/test/features/onboarding/hosted_journeys_test.dart index 0d57deae77..22dcada474 100644 --- a/mobile/test/features/onboarding/hosted_journeys_test.dart +++ b/mobile/test/features/onboarding/hosted_journeys_test.dart @@ -64,6 +64,73 @@ void main() { ); } + for (final selfHosted in [false, true]) { + journey( + 'finishing ${selfHosted ? "self-hosted" : "hosted"} Add restores picker memberships and keeps the new destination', + (tester, f) async { + await f.initialize(); + f.offerProfile = false; + f.rows.addAll([f.row('one'), f.row('two')]); + await f.mount(tester, openEntry: false); + await f.flow.signInAccount(); + await f.settle(tester); + expect(find.text('Select a community'), findsOneWidget); + await tester.tap(find.byKey(const Key('community-switcher-add'))); + await f.settle(tester); + expect(find.text('Add a community'), findsOneWidget); + expect(f.probes, isEmpty); + final discoveries = f.sent('/known-communities/list').length; + f.secondRelayFailures = 1; + if (selfHosted) { + await f.tap(tester, 'Enter a community link'); + await tester.enterText( + find.byKey(const Key('community-link-input')).hitTestable(), + 'my-community.example', + ); + await f.tap(tester, 'Continue'); + await f.tap(tester, 'Saved identity'); + } else { + await f.link(tester, 'three'); + await tester.tap(find.byType(CheckboxListTile)); + await f.settle(tester); + await f.tap(tester, 'Accept and join'); + } + expect(f.flow.stage, HostedFlowStage.blocked); + expect(find.text('Try again').hitTestable(), findsOneWidget); + expect(find.text('Community home').hitTestable(), findsNothing); + final newRelay = selfHosted + ? 'wss://my-community.example' + : 'wss://three.communities.buzz.xyz'; + final partial = await f.storage.loadAll(); + expect( + partial.map((c) => c.relayUrl), + containsAll([newRelay, 'wss://one.communities.buzz.xyz']), + ); + final added = partial.singleWhere((c) => c.relayUrl == newRelay); + expect(await f.storage.loadActiveId(), added.id); + await f.tap(tester, 'Try again'); + expect(f.flow.stage, HostedFlowStage.complete); + expect( + (await f.storage.loadAll()).map((c) => c.relayUrl), + unorderedEquals([ + newRelay, + 'wss://one.communities.buzz.xyz', + 'wss://two.communities.buzz.xyz', + ]), + ); + expect(await f.storage.loadActiveId(), added.id); + expect(f.probes.where((c) => c.relayUrl == newRelay), hasLength(1)); + expect( + f.probes.where((c) => c.relayUrl == 'wss://one.communities.buzz.xyz'), + hasLength(1), + ); + expect(f.sent('/known-communities/list'), hasLength(discoveries)); + expect(find.byType(OnboardingAppEntry), findsNothing); + expect(find.text('Community home').hitTestable(), findsOneWidget); + }, + ); + } + for (final importing in [false, true]) { journey( 'new account ${importing ? 'imports' : 'creates'} and keeps its identity on retry', diff --git a/mobile/test/helpers/hosted_communities_fixture.dart b/mobile/test/helpers/hosted_communities_fixture.dart index fce9a2bfbf..fe6caf7942 100644 --- a/mobile/test/helpers/hosted_communities_fixture.dart +++ b/mobile/test/helpers/hosted_communities_fixture.dart @@ -62,6 +62,7 @@ class WidgetDiscoveryAccount extends BuilderlabAccountClient { Map body, { String? expectedSubject, }) async { + fixture.requests.add(path); if (path.endsWith('/current')) { if (fixture.rejectCurrent) throw AccountFailure.denied; return { diff --git a/mobile/test/shared/hosted/hosted_communities_flow_test.dart b/mobile/test/shared/hosted/hosted_communities_flow_test.dart index b7952ec129..1967b4668e 100644 --- a/mobile/test/shared/hosted/hosted_communities_flow_test.dart +++ b/mobile/test/shared/hosted/hosted_communities_flow_test.dart @@ -20,6 +20,30 @@ void main() { return result; } + for (final changedAccount in [false, true]) { + test( + 'picker snapshot rejects ${changedAccount ? "changed" : "expired"} account', + () async { + final f = HostedCommunitiesFixture(); + await f.initialize(widget: true); + addTearDown(f.dispose); + f.rows = [f.row('one')]; + await f.flow.signInAccount(); + final choices = f.flow.captureCommunityChoices()!; + await f.flow.leave(); + if (changedAccount) { + f.subject = 'other-owner'; + } else { + f.expired = true; + } + expect(f.flow.restoreCommunityChoices(choices), isFalse); + expect(f.flow.stage, HostedFlowStage.idle); + expect(f.flow.communities, isEmpty); + expect(f.admitted, isEmpty); + }, + ); + } + for (final failure in ['binding', 'backup']) { test( '$failure proof failure blocks discovery and preserves saved identity',