diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index 6b5a497928..fed17bc434 100644 --- a/lib/widgets/home.dart +++ b/lib/widgets/home.dart @@ -151,6 +151,11 @@ const kTryAnotherAccountWaitPeriod = Duration(seconds: 5); class _LoadingPlaceholderPage extends StatefulWidget { const _LoadingPlaceholderPage({required this.accountId}); + /// The relevant account for this page. + /// + /// The account is not guaranteed to exist in the global store. This can + /// happen briefly when the account is removed from the database for logout, + /// but before [PerAccountStoreWidget.routeToRemoveOnLogout] is processed. final int accountId; @override @@ -180,9 +185,15 @@ class _LoadingPlaceholderPageState extends State<_LoadingPlaceholderPage> { @override Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); - final realmUrl = GlobalStoreWidget.of(context) - // TODO(#1219) `!` is incorrect - .getAccount(widget.accountId)!.realmUrl; + final account = GlobalStoreWidget.of(context).getAccount(widget.accountId); + + if (account == null) { + // We should only reach this state very briefly. + // See [_LoadingPlaceholderPage.accountId]. + return Scaffold( + appBar: AppBar(), + body: const CircularProgressIndicator()); + } return Scaffold( appBar: AppBar(), @@ -201,7 +212,7 @@ class _LoadingPlaceholderPageState extends State<_LoadingPlaceholderPage> { child: Column( children: [ const SizedBox(height: 16), - Text(zulipLocalizations.tryAnotherAccountMessage(realmUrl.toString())), + Text(zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString())), const SizedBox(height: 8), ElevatedButton( onPressed: () => Navigator.push(context, diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 32d8254d6d..400df9d81a 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -92,6 +92,22 @@ RemoveFcmMessage removeFcmMessage(List zulipMessages, {Account? account }) as RemoveFcmMessage; } +Future openNotification(WidgetTester tester, Account account, Message message) async { + final data = messageFcmMessage(message, account: account); + final intentDataUrl = NotificationOpenPayload( + realmUrl: data.realmUrl, + userId: data.userId, + narrow: switch (data.recipient) { + FcmMessageChannelRecipient(:var streamId, :var topic) => + TopicNarrow(streamId, topic), + FcmMessageDmRecipient(:var allRecipientIds) => + DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), + }).buildUrl(); + unawaited( + WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); + await tester.idle(); // let navigateForNotification find navigator +} + void main() { TestZulipBinding.ensureInitialized(); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; @@ -991,21 +1007,6 @@ void main() { check(pushedRoutes).isEmpty(); } - Future openNotification(WidgetTester tester, Account account, Message message) async { - final data = messageFcmMessage(message, account: account); - final intentDataUrl = NotificationOpenPayload( - realmUrl: data.realmUrl, - userId: data.userId, - narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); - unawaited( - WidgetsBinding.instance.handlePushRoute(intentDataUrl.toString())); - await tester.idle(); // let navigateForNotification find navigator - } void matchesNavigation(Subject> route, Account account, Message message) { route.isA() diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index 4a4698a175..b776ec67fc 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -1,4 +1,3 @@ -import 'dart:async'; import 'dart:convert'; import 'package:checks/checks.dart'; @@ -18,7 +17,8 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/notifications/receive.dart'; import 'package:zulip/widgets/actions.dart'; import 'package:zulip/widgets/app.dart'; -import 'package:zulip/widgets/inbox.dart'; +import 'package:zulip/widgets/home.dart'; +import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import '../api/fake_api.dart'; @@ -27,9 +27,11 @@ import '../model/binding.dart'; import '../model/store_checks.dart'; import '../model/test_store.dart'; import '../model/unreads_checks.dart'; +import '../notifications/display_test.dart'; import '../stdlib_checks.dart'; import '../test_navigation.dart'; import 'dialog_checks.dart'; +import 'page_checks.dart'; import 'test_app.dart'; void main() { @@ -156,68 +158,67 @@ void main() { }); testWidgets("logged-out account's routes removed from nav; other accounts' remain", (tester) async { - Future makeUnreadTopicInInbox(int accountId, String topic) async { + Future prepareStoreWithMessage(int accountId, User user, String topic) async { final stream = eg.stream(); final message = eg.streamMessage(stream: stream, topic: topic); final store = await testBinding.globalStore.perAccount(accountId); await store.addStream(stream); await store.addSubscription(eg.subscription(stream)); await store.addMessage(message); + await store.addUser(user); await tester.pump(); + return message; } addTearDown(testBinding.reset); - final account1 = eg.account(id: 1, user: eg.user()); - final account2 = eg.account(id: 2, user: eg.user()); + final account1 = eg.selfAccount; + final account2 = eg.otherAccount; await testBinding.globalStore.add(account1, eg.initialSnapshot()); await testBinding.globalStore.add(account2, eg.initialSnapshot()); final testNavObserver = TestNavigatorObserver(); - await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); - await tester.pump(); - final navigator = await ZulipApp.navigator; - navigator.popUntil((_) => false); // clear starting routes - await tester.pumpAndSettle(); - - final pushedRoutes = >[]; + final pushedRoutes = >[]; testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); - // TODO(#737): switch to a realistic setup: - // https://github.com/zulip/zulip-flutter/pull/1076#discussion_r1874124363 - final account1Route = MaterialAccountWidgetRoute( - accountId: account1.id, page: const InboxPageBody()); - final account2Route = MaterialAccountWidgetRoute( - accountId: account2.id, page: const InboxPageBody()); - unawaited(navigator.push(account1Route)); - unawaited(navigator.push(account2Route)); - await tester.pumpAndSettle(); - check(pushedRoutes).deepEquals([account1Route, account2Route]); - - await makeUnreadTopicInInbox(account1.id, 'topic in account1'); - final findAccount1PageContent = find.text('topic in account1', skipOffstage: false); - await makeUnreadTopicInInbox(account2.id, 'topic in account2'); - final findAccount2PageContent = find.text('topic in account2', skipOffstage: false); + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + await tester.pump(); + final account1Route = pushedRoutes.single; + check(account1Route).isA().page.isA(); + + final account2Connection = + (await testBinding.globalStore.perAccount(account2.id)).connection as FakeApiConnection; + account2Connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: []).toJson()); + final message = await prepareStoreWithMessage( + account2.id, eg.otherUser, 'topic'); + await openNotification(tester, account2, message); + await tester.pump(); - final findLoadingPage = find.byType(LoadingPlaceholderPage, skipOffstage: false); + final findAccount1PageContent = find.byType(HomePage, skipOffstage: false); + final findAccount2PageContent = find.byType(MessageListPage, skipOffstage: false); + check(pushedRoutes).deepEquals([ + account1Route, + (Subject it) => it.isA() + ..accountId.equals(eg.otherAccount.id) + ..page.isA(), + ]); check(findAccount1PageContent).findsOne(); - check(findLoadingPage).findsNothing(); + check(findAccount2PageContent).findsOne(); - final removedRoutes = >[]; + final removedRoutes = >[]; testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route); final context = tester.element(find.byType(MaterialApp)); final future = logOutAccount(context, account1.id); await tester.pump(TestGlobalStore.removeAccountDuration); await future; + await tester.pumpAndSettle(); // wait for animations, if any check(removedRoutes).single.identicalTo(account1Route); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(account1.id); check(findAccount1PageContent).findsNothing(); - check(findLoadingPage).findsOne(); - - await tester.pump(); - check(findAccount1PageContent).findsNothing(); - check(findLoadingPage).findsNothing(); check(findAccount2PageContent).findsOne(); }); }); diff --git a/test/widgets/home_test.dart b/test/widgets/home_test.dart index 2939358bb8..b1bda73f90 100644 --- a/test/widgets/home_test.dart +++ b/test/widgets/home_test.dart @@ -6,6 +6,7 @@ import 'package:zulip/api/model/events.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/about_zulip.dart'; +import 'package:zulip/widgets/actions.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/app_bar.dart'; import 'package:zulip/widgets/home.dart'; @@ -21,6 +22,7 @@ import '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; +import '../model/store_checks.dart'; import '../model/test_store.dart'; import '../test_navigation.dart'; import 'message_list_checks.dart'; @@ -441,5 +443,23 @@ void main () { // No additional wait for loadPerAccount. checkOnHomePage(tester, expectedAccount: eg.selfAccount); }); + + testWidgets('logging out', (tester) async { + // Regression test for: https://github.com/zulip/zulip-flutter/issues/1219 + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await tester.pumpWidget(const ZulipApp()); + await tester.pump(); // wait for the loading page + await tester.pump(); // wait for store + + final element = tester.element(find.byType(HomePage)); + final future = logOutAccount(element, eg.selfAccount.id); + await tester.pump(TestGlobalStore.removeAccountDuration); + await future; + // No error expected from [_LoadingPlaceholderPage] briefly not having + // access to the account being logged out. + checkOnLoadingPage(); + check(testBinding.globalStore).accountIds.isEmpty(); + }); }); }