ios: Fix bug where tapping a notification didn't navigate to conversation

ios/ZulipMobile/AppDelegate.mm

@@ -11,6 +11,9 @@
 
 #import <React/RCTAppSetupUtils.h>
 
+#import "ExpoModulesCore-Swift.h"
+#import "ZulipMobile-Swift.h"
+
 #if RCT_NEW_ARCH_ENABLED
 #import <React/CoreModulesPlugins.h>
 #import <React/RCTCxxBridgeDelegate.h>
@@ -114,20 +117,14 @@ - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotif
   [RNCPushNotificationIOS didFailToRegisterForRemoteNotificationsWithError:error];
 }
 
-// Required for the notification event. You must call the completion handler after handling the remote notification.
-- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo
-                                                       fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler
-{
-  [RNCPushNotificationIOS didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
-}
-
 // Required for localNotification event
 - (void)userNotificationCenter:(UNUserNotificationCenter *)center
 didReceiveNotificationResponse:(UNNotificationResponse *)response
          withCompletionHandler:(void (^)(void))completionHandler
 {
-  [RNCPushNotificationIOS didReceiveNotificationResponse:response];
-  completionHandler();
+  [ZLPNotificationsEvents userNotificationCenter:center
+                                      didReceive:response
+                           withCompletionHandler:completionHandler];
 }
 
 // Called when a notification is delivered to a foreground app.

ios/ZulipMobile/ZLPNotifications.swift

@@ -1,9 +1,50 @@
 import Foundation
 import UIKit
 import React.RCTBridgeModule
+import React.RCTConvert
+import React.RCTEventEmitter
 
-@objc(ZLPNotifications)
-class ZLPNotifications: NSObject {
+@objc(ZLPNotificationsEvents)
+class ZLPNotificationsEvents: RCTEventEmitter {
+  static var currentInstance: ZLPNotificationsEvents? = nil
+
+  override func startObserving() -> Void {
+    super.startObserving()
+    ZLPNotificationsEvents.currentInstance = self
+  }
+
+  override func stopObserving() -> Void {
+    ZLPNotificationsEvents.currentInstance = nil
+    super.stopObserving()
+  }
+
+  @objc
+  override func supportedEvents() -> [String] {
+    return ["response"]
+  }
+
+  @objc
+  class func userNotificationCenter(
+    _ center: UNUserNotificationCenter,
+    didReceive response: UNNotificationResponse,
+    withCompletionHandler completionHandler: () -> Void
+  ) {
+    currentInstance?.sendEvent(
+      withName: "response",
+
+      // The RCTJSONClean was copied over from
+      // @react-native-community/push-notification-ios; possibly we don't
+      // need it.
+      body: RCTJSONClean(
+        response.notification.request.content.userInfo
+      )
+    )
+    completionHandler()
+  }
+}
+
+@objc(ZLPNotificationsStatus)
+class ZLPNotificationsStatus: NSObject {
   /// Whether the app can receive remote notifications.
   // Ideally we could subscribe to changes in this value, but there
   // doesn't seem to be an API for that. The caller can poll, e.g., by

ios/ZulipMobile/ZLPNotificationsBridge.m

@@ -1,9 +1,13 @@
 #import "React/RCTBridgeModule.h"
+#import "React/RCTEventEmitter.h"
 
-// Register the ZLPNotifications implementation with React Native, needed
-// because ZLPNotifications is in Swift:
+// Register our Swift modules with React Native:
 //   https://reactnative.dev/docs/0.68/native-modules-ios#exporting-swift
-@interface RCT_EXTERN_MODULE(ZLPNotifications, NSObject)
+
+@interface RCT_EXTERN_MODULE(ZLPNotificationsEvents, RCTEventEmitter)
+@end
+
+@interface RCT_EXTERN_MODULE(ZLPNotificationsStatus, NSObject)
 
     RCT_EXTERN_METHOD(areNotificationsAuthorized:
         (RCTPromiseResolveBlock) resolve

src/notification/NotificationListener.js

@@ -1,16 +1,23 @@
 /* @flow strict-local */
-import { DeviceEventEmitter, Platform } from 'react-native';
+import { DeviceEventEmitter, Platform, NativeModules, NativeEventEmitter } from 'react-native';
 import type { PushNotificationEventName } from '@react-native-community/push-notification-ios';
 import PushNotificationIOS from '@react-native-community/push-notification-ios';
+import invariant from 'invariant';
 
 import type { JSONableDict } from '../utils/jsonable';
 import type { GlobalDispatch } from '../types';
 import { androidGetToken, handleDeviceToken } from './notifTokens';
 import type { Notification } from './types';
 import * as logging from '../utils/logging';
-import { fromPushNotificationIOS } from './extract';
+import { fromAPNs } from './extract';
 import { narrowToNotification } from './notifOpen';
 
+// TODO: Could go in a separate file, with some thin wrapper perhaps.
+const iosNativeEventEmitter =
+  Platform.OS === 'ios'
+    ? new NativeEventEmitter<{| +response: [JSONableDict] |}>(NativeModules.ZLPNotificationsEvents)
+    : null;
+
 /**
  * From ios/RNCPushNotificationIOS.m in @rnc/push-notification-ios at 1.2.2.
  */
@@ -43,14 +50,34 @@ export default class NotificationListener {
   }
 
   /** Private. */
-  listenIOS(name: PushNotificationEventName, handler: (...empty) => void | Promise<void>) {
+  listenIOS(
+    args:
+      | {| +name: PushNotificationEventName, +handler: (...empty) => void | Promise<void> |}
+      | {| +name: 'response', +handler: JSONableDict => void |},
+  ) {
+    invariant(
+      iosNativeEventEmitter != null,
+      'NotificationListener: expected `iosNativeEventEmitter` in listenIOS',
+    );
+
+    if (args.name === 'response') {
+      const { name, handler } = args;
+      const sub = iosNativeEventEmitter.addListener(name, handler);
+      this.unsubs.push(() => sub.remove());
+      return;
+    }
+
+    // TODO: Use iosNativeEventEmitter (as above) instead of
+    //   PushNotificationIOS (as below) for all iOS events
+
     // In the native code, the PushNotificationEventName we pass here
     // is mapped to something else (see implementation):
     //
     // 'notification'      -> 'remoteNotificationReceived'
     // 'localNotification' -> 'localNotificationReceived'
     // 'register'          -> 'remoteNotificationsRegistered'
     // 'registrationError' -> 'remoteNotificationRegistrationError'
+    const { name, handler } = args;
     PushNotificationIOS.addEventListener(name, handler);
     this.unsubs.push(() => PushNotificationIOS.removeEventListener(name));
   }
@@ -92,15 +119,18 @@ export default class NotificationListener {
       this.listenAndroid('notificationOpened', this.handleNotificationOpen);
       this.listenAndroid('remoteNotificationsRegistered', this.handleDeviceToken);
     } else {
-      this.listenIOS('notification', (notification: PushNotificationIOS) => {
-        const dataFromAPNs = fromPushNotificationIOS(notification);
-        if (!dataFromAPNs) {
-          return;
-        }
-        this.handleNotificationOpen(dataFromAPNs);
+      this.listenIOS({
+        name: 'response',
+        handler: payload => {
+          const dataFromAPNs = fromAPNs(payload);
+          if (!dataFromAPNs) {
+            return;
+          }
+          this.handleNotificationOpen(dataFromAPNs);
+        },
       });
-      this.listenIOS('register', this.handleDeviceToken);
-      this.listenIOS('registrationError', this.handleIOSRegistrationFailure);
+      this.listenIOS({ name: 'register', handler: this.handleDeviceToken });
+      this.listenIOS({ name: 'registrationError', handler: this.handleIOSRegistrationFailure });
     }
 
     if (Platform.OS === 'android') {

src/notification/extract.js

@@ -38,11 +38,12 @@ export const fromAPNsImpl = (data: ?JSONableDict): Notification | void => {
   //
   // For the format this parses, see `ApnsPayload` in src/api/notificationTypes.js .
   //
-  // Though what it actually receives is more like this:
+  // Though in one case what it actually receives is more like this:
   //   $Rest<ApnsPayload, {| aps: mixed |}>
-  // because the `ApnsPayload` gets parsed by the `PushNotificationIOS`
-  // library, and what it gives us through `getData` is everything but the
-  // `aps` property.
+  // That case is the "initial notification", a notification that launched
+  // the app by being tapped, because the `PushNotificationIOS` library
+  // parses the `ApnsPayload` and gives us (through `getData`) everything
+  // but the `aps` property.
 
   /** Helper function: fail. */
   const err = (style: string) =>
@@ -160,7 +161,7 @@ export const fromAPNsImpl = (data: ?JSONableDict): Notification | void => {
  *
  * @returns A `Notification` on success; `undefined` on failure.
  */
-const fromAPNs = (data: ?JSONableDict): Notification | void => {
+export const fromAPNs = (data: ?JSONableDict): Notification | void => {
   try {
     return fromAPNsImpl(data);
   } catch (errorIllTyped) {

src/settings/NotifTroubleshootingScreen.js

@@ -48,7 +48,7 @@ import { Role } from '../api/permissionsTypes';
 
 const {
   Notifications, // android
-  ZLPNotifications, // ios
+  ZLPNotificationsStatus, // ios
 } = NativeModules;
 
 type Props = $ReadOnly<{|
@@ -92,7 +92,7 @@ function useNativeState() {
     (async () => {
       const systemSettingsEnabled: boolean = await (Platform.OS === 'android'
         ? Notifications.areNotificationsEnabled()
-        : ZLPNotifications.areNotificationsAuthorized());
+        : ZLPNotificationsStatus.areNotificationsAuthorized());
       setResult(r => ({ ...r, systemSettingsEnabled }));
     })();