Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(messaging,ios): keep original UNUserNotificationCenter delegate #3427

Merged
merged 9 commits into from
Apr 22, 2020

Conversation

Salakar
Copy link
Contributor

@Salakar Salakar commented Apr 9, 2020

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call (e.g. non FCM related notifications). This should keep compatibility with other RN modules that set the delegate.

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call. This should keep compatibility with other RN modules that set the delegate.
@Salakar Salakar requested a review from Ehesp April 9, 2020 12:47
@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@4f61188). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #3427   +/-   ##
=========================================
  Coverage          ?   85.25%           
=========================================
  Files             ?      108           
  Lines             ?     3429           
  Branches          ?        0           
=========================================
  Hits              ?     2923           
  Misses            ?      506           
  Partials          ?        0           

russellwheatley
russellwheatley previously approved these changes Apr 9, 2020
@Salakar Salakar marked this pull request as ready for review April 9, 2020 14:16
This reverts commit b355a86
@radko93
Copy link
Contributor

radko93 commented Apr 10, 2020

I tried v6.4.1-alpha.0 and background notifications are working. But sending a local notification is not working. On 6.3 I use react-native-notification to display a local notification when in foreground and it does not work with 6.4.1. I also tested @react-native-community/push-notification-ios to make sure but it also does not work. Another thing is that registerNotificationOpened from react-native-notifications is also not fired (was working on 6.3).

what I've tried:

const unsubscribe = messaging().onMessage(remoteMessage => {
    console.log('notificaiton', remoteMessage) // notifications is correctly reveived
    PushNotificationIOS.presentLocalNotification({
      alertBody: 'test',
      alertTitle: 'test',
    })
  })

@Salakar
Copy link
Contributor Author

Salakar commented Apr 10, 2020

@radko93 can you show me your AppDelegate.m file contents?

@radko93
Copy link
Contributor

radko93 commented Apr 10, 2020

#import "AppDelegate.h"

#import <React/RCTBridge.h>
#import <React/RCTBundleURLProvider.h>
#import <React/RCTRootView.h>
#import "RNNotifications.h"
// RN splash screen
#import "RNSplashScreen.h"

#if DEBUG
#import <FlipperKit/FlipperClient.h>
#import <FlipperKitLayoutPlugin/FlipperKitLayoutPlugin.h>
#import <FlipperKitUserDefaultsPlugin/FKUserDefaultsPlugin.h>
#import <FlipperKitNetworkPlugin/FlipperKitNetworkPlugin.h>
#import <SKIOSNetworkPlugin/SKIOSNetworkAdapter.h>
#import <FlipperKitReactPlugin/FlipperKitReactPlugin.h>

static void InitializeFlipper(UIApplication *application) {
  FlipperClient *client = [FlipperClient sharedClient];
  SKDescriptorMapper *layoutDescriptorMapper = [[SKDescriptorMapper alloc] initWithDefaults];
  [client addPlugin:[[FlipperKitLayoutPlugin alloc] initWithRootNode:application withDescriptorMapper:layoutDescriptorMapper]];
  [client addPlugin:[[FKUserDefaultsPlugin alloc] initWithSuiteName:nil]];
  [client addPlugin:[FlipperKitReactPlugin new]];
  [client addPlugin:[[FlipperKitNetworkPlugin alloc] initWithNetworkAdapter:[SKIOSNetworkAdapter new]]];
  [client start];
}
#endif

// Firebase
@import Firebase;

//Branch
#import <RNBranch/RNBranch.h>

@implementation AppDelegate

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  // Confiture Firebase
  [FIRApp configure];
  // react-native-notifications monitoring
  [RNNotifications startMonitorNotifications];
  // Branch
  // [RNBranch useTestInstance]; // Uncomment this line to use the test key instead of the live one.
  // Flipper
  #if DEBUG
    InitializeFlipper(application);
  #endif
  [RNBranch initSessionWithLaunchOptions:launchOptions isReferrable:YES]; // <-- add this
  RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];
  RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge
                                                   moduleName:@"APPNAME"
                                            initialProperties:nil];

  rootView.backgroundColor = [[UIColor alloc] initWithRed:1.0f green:1.0f blue:1.0f alpha:1];

  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [UIViewController new];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  [self.window makeKeyAndVisible];
  [RNSplashScreen show];  // here
  return YES;
}

// Add the openURL and continueUserActivity functions
- (BOOL)application:(UIApplication *)app openURL:(NSURL *)url options:(NSDictionary<UIApplicationOpenURLOptionsKey,id> *)options {
  if (![RNBranch.branch application:app openURL:url options:options]) {
    // do other deep link routing for the Facebook SDK, Pinterest SDK, etc
  }
  return YES;
}

// Respond to Universal Links
- (BOOL)application:(UIApplication *)application continueUserActivity:(NSUserActivity *)userActivity restorationHandler:(void (^)(NSArray *restorableObjects))restorationHandler {
  BOOL handledByBranch = [[Branch getInstance] continueUserActivity:userActivity];

  return handledByBranch;
}

- (NSURL *)sourceURLForBridge:(RCTBridge *)bridge
{
#if DEBUG
  return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
#else
  return [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"];
#endif
}

@end

With push-notificaiton-ios I just copy-pasted the config from the readme and commented out RN Notifications monitoring.

@Salakar
Copy link
Contributor Author

Salakar commented Apr 10, 2020

Have you requested permission to display notifications and accepted? Can you show some code?

@radko93
Copy link
Contributor

radko93 commented Apr 10, 2020

I'm getting remote notifications in the background so I need to have permissions.

I have something like this (using react-native-permissions, also removed the token handling logic for now):

useEffect(() => {
    let notificationsHandlerListenerUnsubscribe = () => { }
    async function requestPermission() {
      const { status } = await requestNotifications(['alert', 'sound'])
      if (status === 'granted') {
        notificationsHandlerListenerUnsubscribe = notificationsHandler()
        messaging().setBackgroundMessageHandler(() => Promise.resolve())
      }
    }

    async function registerAppWithFCM() {
      await messaging().registerDeviceForRemoteMessages()

      requestPermission()
    }

    registerAppWithFCM()

    return () => {
      notificationsHandlerListenerUnsubscribe()
    }
  }, [])

and notificationHandler is basically (+ Notifications.events().registerNotificationOpened from RNNotifications):

export const notificationsHandler = () => {
  const unsubscribe = messaging().onMessage(remoteMessage => {
    PushNotificationIOS.presentLocalNotification({
      alertBody: 'test',
      alertTitle: 'test',
    })
  })

return unsubscribe
}

@sivakumar-cf
Copy link

Tried to use this PR. Still, I am not getting the AppDelegate willPresentNotification or didReceiveNotificationResponse callbacks. I am using version 6.4.1-alpha.0

@Salakar
Copy link
Contributor Author

Salakar commented Apr 10, 2020

@sivakumar-cf I need to know more information other than 'its not working', show me your app delegate code, your code that calls registerForRemoteMessages, which libraries are you using, device or simulator, etc etc?

@sivakumarjcn
Copy link

I am using https://github.com/zo0r/react-native-push-notification for displaying the local notification.
Debugged in iOS Device iPhone 7
Here is my AppDelegate.m

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
  NSString *filePath;
  filePath = [[NSBundle mainBundle] pathForResource:@"GoogleService-Info" ofType:@"plist" ];
  FIROptions *options = [[FIROptions alloc] initWithContentsOfFile:filePath];
  [FIRApp configureWithOptions:options];

  [FIRDatabase database].persistenceEnabled = YES;
 
  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  [self.window setBackgroundColor:[UIColor blackColor]];
  self.rootViewController = [[UIViewController alloc] init];
  self.rootViewController.view = rootView;
  [self.window setRootViewController:self.rootViewController];
  [self.window makeKeyAndVisible];
  [[UNUserNotificationCenter currentNotificationCenter] setDelegate:self];
  [[UIApplication sharedApplication] setApplicationIconBadgeNumber:0];

  return YES;
}


- (NSURL *)sourceURLForBridge:(RCTBridge *)bridge {
  #if DEBUG
    return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
  #else
    return [CodePush bundleURL];
  #endif
}

- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo
fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler {
  [RNCPushNotificationIOS didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler];
  completionHandler(UIBackgroundFetchResultNoData);
}

- (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings {
  [RNCPushNotificationIOS didRegisterUserNotificationSettings:notificationSettings];
}


- (void)userNotificationCenter:(UNUserNotificationCenter *)center
       willPresentNotification:(UNNotification *)notification
         withCompletionHandler:(void (^)(UNNotificationPresentationOptions))completionHandler {
  
  if(completionHandler) completionHandler(UNNotificationPresentationOptionAlert | UNNotificationPresentationOptionSound);
}

- (void)userNotificationCenter:(UNUserNotificationCenter *)center
didReceiveNotificationResponse:(UNNotificationResponse *)response
         withCompletionHandler:(void (^)(void))completionHandler {
  
  if(completionHandler) completionHandler();
}

- (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification {
  [RNCPushNotificationIOS didReceiveLocalNotification:notification];
}

- (void) application:(UIApplication *)application handleActionWithIdentifier:(NSString *)identifier
forLocalNotification:(UILocalNotification *)notification completionHandler:(void (^)(void))completionHandler {
  if (completionHandler) completionHandler();
}

- (void) application:(UIApplication *)application handleActionWithIdentifier:(NSString *)identifier
forRemoteNotification:(NSDictionary *)userInfo completionHandler:(void (^)(void))completionHandler {
  if (completionHandler) completionHandler();
}

- (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken {
  [RNCPushNotificationIOS didRegisterForRemoteNotificationsWithDeviceToken:deviceToken];
}

- (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error {
   [RNCPushNotificationIOS didFailToRegisterForRemoteNotificationsWithError:error];
}

- (void)applicationDidBecomeActive:(UIApplication *)application {
  [[UIApplication sharedApplication] setApplicationIconBadgeNumber:0];
}

@end


On React native

PushNotification.configure({
      onNotification: notification => {
        if (notification && notification.userInteraction) {
          this.notificationTapped(notification);
        }
        notification.finish(PushNotificationIOS.FetchResult.NoData);
      },
      popInitialNotification: true,
      requestPermissions: false,
    });

    messaging().onNotificationOpenedApp(remoteMessage => {
      this.notificationTapped(remoteMessage);
    });

    messaging()
      .getInitialNotification()
      .then(remoteMessage => {
        if (remoteMessage) {
          this.notificationTapped(remoteMessage);
        }
      })
      .catch(e => {
        console.log("INITIAL NOTIFICATION ERROR", e);
      });

    messaging()
      .registerDeviceForRemoteMessages()
      .then(val => {
        messaging()
          .hasPermission()
          .then(granted => {
            CFAnalytics.logNotificationPermission(granted.toString());
            messaging()
              .requestPermission()
              .then(granted => {
                if (granted) {
                  this._setUpNotificationTokens();
                }
              })
              .catch(e => {
                console.log(e);
              });
          });
      })
      .catch(e => {});

@sivakumar-cf
Copy link

@Salakar I am getting notifications only when the app enters background or in the Killed state, But the AppDelegate notifications delegate callbacks are not getting called due to swizzling. Is there any way I can disable firebase swizzling methods, and manually send the callbacks to react-native-firebase SDK?

@artyorsh
Copy link

@radko93 @Salakar I can confirm the package published to next tag works fine when testing via firebase console. The issue occurs for notifications sent by other services (SendBird in my case).

Setup:

 "@react-native-community/push-notification-ios": "^1.1.1",
 "@react-native-firebase/app": "^6.4.1-alpha.0",
 "@react-native-firebase/crashlytics": "^6.4.1-alpha.0",
 "@react-native-firebase/messaging": "^6.4.1-alpha.0",
 "@react-native-firebase/perf": "^6.4.1-alpha.0",
 "react-native-push-notification": "^3.1.9",
 "sendbird": "3.0.117",

Configuration:

AppDelegate is configured strongly by following guides, with no differences.
Same for configuring messaging.

Note: not sure if it is documented, but you should also upload the certificates to firebase console (done via project overview -> project settings -> cloud messaging).

Result: Push notifications are displayed well if sent by firebase console, but not with other services.

@radko93
Copy link
Contributor

radko93 commented Apr 13, 2020

@artyorsh but does it also work in foregorud?

@artyorsh
Copy link

artyorsh commented Apr 13, 2020

@radko93 yes. Works as expected in case the notification is sent with firebase console

@artyorsh
Copy link

@radko93

Update on #3427 (comment):

That was my issue not reading sendbird docs carefully. What they say - the app state should be handled with their APIs. The screenshot below might be helpful for sendbird users who stuck into same issue)

image

This makes an issue I described invalid.
@Salakar Sorry for posting the wrong information. Now I confirm an alpha release works properly.

@Salakar Salakar added platform: ios plugin: messaging FCM only - ( messaging() ) - do not use for Notifications labels Apr 14, 2020
@nabilfreeman
Copy link

nabilfreeman commented Apr 15, 2020

@sivakumar-cf I also had this question about swizzling, I think RN Firebase now takes stuff straight from native Firebase further up the chain and there is no exposed Obj-c method to trigger notifications like in v5.

So you need to bypass swizzling there.

https://firebase.google.com/docs/cloud-messaging/ios/client#objective-c_6
https://firebase.google.com/docs/cloud-messaging/ios/receive#objective-c-ios-10

I had to do this so I could receive Intercom push notifications as well as Firebase.

Edit: Actually this didn't work, I'm a bit lost to be honest. v6 is very different from v5 and a lot of things I've found common practice over the past few years regarding push seem to be gone.

Edit2: I got it, I was missing await FirebaseMessaging().registerDeviceForRemoteMessages(); on app launch. OMG!! I hate push!! Going to bed. Thanks for this bugfix.

@Salakar Salakar self-assigned this Apr 17, 2020
jamesjara
jamesjara previously approved these changes Apr 18, 2020
@Salakar
Copy link
Contributor Author

Salakar commented Apr 22, 2020

@nabilfreeman Edit2: I got it, I was missing await FirebaseMessaging().registerDeviceForRemoteMessages(); on app launch. OMG!! I hate push!! Going to bed. Thanks for this bugfix.

Seems like you're not the only one missing this - so I've just pushed a change where registration is now done automatically (with the option to opt-out of auto-registration if required)

@nabilfreeman
Copy link

Seems like you're not the only one missing this - so I've just pushed a change where registration is now done automatically (with the option to opt-out of auto-registration if required)

Oh you legend!!! Thanks so much!!!

Salakar and others added 2 commits April 22, 2020 11:39
* fix(messaging): add activity check to getInitialNotification

* fix(messaging): add activity check to getInitialNotification

* Update .spellcheck.dict.txt

Co-authored-by: Mike Diarmid <[email protected]>
@Salakar Salakar merged commit a800cdb into master Apr 22, 2020
@Salakar Salakar deleted the @salakar/original-delegate-fix branch April 22, 2020 11:49
andersondanilo pushed a commit to vixtech/react-native-firebase that referenced this pull request Nov 9, 2020
…ertase#3427)

* fix(messaging,ios): keep original UNUserNotificationCenter delegate

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call. This should keep compatibility with other RN modules that set the delegate.

* v6.4.1-alpha.0

* Revert "v6.4.1-alpha.0"

This reverts commit b355a86

* feat: automatically register with APNs

* docs: typos

* fix: forward delegate call to FIRAuth

Fixes / supersedes invertase#3425

* fix(messaging): add activity check to getInitialNotification (invertase#3495)

* fix(messaging): add activity check to getInitialNotification

* fix(messaging): add activity check to getInitialNotification

* Update .spellcheck.dict.txt

Co-authored-by: Mike Diarmid <[email protected]>

Co-authored-by: Elliot Hesp <[email protected]>
@nabilfreeman
Copy link

Just had a funny moment debugging an RN Firebase push issue, found this issue and my own comments from last year 🤦‍♂️

hmhm2292 pushed a commit to hmhm2292/react-native-firebase that referenced this pull request Jul 13, 2021
…ertase#3427)

* fix(messaging,ios): keep original UNUserNotificationCenter delegate

Keeps a reference to any original UNUserNotificationCenter delegates that are set before we replace the delegate with out own implementation. Internally we will also call the original delegate if our code does not already handle the delegate call. This should keep compatibility with other RN modules that set the delegate.

* v6.4.1-alpha.0

* Revert "v6.4.1-alpha.0"

This reverts commit b355a86

* feat: automatically register with APNs

* docs: typos

* fix: forward delegate call to FIRAuth

Fixes / supersedes invertase#3425

* fix(messaging): add activity check to getInitialNotification (invertase#3495)

* fix(messaging): add activity check to getInitialNotification

* fix(messaging): add activity check to getInitialNotification

* Update .spellcheck.dict.txt

Co-authored-by: Mike Diarmid <[email protected]>

Co-authored-by: Elliot Hesp <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: ios plugin: messaging FCM only - ( messaging() ) - do not use for Notifications
Projects
None yet
9 participants