Skip to content

Commit 3c3491c

Browse files
notif: Use associated account as initial account; if opened from background
Previously, when two accounts (Account-1 and Account-2) were logged in, the app always defaulted to showing the home page of Account-1 on launch. If the app was closed and the user opened a notification from Account-2, the navigation stack would be: HomePage(Account-1) -> MessageListPage(Account-2) This commit fixes that behaviour, now when a notification is opened while the app is closed, the home page will correspond to the account associated with the notification's conversation. This addresses #1210 for background notifications.
1 parent 1c39db7 commit 3c3491c

File tree

3 files changed

+100
-26
lines changed

3 files changed

+100
-26
lines changed

lib/notifications/display.dart

+49-15
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import '../log.dart';
1414
import '../model/binding.dart';
1515
import '../model/localizations.dart';
1616
import '../model/narrow.dart';
17+
import '../model/store.dart';
1718
import '../widgets/app.dart';
1819
import '../widgets/color.dart';
1920
import '../widgets/dialog.dart';
@@ -453,36 +454,69 @@ class NotificationDisplayManager {
453454

454455
static String _personKey(Uri realmUrl, int userId) => "$realmUrl|$userId";
455456

457+
/// Provides the route and the account ID by parsing the notification URL.
458+
///
459+
/// The URL must have been generated using [NotificationOpenPayload.buildUrl]
460+
/// while creating the notification.
461+
///
462+
/// Returns null and shows an error dialog if the associated account is not
463+
/// found in the global store.
464+
static AccountRoute<void>? routeForNotification({
465+
required GlobalStore globalStore,
466+
required Uri url,
467+
}) {
468+
// A helper to avoid making this function an async function,
469+
// allowing it to be called from non-async contexts and where
470+
// BuildContext hasn't been initialized with localizations yet.
471+
void showNotificationErrorDialog() async {
472+
final navigator = await ZulipApp.navigator;
473+
final navigatorContext = navigator.context;
474+
assert(navigatorContext.mounted);
475+
// TODO(linter): this is impossible as there's no actual async gap, but
476+
// the use_build_context_synchronously lint doesn't see that.
477+
if (!navigatorContext.mounted) return;
478+
479+
final zulipLocalizations = ZulipLocalizations.of(navigatorContext);
480+
showErrorDialog(context: navigatorContext,
481+
title: zulipLocalizations.errorNotificationOpenTitle,
482+
message: zulipLocalizations.errorNotificationOpenAccountMissing);
483+
}
484+
485+
assert(debugLog('got notif: url: $url'));
486+
assert(url.scheme == 'zulip' && url.host == 'notification');
487+
final payload = NotificationOpenPayload.parseUrl(url);
488+
489+
final account = globalStore.accounts.firstWhereOrNull((account) =>
490+
account.realmUrl == payload.realmUrl && account.userId == payload.userId);
491+
if (account == null) { // TODO(log)
492+
showNotificationErrorDialog();
493+
return null;
494+
}
495+
496+
return MessageListPage.buildRoute(
497+
accountId: account.id,
498+
// TODO(#82): Open at specific message, not just conversation
499+
narrow: payload.narrow);
500+
}
501+
456502
/// Navigates to the [MessageListPage] of the specific conversation
457503
/// given the `zulip://notification/…` Android intent data URL,
458504
/// generated with [NotificationOpenPayload.buildUrl] while creating
459505
/// the notification.
460506
static Future<void> navigateForNotification(Uri url) async {
461507
assert(debugLog('opened notif: url: $url'));
462508

463-
assert(url.scheme == 'zulip' && url.host == 'notification');
464-
final payload = NotificationOpenPayload.parseUrl(url);
465-
466509
NavigatorState navigator = await ZulipApp.navigator;
467510
final context = navigator.context;
468511
assert(context.mounted);
469512
if (!context.mounted) return; // TODO(linter): this is impossible as there's no actual async gap, but the use_build_context_synchronously lint doesn't see that
470513

471-
final zulipLocalizations = ZulipLocalizations.of(context);
472514
final globalStore = GlobalStoreWidget.of(context);
473-
final account = globalStore.accounts.firstWhereOrNull((account) =>
474-
account.realmUrl == payload.realmUrl && account.userId == payload.userId);
475-
if (account == null) { // TODO(log)
476-
showErrorDialog(context: context,
477-
title: zulipLocalizations.errorNotificationOpenTitle,
478-
message: zulipLocalizations.errorNotificationOpenAccountMissing);
479-
return;
480-
}
515+
final route = routeForNotification(globalStore: globalStore, url: url);
516+
if (route == null) return; // TODO(log)
481517

482518
// TODO(nav): Better interact with existing nav stack on notif open
483-
unawaited(navigator.push(MaterialAccountWidgetRoute<void>(accountId: account.id,
484-
// TODO(#82): Open at specific message, not just conversation
485-
page: MessageListPage(initNarrow: payload.narrow))));
519+
unawaited(navigator.push(route));
486520
}
487521

488522
static Future<Uint8List?> _fetchBitmap(Uri url) async {

lib/widgets/app.dart

+18-9
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,6 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
143143
void initState() {
144144
super.initState();
145145
WidgetsBinding.instance.addObserver(this);
146-
_handleInitialRoute();
147146
}
148147

149148
@override
@@ -156,8 +155,25 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
156155
BuildContext context,
157156
String initialRoute,
158157
) {
159-
// TODO(#524) choose initial account as last one used
160158
final globalStore = GlobalStoreWidget.of(context);
159+
final initialRouteUrl = Uri.tryParse(initialRoute);
160+
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
161+
final route = NotificationDisplayManager.routeForNotification(
162+
globalStore: globalStore,
163+
url: initialRouteUrl);
164+
165+
if (route != null) {
166+
return [
167+
HomePage.buildRoute(accountId: route.accountId),
168+
route,
169+
];
170+
} else {
171+
// The account didn't match any existing accounts,
172+
// fallthrough to show default the route below.
173+
}
174+
}
175+
176+
// TODO(#524) choose initial account as last one used
161177
final initialAccountId = globalStore.accounts.firstOrNull?.id;
162178
return [
163179
if (initialAccountId == null)
@@ -167,13 +183,6 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
167183
];
168184
}
169185

170-
Future<void> _handleInitialRoute() async {
171-
final initialRouteUrl = Uri.parse(WidgetsBinding.instance.platformDispatcher.defaultRouteName);
172-
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
173-
await NotificationDisplayManager.navigateForNotification(initialRouteUrl);
174-
}
175-
}
176-
177186
@override
178187
Future<bool> didPushRouteInformation(routeInformation) async {
179188
switch (routeInformation.uri) {

test/notifications/display_test.dart

+33-2
Original file line numberDiff line numberDiff line change
@@ -960,11 +960,12 @@ void main() {
960960
group('NotificationDisplayManager open', () {
961961
late List<Route<void>> pushedRoutes;
962962

963-
void takeStartingRoutes({bool withAccount = true}) {
963+
void takeStartingRoutes({Account? account, bool withAccount = true}) {
964+
account ??= eg.selfAccount;
964965
final expected = <Condition<Object?>>[
965966
if (withAccount)
966967
(it) => it.isA<MaterialAccountWidgetRoute>()
967-
..accountId.equals(eg.selfAccount.id)
968+
..accountId.equals(account!.id)
968969
..page.isA<HomePage>()
969970
else
970971
(it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>(),
@@ -1130,6 +1131,36 @@ void main() {
11301131
takeStartingRoutes();
11311132
matchesNavigation(check(pushedRoutes).single, account, message);
11321133
});
1134+
1135+
testWidgets('uses associated account as initial account; if initial route', (tester) async {
1136+
addTearDown(testBinding.reset);
1137+
1138+
final accountA = eg.selfAccount;
1139+
final accountB = eg.otherAccount;
1140+
final message = eg.streamMessage();
1141+
final data = messageFcmMessage(message, account: accountB);
1142+
await testBinding.globalStore.add(accountA, eg.initialSnapshot());
1143+
await testBinding.globalStore.add(accountB, eg.initialSnapshot());
1144+
1145+
final intentDataUrl = NotificationOpenPayload(
1146+
realmUrl: data.realmUrl,
1147+
userId: data.userId,
1148+
narrow: switch (data.recipient) {
1149+
FcmMessageChannelRecipient(:var streamId, :var topic) =>
1150+
TopicNarrow(streamId, topic),
1151+
FcmMessageDmRecipient(:var allRecipientIds) =>
1152+
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId),
1153+
}).buildUrl();
1154+
addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue);
1155+
tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString();
1156+
1157+
await prepare(tester, early: true);
1158+
check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet
1159+
1160+
await tester.pump();
1161+
takeStartingRoutes(account: accountB);
1162+
matchesNavigation(check(pushedRoutes).single, accountB, message);
1163+
});
11331164
});
11341165

11351166
group('NotificationOpenPayload', () {

0 commit comments

Comments
 (0)