Skip to content

Commit

Permalink
login: Support logging out of an account
Browse files Browse the repository at this point in the history
Fixes: #463
  • Loading branch information
chrisbobbe authored and gnprice committed Nov 14, 2024
1 parent fcccb94 commit 0344686
Show file tree
Hide file tree
Showing 8 changed files with 399 additions and 6 deletions.
16 changes: 16 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
37 changes: 37 additions & 0 deletions lib/widgets/actions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,53 @@
/// 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';

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<void> 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<void> 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<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
final store = PerAccountStoreWidget.of(context);
final connection = store.connection;
Expand Down
36 changes: 36 additions & 0 deletions lib/widgets/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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))));
}
Expand Down
1 change: 1 addition & 0 deletions lib/widgets/page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ mixin AccountPageRouteMixin<T extends Object?> on PageRoute<T> {
return PerAccountStoreWidget(
accountId: accountId,
placeholder: const LoadingPlaceholderPage(),
routeToRemoveOnLogout: this,
child: super.buildPage(context, animation, secondaryAnimation));
}
}
Expand Down
24 changes: 22 additions & 2 deletions lib/widgets/store.dart
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -195,6 +205,16 @@ class _PerAccountStoreWidgetState extends State<PerAccountStoreWidget> {
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);
Expand All @@ -212,13 +232,13 @@ class _PerAccountStoreWidgetState extends State<PerAccountStoreWidget> {
// 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;
Expand Down
18 changes: 17 additions & 1 deletion test/model/test_store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<int> takeDoRemoveAccountCalls() {
final result = _doRemoveAccountCalls;
_doRemoveAccountCalls = null;
return result ?? [];
}
List<int>? _doRemoveAccountCalls;

@override
Future<void> doRemoveAccount(int accountId) async {
// Nothing to do.
(_doRemoveAccountCalls ??= []).add(accountId);
await Future<void>.delayed(removeAccountDuration);
// Nothing else to do.
}

@override
Expand Down
Loading

0 comments on commit 0344686

Please sign in to comment.