notif: Use associated account as initial account; if opened from background

lib/notifications/display.dart

@@ -453,36 +453,57 @@ 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<void>? 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
   /// the notification.
   static Future<void> 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<void>(accountId: account.id,
-      // TODO(#82): Open at specific message, not just conversation
-      page: MessageListPage(initNarrow: payload.narrow))));
+    unawaited(navigator.push(route));
   }
 
   static Future<Uint8List?> _fetchBitmap(Uri url) async {

lib/widgets/app.dart

@@ -139,6 +139,52 @@ class ZulipApp extends StatefulWidget {
 }
 
 class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
+  @override
+  void initState() {
+    super.initState();
+    WidgetsBinding.instance.addObserver(this);
+  }
+
+  @override
+  void dispose() {
+    WidgetsBinding.instance.removeObserver(this);
+    super.dispose();
+  }
+
+  List<Route<dynamic>> _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<bool> didPushRouteInformation(routeInformation) async {
     switch (routeInformation.uri) {
@@ -152,71 +198,39 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
     return super.didPushRouteInformation(routeInformation);
   }
 
-  Future<void> _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));
   }
 }
 

lib/widgets/home.dart

@@ -31,7 +31,7 @@ enum _HomePageTab {
 class HomePage extends StatefulWidget {
   const HomePage({super.key});
 
-  static Route<void> buildRoute({required int accountId}) {
+  static AccountRoute<void> buildRoute({required int accountId}) {
     return MaterialAccountWidgetRoute(accountId: accountId,
       loadingPlaceholderPage: _LoadingPlaceholderPage(accountId: accountId),
       page: const HomePage());

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<void> buildRoute({int? accountId, BuildContext? context,
+  static AccountRoute<void> buildRoute({int? accountId, BuildContext? context,
       required Narrow narrow}) {
     return MaterialAccountWidgetRoute(accountId: accountId, context: context,
       page: MessageListPage(initNarrow: narrow));

lib/widgets/page.dart

@@ -35,6 +35,12 @@ abstract class WidgetRoute<T extends Object?> extends PageRoute<T> {
   Widget get page;
 }
 
+/// A page route that specifies a particular Zulip account to use, by ID.
+abstract class AccountRoute<T extends Object?> extends PageRoute<T> {
+  /// 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<T extends Object?> extends MaterialPageRoute<T> implem
 }
 
 /// A mixin for providing a given account's per-account store on a page route.
-mixin AccountPageRouteMixin<T extends Object?> on PageRoute<T> {
+mixin AccountPageRouteMixin<T extends Object?> on PageRoute<T> implements AccountRoute<T> {
+  @override
   int get accountId;
+
   Widget? get loadingPlaceholderPage;
 
   @override

lib/widgets/profile.dart

@@ -30,7 +30,7 @@ class ProfilePage extends StatelessWidget {
 
   final int userId;
 
-  static Route<void> buildRoute({int? accountId, BuildContext? context,
+  static AccountRoute<void> buildRoute({int? accountId, BuildContext? context,
       required int userId}) {
     return MaterialAccountWidgetRoute(accountId: accountId, context: context,
       page: ProfilePage(userId: userId));

test/notifications/display_test.dart

@@ -960,11 +960,12 @@ void main() {
   group('NotificationDisplayManager open', () {
     late List<Route<void>> pushedRoutes;
 
-    void takeStartingRoutes({bool withAccount = true}) {
+    void takeStartingRoutes({Account? account, bool withAccount = true}) {
+      account ??= eg.selfAccount;
       final expected = <Condition<Object?>>[
         if (withAccount)
           (it) => it.isA<MaterialAccountWidgetRoute>()
-            ..accountId.equals(eg.selfAccount.id)
+            ..accountId.equals(account!.id)
             ..page.isA<HomePage>()
         else
           (it) => it.isA<WidgetRoute>().page.isA<ChooseAccountPage>(),
@@ -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', () {

test/widgets/page_checks.dart

@@ -6,6 +6,6 @@ extension WidgetRouteChecks<T> on Subject<WidgetRoute<T>> {
   Subject<Widget> get page => has((x) => x.page, 'page');
 }
 
-extension AccountPageRouteMixinChecks<T> on Subject<AccountPageRouteMixin<T>> {
+extension AccountRouteChecks<T> on Subject<AccountRoute<T>> {
   Subject<int> get accountId => has((x) => x.accountId, 'accountId');
 }