invertase / react-native-firebase

πŸ”₯ A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.68k stars 2.21k forks source link

getInitialNotification() message still remains after receiving notification #2618

Closed akhilsanker closed 4 years ago

akhilsanker commented 5 years ago

Issue

After receiving the notification, when the app is closed, the notification message is subscribed with the notifications().getInitialNotification() method.Β But the method returns the previous message again and again even after re-rendering.

    firebase.notifications().getInitialNotification()
      .then((notificationOpen: NotificationOpen) => {
        console.log(notificationOpen, "notificationOpen 3");
        if(notificationOpen.notification._data.module_key == 3) {
            this.props.navigation.navigate('Profile');
        }
        if (notificationOpen) {
          // App was opened by a notification
          // Get the action triggered by the notification being opened
          const action = notificationOpen.action;
          // Get information about the notification that was opened
          const notification: Notification = notificationOpen.notification;  
        }
      });

Project Files

iOS

Click To Expand

#### `ios/Podfile`: - [ ] I'm not using Pods - [x] I'm using Pods and my Podfile looks like: ```ruby # Uncomment the next line to define a global platform for your project platform :ios, '9.0' target 'xxx' do # Uncomment the next line if you're using Swift or would like to use dynamic frameworks # use_frameworks! # Pods for ReactPushNotifications - Add these lines pod 'Firebase/Messaging' target 'xxx' do inherit! :search_paths # Pods for testing end end ``` #### `AppDelegate.m`: ```objc #import "AppDelegate.h" #import #import #import #import #import "RNFirebaseNotifications.h" #import "RNFirebaseMessaging.h" @implementation AppDelegate - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions { [FIRApp configure]; [[UNUserNotificationCenter currentNotificationCenter] setDelegate:self]; [RNFirebaseNotifications configure]; RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions]; RCTRootView *rootView = [[RCTRootView alloc] initWithBridge:bridge moduleName:@"StudentHub" 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]; return YES; } - (NSURL *)sourceURLForBridge:(RCTBridge *)bridge { #if DEBUG return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil]; #else return [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"]; #endif } - (void)application:(UIApplication *)application didReceiveRemoteNotification:(nonnull NSDictionary *)userInfo fetchCompletionHandler:(nonnull void (^)(UIBackgroundFetchResult))completionHandler{ [[RNFirebaseNotifications instance] didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler]; } - (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings { [[RNFirebaseMessaging instance] didRegisterUserNotificationSettings:notificationSettings]; } -(void) userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler { [[RNFirebaseMessaging instance] didReceiveRemoteNotification:response.notification.request.content.userInfo]; completionHandler(); } @end ```


Environment

Click To Expand

**`react-native info` output:** ``` React Native Environment Info: System: OS: macOS 10.14.6 CPU: (4) x64 Intel(R) Core(TM) i5-3210M CPU @ 2.50GHz Memory: 59.52 MB / 8.00 GB Shell: 3.2.57 - /bin/bash Binaries: Node: 12.1.0 - /usr/local/bin/node npm: 6.9.0 - /usr/local/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman SDKs: iOS SDK: Platforms: iOS 12.4, macOS 10.14, tvOS 12.4, watchOS 5.3 Android SDK: API Levels: 23, 26, 27, 28 Build Tools: 28.0.3, 29.0.0 System Images: android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom, android-Q | Google Play Intel x86 Atom IDEs: Android Studio: 3.4 AI-183.6156.11.34.5522156 Xcode: 10.3/10G8 - /usr/bin/xcodebuild npmPackages: react: 16.8.3 => 16.8.3 react-native: 0.59.8 => 0.59.8 npmGlobalPackages: react-native-cli: 2.0.1 react-native-rename: 2.4.1 react-native-video: 4.4.1 ``` - **Platform that you're experiencing the issue on**: - [x] iOS - [ ] Android - [x] **iOS** but have not tested behavior on Android - [ ] **Android** but have not tested behavior on iOS - [ ] Both - **`react-native-firebase` version you're using that has this issue:** - `5.5.6` - **Are you using `TypeScript`?** - `N`

mikehardy commented 5 years ago

you didn't really fill out the template. react-native info output? where are you experiencing the problem? A snippet of the code where you try to do the thing you want and it returns unexpected problems?

akhilsanker commented 5 years ago

Hi @mikehardy,

Any solution?

Thanks

mikehardy commented 5 years ago

Interesting. My solution would be to check that on app bootstrap once, and no where near anything that renders

akhilsanker commented 5 years ago

But getInitialNotification() still returning the notification data, whereas the data in firebase.notifications().onNotificationOpened() is subscribed only once.

mikehardy commented 5 years ago

Are you expecting getInitialNotification to be like a queue, and once you call it you dequeue the data? I'm thinking it is more like a store, and if there was an initial notification the info will always be there. I think there is a misunderstanding of the API. onNotificationOpened() is for when your app is open, and a notification is clicked. getInitialNotification() is for when your app was stopped and is started by clicking on a notification. They are different cases. https://rnfirebase.io/docs/v5.x.x/notifications/receiving-notifications#App-Closed

I might be totally confused about your use case, but I think based on what I'm reading getInitialNotification is just supposed to behave different than what I infer are your expectations

akhilsanker commented 5 years ago

Is there any way to dequeue the data of getInitialNotification() as like of onNotificationOpened()? My scenario is to open a specific page on entering the app, on clicking the received notification. In case of app is in background, it works fine with 'onNotificationOpened()'. But in case of 'getInitialNotification()' the home page redirects to the specific page, but on returning to the home page, the 'getInitialNotification()' gets called again, since its not un subscribed, and the app remains in the specific page. Or is there is any other way to direct the app to specific page on clicking the notification and also to return to home page from that page?

Thanks

suren1525 commented 5 years ago

Any solution?

akhilsanker commented 5 years ago

Not yet πŸ‘Ž

mikehardy commented 5 years ago

As long as your app is running, you can maintain state right? So, set a static boolean somewhere that indicates you have handled the initialNotification data already. Then only redirect if you have not handled it? For me something like ReSub or redux or similar would be perfect for this. Where ever you store state in your app, this is the time to use it.

akhilsanker commented 5 years ago

It would be better to dequeue the data of getInitialNotification(), since the data of getInitialNotification()needed only once when we enter the app by clicking the notification, like the onNotificationOpened().

mikehardy commented 5 years ago

Sure, but don't let an idea of "better" stop you from "make it work" :-). APIs don't always behave the way we want for our project, and I'm sure if you proposed a PR that did this change and it was merged, the comments thread would fill up with people wondering why you changed it, since it worked already for them as it was πŸ€·β€β™‚

akhilsanker commented 5 years ago

@mikehardy , Another scenario in Android.

  1. Closed the app.
  2. Received the notification.
  3. Opened the app by tapping the notification.
  4. The data is subscribed in getInitialNotification().
  5. Closed the app by hardware back button. The app is still in recent used app list.
  6. Open the app from recent used app list. The notification data will again get subscribed in getInitialNotification().
  7. But ,without closing the app, if the app is minimise and open the app from recent used app list, getInitialNotification() is not subscribed again.

    Is there any way to control it using state or any way to unsubscribed getInitialNotification()?

Thanks

mikehardy commented 5 years ago

This is an imprecise description of the Android activity lifecycle https://developer.android.com/guide/components/activities/activity-lifecycle

Additionally, you keep describing getInitialNotification as being subscribed to. It is not an event. There is no subscription. It is just available data that you can fetch repeatedly or not.

When you use the hardware back button you are not closing the app. The activity is pausing. It still exists as a unix process in the android system, and all of its state is preserved, including the data you could fetch from getInitialNotification

It may be stopped (at which point state will be destroyed, including the state of getInitialNotification

There are plenty of ways to control state in your app. I mentioned redux. You could use a Singleton pattern service initialized from bootstrap and toggled as to whether you responded or not. You can use the ReSub package and have a Store you subscribe to with this information.

Each of those is a viable solution, and with the information I've posted above it should be possible to get something working.

39otrebla commented 5 years ago

With getInitialNotification this, unfortunately, happens with CodePush mandatory updates (which forces a restart of the react native instance), in production.

@mikehardy in this case the "static boolean somewhere" you suggested won't work, as the react native instance gets restarted. We are forced to persist this information, which (you'll agree) is far from be the ideal solution.

I too think that getInitialNotifcation should be cleared after the first access, and I don't see how it could break other use cases since apps normally get initialized once.

By the way, this very easily reproducible in dev:

  1. Install a CodePush-enabled app in debug
  2. send a notification
  3. tap on it and let the app open
  4. Cmd + R
  5. getInitialNotification will be called with notification data again
  6. Repeat 4. => you'll get to 5. , forever

This is not the behavior a developer expects, in any use case.

mikehardy commented 5 years ago

Hmm. Interesting. So you're saying that for code-push the native code keeps running but the Javascript bundle restarts so it "feels" (at the javascript layer) like a fresh start programmatically but the user has already seen whatever you wanted from the initial notification (which might have been hours ago) and to workaround you use async storage or something to mark you've consumed it since memory state is not reliable?

I could be wrong about any of that - I'm trying to active listen / restate to make sure I understand.

In that case I could more easily understand why you could want an API you might unambiguously call call consumeInitialNotification or getInitialNotificationOneTime (if you get my meaning).

:thinking:

39otrebla commented 5 years ago

@mikehardy that's correct. You are right when you say that getInitialNotification API should not be seen as a queue. But (in my opinion) it's also true that it should reset the notification data the first time it gets accessed (again, this won't break any use case since apps get normally initialized only once).

mikehardy commented 5 years ago

Well, I have very bad luck with statements like "this won't break any use case" especially when coupled with a definition of "normal" - and by that I just mean to say that I am constantly surprised at the ingenuity of people when solving problems with software, and how they use APIs in unconsidered ways :-). But maybe there is something that can be done like a second "once only" API or something. I am not officially proposing anything, but I personally hadn't thought of the javascript layer as disposable like in the code-push scenario so this has much more merit (whatever that is worth) in my opinion now

39otrebla commented 5 years ago

Another API would work too, of course. I still think a developer expects getInitialNotification to have a value just once per app session, as the name suggests.

mikehardy commented 5 years ago

@Salakar / @Ehesp something philosophical for you - should getInitialNotification consume the notification data, thus only returning it once and returning empty after the first call, or should it be (like it is now) a continuous fountain of the initial notification (if it existed). And if it is continuous, should there be a separate one-shot api. I take no position but you two probably have some experience / thoughts in the area

Dhirajbhujbal commented 5 years ago

Facing the same issue when I open the app by clicking on the notification it wents to the Notification tab As my code navigates use on it but I do 'CMD+R' I am expecting to navigate on the dashboard tab which is default but it still navigating on the 'Notfication' Tab any workaround please.

appli-intramuros commented 5 years ago

A possible workaround is to use global variable (global.MyVar) to ensure getInitialNotification() is called once.

How I did it:

  1. Declare and init the global variable (at the very beginning of App.js for example) :
    
    #App.js

global.notificationInitialized = false;


2. In your notification component,  call `getInitialNotification()` **only** if not initialized :

myNotificationComponent.js

componentDidMount() { ... if (!global.notificationInitialized) { _getInitialNotification(navigation); global.notificationInitialized = true; } ... }


3. Ensure your global variable is reset when exiting app (back button on Android):

App.js

componentWillUnmount() { ... global.notificationInitialized = false; ... }

39otrebla commented 5 years ago

@appli-intramuros this does NOT work in the codepush (i.e. JS reload) case, which is (so far) the most concerning one.

mikehardy commented 5 years ago

On my work project I'm chewing through Deep Linking right now and I thought since they have a similar API (getDynamicLink()) it would be useful to contrast.

In that implementation they clear the link as you fetch it, so as opposed to being a flag, it is more of a queue or stack (1 item, so queue vs stack the same)

https://firebase.google.com/docs/dynamic-links/android/receive#handle_deep_links

You must call getDynamicLink() in every activity that might be launched by the 
link, even though the link might be available from the intent using 
getIntent().getData(). Calling getDynamicLink() retrieves the link and clears that 
data so it is only processed once by your app.

You normally call getDynamicLink() in the main activity as well as any activities
 launched by intent filters that match the link.

So the Google SDK team has definitely taken a side here, but it's important to note that complexity - every Activity has to implement it. Not too bad for react-native where it's normally one activity but important to note.

@Salakar / @Ehesp this might be interesting design-thinking as you shape the API of a v6-compatible notifications package.

mikehardy commented 5 years ago

@39otrebla your current workaround would be async-storage I think (to store the flag consumed-or-not status)

Dhirajbhujbal commented 5 years ago

@appli-intramuros Thanks, But I already thought about the same logic but was just curious that is there is any way to deque the getInitialNotification.

39otrebla commented 5 years ago

@mikehardy yes, we store in AsyncStorage the notificationId of the latest notification opened with getInitialNotification. Then we just compare it with the next one.

async handleInitialNotification(initialNotification) {
  if (initialNotification !== null) {
    try {
      const lastInitialNotificationId = await AsyncStorage.getItem(AS_KEY_LAST_INITIAL_NOTIFICATION_ID)

      if (lastInitialNotificationId !== null) {
        if (lastInitialNotificationId === initialNotification.notification.notificationId) {
          return
        }
      }

      await AsyncStorage.setItem(AS_KEY_LAST_INITIAL_NOTIFICATION_ID, String(initialNotification.notification.notificationId))
    } catch (e) {
      // don't mind, this is a problem only if the current RN instance has been reloaded by a CP mandatory update
    }
  }
}

Thanks for pointing out Google's point of view.

BTW, this workaround has been deployed few weeks ago. Now we can finally kick out campaigns at will, without thinking about whether we have a CP mandatory update rolling out...

lovekothari123 commented 5 years ago

Same problem from my side. i am also not getting any response(payload) while application. In background/killed state i am using this :-

import firebase from "react-native-firebase"; import { Notification, NotificationOpen, RemoteMessage, Firebase } from "react-native-firebase";

firebase .notifications() .getInitialNotification() .then((notificationOpen: NotificationOpen) => { if (notificationOpen) { alert("get data testing done") } });

also i am trying that πŸ‘Ž but did't get any solution

firebase.messaging().onMessage((message: RemoteMessage) => { // Process your message as required console.log("Notification open App closed in open " + message); });

If is there any one who can resolved this issues please update.

Dhirajbhujbal commented 5 years ago

@lovekothari123

As far I know below module will work only when your app is in foreground

firebase.messaging().onMessage((message: RemoteMessage) => { // Process your message as required console.log("Notification open App closed in open " + message); })

lovekothari123 commented 4 years ago

Yes @Dhirajbhujbal i know onMessage() function is working on foreground but when user click on notification while its on foreground or background i did't get any call back as per document they will provide such function for example :- 1) firebase.notifications().getInitialNotification() 2) const notificationOpen: NotificationOpen = await firebase.notifications().getInitialNotification();

This both function provided by the react-native-firebase team but its not working on both os android/ios It will always through an null here is my example :- firebase .notifications() .getInitialNotification() .then((notificationOpen: NotificationOpen) => { console.log("getInitialNotification enter "); console.log(### notificationOpen); .....}

And here is my terminal log data :- 10-23 15:55:34.005 32431 32608 I ReactNativeJS: getInitialNotification enter 10-23 15:55:34.006 32431 32608 I ReactNativeJS: ### null

If any one have a proper solution then please help me for that Thank-you

Dhirajbhujbal commented 4 years ago

@lovekothari123 are you facing issue on android only?

mikehardy commented 4 years ago

@r4mdat it appears you removed your comment connecting #1820 and this one but you may be on to something there. I think they are slightly different, but also related and that PR might fix the issue here. Anyone here want to give it a go with patch-package to test the PR? It was only closed for lack of testing, not any reason it can't merge

r4mdat commented 4 years ago

@mikehardy I was ready to merge the PR to a fork but realized its iOS specific. Experiencing this issue on Android (haven't tested iOS) but regardless, didn't seem like the quick fix I was hoping for.

FWIW, I believe the design of this could be better. getIntiialNotification implies the use of the method in the context of some event occurring (whether the app was opened as the result of a notification action). It's usefulness to reliably determine that doesn't work if there's not a way to dequeue NotificationOpen (whether by the API itself through invocation of the getInitialNotifcation method or an additional method perhaps acknowledgeInitialNotification)

I honestly can't think of a primary use case that supports having the notification constantly returned by the method. And certainly, if there was value in having that, it not's entirely lost by using Async Storage as well -- or having an additional method to manually clear the opened notification.

mikehardy commented 4 years ago

Yes @r4mdat I agree with you after reading the similar getInitialLink API as mentioned here https://github.com/invertase/react-native-firebase/issues/2618#issuecomment-543884753 - at this point my opinion is that getInitialNotification should work once then mirror getInitialLink by not clearing itself. If you can get anything together for #1820 I am at least paying attention :-), cheers

luatnd commented 4 years ago

Hm, I intend to submit a new issue but I found this is exactly my issue: https://github.com/invertase/react-native-firebase/issues/2618#issuecomment-536318853

Then I think this would be the best workaround at this time: https://github.com/invertase/react-native-firebase/issues/2618#issuecomment-544231700

Thanks to @39otrebla πŸ‘ Looking forward to seeing anything like #1820 to be merged

mikehardy commented 4 years ago

@luatnd I think #1820 just needed testing, if you use patch-package and verify the approach + code works we can re-examine and maybe merge it?

luatnd commented 4 years ago

I tested #1820, I don't have experience in iOS objective C so I cannot estimate what will be affected. It seems OK for cases:

Be noted that this issue appears on both iOS & Android, #1820 is just for iOS.

So my safer workaround is the mentioned JS approachment, to keep my production app stable.

stale[bot] commented 4 years ago

Hello πŸ‘‹, to help manage issues we automatically close stale issues. This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs. Thank you for your contributions.

akhilsanker commented 4 years ago

Still require community's attention.

stale[bot] commented 4 years ago

Hello πŸ‘‹, to help manage issues we automatically close stale issues. This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs. Thank you for your contributions.

39otrebla commented 4 years ago

Still requires community's attention.

mikehardy commented 4 years ago

"community" is you, I don't see a PR referenced? Submit one and we can look into it. Note that notifications doesn't exist in v6 though, so doing any work on it (vs using notifee.app or react-native-push-notifications or similar) is a time malinvestment

Ehesp commented 4 years ago

We fixed this issue on Notifee, however the implementation is vastly different & would require a major overhaul on v5.

Happy to accept PRs to sort this on v5.

stale[bot] commented 4 years ago

Hello πŸ‘‹, to help manage issues we automatically close stale issues. This issue has been automatically marked as stale because it has not had activity for quite some time. Has this issue been fixed, or does it still require the community's attention?

This issue will be closed in 15 days if no further activity occurs. Thank you for your contributions.

jetobe95 commented 4 years ago
async handleInitialNotification(initialNotification) {
  if (initialNotification !== null) {
    try {
      const lastInitialNotificationId = await AsyncStorage.getItem(AS_KEY_LAST_INITIAL_NOTIFICATION_ID)

      if (lastInitialNotificationId !== null) {
        if (lastInitialNotificationId === initialNotification.notification.notificationId) {
          return
        }
      }

      await AsyncStorage.setItem(AS_KEY_LAST_INITIAL_NOTIFICATION_ID, String(initialNotification.notification.notificationId))
    } catch (e) {
      // don't mind, this is a problem only if the current RN instance has been reloaded by a CP mandatory update
    }
  }
}

Thank you for your solution!!!

ThanhNguyen140797 commented 4 years ago

@mikehardy yes, we store in AsyncStorage the notificationId of the latest notification opened with getInitialNotification. Then we just compare it with the next one.

async handleInitialNotification(initialNotification) {
  if (initialNotification !== null) {
    try {
      const lastInitialNotificationId = await AsyncStorage.getItem(AS_KEY_LAST_INITIAL_NOTIFICATION_ID)

      if (lastInitialNotificationId !== null) {
        if (lastInitialNotificationId === initialNotification.notification.notificationId) {
          return
        }
      }

      await AsyncStorage.setItem(AS_KEY_LAST_INITIAL_NOTIFICATION_ID, String(initialNotification.notification.notificationId))
    } catch (e) {
      // don't mind, this is a problem only if the current RN instance has been reloaded by a CP mandatory update
    }
  }
}

Thanks for pointing out Google's point of view.

BTW, this workaround has been deployed few weeks ago. Now we can finally kick out campaigns at will, without thinking about whether we have a CP mandatory update rolling out...

Perfect, thanks!

ahmed-khlifi commented 2 years ago

in solved my problem this way, in my case i have "remoteMessage.messageId" as an ID

  const handleQuiteNotification = () => {
    messaging()
      .getInitialNotification()
      .then(async remoteMessage => {
        // check if notification has been opened before
        AsyncStorage.getItem('lastRemoteNotificationID').then(
          lastRemoteNotificationID => {
            if (lastRemoteNotificationID !== remoteMessage.messageId) {
              navigation.navigate(remoteMessage.data.navigateTo, {
                tab: remoteMessage.data.type,
              });
            }
          },
        );
        await AsyncStorage.setItem(
          'lastRemoteNotificationID',
          remoteMessage.messageId,
        );
      });
  };
jihyeonjeong11 commented 6 months ago

in solved my problem this way, in my case i have "remoteMessage.messageId" as an ID

  const handleQuiteNotification = () => {
    messaging()
      .getInitialNotification()
      .then(async remoteMessage => {
        // check if notification has been opened before
        AsyncStorage.getItem('lastRemoteNotificationID').then(
          lastRemoteNotificationID => {
            if (lastRemoteNotificationID !== remoteMessage.messageId) {
              navigation.navigate(remoteMessage.data.navigateTo, {
                tab: remoteMessage.data.type,
              });
            }
          },
        );
        await AsyncStorage.setItem(
          'lastRemoteNotificationID',
          remoteMessage.messageId,
        );
      });
  };

Thanks for the solution!