fixup! nav: Always clear stack before navigating to HomePage

Note to self: be sure to drop this commit message when squashing.

Signed-off-by: Zixuan James Li <[email protected]>

lib/widgets/app.dart

@@ -266,8 +266,7 @@ class ChooseAccountPage extends StatelessWidget {
         // The default trailing padding with M3 is 24px. Decrease by 12 because
         // IconButton (the "…" button) comes with 12px padding on all sides.
         contentPadding: const EdgeInsetsDirectional.only(start: 16, end: 12),
-        onTap: () => Navigator.push(context,
-          HomePage.buildRoute(accountId: accountId))));
+        onTap: () => HomePage.navigate(context, accountId: accountId)));
   }
 
   @override

lib/widgets/home.dart

@@ -36,6 +36,14 @@ class HomePage extends StatefulWidget {
       page: const HomePage());
   }
 
+  /// Navigate to [HomePage], ensuring that its route is at the root level.
+  static void navigate(BuildContext context, {required int accountId}) {
+    final navigator = Navigator.of(context);
+    navigator.popUntil((route) => route.isFirst);
+    unawaited(navigator.pushReplacement(
+      HomePage.buildRoute(accountId: accountId)));
+  }
+
   @override
   State<HomePage> createState() => _HomePageState();
 }

lib/widgets/login.dart

@@ -395,10 +395,7 @@ class _LoginPageState extends State<LoginPage> {
       return;
     }
 
-    unawaited(Navigator.of(context).pushAndRemoveUntil(
-      HomePage.buildRoute(accountId: accountId),
-      (route) => (route is! _LoginSequenceRoute)),
-    );
+    HomePage.navigate(context, accountId: accountId);
   }
 
   Future<int> _getUserId(String email, String apiKey) async {

test/flutter_checks.dart

@@ -44,6 +44,7 @@ extension IconChecks on Subject<Icon> {
 }
 
 extension RouteChecks<T> on Subject<Route<T>> {
+  Subject<bool> get isFirst => has((r) => r.isFirst, 'isFirst');
   Subject<RouteSettings> get settings => has((r) => r.settings, 'settings');
 }
 

test/widgets/app_test.dart

@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:checks/checks.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -157,6 +159,45 @@ void main() {
         ..bottom.isLessThan(2 / 3 * screenHeight);
     });
 
+    testWidgets('choosing an account clears the navigator stack', (tester) async {
+      addTearDown(testBinding.reset);
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+      await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot());
+
+      final pushedRoutes = <Route<dynamic>>[];
+      final poppedRoutes = <Route<dynamic>>[];
+      final testNavObserver = TestNavigatorObserver();
+      testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route);
+      testNavObserver.onPopped = (route, prevRoute) => poppedRoutes.add(route);
+      testNavObserver.onReplaced = (route, prevRoute) {
+        poppedRoutes.add(prevRoute!);
+        pushedRoutes.add(route!);
+      };
+      await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
+      await tester.pump();
+
+      final navigator = await ZulipApp.navigator;
+      unawaited(navigator.push(
+        MaterialWidgetRoute(page: const ChooseAccountPage())));
+      await tester.pumpAndSettle();
+
+      check(poppedRoutes).isEmpty();
+      check(pushedRoutes).deepEquals(<Condition<Object?>>[
+        (it) => it.isA<MaterialAccountWidgetRoute>()
+          ..accountId.equals(eg.selfAccount.id)
+          ..page.isA<HomePage>(),
+        (it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>()
+      ]);
+      pushedRoutes.clear();
+
+      await tester.tap(find.text(eg.otherAccount.email));
+      await tester.pump();
+      check(poppedRoutes).length.equals(2);
+      check(pushedRoutes).single.isA<MaterialAccountWidgetRoute>()
+        ..accountId.equals(eg.otherAccount.id)
+        ..page.isA<HomePage>();
+    });
+
     group('log out', () {
       Future<(Widget, Widget)> prepare(WidgetTester tester, {required Account account}) async {
         await setupChooseAccountPage(tester, accounts: [account]);

test/widgets/home_test.dart

@@ -9,6 +9,7 @@ import 'package:zulip/widgets/app_bar.dart';
 import 'package:zulip/widgets/home.dart';
 import 'package:zulip/widgets/icons.dart';
 import 'package:zulip/widgets/inbox.dart';
+import 'package:zulip/widgets/page.dart';
 import 'package:zulip/widgets/profile.dart';
 import 'package:zulip/widgets/subscription_list.dart';
 import 'package:zulip/widgets/theme.dart';
@@ -18,6 +19,7 @@ import '../example_data.dart' as eg;
 import '../flutter_checks.dart';
 import '../model/binding.dart';
 import '../model/test_store.dart';
+import 'page_checks.dart';
 import 'test_app.dart';
 
 void main () {
@@ -213,4 +215,171 @@ void main () {
       check(find.text(eg.selfUser.fullName)).findsAny();
     });
   });
+
+  group('_LoadingPlaceholderPage', () {
+    void checkOnLoadingPage() {
+      check(find.byType(CircularProgressIndicator).hitTestable()).findsOne();
+      check(find.byType(ChooseAccountPage)).findsNothing();
+      check(find.byType(HomePage)).findsNothing();
+    }
+
+    void checkOnChooseAccountPage() {
+      // Ignore the possible loading page in the background.
+      check(find.byType(CircularProgressIndicator).hitTestable()).findsNothing();
+      check(find.byType(ChooseAccountPage)).findsOne();
+      check(find.byType(HomePage)).findsNothing();
+    }
+
+    ModalRoute<dynamic>? getRouteOf(WidgetTester tester, Finder elementFinder) =>
+      ModalRoute.of(tester.element(elementFinder));
+
+    void checkOnHomePage(WidgetTester tester, {required Account expectedAccount}) {
+      check(find.byType(CircularProgressIndicator)).findsNothing();
+      check(find.byType(ChooseAccountPage)).findsNothing();
+      check(find.byType(HomePage).hitTestable()).findsOne();
+      check(getRouteOf(tester, find.byType(HomePage)))
+        .isA<MaterialAccountWidgetRoute>().accountId.equals(expectedAccount.id);
+    }
+
+    Future<void> prepare(WidgetTester tester) async {
+      addTearDown(testBinding.reset);
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+      await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot());
+      await tester.pumpWidget(const ZulipApp());
+      await tester.pump(Duration.zero); // wait for the loading page
+      checkOnLoadingPage();
+    }
+
+    Future<void> tapChooseAccount(WidgetTester tester) async {
+      await tester.tap(find.text('Try another account'));
+      await tester.pump(Duration.zero); // tap the button
+      await tester.pump(const Duration(milliseconds: 250)); // wait for animation
+      checkOnChooseAccountPage();
+    }
+
+    Future<void> chooseAccountWithEmail(WidgetTester tester, String email) async {
+      await tester.tap(find.text(email));
+      await tester.pump(Duration.zero); // tap the button
+      await tester.pump(const Duration(milliseconds: 350)); // wait for push & pop animations
+      checkOnLoadingPage();
+    }
+
+    testWidgets('smoke', (tester) async {
+      addTearDown(testBinding.reset);
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1);
+      await prepare(tester);
+      await tester.pump(const Duration(seconds: 1));
+      checkOnHomePage(tester, expectedAccount: eg.selfAccount);
+    });
+
+    testWidgets('while loading, go back from ChooseAccountPage', (tester) async {
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1);
+      await prepare(tester);
+      await tapChooseAccount(tester);
+
+      await tester.tap(find.byType(BackButton));
+      await tester.pump(Duration.zero); // tap the button
+      await tester.pump(const Duration(milliseconds: 350)); // wait for pop animation
+      checkOnLoadingPage();
+
+      await tester.pump(const Duration(seconds: 1));
+      checkOnHomePage(tester, expectedAccount: eg.selfAccount);
+    });
+
+    testWidgets('while loading, choose a different account', (tester) async {
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1);
+      await prepare(tester);
+      await tapChooseAccount(tester);
+
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 2);
+      await chooseAccountWithEmail(tester, eg.otherAccount.email);
+
+      // The second loadAccount is still pending.
+      await tester.pump(const Duration(seconds: 1));
+      checkOnLoadingPage();
+
+      // The second loadAccount finished.
+      await tester.pump(const Duration(seconds: 1));
+      checkOnHomePage(tester, expectedAccount: eg.otherAccount);
+    });
+
+    testWidgets('while loading, choosing an account disallow going back', (tester) async {
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1);
+      await prepare(tester);
+      await tapChooseAccount(tester);
+
+      // While still loading, choose a different account.
+      await chooseAccountWithEmail(tester, eg.otherAccount.email);
+
+      // User cannot go back because the navigator stack
+      // was cleared after choosing an account.
+      check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
+        .isNotNull().isFirst.isTrue();
+
+      await tester.pump(const Duration(seconds: 1)); // wait for loadAccount
+      checkOnHomePage(tester, expectedAccount: eg.otherAccount);
+    });
+
+    testWidgets('while loading, go to nested levels of ChooseAccountPage', (tester) async {
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 5);
+      final thirdAccount = eg.account(user: eg.thirdUser);
+      await testBinding.globalStore.add(thirdAccount, eg.initialSnapshot());
+      await prepare(tester);
+
+      // While still loading the first account, choose a different account.
+      await tapChooseAccount(tester);
+      await chooseAccountWithEmail(tester, eg.otherAccount.email);
+      // User cannot go back because the navigator stack
+      // was cleared after choosing an account.
+      check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
+        .isA<MaterialAccountWidgetRoute>()
+          ..isFirst.isTrue()
+          ..accountId.equals(eg.otherAccount.id);
+
+      // While still loading the second account, choose a different account.
+      await tapChooseAccount(tester);
+      await chooseAccountWithEmail(tester, thirdAccount.email);
+      // User cannot go back because the navigator stack
+      // was cleared after choosing an account.
+      check(getRouteOf(tester, find.byType(CircularProgressIndicator)))
+        .isA<MaterialAccountWidgetRoute>()
+          ..isFirst.isTrue()
+          ..accountId.equals(thirdAccount.id);
+
+      await tester.pump(const Duration(seconds: 5)); // wait for loadAccount
+      checkOnHomePage(tester, expectedAccount: thirdAccount);
+    });
+
+    testWidgets('after finishing loading, go back from ChooseAccountPage', (tester) async {
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 1);
+      await prepare(tester);
+      await tapChooseAccount(tester);
+
+      // Stall while on ChoooseAccountPage so that the account finished loading.
+      await tester.pump(const Duration(seconds: 1));
+      checkOnChooseAccountPage();
+
+      await tester.tap(find.byType(BackButton));
+      await tester.pump(Duration.zero); // tap the button
+      await tester.pump(const Duration(milliseconds: 350)); // wait for pop animation
+      checkOnHomePage(tester, expectedAccount: eg.selfAccount);
+    });
+
+    testWidgets('after finishing loading, choose the loaded account', (tester) async {
+      testBinding.globalStore.loadPerAccountDuration = const Duration(seconds: 5);
+      await prepare(tester);
+      await tapChooseAccount(tester);
+
+      // Stall while on ChoooseAccountPage so that the account finished loading.
+      await tester.pump(const Duration(seconds: 5));
+      checkOnChooseAccountPage();
+
+      // Choosing the already loaded account should result in no loading page.
+      await tester.tap(find.text(eg.selfAccount.email));
+      await tester.pump(Duration.zero); // tap the button
+      await tester.pump(const Duration(milliseconds: 350)); // wait for push & pop animations
+      // No additional wait for loadAccount.
+      checkOnHomePage(tester, expectedAccount: eg.selfAccount);
+    });
+  });
 }

test/widgets/login_test.dart

@@ -70,6 +70,7 @@ void main() {
   group('LoginPage', () {
     late FakeApiConnection connection;
     late List<Route<dynamic>> pushedRoutes;
+    late List<Route<dynamic>> poppedRoutes;
 
     void takeStartingRoutes() {
       final expected = <Condition<Object?>>[
@@ -89,8 +90,14 @@ void main() {
         zulipFeatureLevel: serverSettings.zulipFeatureLevel);
 
       pushedRoutes = [];
-      final testNavObserver = TestNavigatorObserver()
-        ..onPushed = (route, prevRoute) => pushedRoutes.add(route);
+      poppedRoutes = [];
+      final testNavObserver = TestNavigatorObserver();
+      testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route);
+      testNavObserver.onPopped = (route, prevRoute) => poppedRoutes.add(route);
+      testNavObserver.onReplaced = (route, prevRoute) {
+        poppedRoutes.add(prevRoute!);
+        pushedRoutes.add(route!);
+      };
       await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
       await tester.pump();
       final navigator = await ZulipApp.navigator;
@@ -144,6 +151,25 @@ void main() {
             id: testBinding.globalStore.accounts.single.id));
       });
 
+      testWidgets('logging into a different account', (tester) async {
+        await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+        final serverSettings = eg.serverSettings();
+        await prepare(tester, serverSettings);
+        check(poppedRoutes).isEmpty();
+        check(pushedRoutes).deepEquals(<Condition<Object?>>[
+          (it) => it.isA<WidgetRoute>().page.isA<HomePage>(),
+          (it) => it.isA<WidgetRoute>().page.isA<LoginPage>(),
+        ]);
+        pushedRoutes.clear();
+
+        await login(tester, eg.otherAccount);
+        final newAccount = testBinding.globalStore.accounts.singleWhere(
+          (account) => account != eg.selfAccount);
+        check(newAccount).equals(eg.otherAccount.copyWith(id: newAccount.id));
+        check(poppedRoutes).length.equals(2);
+        check(pushedRoutes).single.isA<WidgetRoute>().page.isA<HomePage>();
+      });
+
       testWidgets('trims whitespace on username', (tester) async {
         final serverSettings = eg.serverSettings();
         await prepare(tester, serverSettings);
@@ -192,7 +218,6 @@ void main() {
       });
 
       // TODO test validators on the TextFormField widgets
-      // TODO test navigation, i.e. the call to pushAndRemoveUntil
       // TODO test _getUserId case
       // TODO test handling failure in fetchApiKey request
       // TODO test _inProgress logic