app: On launch, go to the last visited account

Fixes: #524

[chris: rebased atop some notification-test refactors in the last
few commits]

Co-authored-by: Chris Bobbe <[email protected]>

Author: sm-sayedi

lib/widgets/app.dart

@@ -211,13 +211,14 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
     }
 
     final globalStore = GlobalStoreWidget.of(context);
-    // TODO(#524) choose initial account as last one used
-    final initialAccountId = globalStore.accounts.firstOrNull?.id;
+    final lastVisitedAccount = globalStore.lastVisitedAccount;
+
     return [
-      if (initialAccountId == null)
+      if (lastVisitedAccount == null)
+        // There are no accounts, or the last-visited account was logged out.
         MaterialWidgetRoute(page: const ChooseAccountPage())
       else
-        HomePage.buildRoute(accountId: initialAccountId),
+        HomePage.buildRoute(accountId: lastVisitedAccount.id),
     ];
   }
 

lib/widgets/share.dart

@@ -60,9 +60,9 @@ class ShareService {
 
     final globalStore = GlobalStoreWidget.of(context);
 
-    // TODO(#524) use last account used, not the first in the list
     // TODO(#1779) allow selecting account, if there are multiple
-    final accountId = globalStore.accounts.firstOrNull?.id;
+    final lastVisitedAccount = globalStore.lastVisitedAccount;
+    final accountId = lastVisitedAccount?.id ?? globalStore.accountIds.firstOrNull;
 
     if (accountId == null) {
       final zulipLocalizations = ZulipLocalizations.of(context);

test/notifications/open_test.dart

@@ -113,12 +113,11 @@ void main() {
         return;
       }
       await tester.pump();
-      final accountIds = testBinding.globalStore.accountIds;
-      final initialAccountId = accountIds.firstOrNull;
-      if (initialAccountId == null) {
-        takeChooseAccountPageRoute();
+      final lastVisitedAccount = testBinding.globalStore.lastVisitedAccount;
+      if (lastVisitedAccount != null) {
+        takeHomePageRouteForAccount(lastVisitedAccount.id);
       } else {
-        takeHomePageRouteForAccount(initialAccountId);
+        takeChooseAccountPageRoute();
       }
       check(pushedRoutes).isEmpty();
     }
@@ -274,7 +273,8 @@ void main() {
 
     testWidgets('wait for app to become ready', (tester) async {
       addTearDown(testBinding.reset);
-      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot(),
+        markLastVisited: false);
       await prepare(tester, early: true);
       final message = eg.streamMessage();
       await openNotification(tester, eg.selfAccount, message);
@@ -287,7 +287,7 @@ void main() {
       // Now let the GlobalStore get loaded and the app's main UI get mounted.
       await tester.pump();
       // The navigator first pushes the starting routes…
-      takeHomePageRouteForAccount(eg.selfAccount.id); // because first in list
+      takeChooseAccountPageRoute(); // (no lastVisitedAccountId in this test)
       // … and then the one the notification leads to.
       matchesNavigation(check(pushedRoutes).single, eg.selfAccount, message);
     }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));

test/widgets/app_test.dart

@@ -37,24 +37,53 @@ void main() {
     }
 
     testWidgets('when no accounts, go to choose account', (tester) async {
+      check(testBinding.globalStore).accounts.isEmpty();
+      check(testBinding.globalStore).lastVisitedAccount.isNull();
       await prepare(tester);
       check(pushedRoutes).deepEquals(<Condition<Object?>>[
         (it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>(),
       ]);
     });
 
-    testWidgets('when have accounts, go to home page for first account', (tester) async {
-      // We'll need per-account data for the account that a page will be opened
-      // for, but not for the other account.
-      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
-      await testBinding.globalStore.insertAccount(eg.otherAccount.toCompanion(false));
-      await prepare(tester);
+    group('when have accounts', () {
+      testWidgets('with account(s) visited, go to home page for the last visited account', (tester) async {
+        await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+        await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
+          realmUsers: [eg.otherUser]));
+        check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
+        await prepare(tester);
+
+        check(pushedRoutes).deepEquals(<Condition<Object?>>[
+          (it) => it.isA<MaterialAccountWidgetRoute>()
+            ..accountId.equals(eg.otherAccount.id)
+            ..page.isA<HomePage>(),
+        ]);
+      });
 
-      check(pushedRoutes).deepEquals(<Condition<Object?>>[
-        (it) => it.isA<MaterialAccountWidgetRoute>()
-          ..accountId.equals(eg.selfAccount.id)
-          ..page.isA<HomePage>(),
-      ]);
+      // testWidgets('when just upgraded to get the last-visited-account feature, '
+      //     'if there is at least one account, open that one, not the choose-account page', (tester) async {
+      //   // The database migration step from9To10 ensures that
+      //   // [IntGlobalSetting.lastVisitedAccountId], thus [GlobalStore.lastVisitedAccount],
+      //   // points to the first account in the list. That migration step has
+      //   // its own tests in test/model/database_test.dart.
+      // });
+
+      testWidgets('with last visited account logged out, go to choose account', (tester) async {
+        await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+        await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
+          realmUsers: [eg.otherUser]));
+        check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
+        final future = logOutAccount(testBinding.globalStore, eg.otherAccount.id);
+        await tester.pump(TestGlobalStore.removeAccountDuration);
+        await future;
+        check(testBinding.globalStore).lastVisitedAccount.isNull();
+        check(testBinding.globalStore).accounts.isNotEmpty();
+        await prepare(tester);
+
+        check(pushedRoutes).deepEquals(<Condition<Object?>>[
+          (it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>(),
+        ]);
+      });
     });
   });
 
@@ -99,6 +128,7 @@ void main() {
     testWidgets('push route when popping last route on stack', (tester) async {
       // Set up the loading of per-account data to fail.
       await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
+      await testBinding.globalStore.setLastVisitedAccount(eg.selfAccount.id);
       testBinding.globalStore.loadPerAccountDuration = Duration.zero;
       testBinding.globalStore.loadPerAccountException = eg.apiExceptionUnauthorized();
       await prepare(tester);
@@ -133,6 +163,7 @@ void main() {
       const loadPerAccountDuration = Duration(seconds: 30);
       assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod);
       await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
+      await testBinding.globalStore.setLastVisitedAccount(eg.selfAccount.id);
       testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
       testBinding.globalStore.loadPerAccountException = eg.apiExceptionUnauthorized();
       await prepare(tester);
@@ -281,8 +312,9 @@ void main() {
     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(
-        realmUsers: [eg.otherUser]));
+      await testBinding.globalStore.add(
+        eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
+        markLastVisited: false);
 
       final pushedRoutes = <Route<void>>[];
       final poppedRoutes = <Route<void>>[];
@@ -322,9 +354,8 @@ void main() {
     testWidgets('choosing an account changes the last visited account', (tester) async {
       addTearDown(testBinding.reset);
       await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
-      await testBinding.globalStore.add(
-        eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
-        markLastVisited: false);
+      await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
+        realmUsers: [eg.otherUser]));
 
       await tester.pumpWidget(ZulipApp());
       await tester.pump();
@@ -334,10 +365,10 @@ void main() {
       await tester.pump();
       await tester.pump();
 
-      check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
-      await tester.tap(find.text(eg.otherAccount.email));
-      await tester.pump();
       check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
+      await tester.tap(find.text(eg.selfAccount.email));
+      await tester.pump();
+      check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
     });
 
     group('log out', () {

test/widgets/home_test.dart

@@ -333,8 +333,9 @@ void main () {
       pushedRoutes = [];
       lastPoppedRoute = null;
       await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
-      await testBinding.globalStore.add(eg.otherAccount, eg.initialSnapshot(
-        realmUsers: [eg.otherUser]));
+      await testBinding.globalStore.add(
+        eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
+        markLastVisited: false);
       await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
       await tester.pump(Duration.zero); // wait for the loading page
       checkOnLoadingPage();