home: Stop assuming account existence from loading page

We could pass realmUrl when initializing the `_LoadingPlaceholderPage`, but that will still require a check for the existence of the account. The loading page will be blank when the account does not exist. The user can't easily reach this page because they can only logout from `ChooseAccountPage`, until we start invalidating API keys. Even then, they will only see the blank page briefly before getting navigated, so we should not show any text at all. Fixes: #1219 Signed-off-by: Zixuan James Li <[email protected]>
zulip · Jan 7, 2025 · 4ac4595 · 4ac4595
1 parent 0b90e42
commit 4ac4595
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 26 deletions.
diff --git 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,35 +185,37 @@ 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);
 
     return Scaffold(
       appBar: AppBar(),
-      body: Center(
-        child: Column(
-          mainAxisSize: MainAxisSize.min,
-          children: [
-            const CircularProgressIndicator(),
-            Visibility(
-              visible: showTryAnotherAccount,
-              maintainSize: true,
-              maintainAnimation: true,
-              maintainState: true,
-              child: Padding(
-                padding: const EdgeInsets.symmetric(horizontal: 8),
-                child: Column(
-                  children: [
-                    const SizedBox(height: 16),
-                    Text(zulipLocalizations.tryAnotherAccountMessage(realmUrl.toString())),
-                    const SizedBox(height: 8),
-                    ElevatedButton(
-                      onPressed: () => Navigator.push(context,
-                        MaterialWidgetRoute(page: const ChooseAccountPage())),
-                      child: Text(zulipLocalizations.tryAnotherAccountButton)),
-                  ]))),
-          ])));
+      body: (account == null)
+        // We should only reach this state very briefly.
+        // See [_LoadingPlaceholderPage.accountId].
+        ? const SizedBox.shrink()
+
+        : Center(child: Column(
+            mainAxisSize: MainAxisSize.min,
+            children: [
+              const CircularProgressIndicator(),
+              Visibility(
+                visible: showTryAnotherAccount,
+                maintainSize: true,
+                maintainAnimation: true,
+                maintainState: true,
+                child: Padding(
+                  padding: const EdgeInsets.symmetric(horizontal: 8),
+                  child: Column(
+                    children: [
+                      const SizedBox(height: 16),
+                      Text(zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString())),
+                      const SizedBox(height: 8),
+                      ElevatedButton(
+                        onPressed: () => Navigator.push(context,
+                          MaterialWidgetRoute(page: const ChooseAccountPage())),
+                        child: Text(zulipLocalizations.tryAnotherAccountButton)),
+                    ]))),
+            ])));
   }
 }
 

diff --git 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,22 @@ 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(Duration.zero); // wait for the loading page
+      await tester.pump(loadPerAccountDuration);
+
+      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.
+      check(testBinding.globalStore).accountIds.isEmpty();
+    });
   });
 }