Skip to content

Commit 21a347c

Browse files
committed
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 contain a CircularLoadingIndicator 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 page briefly before getting navigated, so we should not show any text at all. Fixes: #1219 Signed-off-by: Zixuan James Li <[email protected]>
1 parent efcf201 commit 21a347c

File tree

2 files changed

+35
-4
lines changed

2 files changed

+35
-4
lines changed

lib/widgets/home.dart

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ const kTryAnotherAccountWaitPeriod = Duration(seconds: 5);
151151
class _LoadingPlaceholderPage extends StatefulWidget {
152152
const _LoadingPlaceholderPage({required this.accountId});
153153

154+
/// The relevant account for this page.
155+
///
156+
/// The account is not guaranteed to exist in the global store. This can
157+
/// happen briefly when the account is removed from the database for logout,
158+
/// but before [PerAccountStoreWidget.routeToRemoveOnLogout] is processed.
154159
final int accountId;
155160

156161
@override
@@ -180,9 +185,15 @@ class _LoadingPlaceholderPageState extends State<_LoadingPlaceholderPage> {
180185
@override
181186
Widget build(BuildContext context) {
182187
final zulipLocalizations = ZulipLocalizations.of(context);
183-
final realmUrl = GlobalStoreWidget.of(context)
184-
// TODO(#1219) `!` is incorrect
185-
.getAccount(widget.accountId)!.realmUrl;
188+
final account = GlobalStoreWidget.of(context).getAccount(widget.accountId);
189+
190+
if (account == null) {
191+
// We should only reach this state very briefly.
192+
// See [_LoadingPlaceholderPage.accountId].
193+
return Scaffold(
194+
appBar: AppBar(),
195+
body: const CircularProgressIndicator());
196+
}
186197

187198
return Scaffold(
188199
appBar: AppBar(),
@@ -201,7 +212,7 @@ class _LoadingPlaceholderPageState extends State<_LoadingPlaceholderPage> {
201212
child: Column(
202213
children: [
203214
const SizedBox(height: 16),
204-
Text(zulipLocalizations.tryAnotherAccountMessage(realmUrl.toString())),
215+
Text(zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString())),
205216
const SizedBox(height: 8),
206217
ElevatedButton(
207218
onPressed: () => Navigator.push(context,

test/widgets/home_test.dart

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:zulip/api/model/events.dart';
66
import 'package:zulip/model/narrow.dart';
77
import 'package:zulip/model/store.dart';
88
import 'package:zulip/widgets/about_zulip.dart';
9+
import 'package:zulip/widgets/actions.dart';
910
import 'package:zulip/widgets/app.dart';
1011
import 'package:zulip/widgets/app_bar.dart';
1112
import 'package:zulip/widgets/home.dart';
@@ -21,6 +22,7 @@ import '../api/fake_api.dart';
2122
import '../example_data.dart' as eg;
2223
import '../flutter_checks.dart';
2324
import '../model/binding.dart';
25+
import '../model/store_checks.dart';
2426
import '../model/test_store.dart';
2527
import '../test_navigation.dart';
2628
import 'message_list_checks.dart';
@@ -441,5 +443,23 @@ void main () {
441443
// No additional wait for loadPerAccount.
442444
checkOnHomePage(tester, expectedAccount: eg.selfAccount);
443445
});
446+
447+
testWidgets('logging out', (tester) async {
448+
// Regression test for: https://github.com/zulip/zulip-flutter/issues/1219
449+
addTearDown(testBinding.reset);
450+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
451+
await tester.pumpWidget(const ZulipApp());
452+
await tester.pump(); // wait for the loading page
453+
await tester.pump(); // wait for store
454+
455+
final element = tester.element(find.byType(HomePage));
456+
final future = logOutAccount(element, eg.selfAccount.id);
457+
await tester.pump(TestGlobalStore.removeAccountDuration);
458+
await future;
459+
// No error expected from [_LoadingPlaceholderPage] briefly not having
460+
// access to the account being logged out.
461+
checkOnLoadingPage();
462+
check(testBinding.globalStore).accountIds.isEmpty();
463+
});
444464
});
445465
}

0 commit comments

Comments
 (0)