Skip to content

Commit 4ac4595

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: zulip#1219 Signed-off-by: Zixuan James Li <[email protected]>
1 parent 0b90e42 commit 4ac4595

File tree

2 files changed

+52
-26
lines changed

2 files changed

+52
-26
lines changed

lib/widgets/home.dart

Lines changed: 33 additions & 26 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,35 +185,37 @@ 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);
186189

187190
return Scaffold(
188191
appBar: AppBar(),
189-
body: Center(
190-
child: Column(
191-
mainAxisSize: MainAxisSize.min,
192-
children: [
193-
const CircularProgressIndicator(),
194-
Visibility(
195-
visible: showTryAnotherAccount,
196-
maintainSize: true,
197-
maintainAnimation: true,
198-
maintainState: true,
199-
child: Padding(
200-
padding: const EdgeInsets.symmetric(horizontal: 8),
201-
child: Column(
202-
children: [
203-
const SizedBox(height: 16),
204-
Text(zulipLocalizations.tryAnotherAccountMessage(realmUrl.toString())),
205-
const SizedBox(height: 8),
206-
ElevatedButton(
207-
onPressed: () => Navigator.push(context,
208-
MaterialWidgetRoute(page: const ChooseAccountPage())),
209-
child: Text(zulipLocalizations.tryAnotherAccountButton)),
210-
]))),
211-
])));
192+
body: (account == null)
193+
// We should only reach this state very briefly.
194+
// See [_LoadingPlaceholderPage.accountId].
195+
? const SizedBox.shrink()
196+
197+
: Center(child: Column(
198+
mainAxisSize: MainAxisSize.min,
199+
children: [
200+
const CircularProgressIndicator(),
201+
Visibility(
202+
visible: showTryAnotherAccount,
203+
maintainSize: true,
204+
maintainAnimation: true,
205+
maintainState: true,
206+
child: Padding(
207+
padding: const EdgeInsets.symmetric(horizontal: 8),
208+
child: Column(
209+
children: [
210+
const SizedBox(height: 16),
211+
Text(zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString())),
212+
const SizedBox(height: 8),
213+
ElevatedButton(
214+
onPressed: () => Navigator.push(context,
215+
MaterialWidgetRoute(page: const ChooseAccountPage())),
216+
child: Text(zulipLocalizations.tryAnotherAccountButton)),
217+
]))),
218+
])));
212219
}
213220
}
214221

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)