Skip to content

Commit f9cd2f2

Browse files
notif [nfc]: Refactor NotificationOpenService.routeForNotification
Update it to receive `NotificationOpenPayload` as an argument instead of the Android Notification URL. Also rename `navigateForNotification` to `navigateForAndroidNotificationUrl`, making it more explicit.
1 parent a79adc2 commit f9cd2f2

File tree

2 files changed

+33
-31
lines changed

2 files changed

+33
-31
lines changed

lib/notifications/open.dart

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,23 @@ import '../widgets/store.dart';
1717
/// Responds to the user opening a notification.
1818
class NotificationOpenService {
1919

20-
/// Provides the route and the account ID by parsing the notification URL.
21-
///
22-
/// The URL must have been generated using
23-
/// [NotificationOpenPayload.buildAndroidNotificationUrl] while creating the
24-
/// notification.
20+
/// Provides the route to open by parsing the notification payload.
2521
///
2622
/// Returns null and shows an error dialog if the associated account is not
2723
/// found in the global store.
2824
///
2925
/// The context argument should be a descendant of the app's main [Navigator].
3026
static AccountRoute<void>? routeForNotification({
3127
required BuildContext context,
32-
required Uri url,
28+
required NotificationOpenPayload data,
3329
}) {
3430
assert(defaultTargetPlatform == TargetPlatform.android);
3531

3632
final globalStore = GlobalStoreWidget.of(context);
3733

38-
assert(debugLog('got notif: url: $url'));
39-
assert(url.scheme == 'zulip' && url.host == 'notification');
40-
final payload = NotificationOpenPayload.parseAndroidNotificationUrl(url);
41-
4234
final account = globalStore.accounts.firstWhereOrNull(
43-
(account) => account.realmUrl.origin == payload.realmUrl.origin
44-
&& account.userId == payload.userId);
35+
(account) => account.realmUrl.origin == data.realmUrl.origin
36+
&& account.userId == data.userId);
4537
if (account == null) { // TODO(log)
4638
final zulipLocalizations = ZulipLocalizations.of(context);
4739
showErrorDialog(context: context,
@@ -53,14 +45,14 @@ class NotificationOpenService {
5345
return MessageListPage.buildRoute(
5446
accountId: account.id,
5547
// TODO(#82): Open at specific message, not just conversation
56-
narrow: payload.narrow);
48+
narrow: data.narrow);
5749
}
5850

5951
/// Navigates to the [MessageListPage] of the specific conversation
6052
/// given the `zulip://notification/…` Android intent data URL,
6153
/// generated with [NotificationOpenPayload.buildAndroidNotificationUrl]
6254
/// while creating the notification.
63-
static Future<void> navigateForNotification(Uri url) async {
55+
static Future<void> navigateForAndroidNotificationUrl(Uri url) async {
6456
assert(defaultTargetPlatform == TargetPlatform.android);
6557
assert(debugLog('opened notif: url: $url'));
6658

@@ -69,7 +61,9 @@ class NotificationOpenService {
6961
assert(context.mounted);
7062
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
7163

72-
final route = routeForNotification(context: context, url: url);
64+
assert(url.scheme == 'zulip' && url.host == 'notification');
65+
final data = NotificationOpenPayload.parseAndroidNotificationUrl(url);
66+
final route = routeForNotification(context: context, data: data);
7367
if (route == null) return; // TODO(log)
7468

7569
// TODO(nav): Better interact with existing nav stack on notif open

lib/widgets/app.dart

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -168,27 +168,35 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
168168
super.dispose();
169169
}
170170

171+
AccountRoute<void>? _initialRouteAndroid(
172+
BuildContext context,
173+
String initialRoute,
174+
) {
175+
final initialRouteUrl = Uri.tryParse(initialRoute);
176+
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
177+
assert(debugLog('got notif: url: $initialRouteUrl'));
178+
final data =
179+
NotificationOpenPayload.parseAndroidNotificationUrl(initialRouteUrl);
180+
return NotificationOpenService.routeForNotification(
181+
context: context,
182+
data: data);
183+
}
184+
185+
return null;
186+
}
187+
171188
List<Route<dynamic>> _handleGenerateInitialRoutes(String initialRoute) {
172189
// The `_ZulipAppState.context` lacks the required ancestors. Instead
173190
// we use the Navigator which should be available when this callback is
174191
// called and it's context should have the required ancestors.
175192
final context = ZulipApp.navigatorKey.currentContext!;
176193

177-
final initialRouteUrl = Uri.tryParse(initialRoute);
178-
if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) {
179-
final route = NotificationOpenService.routeForNotification(
180-
context: context,
181-
url: initialRouteUrl);
182-
183-
if (route != null) {
184-
return [
185-
HomePage.buildRoute(accountId: route.accountId),
186-
route,
187-
];
188-
} else {
189-
// The account didn't match any existing accounts,
190-
// fall through to show the default route below.
191-
}
194+
final route = _initialRouteAndroid(context, initialRoute);
195+
if (route != null) {
196+
return [
197+
HomePage.buildRoute(accountId: route.accountId),
198+
route,
199+
];
192200
}
193201

194202
final globalStore = GlobalStoreWidget.of(context);
@@ -209,7 +217,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
209217
await LoginPage.handleWebAuthUrl(url);
210218
return true;
211219
case Uri(scheme: 'zulip', host: 'notification') && var url:
212-
await NotificationOpenService.navigateForNotification(url);
220+
await NotificationOpenService.navigateForAndroidNotificationUrl(url);
213221
return true;
214222
}
215223
return super.didPushRouteInformation(routeInformation);

0 commit comments

Comments
 (0)