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

[question] APNs integration #159

Closed
vonovak opened this issue Jun 19, 2020 · 15 comments
Closed

[question] APNs integration #159

vonovak opened this issue Jun 19, 2020 · 15 comments
Assignees

Comments

@vonovak
Copy link
Contributor

vonovak commented Jun 19, 2020

Hello!
I've got some questions regarding integrating with APNs because I'm not sure where "notifee's responsibility" starts and ends.

The idea is following: on iOS, I'm not using firebase and so I'd like to use https://github.com/react-native-community/push-notification-ios to get APNs token only (and deliver it to a server which will send the remote notifications). That means I only implement

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

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

and then I want to use Notifee to react to when notification is received (both when app is in background and foreground) and when it is pressed by user, including actions offered in the notification. It appears that events should handle just that - ?

So far I was able to get notification delivered on android with fcm, and the onForegroundEvent as well as onBackgroundEvent are fired in JS (I just made a console.log there and found it in logcat).

When I tried to do the same on ios, I only managed to get the push notification delivered when the app is in background / killed (so remote notifications are delivered, getting the APNs token seems to work). Now, when I try to subscribe for the events in JS, I unfortunately don't receive any :/

I'm aware of this comment and the implementation.

Unfortunately, since the native code is not open, I'm not quite sure what is going on inside and I'm not sure if my expectations are correct.

in device logs, I found

Warning: UNUserNotificationCenter delegate received call to -userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler: but the completion handler was never called.

which I'm guessing might be an issue?

I'd like to ask

(1) are my expectations sensible/correct and should my JS handlers (at least the foreground one) be called? I'm registering the handlers early in the app

(2) is it theoretically possible that the event is fired from native before a listener is attached in JS? (I would understand that for the background event but not for the foreground one?). Does notifee's native code somehow wait until JS "is ready"?

(3) does notifee implement willPresentNotification and didReceiveNotificationResponse for its events functionality?

(4) any ETA for proper onBackgroundEvent support on iOS?

(5) how is onBackgroundEvent different from RNFB's messaging().setBackgroundMessageHandler? Maybe I'm mixing local and remote notifications and asking stuff that should be obvious but I didn't feel like I found answers for these in the docs. Generally, when reading docs, I keep noticing it's always documented what some library is but it's often not documented what the library is not. But otherwise the docs are really nice (I really like click-though type definitions)!

Lastly, can someone confirm that this works for them using RN 60? Maybe I should upgrade first...

@Salakar Salakar self-assigned this Jun 22, 2020
@Salakar
Copy link
Member

Salakar commented Jun 22, 2020

Some quick answers;

(2) is it theoretically possible that the event is fired from native before a listener is attached in JS? (I would understand that for the background event but not for the foreground one?). Does notifee's native code somehow wait until JS "is ready"?

On RNFirebase we have a temporary buffer of all events that are sent while JS is not ready that is uncorked once JS is ready, this is something we need to apply to Notifee still; though in our early Notifee testing we couldn't get events to fire before JS was ready

(3) does notifee implement willPresentNotification and didReceiveNotificationResponse for its events functionality?

Yes, but it only handles notifications that were created explicitly by Notifee, Notifee's handlers for willPresentNotification and didReceiveNotificationResponse ignore everything else, leaving things like RNFirebase messaging to handle it. Though if you're having issues here then I suspect we may need to do some more work here 🤔

(4) any ETA for proper onBackgroundEvent support on iOS?

Hoping to look at it this week.

(5) how is onBackgroundEvent different from RNFB's messaging().setBackgroundMessageHandler? Maybe I'm mixing local and remote notifications and asking stuff that should be obvious but I didn't feel like I found answers for these in the docs.

Notifee onBackgroundEvent and RNFB setBackgroundMessageHandler are pretty much identical in behaviour - however Notifee is local notifications only, so only events from notifications created via Notifee APIs will come through to it. Whereas RNFB is FCM (remote) messages only and will ignore Notifee events as they're not FCM.

Generally, when reading docs, I keep noticing it's always documented what some library is but it's often not documented what the library is not.

This is a good point, cc @Ehesp - we could be clearer on what Notifee is not, e.g. not for remote notifications, not a FCM replacement, etc. I've also open sourced our documentation so if you feel any of the docs aren't clear feel free to raise an issue on the docs and we can look into it (or suggest a change via a PR): https://github.com/notifee/documentation

But otherwise the docs are really nice (I really like click-though type definitions)!

Thank you!


If you want to go through anything in more depth though and debug any issues, feel free to ping me on Discord 👍

@vonovak
Copy link
Contributor Author

vonovak commented Jun 23, 2020

Yes, but it only handles notifications that were created explicitly by Notifee, Notifee's handlers for willPresentNotification and didReceiveNotificationResponse ignore everything else, leaving things like RNFirebase messaging to handle it.

okay, this makes it clear now. I wanted to get events fo remote notifications which is not supported.

Any plan to allow responding to remote notifications too, or do you to purposely avoid it and leave in the scope of other libraries? I guess I'll try my luck with react-native-notifications (I'm not using fcm on ios), it just feels funny I have to add another dep because it seems Notifee could get the job done too.

Thanks!

@Salakar
Copy link
Member

Salakar commented Jun 23, 2020

Its probably something we should add otherwise you'd need yet another library, I think the initial issue was that if it didn't go through Notifee then we only had access to a limited amount of data from the notification and many fields could not be read/populated for sending the event - though in this scenario I think as long as you have notification data & action ids that should cover everything needed.

Will look into it

@jyliang
Copy link

jyliang commented Jul 29, 2020

@Salakar could you please let me know if Notifee becomes the delegate for UNUserNotificationCenter .currentNotificationCenter() ? If so, it looks like some of the events might be stopping inside Notifee.

For example I get didReceiveNotificationResponse called inside RNFBMessaging+UNUserNotificationCenter.m, it appears the originalDelegate is NotifeeCoreUNUserNotificationCenter. There is a local notification I scheduled on the native side, so I don't expect Notifee to pick it up. However my original delegate is not called. I can open up another ticket if needed

@vonovak
Copy link
Contributor Author

vonovak commented Jul 29, 2020

@jyliang yes, from my experience, notifee overwrites the delegate you may have set previously, and so your delegate does not learn the events it should. I hope this will be addressed at some point soon.

@helenaford
Copy link
Member

Notifee now calls the original delegate in the latest release v0.11.1.

@vonovak
Copy link
Contributor Author

vonovak commented Oct 5, 2020

hello!
I'd like to ask if there's any ETA on exposing didReceiveNotificationResponse (support for JS events that correspond to responses to remote notifications) to JS? We have discussed this with @Salakar on discord already a few months back and it seemed like the support for this was going to be added soon (Mike said it'd be easy to add) - should I count on this or should I somehow work around this? I'm available to add the feature to notifee, too.

Thanks!

@vonovak
Copy link
Contributor Author

vonovak commented Oct 12, 2020

@Salakar @helenaford hello and sorry for pinging again, could you please provide an answer? Thank you!

@helenaford
Copy link
Member

Hey, @vonovak sorry for the delayed response. We are talking it over internally and can back to you at the end of this week. From initial talks, I think the answer is yes, we can implement it.

Also, from reading your initial comment on this thread, do you still need to know when a remote notification is delivered? Event could be something like `REMOTE_NOTIFICATION_DELIVERED'.

And, do you only need iOS support?

@vonovak
Copy link
Contributor Author

vonovak commented Oct 14, 2020

@helenaford thanks for the reply. I thought about this again for a bit; I know you folks probably don't want to get into handling too much "remote notifications" stuff.

At the same time, as a consumer of the library and by looking at the docs (https://notifee.app/react-native/docs/events#foreground-events) I had the impression that if I tap one of the actions in the notification, then notifee will handle it. When I respond to notification action on android, the notifee.onForegroundEvent as well as notifee.onBackgroundEvent do get called, however it was not the case on iOS. So in this case, it was the matter of having consistent behavior on both platforms - that it the main issue I had with current state of things, so think about it and you'll see.

do you still need to know when a remote notification is delivered?

actually, I do not. At least not now 😆 I'm "juggling" several things here: data-only notifications on android, but "normal notification" (without content-available: 1) on iOS, react native firebase on android, but not on iOS (just using apns directly because they don't want to go through any extra layers of abstraction), so it's a bit of a complicated story. But given that

Notifee now calls the original delegate in the latest release v0.11.1.

I guess I'll be able to work out potential issues somehow by myself.

Thank you!

@vonovak
Copy link
Contributor Author

vonovak commented Oct 14, 2020

ah, I'm sorry, I just realized - because it's a data-only notification on Android, I use notifee.displayNotification to present it. And so when user responds to the notification, the notifee handler is called. But on iOS, we use plain notification, which is why notifee ignores it (it was not displayed by notifee). It makes sense to me now, sorry for my ignorance. Maybe a little bit of clarification in the docs would be nice. (eg. "notifee will only handle notifications that were presented by notifee")

@mikehardy
Copy link
Contributor

@vonovak where in the documentation would you put it? I think on the handling events - right here? https://notifee.app/react-native/docs/events#listening-for-events

Also, it is perhaps surprising but then understandable that the library only emits events for notifications it posts, but...I wonder if there is room philosophically / ability technically to mark a custom generated notification as being "Notifee Event Capable" such that even if Notifee did not generate the notification, it would react to it. That might tie everything up for you? I'm not saying it is possible, this more of a 🤔 question to see if it would be useful even

@helenaford
Copy link
Member

@vonovak no problem, just having this conversation is helpful to see what can be improved...

Think we're on the same lines @mikehardy. Could be possible to put something in the apns payload to let notifee know they can handle the notification and send events. I think it's simple on iOS but the challenge is to make sure both platforms are consistent. So, if we do support these extra events on iOS, we'd need to make sure Android does too.

@vonovak
Copy link
Contributor Author

vonovak commented Oct 21, 2020

where in the documentation would you put it? I think on the handling events - right here? https://notifee.app/react-native/docs/events#listening-for-events

yes, that sounds like the best place

I wonder if there is room philosophically / ability technically to mark a custom generated notification as being "Notifee Event Capable" such that even if Notifee did not generate the notification, it would react to it. That might tie everything up for you?

I think I just approached the library with wrong expectations from the start. Eg. if I look at userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler: I thought I would receive the user's response in one of the notifee's event handlers regardless of whether the notification was a remote one or one from notifee, because I thought "why would a library differentiate between them?". And if it only handles responses only to its own notifications, how am I supposed to handle the other ones? Should I need to use another library for that? Why? Just explaining what my initial expectations were, and somehow the docs didn't correct that mental model :)

@Salakar Salakar transferred this issue from invertase/react-native-notifee Sep 24, 2021
@github-actions
Copy link

github-actions bot commented Dec 6, 2022

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 attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants