diff --git a/lib/notifications/display.dart b/lib/notifications/display.dart index 1b85ccaebc..1d74db6854 100644 --- a/lib/notifications/display.dart +++ b/lib/notifications/display.dart @@ -453,6 +453,40 @@ class NotificationDisplayManager { static String _personKey(Uri realmUrl, int userId) => "$realmUrl|$userId"; + /// Provides the route and the account ID by parsing the notification URL. + /// + /// The URL must have been generated using [NotificationOpenPayload.buildUrl] + /// while creating the notification. + /// + /// Returns null and shows an error dialog if the associated account is not + /// found in the global store. + static AccountRoute? routeForNotification({ + required BuildContext context, + required Uri url, + }) { + final globalStore = GlobalStoreWidget.of(context); + + assert(debugLog('got notif: url: $url')); + assert(url.scheme == 'zulip' && url.host == 'notification'); + final payload = NotificationOpenPayload.parseUrl(url); + + final account = globalStore.accounts.firstWhereOrNull( + (account) => account.realmUrl.origin == payload.realmUrl.origin + && account.userId == payload.userId); + if (account == null) { // TODO(log) + final zulipLocalizations = ZulipLocalizations.of(context); + showErrorDialog(context: context, + title: zulipLocalizations.errorNotificationOpenTitle, + message: zulipLocalizations.errorNotificationOpenAccountMissing); + return null; + } + + return MessageListPage.buildRoute( + accountId: account.id, + // TODO(#82): Open at specific message, not just conversation + narrow: payload.narrow); + } + /// Navigates to the [MessageListPage] of the specific conversation /// given the `zulip://notification/…` Android intent data URL, /// generated with [NotificationOpenPayload.buildUrl] while creating @@ -460,29 +494,16 @@ class NotificationDisplayManager { static Future navigateForNotification(Uri url) async { assert(debugLog('opened notif: url: $url')); - assert(url.scheme == 'zulip' && url.host == 'notification'); - final payload = NotificationOpenPayload.parseUrl(url); - NavigatorState navigator = await ZulipApp.navigator; final context = navigator.context; assert(context.mounted); 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 - final zulipLocalizations = ZulipLocalizations.of(context); - final globalStore = GlobalStoreWidget.of(context); - final account = globalStore.accounts.firstWhereOrNull((account) => - account.realmUrl == payload.realmUrl && account.userId == payload.userId); - if (account == null) { // TODO(log) - showErrorDialog(context: context, - title: zulipLocalizations.errorNotificationOpenTitle, - message: zulipLocalizations.errorNotificationOpenAccountMissing); - return; - } + final route = routeForNotification(context: context, url: url); + if (route == null) return; // TODO(log) // TODO(nav): Better interact with existing nav stack on notif open - unawaited(navigator.push(MaterialAccountWidgetRoute(accountId: account.id, - // TODO(#82): Open at specific message, not just conversation - page: MessageListPage(initNarrow: payload.narrow)))); + unawaited(navigator.push(route)); } static Future _fetchBitmap(Uri url) async { diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index f20e9f8fcc..9fa82144a1 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -139,6 +139,52 @@ class ZulipApp extends StatefulWidget { } class _ZulipAppState extends State with WidgetsBindingObserver { + @override + void initState() { + super.initState(); + WidgetsBinding.instance.addObserver(this); + } + + @override + void dispose() { + WidgetsBinding.instance.removeObserver(this); + super.dispose(); + } + + List> _handleGenerateInitialRoutes(String initialRoute) { + // The `_ZulipAppState.context` lacks the required ancestors. Instead + // we use the Navigator which should be available when this callback is + // called and it's context should have the required ancestors. + final context = ZulipApp.navigatorKey.currentContext!; + + final initialRouteUrl = Uri.tryParse(initialRoute); + if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) { + final route = NotificationDisplayManager.routeForNotification( + context: context, + url: initialRouteUrl); + + if (route != null) { + return [ + HomePage.buildRoute(accountId: route.accountId), + route, + ]; + } else { + // The account didn't match any existing accounts, + // fall through to show the default route below. + } + } + + final globalStore = GlobalStoreWidget.of(context); + // TODO(#524) choose initial account as last one used + final initialAccountId = globalStore.accounts.firstOrNull?.id; + return [ + if (initialAccountId == null) + MaterialWidgetRoute(page: const ChooseAccountPage()) + else + HomePage.buildRoute(accountId: initialAccountId), + ]; + } + @override Future didPushRouteInformation(routeInformation) async { switch (routeInformation.uri) { @@ -152,71 +198,39 @@ class _ZulipAppState extends State with WidgetsBindingObserver { return super.didPushRouteInformation(routeInformation); } - Future _handleInitialRoute() async { - final initialRouteUrl = Uri.parse(WidgetsBinding.instance.platformDispatcher.defaultRouteName); - if (initialRouteUrl case Uri(scheme: 'zulip', host: 'notification')) { - await NotificationDisplayManager.navigateForNotification(initialRouteUrl); - } - } - - @override - void initState() { - super.initState(); - WidgetsBinding.instance.addObserver(this); - _handleInitialRoute(); - } - - @override - void dispose() { - WidgetsBinding.instance.removeObserver(this); - super.dispose(); - } - @override Widget build(BuildContext context) { final themeData = zulipThemeData(context); return GlobalStoreWidget( - child: Builder(builder: (context) { - final globalStore = GlobalStoreWidget.of(context); - // TODO(#524) choose initial account as last one used - final initialAccountId = globalStore.accounts.firstOrNull?.id; - return MaterialApp( - onGenerateTitle: (BuildContext context) { - return ZulipLocalizations.of(context).zulipAppTitle; - }, - localizationsDelegates: ZulipLocalizations.localizationsDelegates, - supportedLocales: ZulipLocalizations.supportedLocales, - theme: themeData, - - navigatorKey: ZulipApp.navigatorKey, - navigatorObservers: widget.navigatorObservers ?? const [], - builder: (BuildContext context, Widget? child) { - if (!ZulipApp.ready.value) { - SchedulerBinding.instance.addPostFrameCallback( - (_) => widget._declareReady()); - } - GlobalLocalizations.zulipLocalizations = ZulipLocalizations.of(context); - return child!; - }, - - // We use onGenerateInitialRoutes for the real work of specifying the - // initial nav state. To do that we need [MaterialApp] to decide to - // build a [Navigator]... which means specifying either `home`, `routes`, - // `onGenerateRoute`, or `onUnknownRoute`. Make it `onGenerateRoute`. - // It never actually gets called, though: `onGenerateInitialRoutes` - // handles startup, and then we always push whole routes with methods - // like [Navigator.push], never mere names as with [Navigator.pushNamed]. - onGenerateRoute: (_) => null, - - onGenerateInitialRoutes: (_) { - return [ - if (initialAccountId == null) - MaterialWidgetRoute(page: const ChooseAccountPage()) - else - HomePage.buildRoute(accountId: initialAccountId), - ]; - }); - })); + child: MaterialApp( + onGenerateTitle: (BuildContext context) { + return ZulipLocalizations.of(context).zulipAppTitle; + }, + localizationsDelegates: ZulipLocalizations.localizationsDelegates, + supportedLocales: ZulipLocalizations.supportedLocales, + theme: themeData, + + navigatorKey: ZulipApp.navigatorKey, + navigatorObservers: widget.navigatorObservers ?? const [], + builder: (BuildContext context, Widget? child) { + if (!ZulipApp.ready.value) { + SchedulerBinding.instance.addPostFrameCallback( + (_) => widget._declareReady()); + } + GlobalLocalizations.zulipLocalizations = ZulipLocalizations.of(context); + return child!; + }, + + // We use onGenerateInitialRoutes for the real work of specifying the + // initial nav state. To do that we need [MaterialApp] to decide to + // build a [Navigator]... which means specifying either `home`, `routes`, + // `onGenerateRoute`, or `onUnknownRoute`. Make it `onGenerateRoute`. + // It never actually gets called, though: `onGenerateInitialRoutes` + // handles startup, and then we always push whole routes with methods + // like [Navigator.push], never mere names as with [Navigator.pushNamed]. + onGenerateRoute: (_) => null, + + onGenerateInitialRoutes: _handleGenerateInitialRoutes)); } } diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index ad70b57c32..d7b1585022 100644 --- a/lib/widgets/home.dart +++ b/lib/widgets/home.dart @@ -31,7 +31,7 @@ enum _HomePageTab { class HomePage extends StatefulWidget { const HomePage({super.key}); - static Route buildRoute({required int accountId}) { + static AccountRoute buildRoute({required int accountId}) { return MaterialAccountWidgetRoute(accountId: accountId, loadingPlaceholderPage: _LoadingPlaceholderPage(accountId: accountId), page: const HomePage()); diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 4263041dbc..25993efa30 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -188,7 +188,7 @@ abstract class MessageListPageState { class MessageListPage extends StatefulWidget { const MessageListPage({super.key, required this.initNarrow}); - static Route buildRoute({int? accountId, BuildContext? context, + static AccountRoute buildRoute({int? accountId, BuildContext? context, required Narrow narrow}) { return MaterialAccountWidgetRoute(accountId: accountId, context: context, page: MessageListPage(initNarrow: narrow)); diff --git a/lib/widgets/page.dart b/lib/widgets/page.dart index 0ba65fb3d4..a2c6fe52a1 100644 --- a/lib/widgets/page.dart +++ b/lib/widgets/page.dart @@ -35,6 +35,12 @@ abstract class WidgetRoute extends PageRoute { Widget get page; } +/// A page route that specifies a particular Zulip account to use, by ID. +abstract class AccountRoute extends PageRoute { + /// The [Account.id] of the account to use for this page. + int get accountId; +} + /// A [MaterialPageRoute] that always builds the same widget. /// /// This is useful for making the route more transparent for a test to inspect. @@ -56,8 +62,10 @@ class MaterialWidgetRoute extends MaterialPageRoute implem } /// A mixin for providing a given account's per-account store on a page route. -mixin AccountPageRouteMixin on PageRoute { +mixin AccountPageRouteMixin on PageRoute implements AccountRoute { + @override int get accountId; + Widget? get loadingPlaceholderPage; @override diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart index c4f8970ff0..327910f6c0 100644 --- a/lib/widgets/profile.dart +++ b/lib/widgets/profile.dart @@ -30,7 +30,7 @@ class ProfilePage extends StatelessWidget { final int userId; - static Route buildRoute({int? accountId, BuildContext? context, + static AccountRoute buildRoute({int? accountId, BuildContext? context, required int userId}) { return MaterialAccountWidgetRoute(accountId: accountId, context: context, page: ProfilePage(userId: userId)); diff --git a/test/notifications/display_test.dart b/test/notifications/display_test.dart index 32d8254d6d..b1c56b55b1 100644 --- a/test/notifications/display_test.dart +++ b/test/notifications/display_test.dart @@ -960,11 +960,12 @@ void main() { group('NotificationDisplayManager open', () { late List> pushedRoutes; - void takeStartingRoutes({bool withAccount = true}) { + void takeStartingRoutes({Account? account, bool withAccount = true}) { + account ??= eg.selfAccount; final expected = >[ if (withAccount) (it) => it.isA() - ..accountId.equals(eg.selfAccount.id) + ..accountId.equals(account!.id) ..page.isA() else (it) => it.isA().page.isA(), @@ -1036,6 +1037,21 @@ void main() { eg.dmMessage(from: eg.otherUser, to: [eg.selfUser])); }); + testWidgets('account queried by realmUrl origin component', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add( + eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), + eg.initialSnapshot()); + await prepare(tester); + + await checkOpenNotification(tester, + eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example/')), + eg.streamMessage()); + await checkOpenNotification(tester, + eg.selfAccount.copyWith(realmUrl: Uri.parse('http://chat.example')), + eg.streamMessage()); + }); + testWidgets('no accounts', (tester) async { await prepare(tester, withAccount: false); await openNotification(tester, eg.selfAccount, eg.streamMessage()); @@ -1112,11 +1128,12 @@ void main() { realmUrl: data.realmUrl, userId: data.userId, narrow: switch (data.recipient) { - FcmMessageChannelRecipient(:var streamId, :var topic) => - TopicNarrow(streamId, topic), - FcmMessageDmRecipient(:var allRecipientIds) => - DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), - }).buildUrl(); + FcmMessageChannelRecipient(:var streamId, :var topic) => + TopicNarrow(streamId, topic), + FcmMessageDmRecipient(:var allRecipientIds) => + DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), + }).buildUrl(); + addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); // Now start the app. @@ -1129,6 +1146,36 @@ void main() { takeStartingRoutes(); matchesNavigation(check(pushedRoutes).single, account, message); }); + + testWidgets('uses associated account as initial account; if initial route', (tester) async { + addTearDown(testBinding.reset); + + final accountA = eg.selfAccount; + final accountB = eg.otherAccount; + final message = eg.streamMessage(); + final data = messageFcmMessage(message, account: accountB); + await testBinding.globalStore.add(accountA, eg.initialSnapshot()); + await testBinding.globalStore.add(accountB, eg.initialSnapshot()); + + final intentDataUrl = NotificationOpenPayload( + realmUrl: data.realmUrl, + userId: data.userId, + narrow: switch (data.recipient) { + FcmMessageChannelRecipient(:var streamId, :var topic) => + TopicNarrow(streamId, topic), + FcmMessageDmRecipient(:var allRecipientIds) => + DmNarrow(allRecipientIds: allRecipientIds, selfUserId: data.userId), + }).buildUrl(); + addTearDown(tester.binding.platformDispatcher.clearDefaultRouteNameTestValue); + tester.binding.platformDispatcher.defaultRouteNameTestValue = intentDataUrl.toString(); + + await prepare(tester, early: true); + check(pushedRoutes).isEmpty(); // GlobalStore hasn't loaded yet + + await tester.pump(); + takeStartingRoutes(account: accountB); + matchesNavigation(check(pushedRoutes).single, accountB, message); + }); }); group('NotificationOpenPayload', () { diff --git a/test/widgets/page_checks.dart b/test/widgets/page_checks.dart index 412a59fc49..a3692273bf 100644 --- a/test/widgets/page_checks.dart +++ b/test/widgets/page_checks.dart @@ -6,6 +6,6 @@ extension WidgetRouteChecks on Subject> { Subject get page => has((x) => x.page, 'page'); } -extension AccountPageRouteMixinChecks on Subject> { +extension AccountRouteChecks on Subject> { Subject get accountId => has((x) => x.accountId, 'accountId'); }