Skip to content

Commit 54adf40

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 3ff7096 commit 54adf40

File tree

3 files changed

+121
-31
lines changed

3 files changed

+121
-31
lines changed

lib/notifications/display.dart

+32-10
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

+44-16
Original file line numberDiff line numberDiff line change
@@ -152,18 +152,56 @@ 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 navigatorContext = navigator.context;
159+
assert(navigatorContext.mounted);
160+
// TODO(linter): this is impossible as there's no actual async gap, but
161+
// the use_build_context_synchronously lint doesn't see that.
162+
if (!navigatorContext.mounted) return;
163+
164+
final zulipLocalizations = ZulipLocalizations.of(navigatorContext);
165+
showErrorDialog(context: navigatorContext,
166+
title: zulipLocalizations.errorNotificationOpenTitle,
167+
message: zulipLocalizations.errorNotificationOpenAccountMissing);
159168
}
169+
170+
final globalStore = GlobalStoreWidget.of(context);
171+
// TODO(#524) choose initial account as last one used
172+
final initialAccountId = globalStore.accounts.firstOrNull?.id;
173+
174+
return (String initialRoute) {
175+
final initialRouteUrl = Uri.parse(initialRoute);
176+
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
177+
final notificationResult = NotificationDisplayManager.routeForNotification(
178+
globalStore: globalStore,
179+
url: initialRouteUrl);
180+
181+
if (notificationResult != null) {
182+
return [
183+
HomePage.buildRoute(accountId: notificationResult.accountId),
184+
notificationResult.route,
185+
];
186+
} else {
187+
showNotificationErrorDialog();
188+
// Fallthrough to show default route below.
189+
}
190+
}
191+
192+
return [
193+
if (initialAccountId == null)
194+
MaterialWidgetRoute(page: const ChooseAccountPage())
195+
else
196+
HomePage.buildRoute(accountId: initialAccountId),
197+
];
198+
};
160199
}
161200

162201
@override
163202
void initState() {
164203
super.initState();
165204
WidgetsBinding.instance.addObserver(this);
166-
_handleInitialRoute();
167205
}
168206

169207
@override
@@ -177,9 +215,6 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
177215
final themeData = zulipThemeData(context);
178216
return GlobalStoreWidget(
179217
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;
183218
return MaterialApp(
184219
title: 'Zulip',
185220
localizationsDelegates: ZulipLocalizations.localizationsDelegates,
@@ -206,14 +241,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
206241
// like [Navigator.push], never mere names as with [Navigator.pushNamed].
207242
onGenerateRoute: (_) => null,
208243

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

test/notifications/display_test.dart

+45-5
Original file line numberDiff line numberDiff line change
@@ -1070,6 +1070,7 @@ void main() {
10701070

10711071
testWidgets('at app launch', (tester) async {
10721072
addTearDown(testBinding.reset);
1073+
addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue);
10731074
// Set up a value for `PlatformDispatcher.defaultRouteName` to return,
10741075
// for determining the intial route.
10751076
final account = eg.selfAccount;
@@ -1079,11 +1080,11 @@ void main() {
10791080
realmUrl: data.realmUrl,
10801081
userId: data.userId,
10811082
narrow: switch (data.recipient) {
1082-
FcmMessageChannelRecipient(:var streamId, :var topic) =>
1083-
TopicNarrow(streamId, topic),
1084-
FcmMessageDmRecipient(:var allRecipientIds) =>
1085-
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId),
1086-
}).buildUrl();
1083+
FcmMessageChannelRecipient(:var streamId, :var topic) =>
1084+
TopicNarrow(streamId, topic),
1085+
FcmMessageDmRecipient(:var allRecipientIds) =>
1086+
DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId),
1087+
}).buildUrl();
10871088
tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString();
10881089

10891090
// Now start the app.
@@ -1096,6 +1097,45 @@ void main() {
10961097
takeStartingRoutes();
10971098
matchesNavigation(check(pushedRoutes).single, account, message);
10981099
});
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+
});
10991139
});
11001140

11011141
group('NotificationOpenPayload', () {

0 commit comments

Comments
 (0)