Skip to content

Commit e0ee84d

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 92240c4 commit e0ee84d

File tree

3 files changed

+102
-18
lines changed

3 files changed

+102
-18
lines changed

lib/notifications/display.dart

+32-10
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ 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';
2021
import '../widgets/message_list.dart';
21-
import '../widgets/page.dart';
2222
import '../widgets/store.dart';
2323
import '../widgets/theme.dart';
2424

@@ -452,36 +452,58 @@ class NotificationDisplayManager {
452452

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

455+
/// Provides the route and the account ID by parsing the notification URL.
456+
///
457+
/// The URL must have been generated using [NotificationOpenPayload.buildUrl]
458+
/// while creating the notification.
459+
///
460+
/// Returns null if the associated account is not found in the global
461+
/// store.
462+
static ({Route<void> route, int accountId})? routeForNotification({
463+
required GlobalStore globalStore,
464+
required Uri url,
465+
}) {
466+
assert(debugLog('got notif: url: $url'));
467+
assert(url.scheme == 'zulip' && url.host == 'notification');
468+
final payload = NotificationOpenPayload.parseUrl(url);
469+
470+
final account = globalStore.accounts.firstWhereOrNull((account) =>
471+
account.realmUrl == payload.realmUrl && account.userId == payload.userId);
472+
if (account == null) return null;
473+
474+
final route = MessageListPage.buildRoute(
475+
accountId: account.id,
476+
// TODO(#82): Open at specific message, not just conversation
477+
narrow: payload.narrow);
478+
return (route: route, accountId: account.id);
479+
}
480+
455481
/// Navigates to the [MessageListPage] of the specific conversation
456482
/// given the `zulip://notification/…` Android intent data URL,
457483
/// generated with [NotificationOpenPayload.buildUrl] while creating
458484
/// the notification.
459485
static Future<void> navigateForNotification(Uri url) async {
460486
assert(debugLog('opened notif: url: $url'));
461487

462-
assert(url.scheme == 'zulip' && url.host == 'notification');
463-
final payload = NotificationOpenPayload.parseUrl(url);
464-
465488
NavigatorState navigator = await ZulipApp.navigator;
466489
final context = navigator.context;
467490
assert(context.mounted);
468491
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
469492

470493
final zulipLocalizations = ZulipLocalizations.of(context);
471494
final globalStore = GlobalStoreWidget.of(context);
472-
final account = globalStore.accounts.firstWhereOrNull((account) =>
473-
account.realmUrl == payload.realmUrl && account.userId == payload.userId);
474-
if (account == null) { // TODO(log)
495+
496+
final notificationResult =
497+
routeForNotification(globalStore: globalStore, url: url);
498+
if (notificationResult == null) { // TODO(log)
475499
showErrorDialog(context: context,
476500
title: zulipLocalizations.errorNotificationOpenTitle,
477501
message: zulipLocalizations.errorNotificationOpenAccountMissing);
478502
return;
479503
}
480504

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

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

lib/widgets/app.dart

+31-8
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,38 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
155155
InitialRouteListFactory _handleGenerateInitialRoutes(BuildContext context) {
156156
final globalStore = GlobalStoreWidget.of(context);
157157

158+
void showNotificationErrorDialog() async {
159+
final navigator = await ZulipApp.navigator;
160+
final navigatorContext = navigator.context;
161+
assert(navigatorContext.mounted);
162+
// TODO(linter): this is impossible as there's no actual async gap, but
163+
// the use_build_context_synchronously lint doesn't see that.
164+
if (!navigatorContext.mounted) return;
165+
166+
final zulipLocalizations = ZulipLocalizations.of(navigatorContext);
167+
showErrorDialog(context: navigatorContext,
168+
title: zulipLocalizations.errorNotificationOpenTitle,
169+
message: zulipLocalizations.errorNotificationOpenAccountMissing);
170+
}
171+
158172
return (String initialRoute) {
173+
final initialRouteUrl = Uri.parse(initialRoute);
174+
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
175+
final notificationResult = NotificationDisplayManager.routeForNotification(
176+
globalStore: globalStore,
177+
url: initialRouteUrl);
178+
179+
if (notificationResult != null) {
180+
return [
181+
HomePage.buildRoute(accountId: notificationResult.accountId),
182+
notificationResult.route,
183+
];
184+
} else {
185+
showNotificationErrorDialog();
186+
// Fallthrough to show default route below.
187+
}
188+
}
189+
159190
// TODO(#524) choose initial account as last one used
160191
final initialAccountId = globalStore.accounts.firstOrNull?.id;
161192
return [
@@ -167,18 +198,10 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
167198
};
168199
}
169200

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-
177201
@override
178202
void initState() {
179203
super.initState();
180204
WidgetsBinding.instance.addObserver(this);
181-
_handleInitialRoute();
182205
}
183206

184207
@override

test/notifications/display_test.dart

+39
Original file line numberDiff line numberDiff line change
@@ -1130,6 +1130,45 @@ void main() {
11301130
takeStartingRoutes();
11311131
matchesNavigation(check(pushedRoutes).single, account, message);
11321132
});
1133+
1134+
testWidgets('uses associated account as initial account; if initial route', (tester) async {
1135+
addTearDown(testBinding.reset);
1136+
addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue);
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+
tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString();
1155+
1156+
await prepare(tester, early: true);
1157+
check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet
1158+
1159+
await tester.pump();
1160+
check(pushedRoutes).deepEquals(<Condition<Object?>>[
1161+
(it) => it.isA<MaterialAccountWidgetRoute>()
1162+
..accountId.equals(accountB.id)
1163+
..page.isA<HomePage>(),
1164+
(it) => it.isA<MaterialAccountWidgetRoute>()
1165+
..accountId.equals(accountB.id)
1166+
..page.isA<MessageListPage>()
1167+
.initNarrow.equals(SendableNarrow.ofMessage(message,
1168+
selfUserId: accountB.userId))
1169+
]);
1170+
pushedRoutes.clear();
1171+
});
11331172
});
11341173

11351174
group('NotificationOpenPayload', () {

0 commit comments

Comments
 (0)