From cdfb0d90edf0b0bdfb109656545bc8adc6b58e64 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Sun, 29 Dec 2024 20:02:59 -0500 Subject: [PATCH] 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 --- lib/widgets/home.dart | 19 +++++++++++++++---- test/widgets/home_test.dart | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index 6b5a4979282..ad70b57c32b 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 SizedBox.shrink()); + } 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/widgets/home_test.dart b/test/widgets/home_test.dart index a86bc5c34e3..beb465a4e59 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,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(); + }); }); }