-
Notifications
You must be signed in to change notification settings - Fork 94
Add FCM support with addNotification in node-gcm 1.4.4 #146
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
Conversation
|
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Added tests to prevent regressions as per request in #141. cc @superkhau |
|
@zbarbuto TY for following up with a test to prevent regressions. 👍 |
|
@slnode test please |
|
@superkhau Anything else I can do to help get this merged? Does someone else need to review? |
|
@zbarbuto Nope, it's LGTM now. TY for the contribution. ;) |
Description
Notifications aren't sent to the new FCM system with the latest version of
node-gcm. This uses theaddNotification()method to add the expected notification properties to the message to be sent.The properties used are
messageFromfor the title andalertfor the body as this makes it fully compatible with the existing docs on notifications (ie. the result on the device will be the same on both iOS and Android)Related issues
Checklist
guide