Skip to content

Commit

Permalink
home: Stop assuming account existence from loading page
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
PIG208 committed Jan 2, 2025
1 parent 06c5543 commit 9f19caf
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
19 changes: 15 additions & 4 deletions lib/widgets/home.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
Expand Down
19 changes: 19 additions & 0 deletions test/widgets/home_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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();
});
});
}

0 comments on commit 9f19caf

Please sign in to comment.