-
Notifications
You must be signed in to change notification settings - Fork 13.1k
[FIX] Push: fix notification priority for google (FCM) #15803
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] Push: fix notification priority for google (FCM) #15803
Conversation
|
Isn't the 'priority' field for FCM accept a string of either "normal" or "high"? https://firebase.google.com/docs/cloud-messaging/http-server-ref Wondering cause right now when phone enters doze mode, android app is not able to receive notifs. You can test this by forcing doze mode using adb Then, if you send a message, the android client does not receive it until you disable doze mode with |
|
interesting.. I'll take a look on this @xtilyn , thanks for letting us know.. in your opinion then rocket.chat notifications should show up even if the device is on idle? |
|
Not sure, but looking at Messenger and Discord, message notifs are always received even if device is on idle. Not sure about slack. I think maybe we could give user the choice in settings in case they don't want to miss important messages for certain contacts? |
@sampaiodiego yes I believe that is the expectation. Even when the user puts "do not disturb" mode on, the notifications should still be received in background, however they are muted and without vibration. (ref: https://finance.yahoo.com/news/learn-not-disturb-mode-android-131551624.html ) |
|
Related to RocketChat/push#2 . I can confirm with Rocket.Chat 3.6.3 and Rocket.Chat.ReactNative 4.10.0 that this is an issue. It happens when the device is in Doze mode, and as @xtilyn helpfully pointed out, it's easy to force this mode manually and test this Google doc that explains Doze mode and App Standby mode: https://developer.android.com/training/monitoring-device-state/doze-standby#exemption-cases |
|
According to node-gcm, it should be like this: var message = new gcm.Message({
collapseKey: 'demo',
priority: 'high', # <-- this is important
... |
According to https://www.npmjs.com/package/node-gcm#usage and recommendation in https://developer.android.com/training/monitoring-device-state/doze-standby Continuation of RocketChat#15803 (comment)
|
@sampaiodiego I've created a PR implementing @xtilyn 's proposal here: #19061 |
According to https://www.npmjs.com/package/node-gcm#usage and recommendation in https://developer.android.com/training/monitoring-device-state/doze-standby Continuation of RocketChat#15803 (comment)
According to https://www.npmjs.com/package/node-gcm#usage and recommendation in https://developer.android.com/training/monitoring-device-state/doze-standby Continuation of RocketChat#15803 (comment)
According to https://www.npmjs.com/package/node-gcm#usage and recommendation in https://developer.android.com/training/monitoring-device-state/doze-standby Continuation of RocketChat#15803 (comment)
Currently Rocket.Chat is sending notifications using FCM by the default priority of data messages [1]. Those messages have no service-level objective. This PR changes the priority to '10' which should be the default for instant messaging (as defined in the FCM documentation). The same priority of '10' is also recommended for apple [2]. However for some reason the "push" lib already defaults to '10' for apple [3]. This change relies on [4] to be functional. Yet, it shouldn't brick anything without it.
[1] https://firebase.google.com/docs/cloud-messaging/http-server-ref
[2] https://developer.apple.com/documentation/usernotifications/setting_up_a_remote_notification_server/sending_notification_requests_to_apns
[3] https://github.com/RocketChat/push/blob/master/lib/server/push.api.js#L185
[4] RocketChat/push#2