Skip to content

Commit 0c0bdb1

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 3ff7096 commit 0c0bdb1

File tree

3 files changed

+112
-26
lines changed

3 files changed

+112
-26
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: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -152,18 +152,54 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
152152
return super.didPushRouteInformation(routeInformation);
153153
}
154154

155-
Future<void> _handleInitialRoute() async {
156-
final initialRouteUrl = Uri.parse(WidgetsBinding.instance.platformDispatcher.defaultRouteName);
157-
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
158-
await NotificationDisplayManager.navigateForNotification(initialRouteUrl);
155+
InitialRouteListFactory _handleGenerateInitialRoutes(BuildContext context) {
156+
void showNotificationErrorDialog() async {
157+
final navigator = await ZulipApp.navigator;
158+
final context = navigator.context;
159+
assert(context.mounted);
160+
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
161+
162+
final zulipLocalizations = ZulipLocalizations.of(context);
163+
showErrorDialog(context: context,
164+
title: zulipLocalizations.errorNotificationOpenTitle,
165+
message: zulipLocalizations.errorNotificationOpenAccountMissing);
159166
}
167+
168+
final globalStore = GlobalStoreWidget.of(context);
169+
// TODO(#524) choose initial account as last one used
170+
final initialAccountId = globalStore.accounts.firstOrNull?.id;
171+
172+
return (String intialRoute) {
173+
final initialRouteUrl = Uri.parse(intialRoute);
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+
190+
return [
191+
if (initialAccountId == null)
192+
MaterialWidgetRoute(page: const ChooseAccountPage())
193+
else
194+
HomePage.buildRoute(accountId: initialAccountId),
195+
];
196+
};
160197
}
161198

162199
@override
163200
void initState() {
164201
super.initState();
165202
WidgetsBinding.instance.addObserver(this);
166-
_handleInitialRoute();
167203
}
168204

169205
@override
@@ -177,9 +213,6 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
177213
final themeData = zulipThemeData(context);
178214
return GlobalStoreWidget(
179215
child: Builder(builder: (context) {
180-
final globalStore = GlobalStoreWidget.of(context);
181-
// TODO(#524) choose initial account as last one used
182-
final initialAccountId = globalStore.accounts.firstOrNull?.id;
183216
return MaterialApp(
184217
title: 'Zulip',
185218
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
@@ -206,14 +239,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
206239
// like [Navigator.push], never mere names as with [Navigator.pushNamed].
207240
onGenerateRoute: (_) => null,
208241

209-
onGenerateInitialRoutes: (_) {
210-
return [
211-
if (initialAccountId == null)
212-
MaterialWidgetRoute(page: const ChooseAccountPage())
213-
else
214-
HomePage.buildRoute(accountId: initialAccountId),
215-
];
216-
});
242+
onGenerateInitialRoutes: _handleGenerateInitialRoutes(context));
217243
}));
218244
}
219245
}

test/notifications/display_test.dart

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

11011139
group('NotificationOpenPayload', () {

0 commit comments

Comments
 (0)