From 4ac4595cac1010465efe9361b283a8abd7a6f62e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Sun, 29 Dec 2024 20:02:59 -0500 Subject: [PATCH 1/9] 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 | 59 +++++++++++++++++++++---------------- test/widgets/home_test.dart | 19 ++++++++++++ 2 files changed, 52 insertions(+), 26 deletions(-) diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index 6b5a497928..198d09e9d5 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,35 +185,37 @@ 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); return Scaffold( appBar: AppBar(), - body: Center( - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - const CircularProgressIndicator(), - Visibility( - visible: showTryAnotherAccount, - maintainSize: true, - maintainAnimation: true, - maintainState: true, - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 8), - child: Column( - children: [ - const SizedBox(height: 16), - Text(zulipLocalizations.tryAnotherAccountMessage(realmUrl.toString())), - const SizedBox(height: 8), - ElevatedButton( - onPressed: () => Navigator.push(context, - MaterialWidgetRoute(page: const ChooseAccountPage())), - child: Text(zulipLocalizations.tryAnotherAccountButton)), - ]))), - ]))); + body: (account == null) + // We should only reach this state very briefly. + // See [_LoadingPlaceholderPage.accountId]. + ? const SizedBox.shrink() + + : Center(child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + const CircularProgressIndicator(), + Visibility( + visible: showTryAnotherAccount, + maintainSize: true, + maintainAnimation: true, + maintainState: true, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 8), + child: Column( + children: [ + const SizedBox(height: 16), + Text(zulipLocalizations.tryAnotherAccountMessage(account.realmUrl.toString())), + const SizedBox(height: 8), + ElevatedButton( + onPressed: () => Navigator.push(context, + MaterialWidgetRoute(page: const ChooseAccountPage())), + child: Text(zulipLocalizations.tryAnotherAccountButton)), + ]))), + ]))); } } diff --git a/test/widgets/home_test.dart b/test/widgets/home_test.dart index a86bc5c34e..beb465a4e5 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(); + }); }); } From 1eed66b2b5fccf477c2265e757972461c945b774 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 2 Jan 2025 02:10:17 -0500 Subject: [PATCH 2/9] l10n [nfc]: Use a generalized name for errorCouldNotConnectTitle Signed-off-by: Zixuan James Li --- assets/l10n/app_en.arb | 4 ++-- lib/generated/l10n/zulip_localizations.dart | 2 +- lib/generated/l10n/zulip_localizations_ar.dart | 2 +- lib/generated/l10n/zulip_localizations_en.dart | 2 +- lib/generated/l10n/zulip_localizations_fr.dart | 2 +- lib/generated/l10n/zulip_localizations_ja.dart | 2 +- lib/generated/l10n/zulip_localizations_pl.dart | 2 +- lib/generated/l10n/zulip_localizations_ru.dart | 2 +- lib/widgets/login.dart | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 58822303fd..94db8303aa 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -186,8 +186,8 @@ "url": {"type": "String", "example": "http://example.com/"} } }, - "errorLoginCouldNotConnectTitle": "Could not connect", - "@errorLoginCouldNotConnectTitle": { + "errorCouldNotConnectTitle": "Could not connect", + "@errorCouldNotConnectTitle": { "description": "Error title when the app could not connect to the server." }, "errorMessageDoesNotSeemToExist": "That message does not seem to exist.", diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 00d7cfde72..291863e21c 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -359,7 +359,7 @@ abstract class ZulipLocalizations { /// /// In en, this message translates to: /// **'Could not connect'** - String get errorLoginCouldNotConnectTitle; + String get errorCouldNotConnectTitle; /// Error message when loading a message that does not exist. /// diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 542b85031b..01f1775c7f 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -159,7 +159,7 @@ class ZulipLocalizationsAr extends ZulipLocalizations { } @override - String get errorLoginCouldNotConnectTitle => 'Could not connect'; + String get errorCouldNotConnectTitle => 'Could not connect'; @override String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index b6bc9f72e7..ed6531b164 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -159,7 +159,7 @@ class ZulipLocalizationsEn extends ZulipLocalizations { } @override - String get errorLoginCouldNotConnectTitle => 'Could not connect'; + String get errorCouldNotConnectTitle => 'Could not connect'; @override String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.'; diff --git a/lib/generated/l10n/zulip_localizations_fr.dart b/lib/generated/l10n/zulip_localizations_fr.dart index c857da2c82..5373cb9a12 100644 --- a/lib/generated/l10n/zulip_localizations_fr.dart +++ b/lib/generated/l10n/zulip_localizations_fr.dart @@ -159,7 +159,7 @@ class ZulipLocalizationsFr extends ZulipLocalizations { } @override - String get errorLoginCouldNotConnectTitle => 'Could not connect'; + String get errorCouldNotConnectTitle => 'Could not connect'; @override String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 7adbc9ae8a..3b2af456f1 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -159,7 +159,7 @@ class ZulipLocalizationsJa extends ZulipLocalizations { } @override - String get errorLoginCouldNotConnectTitle => 'Could not connect'; + String get errorCouldNotConnectTitle => 'Could not connect'; @override String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 07746b3f27..d69cae8d5e 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -159,7 +159,7 @@ class ZulipLocalizationsPl extends ZulipLocalizations { } @override - String get errorLoginCouldNotConnectTitle => 'Nie można połączyć'; + String get errorCouldNotConnectTitle => 'Could not connect'; @override String get errorMessageDoesNotSeemToExist => 'Taka wiadomość raczej nie istnieje.'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 9c2065376b..47cc86a760 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -159,7 +159,7 @@ class ZulipLocalizationsRu extends ZulipLocalizations { } @override - String get errorLoginCouldNotConnectTitle => 'Could not connect'; + String get errorCouldNotConnectTitle => 'Could not connect'; @override String get errorMessageDoesNotSeemToExist => 'That message does not seem to exist.'; diff --git a/lib/widgets/login.dart b/lib/widgets/login.dart index ce1cf09438..64a0210525 100644 --- a/lib/widgets/login.dart +++ b/lib/widgets/login.dart @@ -176,7 +176,7 @@ class _AddAccountPageState extends State { // TODO(#105) give more helpful feedback; see `fetchServerSettings` // in zulip-mobile's src/message/fetchActions.js. showErrorDialog(context: context, - title: zulipLocalizations.errorLoginCouldNotConnectTitle, + title: zulipLocalizations.errorCouldNotConnectTitle, message: zulipLocalizations.errorLoginCouldNotConnect(url.toString())); return; } From 191d85bfaa77eed692d32ab23b8d4b259346b03f Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 2 Jan 2025 02:20:29 -0500 Subject: [PATCH 3/9] log [nfc]: Rename ReportErrorCallback to ReportErrorCancellablyCallback This highlights the API choice that the callback signature allows the caller to clear/cancel the reported errors, drawing distinction from a later added variant that does not allow this. Signed-off-by: Zixuan James Li --- lib/log.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/log.dart b/lib/log.dart index e3261f8cba..86f104db14 100644 --- a/lib/log.dart +++ b/lib/log.dart @@ -31,7 +31,7 @@ bool debugLog(String message) { return true; } -typedef ReportErrorCallback = void Function(String? message, {String? details}); +typedef ReportErrorCancellablyCallback = void Function(String? message, {String? details}); /// Show the user an error message, without requiring them to interact with it. /// @@ -48,7 +48,7 @@ typedef ReportErrorCallback = void Function(String? message, {String? details}); // This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart` // from importing widget code, because the file is a dependency for the rest of // the app. -ReportErrorCallback reportErrorToUserBriefly = defaultReportErrorToUserBriefly; +ReportErrorCancellablyCallback reportErrorToUserBriefly = defaultReportErrorToUserBriefly; void defaultReportErrorToUserBriefly(String? message, {String? details}) { // Error dismissing is a no-op to the default handler. From 0af16f9eb29e38bb9a9cd34bb758e1aa956a423e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 3 Jan 2025 10:10:53 +0800 Subject: [PATCH 4/9] log [nfc]: Rename default report error helper to reportErrorToConsole Signed-off-by: Zixuan James Li --- lib/log.dart | 4 ++-- lib/widgets/app.dart | 2 +- test/model/store_test.dart | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/log.dart b/lib/log.dart index 86f104db14..516e84d5f0 100644 --- a/lib/log.dart +++ b/lib/log.dart @@ -48,9 +48,9 @@ typedef ReportErrorCancellablyCallback = void Function(String? message, {String? // This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart` // from importing widget code, because the file is a dependency for the rest of // the app. -ReportErrorCancellablyCallback reportErrorToUserBriefly = defaultReportErrorToUserBriefly; +ReportErrorCancellablyCallback reportErrorToUserBriefly = reportErrorToConsole; -void defaultReportErrorToUserBriefly(String? message, {String? details}) { +void reportErrorToConsole(String? message, {String? details}) { // Error dismissing is a no-op to the default handler. if (message == null) return; // If this callback is still in place, then the app's widget tree diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 75fec7bc8b..1c50f1bf1f 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -84,7 +84,7 @@ class ZulipApp extends StatefulWidget { @visibleForTesting static void debugReset() { _snackBarCount = 0; - reportErrorToUserBriefly = defaultReportErrorToUserBriefly; + reportErrorToUserBriefly = reportErrorToConsole; _ready.dispose(); _ready = ValueNotifier(false); } diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 10e8698360..15674d7c10 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -858,7 +858,7 @@ void main() { Future prepare() async { reportErrorToUserBriefly = logReportedError; - addTearDown(() => reportErrorToUserBriefly = defaultReportErrorToUserBriefly); + addTearDown(() => reportErrorToUserBriefly = reportErrorToConsole); await preparePoll(lastEventId: 1); } From 54664aa15d98a6c14899a01dab9bf5b50f96ed98 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 30 Dec 2024 13:50:37 -0500 Subject: [PATCH 5/9] test [nfc]: Factor out `checkNoErrorDialog` helper With #996, these tests will have to start checking for separate per-platform flavors of alert dialog. Best if they all do so through this one codepath. --- test/widgets/app_test.dart | 4 ++-- test/widgets/compose_box_test.dart | 9 +++------ test/widgets/dialog_checks.dart | 7 +++++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index 8c992533ee..af386bfdf1 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -280,14 +280,14 @@ void main() { check(ZulipApp.ready).value.isFalse(); await tester.pump(); check(findSnackBarByText(message).evaluate()).isEmpty(); - check(find.byType(AlertDialog).evaluate()).isEmpty(); + checkNoErrorDialog(tester); check(ZulipApp.ready).value.isTrue(); // After app startup, reportErrorToUserBriefly displays a SnackBar. reportErrorToUserBriefly(message, details: details); await tester.pumpAndSettle(); check(findSnackBarByText(message).evaluate()).single; - check(find.byType(AlertDialog).evaluate()).isEmpty(); + checkNoErrorDialog(tester); // Open the error details dialog. await tester.tap(find.text('Details')); diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 077134e7d1..e5bd1fc316 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -427,8 +427,7 @@ void main() { await setupAndTapSend(tester, prepareResponse: (int messageId) { connection.prepare(json: SendMessageResult(id: messageId).toJson()); }); - final errorDialogs = tester.widgetList(find.byType(AlertDialog)); - check(errorDialogs).isEmpty(); + checkNoErrorDialog(tester); }); testWidgets('ZulipApiException', (tester) async { @@ -497,8 +496,7 @@ void main() { check(call.allowMultiple).equals(true); check(call.type).equals(FileType.media); - final errorDialogs = tester.widgetList(find.byType(AlertDialog)); - check(errorDialogs).isEmpty(); + checkNoErrorDialog(tester); check(controller!.content.text) .equals('see image: [Uploading image.jpg…]()\n\n'); @@ -557,8 +555,7 @@ void main() { check(call.source).equals(ImageSource.camera); check(call.requestFullMetadata).equals(false); - final errorDialogs = tester.widgetList(find.byType(AlertDialog)); - check(errorDialogs).isEmpty(); + checkNoErrorDialog(tester); check(controller!.content.text) .equals('see image: [Uploading image.jpg…]()\n\n'); diff --git a/test/widgets/dialog_checks.dart b/test/widgets/dialog_checks.dart index 504042d07e..af0c6e2963 100644 --- a/test/widgets/dialog_checks.dart +++ b/test/widgets/dialog_checks.dart @@ -1,4 +1,6 @@ +import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/widgets/dialog.dart'; @@ -26,6 +28,11 @@ Widget checkErrorDialog(WidgetTester tester, { matching: find.widgetWithText(TextButton, 'OK'))); } +// TODO(#996) update this to check for per-platform flavors of alert dialog +void checkNoErrorDialog(WidgetTester tester) { + check(find.byType(AlertDialog)).findsNothing(); +} + /// In a widget test, check that [showSuggestedActionDialog] was called /// with the right text. /// From da9cbb679b1177942b385155ba1092140965501f Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 2 Jan 2025 02:12:20 -0500 Subject: [PATCH 6/9] log: Add reportErrorModally Signed-off-by: Zixuan James Li --- lib/log.dart | 12 ++++++++++++ lib/widgets/app.dart | 12 ++++++++++++ test/widgets/app_test.dart | 19 +++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/lib/log.dart b/lib/log.dart index 516e84d5f0..bfcc573651 100644 --- a/lib/log.dart +++ b/lib/log.dart @@ -32,6 +32,7 @@ bool debugLog(String message) { } typedef ReportErrorCancellablyCallback = void Function(String? message, {String? details}); +typedef ReportErrorCallback = void Function(String message, {String? details}); /// Show the user an error message, without requiring them to interact with it. /// @@ -50,6 +51,17 @@ typedef ReportErrorCancellablyCallback = void Function(String? message, {String? // the app. ReportErrorCancellablyCallback reportErrorToUserBriefly = reportErrorToConsole; +/// Show the user a dismissable error message in a modal popup. +/// +/// Typically this shows a [AlertDialog] containing the message. +/// If called before the app's widget tree is ready (see [ZulipApp.ready]), +/// then we give up on showing the message to the user, +/// and just log the message to the console. +// This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart` +// from importing widget code, because the file is a dependency for the rest of +// the app. +ReportErrorCallback reportErrorToUserModally = reportErrorToConsole; + void reportErrorToConsole(String? message, {String? details}) { // Error dismissing is a no-op to the default handler. if (message == null) return; diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 1c50f1bf1f..c1fe713a41 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -85,6 +85,7 @@ class ZulipApp extends StatefulWidget { static void debugReset() { _snackBarCount = 0; reportErrorToUserBriefly = reportErrorToConsole; + reportErrorToUserModally = reportErrorToConsole; _ready.dispose(); _ready = ValueNotifier(false); } @@ -128,10 +129,21 @@ class ZulipApp extends StatefulWidget { newSnackBar.closed.whenComplete(() => _snackBarCount--); } + /// The callback we normally use as [reportErrorToUserModally]. + static void _reportErrorToUserModally(String message, {String? details}) { + assert(_ready.value); + + showErrorDialog( + context: navigatorKey.currentContext!, + title: message, + message: details); + } + void _declareReady() { assert(navigatorKey.currentContext != null); _ready.value = true; reportErrorToUserBriefly = _reportErrorToUserBriefly; + reportErrorToUserModally = _reportErrorToUserModally; } @override diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index af386bfdf1..a53d07a044 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -361,5 +361,24 @@ void main() { await tester.pumpAndSettle(); check(findSnackBarByText('unrelated').evaluate()).single; }); + + testWidgets('reportErrorToUserModally', (tester) async { + addTearDown(testBinding.reset); + await tester.pumpWidget(const ZulipApp()); + const message = 'test error message'; + const details = 'details'; + + // Prior to app startup, reportErrorToUserModally only logs. + reportErrorToUserModally(message, details: details); + check(ZulipApp.ready).value.isFalse(); + await tester.pump(); + checkNoErrorDialog(tester); + + check(ZulipApp.ready).value.isTrue(); + // After app startup, reportErrorToUserModally displays an [AlertDialog]. + reportErrorToUserModally(message, details: details); + await tester.pump(); + checkErrorDialog(tester, expectedTitle: message, expectedMessage: details); + }); }); } From 11f4797e99003c51a6913965c886f7ff0d27c62c Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 6 Jan 2025 17:35:53 +0800 Subject: [PATCH 7/9] login [nfc]: Pass GlobalStore to logOutAccount This allows us to call it from model code when GlobalStore is available. Signed-off-by: Zixuan James Li --- lib/widgets/actions.dart | 4 +--- lib/widgets/app.dart | 2 +- test/widgets/actions_test.dart | 7 ++++--- test/widgets/home_test.dart | 3 ++- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/widgets/actions.dart b/lib/widgets/actions.dart index 86c6e44875..5ac1d732c8 100644 --- a/lib/widgets/actions.dart +++ b/lib/widgets/actions.dart @@ -20,9 +20,7 @@ import '../notifications/receive.dart'; import 'dialog.dart'; import 'store.dart'; -Future logOutAccount(BuildContext context, int accountId) async { - final globalStore = GlobalStoreWidget.of(context); - +Future logOutAccount(GlobalStore globalStore, int accountId) async { final account = globalStore.getAccount(accountId); if (account == null) return; // TODO(log) diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index c1fe713a41..643666693c 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -261,7 +261,7 @@ class ChooseAccountPage extends StatelessWidget { actionButtonText: zulipLocalizations.logOutConfirmationDialogConfirmButton, onActionButtonPress: () { // TODO error handling if db write fails? - logOutAccount(context, accountId); + logOutAccount(GlobalStoreWidget.of(context), accountId); }); }, child: Text(zulipLocalizations.chooseAccountPageLogOutButton)), diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index 4a4698a175..0da9afdae0 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -20,6 +20,7 @@ import 'package:zulip/widgets/actions.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/page.dart'; +import 'package:zulip/widgets/store.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -106,7 +107,7 @@ void main() { final newConnection = separateConnection() ..prepare(delay: unregisterDelay, json: {'msg': '', 'result': 'success'}); - final future = logOutAccount(context, eg.selfAccount.id); + final future = logOutAccount(GlobalStoreWidget.of(context), eg.selfAccount.id); // Unregister-token request and account removal dispatched together checkSingleUnregisterRequest(newConnection); check(testBinding.globalStore.takeDoRemoveAccountCalls()) @@ -138,7 +139,7 @@ void main() { final newConnection = separateConnection() ..prepare(delay: unregisterDelay, exception: exception); - final future = logOutAccount(context, eg.selfAccount.id); + final future = logOutAccount(GlobalStoreWidget.of(context), eg.selfAccount.id); // Unregister-token request and account removal dispatched together checkSingleUnregisterRequest(newConnection); check(testBinding.globalStore.takeDoRemoveAccountCalls()) @@ -208,7 +209,7 @@ void main() { testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route); final context = tester.element(find.byType(MaterialApp)); - final future = logOutAccount(context, account1.id); + final future = logOutAccount(GlobalStoreWidget.of(context), account1.id); await tester.pump(TestGlobalStore.removeAccountDuration); await future; check(removedRoutes).single.identicalTo(account1Route); diff --git a/test/widgets/home_test.dart b/test/widgets/home_test.dart index beb465a4e5..82306701bb 100644 --- a/test/widgets/home_test.dart +++ b/test/widgets/home_test.dart @@ -15,6 +15,7 @@ import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/profile.dart'; +import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/subscription_list.dart'; import 'package:zulip/widgets/theme.dart'; @@ -453,7 +454,7 @@ void main () { await tester.pump(loadPerAccountDuration); final element = tester.element(find.byType(HomePage)); - final future = logOutAccount(element, eg.selfAccount.id); + final future = logOutAccount(GlobalStoreWidget.of(element), eg.selfAccount.id); await tester.pump(TestGlobalStore.removeAccountDuration); await future; // No error expected from [_LoadingPlaceholderPage] briefly not having From 90e4159a72538287ba05a052848ed46fb9a7dc55 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 7 Jan 2025 17:20:48 +0800 Subject: [PATCH 8/9] action test [nfc]: Remove irrelevant issue reference Coming up with a realistic test case doesn't actually require invalidating API key. Because the goal is to use routes that exist in the app (`InboxPageBody` has become a part of `HomePage` and doesn't exist on its own), we can set up HomePage and MessageListPage instead. Signed-off-by: Zixuan James Li --- test/widgets/actions_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index 0da9afdae0..e75bc84df2 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -183,7 +183,7 @@ void main() { final pushedRoutes = >[]; testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); - // TODO(#737): switch to a realistic setup: + // TODO: switch to a realistic setup: // https://github.com/zulip/zulip-flutter/pull/1076#discussion_r1874124363 final account1Route = MaterialAccountWidgetRoute( accountId: account1.id, page: const InboxPageBody()); From 501aaa6b3173749a169a653c99983f1f391d7ea3 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 19 Dec 2024 16:29:41 -0500 Subject: [PATCH 9/9] store: Handle invalid API key on register-queue Fixes: #737 Signed-off-by: Zixuan James Li --- assets/l10n/app_en.arb | 7 ++ lib/generated/l10n/zulip_localizations.dart | 6 ++ .../l10n/zulip_localizations_ar.dart | 5 + .../l10n/zulip_localizations_en.dart | 5 + .../l10n/zulip_localizations_fr.dart | 5 + .../l10n/zulip_localizations_ja.dart | 5 + .../l10n/zulip_localizations_pl.dart | 5 + .../l10n/zulip_localizations_ru.dart | 5 + lib/model/store.dart | 43 +++++++-- lib/widgets/app.dart | 37 +++++++- test/model/store_test.dart | 34 +++++++ test/model/test_store.dart | 4 + test/widgets/app_test.dart | 95 +++++++++++++++++++ test/widgets/store_test.dart | 89 +++++++++++++++++ 14 files changed, 338 insertions(+), 7 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 94db8303aa..7aa1898063 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -458,6 +458,13 @@ "@topicValidationErrorMandatoryButEmpty": { "description": "Topic validation error when topic is required but was empty." }, + "errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.", + "@errorInvalidApiKeyMessage": { + "description": "Error message in the dialog for invalid API key.", + "placeholders": { + "url": {"type": "String", "example": "http://chat.example.com/"} + } + }, "errorInvalidResponse": "The server sent an invalid response", "@errorInvalidResponse": { "description": "Error message when an API call returned an invalid response." diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 291863e21c..c72fd71505 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -721,6 +721,12 @@ abstract class ZulipLocalizations { /// **'Topics are required in this organization.'** String get topicValidationErrorMandatoryButEmpty; + /// Error message in the dialog for invalid API key. + /// + /// In en, this message translates to: + /// **'Your account at {url} could not be authenticated. Please try logging in again or use another account.'** + String errorInvalidApiKeyMessage(String url); + /// Error message when an API call returned an invalid response. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 01f1775c7f..c40b5c98a4 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -357,6 +357,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String errorInvalidApiKeyMessage(String url) { + return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; + } + @override String get errorInvalidResponse => 'The server sent an invalid response'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index ed6531b164..cd3e5e407a 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -357,6 +357,11 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String errorInvalidApiKeyMessage(String url) { + return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; + } + @override String get errorInvalidResponse => 'The server sent an invalid response'; diff --git a/lib/generated/l10n/zulip_localizations_fr.dart b/lib/generated/l10n/zulip_localizations_fr.dart index 5373cb9a12..d3a6a46d5c 100644 --- a/lib/generated/l10n/zulip_localizations_fr.dart +++ b/lib/generated/l10n/zulip_localizations_fr.dart @@ -357,6 +357,11 @@ class ZulipLocalizationsFr extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String errorInvalidApiKeyMessage(String url) { + return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; + } + @override String get errorInvalidResponse => 'The server sent an invalid response'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 3b2af456f1..45f3f35b2a 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -357,6 +357,11 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String errorInvalidApiKeyMessage(String url) { + return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; + } + @override String get errorInvalidResponse => 'The server sent an invalid response'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index d69cae8d5e..6510f428b2 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -357,6 +357,11 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.'; + @override + String errorInvalidApiKeyMessage(String url) { + return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; + } + @override String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 47cc86a760..46d2c08a19 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -357,6 +357,11 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.'; + @override + String errorInvalidApiKeyMessage(String url) { + return 'Your account at $url could not be authenticated. Please try logging in again or use another account.'; + } + @override String get errorInvalidResponse => 'The server sent an invalid response'; diff --git a/lib/model/store.dart b/lib/model/store.dart index 58a8a70615..aab1d6ca61 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -19,6 +19,7 @@ import '../api/backoff.dart'; import '../api/route/realm.dart'; import '../log.dart'; import '../notifications/receive.dart'; +import '../widgets/actions.dart'; import 'autocomplete.dart'; import 'database.dart'; import 'emoji.dart'; @@ -149,13 +150,31 @@ abstract class GlobalStore extends ChangeNotifier { /// and/or [perAccountSync]. Future loadPerAccount(int accountId) async { assert(_accounts.containsKey(accountId)); - final store = await doLoadPerAccount(accountId); + PerAccountStore? store; + try { + store = await doLoadPerAccount(accountId); + } catch (e) { + switch (e) { + case ZulipApiException(code: 'INVALID_API_KEY'): + // The API key is invalid and the store can never be loaded + // unless the user retries manually. + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + final account = getAccount(accountId)!; + reportErrorToUserModally( + zulipLocalizations.errorCouldNotConnectTitle, + details: zulipLocalizations.errorInvalidApiKeyMessage( + account.realmUrl.toString())); + await logOutAccount(this, accountId); + default: + rethrow; + } + } if (!_accounts.containsKey(accountId)) { - // [removeAccount] was called during [doLoadPerAccount]. - store.dispose(); + // [removeAccount] was called during or after [doLoadPerAccount]. + store?.dispose(); throw AccountNotFoundException(); } - return store; + return store!; } /// Load per-account data for the given account, unconditionally. @@ -912,7 +931,13 @@ class UpdateMachine { // at 1 kiB (at least on Android), and stack can be longer than that. assert(debugLog('Stack:\n$s')); assert(debugLog('Backing off, then will retry…')); - // TODO tell user if initial-fetch errors persist, or look non-transient + // TODO(#890): tell user if initial-fetch errors persist, or look non-transient + switch (e) { + case ZulipApiException(code: 'INVALID_API_KEY'): + // We cannot recover from this error through retrying. + // Leave it to [GlobalStore.loadPerAccount]. + rethrow; + } await (backoffMachine ??= BackoffMachine()).wait(); assert(debugLog('… Backoff wait complete, retrying initial fetch.')); } @@ -1044,7 +1069,13 @@ class UpdateMachine { } } catch (e) { if (_disposed) return; - await _handlePollError(e); + try { + await _handlePollError(e); + } on AccountNotFoundException { + // Cannot recover by replacing the store because the account + // was logged out. + return; + } assert(_disposed); return; } diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 643666693c..d76afb02ae 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -189,6 +189,11 @@ class _ZulipAppState extends State with WidgetsBindingObserver { final themeData = zulipThemeData(context); return GlobalStoreWidget( child: Builder(builder: (context) { + final effectiveNavigatorObservers = [ + _EmptyStackNavigatorObserver(), + if (widget.navigatorObservers != null) + ...widget.navigatorObservers!, + ]; final globalStore = GlobalStoreWidget.of(context); // TODO(#524) choose initial account as last one used final initialAccountId = globalStore.accounts.firstOrNull?.id; @@ -199,7 +204,7 @@ class _ZulipAppState extends State with WidgetsBindingObserver { theme: themeData, navigatorKey: ZulipApp.navigatorKey, - navigatorObservers: widget.navigatorObservers ?? const [], + navigatorObservers: effectiveNavigatorObservers, builder: (BuildContext context, Widget? child) { if (!ZulipApp.ready.value) { SchedulerBinding.instance.addPostFrameCallback( @@ -230,6 +235,36 @@ class _ZulipAppState extends State with WidgetsBindingObserver { } } +class _EmptyStackNavigatorObserver extends NavigatorObserver { + void _pushIfEmpty() async { + final navigator = await ZulipApp.navigator; + bool isEmptyStack = true; + // TODO: find a better way to inspect the navigator stack + navigator.popUntil((route) { + isEmptyStack = false; + return true; // never actually pops + }); + if (isEmptyStack) { + unawaited(navigator.push( + MaterialWidgetRoute(page: const ChooseAccountPage()))); + } + } + + @override + void didRemove(Route route, Route? previousRoute) async { + if (previousRoute == null) { + _pushIfEmpty(); + } + } + + @override + void didPop(Route route, Route? previousRoute) async { + if (previousRoute == null) { + _pushIfEmpty(); + } + } +} + class ChooseAccountPage extends StatelessWidget { const ChooseAccountPage({super.key}); diff --git a/test/model/store_test.dart b/test/model/store_test.dart index 15674d7c10..2e64e28757 100644 --- a/test/model/store_test.dart +++ b/test/model/store_test.dart @@ -7,6 +7,7 @@ import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; import 'package:test/scaffolding.dart'; import 'package:zulip/api/core.dart'; +import 'package:zulip/api/exception.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/events.dart'; @@ -500,6 +501,20 @@ void main() { users.map((expected) => (it) => it.fullName.equals(expected.fullName))); })); + test('GlobalStore.perAccount on INVALID_API_KEY', () => awaitFakeAsync((async) async { + addTearDown(testBinding.reset); + + await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false)); + testBinding.globalStore.loadPerAccountException = ZulipApiException( + routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400, + data: {}, message: ''); + await check(testBinding.globalStore.perAccount(eg.selfAccount.id)) + .throws(); + + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + })); + // TODO test UpdateMachine.load starts polling loop // TODO test UpdateMachine.load calls registerNotificationToken }); @@ -843,6 +858,25 @@ void main() { check(store.debugMessageListViews).isEmpty(); })); + test('log out if fail to reload on unexpected errors', () => awaitFakeAsync((async) async { + await preparePoll(); + + prepareUnexpectedLoopError(); + updateMachine.debugAdvanceLoop(); + async.elapse(Duration.zero); + check(store).isLoading.isTrue(); + + globalStore.loadPerAccountException = ZulipApiException( + routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400, + data: {}, message: ''); + // The reload doesn't happen immediately; there's a timer. + check(async.pendingTimers).length.equals(1); + async.flushTimers(); + + check(globalStore.takeDoRemoveAccountCalls().single) + .equals(eg.selfAccount.id); + })); + group('report error', () { String? lastReportedError; String? takeLastReportedError() { diff --git a/test/model/test_store.dart b/test/model/test_store.dart index 2322e78d7a..de94ab2487 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -126,6 +126,7 @@ class TestGlobalStore extends GlobalStore { static const Duration removeAccountDuration = Duration(milliseconds: 1); Duration? loadPerAccountDuration; + Object? loadPerAccountException; /// Consume the log of calls made to [doRemoveAccount]. List takeDoRemoveAccountCalls() { @@ -147,6 +148,9 @@ class TestGlobalStore extends GlobalStore { if (loadPerAccountDuration != null) { await Future.delayed(loadPerAccountDuration!); } + if (loadPerAccountException != null) { + throw loadPerAccountException!; + } final initialSnapshot = _initialSnapshots[accountId]!; final store = PerAccountStore.fromInitialSnapshot( globalStore: this, diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index a53d07a044..9d1fbc8f31 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -3,8 +3,10 @@ import 'dart:async'; import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/exception.dart'; import 'package:zulip/log.dart'; import 'package:zulip/model/database.dart'; +import 'package:zulip/widgets/actions.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/home.dart'; import 'package:zulip/widgets/page.dart'; @@ -57,6 +59,99 @@ void main() { }); }); + group('_EmptyStackNavigatorObserver', () { + late List> pushedRoutes; + late List> removedRoutes; + + Future prepare(WidgetTester tester) async { + addTearDown(testBinding.reset); + + pushedRoutes = []; + removedRoutes = []; + final testNavObserver = TestNavigatorObserver(); + testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); + testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route); + + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + await tester.pump(); // start to load account + check(pushedRoutes).single.isA().page.isA(); + pushedRoutes.clear(); + } + + testWidgets('do not push route to non-empty navigator stack', (tester) async { + const loadPerAccountDuration = Duration(seconds: 30); + assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod); + testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration; + testBinding.globalStore.loadPerAccountException = ZulipApiException( + routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400, + data: {}, message: ''); + await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false)); + await prepare(tester); + + await tester.pump(kTryAnotherAccountWaitPeriod); + await tester.tap(find.text('Try another account')); + await tester.pump(); // tap the button + check(pushedRoutes).single.isA().page.isA(); + pushedRoutes.clear(); + + await tester.pump(loadPerAccountDuration); // got the error + await tester.pump(TestGlobalStore.removeAccountDuration); + check(pushedRoutes).single.isA>(); + pushedRoutes.clear(); + check(removedRoutes).single.isA().page.isA(); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: 'Could not connect', + expectedMessage: + 'Your account at https://chat.example/ could not be authenticated.' + ' Please try logging in again or use another account.'))); + // No more routes are pushed after dismissing the error dialog, + // because the navigator stack was non-empty. + check(pushedRoutes).isEmpty(); + }); + + testWidgets('push route when popping last route on stack', (tester) async { + testBinding.globalStore.loadPerAccountDuration = Duration.zero; + testBinding.globalStore.loadPerAccountException = ZulipApiException( + routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400, + data: {}, message: ''); + await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false)); + await prepare(tester); + + await tester.pump(Duration.zero); // got the error + await tester.pump(TestGlobalStore.removeAccountDuration); + check(pushedRoutes).single.isA>(); + pushedRoutes.clear(); + check(removedRoutes).single.isA().page.isA(); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: 'Could not connect', + expectedMessage: + 'Your account at https://chat.example/ could not be authenticated.' + ' Please try logging in again or use another account.'))); + // The navigator stack became empty after dismissing the error dialog, + // so a choose-account page route was pushed. + check(pushedRoutes).single.isA().page.isA(); + }); + + testWidgets('push route when removing last route on stack', (tester) async { + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await prepare(tester); + + final future = logOutAccount(testBinding.globalStore, eg.selfAccount.id); + await tester.pump(TestGlobalStore.removeAccountDuration); + await future; + check(pushedRoutes).single.isA().page.isA(); + check(removedRoutes).single.isA().page.isA(); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + }); + }); + group('ChooseAccountPage', () { Future setupChooseAccountPage(WidgetTester tester, { required List accounts, diff --git a/test/widgets/store_test.dart b/test/widgets/store_test.dart index e2d7821ae3..b4892aa42d 100644 --- a/test/widgets/store_test.dart +++ b/test/widgets/store_test.dart @@ -1,13 +1,21 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/exception.dart'; import 'package:zulip/model/store.dart'; +import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/home.dart'; +import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/store.dart'; import '../flutter_checks.dart'; import '../model/binding.dart'; import '../example_data.dart' as eg; import '../model/store_checks.dart'; +import '../model/test_store.dart'; +import '../test_navigation.dart'; +import 'dialog_checks.dart'; +import 'page_checks.dart'; /// A widget whose state uses [PerAccountStoreAwareStateMixin]. class MyWidgetWithMixin extends StatefulWidget { @@ -113,6 +121,87 @@ void main() { .contains('consider MaterialAccountWidgetRoute'); }); + testWidgets('PerAccountStoreWidget when log out, do not push route to non-empty navigator stack', (tester) async { + addTearDown(testBinding.reset); + + final testNavObserver = TestNavigatorObserver(); + final pushedRoutes = >[]; + testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); + + const loadPerAccountDuration = Duration(seconds: 30); + assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod); + testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration; + testBinding.globalStore.loadPerAccountException = ZulipApiException( + routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400, + data: {}, message: ''); + await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false)); + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + await tester.pump(); // start to load account + check(pushedRoutes).single.isA().page.isA(); + pushedRoutes.clear(); + + final removedRoutes = >[]; + testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route); + await tester.pump(kTryAnotherAccountWaitPeriod); + await tester.tap(find.text('Try another account')); + await tester.pump(); // tap the button + check(pushedRoutes).single.isA().page.isA(); + pushedRoutes.clear(); + + await tester.pump(loadPerAccountDuration); // got the error + await tester.pump(TestGlobalStore.removeAccountDuration); + check(pushedRoutes).single.isA>(); + pushedRoutes.clear(); + check(removedRoutes).single.isA().page.isA(); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: 'Could not connect', + expectedMessage: + 'Your account at https://chat.example/ could not be authenticated.' + ' Please try logging in again or use another account.'))); + // No more routes are pushed after dismissing the error dialog. + check(pushedRoutes).isEmpty(); + }); + + testWidgets('PerAccountStoreWidget when log out, push route when popping root level route', (tester) async { + addTearDown(testBinding.reset); + + final testNavObserver = TestNavigatorObserver(); + final pushedRoutes = >[]; + testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); + + testBinding.globalStore.loadPerAccountDuration = Duration.zero; + testBinding.globalStore.loadPerAccountException = ZulipApiException( + routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400, + data: {}, message: ''); + await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false)); + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + await tester.pump(); // start to load account + check(pushedRoutes).single.isA().page.isA(); + pushedRoutes.clear(); + + final removedRoutes = >[]; + testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route); + await tester.pump(); // got the error + await tester.pump(TestGlobalStore.removeAccountDuration); + check(pushedRoutes).single.isA>(); + pushedRoutes.clear(); + check(removedRoutes).single.isA().page.isA(); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + + await tester.tap(find.byWidget(checkErrorDialog(tester, + expectedTitle: 'Could not connect', + expectedMessage: + 'Your account at https://chat.example/ could not be authenticated.' + ' Please try logging in again or use another account.'))); + // The navigator stack became empty after dismissing the error dialog, + // so a choose-account page route was pushed. + check(pushedRoutes).single.isA().page.isA(); + }); + testWidgets('PerAccountStoreWidget immediate data after first loaded', (tester) async { await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); addTearDown(testBinding.reset);