Skip to content

Commit ae30cd3

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 4d68974 commit ae30cd3

File tree

3 files changed

+102
-18
lines changed

3 files changed

+102
-18
lines changed

lib/notifications/display.dart

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ import '../log.dart';
1313
import '../model/binding.dart';
1414
import '../model/localizations.dart';
1515
import '../model/narrow.dart';
16+
import '../model/store.dart';
1617
import '../widgets/app.dart';
1718
import '../widgets/color.dart';
1819
import '../widgets/dialog.dart';
1920
import '../widgets/message_list.dart';
20-
import '../widgets/page.dart';
2121
import '../widgets/store.dart';
2222
import '../widgets/theme.dart';
2323

@@ -456,36 +456,58 @@ class NotificationDisplayManager {
456456

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

459+
/// Provides the route and the account ID by parsing the notification URL.
460+
///
461+
/// The URL must have been generated using [NotificationOpenPayload.buildUrl]
462+
/// while creating the notification.
463+
///
464+
/// Returns null if the associated account is not found in the global
465+
/// store.
466+
static ({Route<void> route, int accountId})? routeForNotification({
467+
required GlobalStore globalStore,
468+
required Uri url,
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((account) =>
475+
account.realmUrl == payload.realmUrl && account.userId == payload.userId);
476+
if (account == null) return null;
477+
478+
final route = MessageListPage.buildRoute(
479+
accountId: account.id,
480+
// TODO(#82): Open at specific message, not just conversation
481+
narrow: payload.narrow);
482+
return (route: route, accountId: account.id);
483+
}
484+
459485
/// Navigates to the [MessageListPage] of the specific conversation
460486
/// given the `zulip://notification/…` Android intent data URL,
461487
/// generated with [NotificationOpenPayload.buildUrl] while creating
462488
/// the notification.
463489
static Future<void> navigateForNotification(Uri url) async {
464490
assert(debugLog('opened notif: url: $url'));
465491

466-
assert(url.scheme == 'zulip' && url.host == 'notification');
467-
final payload = NotificationOpenPayload.parseUrl(url);
468-
469492
NavigatorState navigator = await ZulipApp.navigator;
470493
final context = navigator.context;
471494
assert(context.mounted);
472495
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
473496

474497
final zulipLocalizations = ZulipLocalizations.of(context);
475498
final globalStore = GlobalStoreWidget.of(context);
476-
final account = globalStore.accounts.firstWhereOrNull((account) =>
477-
account.realmUrl == payload.realmUrl && account.userId == payload.userId);
478-
if (account == null) { // TODO(log)
499+
500+
final notificationResult =
501+
routeForNotification(globalStore: globalStore, url: url);
502+
if (notificationResult == null) { // TODO(log)
479503
showErrorDialog(context: context,
480504
title: zulipLocalizations.errorNotificationOpenTitle,
481505
message: zulipLocalizations.errorNotificationOpenAccountMissing);
482506
return;
483507
}
484508

485509
// TODO(nav): Better interact with existing nav stack on notif open
486-
unawaited(navigator.push(MaterialAccountWidgetRoute<void>(accountId: account.id,
487-
// TODO(#82): Open at specific message, not just conversation
488-
page: MessageListPage(initNarrow: payload.narrow))));
510+
unawaited(navigator.push(notificationResult.route));
489511
}
490512

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

lib/widgets/app.dart

Lines changed: 31 additions & 8 deletions
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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,45 @@ void main() {
10971097
takeStartingRoutes();
10981098
matchesNavigation(check(pushedRoutes).single, account, message);
10991099
});
1100+
1101+
testWidgets('uses associated account as initial account; if initial route', (tester) async {
1102+
addTearDown(testBinding.reset);
1103+
addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue);
1104+
1105+
final accountA = eg.selfAccount;
1106+
final accountB = eg.otherAccount;
1107+
final message = eg.streamMessage();
1108+
final data = messageFcmMessage(message, account: accountB);
1109+
await testBinding.globalStore.add(accountA, eg.initialSnapshot());
1110+
await testBinding.globalStore.add(accountB, eg.initialSnapshot());
1111+
1112+
final intentDataUrl = NotificationOpenPayload(
1113+
realmUrl: data.realmUrl,
1114+
userId: data.userId,
1115+
narrow: switch (data.recipient) {
1116+
FcmMessageChannelRecipient(:var streamId, :var topic) =>
1117+
TopicNarrow(streamId, topic),
1118+
FcmMessageDmRecipient(:var allRecipientIds) =>
1119+
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId),
1120+
}).buildUrl();
1121+
tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString();
1122+
1123+
await prepare(tester, early: true);
1124+
check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet
1125+
1126+
await tester.pump();
1127+
check(pushedRoutes).deepEquals(<Condition<Object?>>[
1128+
(it) => it.isA<MaterialAccountWidgetRoute>()
1129+
..accountId.equals(accountB.id)
1130+
..page.isA<HomePage>(),
1131+
(it) => it.isA<MaterialAccountWidgetRoute>()
1132+
..accountId.equals(accountB.id)
1133+
..page.isA<MessageListPage>()
1134+
.initNarrow.equals(SendableNarrow.ofMessage(message,
1135+
selfUserId: accountB.userId))
1136+
]);
1137+
pushedRoutes.clear();
1138+
});
11001139
});
11011140

11021141
group('NotificationOpenPayload', () {

0 commit comments

Comments
 (0)