From 2ca77747d11bea5fa8896d49c6dfefc8bd5400bc Mon Sep 17 00:00:00 2001 From: klopez4212 Date: Sat, 31 Oct 2026 16:01:15 +0000 Subject: [PATCH] Fix mobile Add Community loading and membership restoration (#8796) ## Summary Open Add Community immediately from the post-login picker and return directly on Back, without membership restoration or repeated discovery during navigation. Completing Add restores the saved selection while keeping the newly added community active; failures retain progress and offer Retry. Follow-up to #8729. Mobile app update only; no relay deployment required. ## Testing `just mobile-install mobile-check mobile-test` passed (4,025 standard tests), plus 4,049 Builderlab-enabled tests on the merged revision. Production-path journeys cover hosted and self-hosted completion, partial restoration and Retry, persisted memberships, and the active destination. Removing the completion hook makes the new journeys fail. Independent agent review found no blockers. Native walkthrough and human verification remain pending. --------- Signed-off-by: kenny lopez Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> --- .../hosted_community_selection.dart | 17 ++- mobile/lib/onboarding_app_entry.dart | 9 +- mobile/lib/shared/hosted/hosted_flow.dart | 100 ++++++++++++++---- .../shared/hosted/hosted_flow_provider.dart | 36 ++++++- .../fullscreen_onboarding_test.dart | 70 ++++-------- .../onboarding/hosted_journeys_test.dart | 67 ++++++++++++ .../helpers/hosted_communities_fixture.dart | 1 + .../hosted/hosted_communities_flow_test.dart | 24 +++++ 8 files changed, 251 insertions(+), 73 deletions(-) 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',