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/model/test_store.dart

@@ -169,13 +169,24 @@ class TestGlobalStore extends GlobalStore with _ApiConnectionsMixin, _DatabaseMi
   /// The given initial snapshot will be used to initialize a corresponding
   /// [PerAccountStore] when [perAccount] is subsequently called for this
   /// account, in particular when a [PerAccountStoreWidget] is mounted.
-  Future<void> add(Account account, InitialSnapshot initialSnapshot) async {
+  ///
+  /// By default, the account is marked as [IntGlobalSetting.lastVisitedAccountId].
+  /// Pass false for [markLastVisited] to unmark it.
+  Future<void> add(
+    Account account,
+    InitialSnapshot initialSnapshot, {
+    bool markLastVisited = true,
+  }) async {
     assert(initialSnapshot.zulipVersion == account.zulipVersion);
     assert(initialSnapshot.zulipMergeBase == account.zulipMergeBase);
     assert(initialSnapshot.zulipFeatureLevel == account.zulipFeatureLevel);
     await insertAccount(account.toCompanion(false));
     assert(!_initialSnapshots.containsKey(account.id));
     _initialSnapshots[account.id] = initialSnapshot;
+
+    if (markLastVisited) {
+      await setLastVisitedAccount(account.id);
+    }
   }
 
   Duration? loadPerAccountDuration;

test/notifications/open_test.dart

@@ -20,6 +20,7 @@ import 'package:zulip/widgets/page.dart';
 import '../example_data.dart' as eg;
 import '../model/binding.dart';
 import '../model/narrow_checks.dart';
+import '../model/store_checks.dart';
 import '../stdlib_checks.dart';
 import '../test_navigation.dart';
 import '../widgets/checks.dart';
@@ -112,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();
     }
@@ -273,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);
@@ -286,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}));
@@ -326,6 +327,44 @@ void main() {
       takeHomePageRouteForAccount(accountB.id); // because associated account
       matchesNavigation(check(pushedRoutes).single, accountB, message);
     }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
+
+    group('changes last visited account', () {
+      testWidgets('app already opened, then notification is opened', (tester) async {
+        addTearDown(testBinding.reset);
+        await testBinding.globalStore.add(
+          eg.selfAccount, eg.initialSnapshot(realmUsers: [eg.selfUser]));
+        await testBinding.globalStore.add(
+          eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
+          markLastVisited: false);
+        await prepare(tester);
+        check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
+
+        await checkOpenNotification(tester, eg.otherAccount, eg.streamMessage());
+        check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
+      }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
+
+      testWidgets('app is opened through notification', (tester) async {
+        addTearDown(testBinding.reset);
+
+        final accountA = eg.selfAccount;
+        final accountB = eg.otherAccount;
+        final message = eg.streamMessage();
+        await testBinding.globalStore.add(accountA, eg.initialSnapshot());
+        await testBinding.globalStore.add(
+          accountB, eg.initialSnapshot(realmUsers: [eg.otherUser]),
+          markLastVisited: false);
+        check(testBinding.globalStore).lastVisitedAccount.equals(eg.selfAccount);
+        setupNotificationDataForLaunch(tester, accountB, message);
+
+        await prepare(tester, early: true);
+        check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet
+
+        await tester.pump();
+        takeHomePageRouteForAccount(accountB.id); // because associated account
+        matchesNavigation(check(pushedRoutes).single, accountB, message);
+        check(testBinding.globalStore).lastVisitedAccount.equals(eg.otherAccount);
+      }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS}));
+    });
   });
 
   group('NotificationOpenPayload', () {

test/widgets/app_test.dart

@@ -43,18 +43,50 @@ void main() {
       ]);
     });
 
-    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]));
+        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('with no account visited, go to home page for the first account', (tester) async {
+        await testBinding.globalStore.add(
+          eg.selfAccount, eg.initialSnapshot(),
+          markLastVisited: false);
+        await testBinding.globalStore.add(
+          eg.otherAccount, eg.initialSnapshot(realmUsers: [eg.otherUser]),
+          markLastVisited: false);
+        // We assume this is set by the database migration step from v9 to v10.
+        // That migration step integrity is excercised in its own test case
+        // in test/model/database_test.dart.
+        await testBinding.globalStore.setLastVisitedAccount(eg.selfAccount.id);
+        await prepare(tester);
+
+        check(pushedRoutes).deepEquals(<Condition<Object?>>[
+          (it) => it.isA<MaterialAccountWidgetRoute>()
+            ..accountId.equals(eg.selfAccount.id)
+            ..page.isA<HomePage>(),
+        ]);
+      });
+
+      testWidgets('with last visited account id matching none of the accounts, go to choose account', (tester) async {
+        await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+        await testBinding.globalStore.setLastVisitedAccount(eg.otherAccount.id);
+        check(testBinding.globalStore).getAccount(eg.otherAccount.id).isNull();
+        await prepare(tester);
+
+        check(pushedRoutes).deepEquals(<Condition<Object?>>[
+          (it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>(),
+        ]);
+      });
     });
   });
 
@@ -99,6 +131,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 +166,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 +315,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>>[];
@@ -319,6 +354,27 @@ void main() {
         ..page.isA<HomePage>();
     });
 
+    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 tester.pumpWidget(ZulipApp());
+      await tester.pump();
+
+      final navigator = await ZulipApp.navigator;
+      unawaited(navigator.push(MaterialWidgetRoute(page: const ChooseAccountPage())));
+      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);
+    });
+
     group('log out', () {
       Future<(Widget, Widget)> prepare(WidgetTester tester, {required Account account}) async {
         await setupChooseAccountPage(tester, accounts: [account]);

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();