Skip to content

Commit 652c332

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 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]>
1 parent c5fc2d9 commit 652c332

File tree

2 files changed

+34
-4
lines changed

2 files changed

+34
-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 SizedBox.shrink());
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: 19 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,22 @@ 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(Duration.zero); // wait for the loading page
453+
await tester.pump(loadPerAccountDuration);
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+
check(testBinding.globalStore).accountIds.isEmpty();
462+
});
444463
});
445464
}

0 commit comments

Comments
 (0)