Skip to content

Commit 6c35bea

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 zulip#1210 for background notifications.
1 parent 28fda7d commit 6c35bea

File tree

3 files changed

+88
-27
lines changed

3 files changed

+88
-27
lines changed

lib/notifications/display.dart

+37-16
Original file line numberDiff line numberDiff line change
@@ -453,36 +453,57 @@ class NotificationDisplayManager {
453453

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

456+
/// Provides the route and the account ID by parsing the notification URL.
457+
///
458+
/// The URL must have been generated using [NotificationOpenPayload.buildUrl]
459+
/// while creating the notification.
460+
///
461+
/// Returns null and shows an error dialog if the associated account is not
462+
/// found in the global store.
463+
static AccountRoute<void>? routeForNotification({
464+
required BuildContext context,
465+
required Uri url,
466+
}) {
467+
final globalStore = GlobalStoreWidget.of(context);
468+
final zulipLocalizations = ZulipLocalizations.of(context);
469+
470+
assert(debugLog('got notif: url: $url'));
471+
assert(url.scheme == 'zulip' && url.host == 'notification');
472+
final payload = NotificationOpenPayload.parseUrl(url);
473+
474+
final account = globalStore.accounts.firstWhereOrNull(
475+
(account) => account.realmUrl.origin == payload.realmUrl.origin
476+
&& account.userId == payload.userId);
477+
if (account == null) { // TODO(log)
478+
showErrorDialog(context: context,
479+
title: zulipLocalizations.errorNotificationOpenTitle,
480+
message: zulipLocalizations.errorNotificationOpenAccountMissing);
481+
return null;
482+
}
483+
484+
return MessageListPage.buildRoute(
485+
accountId: account.id,
486+
// TODO(#82): Open at specific message, not just conversation
487+
narrow: payload.narrow);
488+
}
489+
456490
/// Navigates to the [MessageListPage] of the specific conversation
457491
/// given the `zulip://notification/…` Android intent data URL,
458492
/// generated with [NotificationOpenPayload.buildUrl] while creating
459493
/// the notification.
460494
static Future<void> navigateForNotification(Uri url) async {
461495
assert(debugLog('opened notif: url: $url'));
462496

463-
assert(url.scheme == 'zulip' && url.host == 'notification');
464-
final payload = NotificationOpenPayload.parseUrl(url);
465-
466497
NavigatorState navigator = await ZulipApp.navigator;
467498
final context = navigator.context;
468499
assert(context.mounted);
469500
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
470501

471-
final zulipLocalizations = ZulipLocalizations.of(context);
472-
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-
}
502+
final route = routeForNotification(context: context, url: url);
503+
if (route == null) return; // TODO(log)
481504

482505
// 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))));
506+
unawaited(navigator.push(route));
486507
}
487508

488509
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
@@ -157,8 +156,25 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
157156
// we use the Navigator which should be available when this callback is
158157
// called and it's context should have the required dependencies.
159158
final context = ZulipApp.navigatorKey.currentContext!;
160-
final globalStore = GlobalStoreWidget.of(context);
161159

160+
final initialRouteUrl = Uri.tryParse(initialRoute);
161+
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
162+
final route = NotificationDisplayManager.routeForNotification(
163+
context: context,
164+
url: initialRouteUrl);
165+
166+
if (route != null) {
167+
return [
168+
HomePage.buildRoute(accountId: route.accountId),
169+
route,
170+
];
171+
} else {
172+
// The account didn't match any existing accounts,
173+
// fall through to show the default route below.
174+
}
175+
}
176+
177+
final globalStore = GlobalStoreWidget.of(context);
162178
// TODO(#524) choose initial account as last one used
163179
final initialAccountId = globalStore.accounts.firstOrNull?.id;
164180
return [
@@ -169,13 +185,6 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
169185
];
170186
}
171187

172-
Future<void> _handleInitialRoute() async {
173-
final initialRouteUrl = Uri.parse(WidgetsBinding.instance.platformDispatcher.defaultRouteName);
174-
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
175-
await NotificationDisplayManager.navigateForNotification(initialRouteUrl);
176-
}
177-
}
178-
179188
@override
180189
Future<bool> didPushRouteInformation(routeInformation) async {
181190
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)