From e95e112f909ff9c08253328e83b84dba8fcca75a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 24 Oct 2024 18:31:03 -0700 Subject: [PATCH 1/7] app: Update three-dots button to use new M3 MenuAnchor widget Greg spotted this opportunity to be a bit more modern: https://github.com/zulip/zulip-flutter/pull/1010#discussion_r1807138445 --- lib/widgets/app.dart | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 7091a2a927..7283311b37 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -275,24 +275,32 @@ class ChooseAccountPage extends StatelessWidget { } } -enum ChooseAccountPageOverflowMenuItem { aboutZulip } - class ChooseAccountPageOverflowButton extends StatelessWidget { const ChooseAccountPageOverflowButton({super.key}); @override Widget build(BuildContext context) { - return PopupMenuButton( - itemBuilder: (BuildContext context) => const [ - PopupMenuItem( - value: ChooseAccountPageOverflowMenuItem.aboutZulip, - child: Text('About Zulip')), - ], - onSelected: (item) { - switch (item) { - case ChooseAccountPageOverflowMenuItem.aboutZulip: + final designVariables = DesignVariables.of(context); + final materialLocalizations = MaterialLocalizations.of(context); + return MenuAnchor( + menuChildren: [ + MenuItemButton( + onPressed: () { Navigator.push(context, AboutZulipPage.buildRoute(context)); - } + }, + child: const Text('About Zulip')), // TODO(i18n) + ], + builder: (BuildContext context, MenuController controller, Widget? child) { + return IconButton( + tooltip: materialLocalizations.showMenuTooltip, // "Show menu" + onPressed: () { + if (controller.isOpen) { + controller.close(); + } else { + controller.open(); + } + }, + icon: Icon(Icons.adaptive.more, color: designVariables.icon)); }); } } From 89adaf3b15923aae06c7d65c187f63d902c8e27f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 5 Sep 2024 16:49:01 -0700 Subject: [PATCH 2/7] api: Add remove{Apns,Fcm}Token --- lib/api/route/notifications.dart | 19 +++++++++++- test/api/route/notifications_test.dart | 42 ++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/lib/api/route/notifications.dart b/lib/api/route/notifications.dart index 26c022afc7..a8c7b23b36 100644 --- a/lib/api/route/notifications.dart +++ b/lib/api/route/notifications.dart @@ -1,4 +1,3 @@ - import '../core.dart'; /// https://zulip.com/api/add-fcm-token @@ -10,6 +9,15 @@ Future addFcmToken(ApiConnection connection, { }); } +/// https://zulip.com/api/remove-fcm-token +Future removeFcmToken(ApiConnection connection, { + required String token, +}) { + return connection.delete('removeFcmToken', (_) {}, 'users/me/android_gcm_reg_id', { + 'token': RawParameter(token), + }); +} + /// https://zulip.com/api/add-apns-token Future addApnsToken(ApiConnection connection, { required String token, @@ -20,3 +28,12 @@ Future addApnsToken(ApiConnection connection, { 'appid': RawParameter(appid), }); } + +/// https://zulip.com/api/remove-apns-token +Future removeApnsToken(ApiConnection connection, { + required String token, +}) { + return connection.delete('removeApnsToken', (_) {}, 'users/me/apns_device_token', { + 'token': RawParameter(token), + }); +} diff --git a/test/api/route/notifications_test.dart b/test/api/route/notifications_test.dart index 8c11dc12d0..bc3d16d786 100644 --- a/test/api/route/notifications_test.dart +++ b/test/api/route/notifications_test.dart @@ -28,6 +28,27 @@ void main() { }); }); + group('removeFcmToken', () { + Future checkRemoveFcmToken(FakeApiConnection connection, { + required String token, + }) async { + connection.prepare(json: {}); + await removeFcmToken(connection, token: token); + check(connection.lastRequest).isA() + ..method.equals('DELETE') + ..url.path.equals('/api/v1/users/me/android_gcm_reg_id') + ..bodyFields.deepEquals({ + 'token': token, + }); + } + + test('smoke', () { + return FakeApiConnection.with_((connection) async { + await checkRemoveFcmToken(connection, token: 'asdf'); + }); + }); + }); + group('addApnsToken', () { Future checkAddApnsToken(FakeApiConnection connection, { required String token, @@ -50,4 +71,25 @@ void main() { }); }); }); + + group('removeApnsToken', () { + Future checkRemoveApnsToken(FakeApiConnection connection, { + required String token, + }) async { + connection.prepare(json: {}); + await removeApnsToken(connection, token: token); + check(connection.lastRequest).isA() + ..method.equals('DELETE') + ..url.path.equals('/api/v1/users/me/apns_device_token') + ..bodyFields.deepEquals({ + 'token': token, + }); + } + + test('smoke', () { + return FakeApiConnection.with_((connection) async { + await checkRemoveApnsToken(connection, token: 'asdf'); + }); + }); + }); } From f830e8c1c4182702e58038fe7e60e1ba9d57ffe2 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 10 Sep 2024 11:39:19 -0700 Subject: [PATCH 3/7] notif: Implement NotificationService.unregisterToken --- lib/notifications/receive.dart | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/notifications/receive.dart b/lib/notifications/receive.dart index 23ca31d414..cc24d763d4 100644 --- a/lib/notifications/receive.dart +++ b/lib/notifications/receive.dart @@ -159,6 +159,22 @@ class NotificationService { } } + static Future unregisterToken(ApiConnection connection, {required String token}) async { + switch (defaultTargetPlatform) { + case TargetPlatform.android: + await removeFcmToken(connection, token: token); + + case TargetPlatform.iOS: + await removeApnsToken(connection, token: token); + + case TargetPlatform.linux: + case TargetPlatform.macOS: + case TargetPlatform.windows: + case TargetPlatform.fuchsia: + assert(false); + } + } + static void _onForegroundMessage(FirebaseRemoteMessage message) { assert(debugLog("notif message: ${message.data}")); _onRemoteMessage(message); From 7aff885706d0378fd040426d672e4ec24a3d42f6 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 28 Oct 2024 18:47:02 -0700 Subject: [PATCH 4/7] dialog: Dismiss suggested-action dialog on pressing action button So far this function just has two callers in uncommon paths involving app permissions. For those, and for the new caller we're about to add -- logging out an account -- it's unhelpful for the dialog to linger unchanged after the action button is tapped. Better to make it go away, preventing confusion about whether the action was actually done. --- lib/widgets/dialog.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index c6bdc4ef65..27d8b1da27 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -71,7 +71,10 @@ void showSuggestedActionDialog({ onPressed: () => Navigator.pop(context), child: _dialogActionText(zulipLocalizations.dialogCancel)), TextButton( - onPressed: onActionButtonPress, + onPressed: () { + onActionButtonPress(); + Navigator.pop(context); + }, child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)), ])); } From 38213a8f8a414bfaf87bfea0bb420ded0003de79 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 8 Nov 2024 20:23:38 -0800 Subject: [PATCH 5/7] widget tests: Add `checkSuggestedActionDialog` Like we added `checkErrorDialog` in 02fd562b8. --- test/widgets/dialog_checks.dart | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/widgets/dialog_checks.dart b/test/widgets/dialog_checks.dart index 84593fb59d..504042d07e 100644 --- a/test/widgets/dialog_checks.dart +++ b/test/widgets/dialog_checks.dart @@ -1,5 +1,6 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/widgets/dialog.dart'; /// In a widget test, check that showErrorDialog was called with the right text. /// @@ -24,3 +25,34 @@ Widget checkErrorDialog(WidgetTester tester, { find.descendant(of: find.byWidget(dialog), matching: find.widgetWithText(TextButton, 'OK'))); } + +/// In a widget test, check that [showSuggestedActionDialog] was called +/// with the right text. +/// +/// Checks for a suggested-action dialog matching an expected title and message. +/// Fails if none is found. +/// +/// On success, returns a Record with the widget's action button first +/// and its cancel button second. +/// Tap the action button by calling `tester.tap(find.byWidget(actionButton))`. +(Widget, Widget) checkSuggestedActionDialog(WidgetTester tester, { + required String expectedTitle, + required String expectedMessage, + String? expectedActionButtonText, +}) { + final dialog = tester.widget(find.byType(AlertDialog)); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.title!), matching: find.text(expectedTitle))); + tester.widget(find.descendant(matchRoot: true, + of: find.byWidget(dialog.content!), matching: find.text(expectedMessage))); + + final actionButton = tester.widget( + find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(TextButton, expectedActionButtonText ?? 'Continue'))); + + final cancelButton = tester.widget( + find.descendant(of: find.byWidget(dialog), + matching: find.widgetWithText(TextButton, 'Cancel'))); + + return (actionButton, cancelButton); +} From fcccb94b6d375d4caf9d2aa68d1256e3e6c05d9c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 7 Nov 2024 18:44:17 -0800 Subject: [PATCH 6/7] test store: Make TestGlobalStore.doInsertAccount not drop ackedPushToken Without this line, if a test `globalStore.add`s an account with a non-null `ackedPushToken`, the field will be null in app code or test code that retrieves the account from `globalStore`. Thankfully no tests have been doing that, but it'll be convenient to do that soon, for testing the unregister-token part of logging out. --- test/model/test_store.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/test/model/test_store.dart b/test/model/test_store.dart index d6e201a6ad..ad6d7a729b 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -111,6 +111,7 @@ class TestGlobalStore extends GlobalStore { zulipFeatureLevel: data.zulipFeatureLevel.value, zulipVersion: data.zulipVersion.value, zulipMergeBase: data.zulipMergeBase.value, + ackedPushToken: data.ackedPushToken.value, ); } From 0344686c73d6324791f7251704af0adc273fa405 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 8 Nov 2024 20:29:53 -0800 Subject: [PATCH 7/7] login: Support logging out of an account Fixes: #463 --- assets/l10n/app_en.arb | 16 +++ lib/widgets/actions.dart | 37 +++++ lib/widgets/app.dart | 36 +++++ lib/widgets/page.dart | 1 + lib/widgets/store.dart | 24 +++- test/model/test_store.dart | 18 ++- test/widgets/actions_test.dart | 237 ++++++++++++++++++++++++++++++++- test/widgets/app_test.dart | 36 +++++ 8 files changed, 399 insertions(+), 6 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index aea0c83d38..c14d06fa22 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -19,6 +19,22 @@ "@chooseAccountPageTitle": { "description": "Title for ChooseAccountPage" }, + "chooseAccountPageLogOutButton": "Log out", + "@chooseAccountPageLogOutButton": { + "description": "Label for the 'Log out' button for an account on the choose-account page" + }, + "logOutConfirmationDialogTitle": "Log out?", + "@logOutConfirmationDialogTitle": { + "description": "Title for a confirmation dialog for logging out." + }, + "logOutConfirmationDialogMessage": "To use this account in the future, you will have to re-enter the URL for your organization and your account information.", + "@logOutConfirmationDialogMessage": { + "description": "Message for a confirmation dialog for logging out." + }, + "logOutConfirmationDialogConfirmButton": "Log out", + "@logOutConfirmationDialogConfirmButton": { + "description": "Label for the 'Log out' button on a confirmation dialog for logging out." + }, "chooseAccountButtonAddAnAccount": "Add an account", "@chooseAccountButtonAddAnAccount": { "description": "Label for ChooseAccountPage button to add an account" diff --git a/lib/widgets/actions.dart b/lib/widgets/actions.dart index 934e04934e..c9b7fdcb48 100644 --- a/lib/widgets/actions.dart +++ b/lib/widgets/actions.dart @@ -6,6 +6,8 @@ /// in order to present success or error feedback to the user through the UI. library; +import 'dart:async'; + import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; @@ -13,9 +15,44 @@ import '../api/model/model.dart'; import '../api/model/narrow.dart'; import '../api/route/messages.dart'; import '../model/narrow.dart'; +import '../model/store.dart'; +import '../notifications/receive.dart'; import 'dialog.dart'; import 'store.dart'; +Future logOutAccount(BuildContext context, int accountId) async { + final globalStore = GlobalStoreWidget.of(context); + + final account = globalStore.getAccount(accountId); + if (account == null) return; // TODO(log) + + // Unawaited, to not block removing the account on this request. + unawaited(unregisterToken(globalStore, accountId)); + + await globalStore.removeAccount(accountId); +} + +Future unregisterToken(GlobalStore globalStore, int accountId) async { + final account = globalStore.getAccount(accountId); + if (account == null) return; // TODO(log) + + // TODO(#322) use actual acked push token; until #322, this is just null. + final token = account.ackedPushToken + // Try the current token as a fallback; maybe the server has registered + // it and we just haven't recorded that fact in the client. + ?? NotificationService.instance.token.value; + if (token == null) return; + + final connection = globalStore.apiConnectionFromAccount(account); + try { + await NotificationService.unregisterToken(connection, token: token); + } catch (e) { + // TODO retry? handle failures? + } finally { + connection.close(); + } +} + Future markNarrowAsRead(BuildContext context, Narrow narrow) async { final store = PerAccountStoreWidget.of(context); final connection = store.connection; diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 7283311b37..d0944d663b 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -8,8 +8,10 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import '../log.dart'; import '../model/localizations.dart'; import '../model/narrow.dart'; +import '../model/store.dart'; import '../notifications/display.dart'; import 'about_zulip.dart'; +import 'actions.dart'; import 'app_bar.dart'; import 'dialog.dart'; import 'inbox.dart'; @@ -232,11 +234,45 @@ class ChooseAccountPage extends StatelessWidget { required Widget title, Widget? subtitle, }) { + final designVariables = DesignVariables.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); + final materialLocalizations = MaterialLocalizations.of(context); return Card( clipBehavior: Clip.hardEdge, child: ListTile( title: title, subtitle: subtitle, + trailing: MenuAnchor( + menuChildren: [ + MenuItemButton( + onPressed: () { + showSuggestedActionDialog(context: context, + title: zulipLocalizations.logOutConfirmationDialogTitle, + message: zulipLocalizations.logOutConfirmationDialogMessage, + // TODO(#1032) "destructive" style for action button + actionButtonText: zulipLocalizations.logOutConfirmationDialogConfirmButton, + onActionButtonPress: () { + // TODO error handling if db write fails? + logOutAccount(context, accountId); + }); + }, + child: Text(zulipLocalizations.chooseAccountPageLogOutButton)), + ], + builder: (BuildContext context, MenuController controller, Widget? child) { + return IconButton( + tooltip: materialLocalizations.showMenuTooltip, // "Show menu" + onPressed: () { + if (controller.isOpen) { + controller.close(); + } else { + controller.open(); + } + }, + icon: Icon(Icons.adaptive.more, color: designVariables.icon)); + }), + // The default trailing padding with M3 is 24px. Decrease by 12 because + // IconButton (the "…" button) comes with 12px padding on all sides. + contentPadding: const EdgeInsetsDirectional.only(start: 16, end: 12), onTap: () => Navigator.push(context, HomePage.buildRoute(accountId: accountId)))); } diff --git a/lib/widgets/page.dart b/lib/widgets/page.dart index a039769d4f..2008c627a1 100644 --- a/lib/widgets/page.dart +++ b/lib/widgets/page.dart @@ -40,6 +40,7 @@ mixin AccountPageRouteMixin on PageRoute { return PerAccountStoreWidget( accountId: accountId, placeholder: const LoadingPlaceholderPage(), + routeToRemoveOnLogout: this, child: super.buildPage(context, animation, secondaryAnimation)); } } diff --git a/lib/widgets/store.dart b/lib/widgets/store.dart index 12100efd03..1d37bcfd17 100644 --- a/lib/widgets/store.dart +++ b/lib/widgets/store.dart @@ -1,7 +1,9 @@ import 'package:flutter/material.dart'; +import 'package:flutter/scheduler.dart'; import '../model/binding.dart'; import '../model/store.dart'; +import 'page.dart'; /// Provides access to the app's data. /// @@ -112,11 +114,19 @@ class PerAccountStoreWidget extends StatefulWidget { super.key, required this.accountId, this.placeholder = const LoadingPlaceholder(), + this.routeToRemoveOnLogout, required this.child, }); final int accountId; final Widget placeholder; + + /// A per-account [Route] that should be removed on logout. + /// + /// Use this when the widget is a page on a route that should go away + /// when the account is logged out, instead of lingering with [placeholder]. + final AccountPageRouteMixin? routeToRemoveOnLogout; + final Widget child; /// The user's data for the relevant Zulip account for this widget. @@ -195,6 +205,16 @@ class _PerAccountStoreWidgetState extends State { void didChangeDependencies() { super.didChangeDependencies(); final globalStore = GlobalStoreWidget.of(context); + final accountExists = globalStore.getAccount(widget.accountId) != null; + if (!accountExists) { + // logged out + _setStore(null); + if (widget.routeToRemoveOnLogout != null) { + SchedulerBinding.instance.addPostFrameCallback( + (_) => Navigator.of(context).removeRoute(widget.routeToRemoveOnLogout!)); + } + return; + } // If we already have data, get it immediately. This avoids showing one // frame of loading indicator each time we have a new PerAccountStoreWidget. final store = globalStore.perAccountSync(widget.accountId); @@ -212,13 +232,13 @@ class _PerAccountStoreWidgetState extends State { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI - // (for example by removing a per-account route from the nav). + // (usually by using [routeToRemoveOnLogout]). } }(); } } - void _setStore(PerAccountStore store) { + void _setStore(PerAccountStore? store) { if (store != this.store) { setState(() { this.store = store; diff --git a/test/model/test_store.dart b/test/model/test_store.dart index ad6d7a729b..964c22669f 100644 --- a/test/model/test_store.dart +++ b/test/model/test_store.dart @@ -47,6 +47,10 @@ class TestGlobalStore extends GlobalStore { /// but that breach is sometimes convenient for tests. bool useCachedApiConnections = false; + void clearCachedApiConnections() { + _apiConnections.clear(); + } + /// Get or construct a [FakeApiConnection] with the given arguments. /// /// To access the same [FakeApiConnection] that the code under test will get, @@ -120,9 +124,21 @@ class TestGlobalStore extends GlobalStore { // Nothing to do. } + static const Duration removeAccountDuration = Duration(milliseconds: 1); + + /// Consume the log of calls made to [doRemoveAccount]. + List takeDoRemoveAccountCalls() { + final result = _doRemoveAccountCalls; + _doRemoveAccountCalls = null; + return result ?? []; + } + List? _doRemoveAccountCalls; + @override Future doRemoveAccount(int accountId) async { - // Nothing to do. + (_doRemoveAccountCalls ??= []).add(accountId); + await Future.delayed(removeAccountDuration); + // Nothing else to do. } @override diff --git a/test/widgets/actions_test.dart b/test/widgets/actions_test.dart index 68d192b903..8b4a355b3e 100644 --- a/test/widgets/actions_test.dart +++ b/test/widgets/actions_test.dart @@ -1,9 +1,13 @@ +import 'dart:async'; import 'dart:convert'; import 'package:checks/checks.dart'; +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; +import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; +import 'package:zulip/api/exception.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/narrow.dart'; @@ -11,13 +15,20 @@ import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; +import 'package:zulip/notifications/receive.dart'; 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 '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../model/binding.dart'; +import '../model/store_checks.dart'; +import '../model/test_store.dart'; import '../model/unreads_checks.dart'; import '../stdlib_checks.dart'; +import '../test_navigation.dart'; import 'dialog_checks.dart'; import 'test_app.dart'; @@ -30,19 +41,239 @@ void main() { Future prepare(WidgetTester tester, { UnreadMessagesSnapshot? unreadMsgs, + String? ackedPushToken = '123', }) async { addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot( + final selfAccount = eg.selfAccount.copyWith(ackedPushToken: Value(ackedPushToken)); + await testBinding.globalStore.add(selfAccount, eg.initialSnapshot( unreadMsgs: unreadMsgs)); - store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + store = await testBinding.globalStore.perAccount(selfAccount.id); connection = store.connection as FakeApiConnection; - await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id, child: const Scaffold(body: Placeholder()))); await tester.pump(); context = tester.element(find.byType(Placeholder)); } + /// Creates and caches a new [FakeApiConnection] in [TestGlobalStore]. + /// + /// In live code, [unregisterToken] makes a new [ApiConnection] for the + /// unregister-token request instead of reusing the store's connection. + /// To enable callers to prepare responses for that request, this function + /// creates a new [FakeApiConnection] and caches it in [TestGlobalStore] + /// for [unregisterToken] to pick up. + /// + /// Call this instead of just turning on + /// [TestGlobalStore.useCachedApiConnections] so that [unregisterToken] + /// doesn't try to call `close` twice on the same connection instance, + /// which isn't allowed. (Once by the unregister-token code + /// and once as part of removing the account.) + FakeApiConnection separateConnection() { + testBinding.globalStore + ..clearCachedApiConnections() + ..useCachedApiConnections = true; + return testBinding.globalStore + .apiConnectionFromAccount(eg.selfAccount) as FakeApiConnection; + } + + String unregisterApiPathForPlatform(TargetPlatform platform) { + return switch (platform) { + TargetPlatform.android => '/api/v1/users/me/android_gcm_reg_id', + TargetPlatform.iOS => '/api/v1/users/me/apns_device_token', + _ => throw Error(), + }; + } + + void checkSingleUnregisterRequest( + FakeApiConnection connection, { + String? expectedToken, + }) { + final subject = check(connection.takeRequests()).single.isA() + ..method.equals('DELETE') + ..url.path.equals(unregisterApiPathForPlatform(defaultTargetPlatform)); + if (expectedToken != null) { + subject.bodyFields.deepEquals({'token': expectedToken}); + } + } + + group('logOutAccount', () { + testWidgets('smoke', (tester) async { + await prepare(tester); + check(testBinding.globalStore).accountIds.single.equals(eg.selfAccount.id); + const unregisterDelay = Duration(seconds: 5); + assert(unregisterDelay > TestGlobalStore.removeAccountDuration); + final newConnection = separateConnection() + ..prepare(delay: unregisterDelay, json: {'msg': '', 'result': 'success'}); + + final future = logOutAccount(context, eg.selfAccount.id); + // Unregister-token request and account removal dispatched together + checkSingleUnregisterRequest(newConnection); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + + await tester.pump(TestGlobalStore.removeAccountDuration); + await future; + // Account removal not blocked on unregister-token response + check(testBinding.globalStore).accountIds.isEmpty(); + check(connection.isOpen).isFalse(); + check(newConnection.isOpen).isTrue(); // still busy with unregister-token + + await tester.pump(unregisterDelay - TestGlobalStore.removeAccountDuration); + check(newConnection.isOpen).isFalse(); + }); + + testWidgets('unregister request has an error', (tester) async { + await prepare(tester); + check(testBinding.globalStore).accountIds.single.equals(eg.selfAccount.id); + const unregisterDelay = Duration(seconds: 5); + assert(unregisterDelay > TestGlobalStore.removeAccountDuration); + final exception = ZulipApiException( + httpStatus: 401, + code: 'UNAUTHORIZED', + data: {"result": "error", "msg": "Invalid API key", "code": "UNAUTHORIZED"}, + routeName: 'removeEtcEtcToken', + message: 'Invalid API key', + ); + final newConnection = separateConnection() + ..prepare(delay: unregisterDelay, exception: exception); + + final future = logOutAccount(context, eg.selfAccount.id); + // Unregister-token request and account removal dispatched together + checkSingleUnregisterRequest(newConnection); + check(testBinding.globalStore.takeDoRemoveAccountCalls()) + .single.equals(eg.selfAccount.id); + + await tester.pump(TestGlobalStore.removeAccountDuration); + await future; + // Account removal not blocked on unregister-token response + check(testBinding.globalStore).accountIds.isEmpty(); + check(connection.isOpen).isFalse(); + check(newConnection.isOpen).isTrue(); // for the unregister-token request + + await tester.pump(unregisterDelay - TestGlobalStore.removeAccountDuration); + check(newConnection.isOpen).isFalse(); + }); + + testWidgets("logged-out account's routes removed from nav; other accounts' remain", (tester) async { + Future makeUnreadTopicInInbox(int accountId, String topic) async { + final stream = eg.stream(); + final message = eg.streamMessage(stream: stream, topic: topic); + final store = await testBinding.globalStore.perAccount(accountId); + await store.addStream(stream); + await store.addSubscription(eg.subscription(stream)); + await store.addMessage(message); + await tester.pump(); + } + + addTearDown(testBinding.reset); + + final account1 = eg.account(id: 1, user: eg.user()); + final account2 = eg.account(id: 2, user: eg.user()); + await testBinding.globalStore.add(account1, eg.initialSnapshot()); + await testBinding.globalStore.add(account2, eg.initialSnapshot()); + + final testNavObserver = TestNavigatorObserver(); + await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver])); + await tester.pump(); + final navigator = await ZulipApp.navigator; + navigator.popUntil((_) => false); // clear starting routes + await tester.pumpAndSettle(); + + final pushedRoutes = >[]; + testNavObserver.onPushed = (route, prevRoute) => pushedRoutes.add(route); + final account1Route = InboxPage.buildRoute(accountId: account1.id); + final account2Route = InboxPage.buildRoute(accountId: account2.id); + unawaited(navigator.push(account1Route)); + unawaited(navigator.push(account2Route)); + await tester.pumpAndSettle(); + check(pushedRoutes).deepEquals([account1Route, account2Route]); + + await makeUnreadTopicInInbox(account1.id, 'topic in account1'); + final findAccount1PageContent = find.text('topic in account1', skipOffstage: false); + + await makeUnreadTopicInInbox(account2.id, 'topic in account2'); + final findAccount2PageContent = find.text('topic in account2', skipOffstage: false); + + final findLoadingPage = find.byType(LoadingPlaceholderPage, skipOffstage: false); + + check(findAccount1PageContent).findsOne(); + check(findLoadingPage).findsNothing(); + + final removedRoutes = >[]; + testNavObserver.onRemoved = (route, prevRoute) => removedRoutes.add(route); + + final context = tester.element(find.byType(MaterialApp)); + final future = logOutAccount(context, account1.id); + await tester.pump(TestGlobalStore.removeAccountDuration); + await future; + check(removedRoutes).single.identicalTo(account1Route); + check(findAccount1PageContent).findsNothing(); + check(findLoadingPage).findsOne(); + + await tester.pump(); + check(findAccount1PageContent).findsNothing(); + check(findLoadingPage).findsNothing(); + check(findAccount2PageContent).findsOne(); + }); + }); + + group('unregisterToken', () { + testWidgets('smoke, happy path', (tester) async { + await prepare(tester, ackedPushToken: '123'); + + final newConnection = separateConnection() + ..prepare(json: {'msg': '', 'result': 'success'}); + final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id); + await tester.pump(Duration.zero); + await future; + checkSingleUnregisterRequest(newConnection, expectedToken: '123'); + check(newConnection.isOpen).isFalse(); + }, variant: const TargetPlatformVariant({TargetPlatform.android, TargetPlatform.iOS})); + + testWidgets('fallback to current token if acked is missing', (tester) async { + await prepare(tester, ackedPushToken: null); + NotificationService.instance.token = ValueNotifier('asdf'); + + final newConnection = separateConnection() + ..prepare(json: {'msg': '', 'result': 'success'}); + final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id); + await tester.pump(Duration.zero); + await future; + checkSingleUnregisterRequest(newConnection, expectedToken: 'asdf'); + check(newConnection.isOpen).isFalse(); + }); + + testWidgets('no error if acked token and current token both missing', (tester) async { + await prepare(tester, ackedPushToken: null); + NotificationService.instance.token = ValueNotifier(null); + + final newConnection = separateConnection(); + final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id); + await tester.pumpAndSettle(); + await future; + check(newConnection.takeRequests()).isEmpty(); + }); + + testWidgets('connection closed if request errors', (tester) async { + await prepare(tester, ackedPushToken: '123'); + + final newConnection = separateConnection() + ..prepare(exception: ZulipApiException( + httpStatus: 401, + code: 'UNAUTHORIZED', + data: {"result": "error", "msg": "Invalid API key", "code": "UNAUTHORIZED"}, + routeName: 'removeEtcEtcToken', + message: 'Invalid API key', + )); + final future = unregisterToken(testBinding.globalStore, eg.selfAccount.id); + await tester.pump(Duration.zero); + await future; + checkSingleUnregisterRequest(newConnection, expectedToken: '123'); + check(newConnection.isOpen).isFalse(); + }); + }); + group('markNarrowAsRead', () { testWidgets('smoke test on modern server', (tester) async { final narrow = TopicNarrow.ofMessage(eg.streamMessage()); diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index 683298f017..c21ac1915e 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -10,6 +10,8 @@ import 'package:zulip/widgets/page.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 'dialog_checks.dart'; import 'page_checks.dart'; @@ -158,6 +160,40 @@ void main() { ..top.isGreaterThan(1 / 3 * screenHeight) ..bottom.isLessThan(2 / 3 * screenHeight); }); + + group('log out', () { + Future<(Widget, Widget)> prepare(WidgetTester tester, {required Account account}) async { + await setupChooseAccountPage(tester, accounts: [account]); + + final findThreeDotsButton = find.descendant( + of: find.widgetWithText(Card, eg.selfAccount.realmUrl.toString()), + matching: find.byIcon(Icons.adaptive.more)); + + await tester.tap(findThreeDotsButton); + await tester.pump(); + await tester.tap(find.descendant( + of: find.byType(MenuItemButton), matching: find.text('Log out'))); + await tester.pumpAndSettle(); // TODO just `pump`? But the dialog doesn't appear. + return checkSuggestedActionDialog(tester, + expectedTitle: 'Log out?', + expectedMessage: 'To use this account in the future, you will have to re-enter the URL for your organization and your account information.', + expectedActionButtonText: 'Log out'); + } + + testWidgets('user confirms logging out', (tester) async { + final (actionButton, _) = await prepare(tester, account: eg.selfAccount); + await tester.tap(find.byWidget(actionButton)); + await tester.pump(TestGlobalStore.removeAccountDuration); + check(testBinding.globalStore).accounts.isEmpty(); + }); + + testWidgets('user cancels logging out', (tester) async { + final (_, cancelButton) = await prepare(tester, account: eg.selfAccount); + await tester.tap(find.byWidget(cancelButton)); + await tester.pumpAndSettle(); + check(testBinding.globalStore).accounts.deepEquals([eg.selfAccount]); + }); + }); }); group('scaffoldMessenger', () {