Skip to content

Conversation

@zbarbuto
Copy link
Member

@zbarbuto zbarbuto commented Feb 7, 2017

Description

Notifications aren't sent to the new FCM system with the latest version of node-gcm. This uses the addNotification() method to add the expected notification properties to the message to be sent.

The properties used are messageFrom for the title and alert for 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

  • None

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

@slnode
Copy link

slnode commented Feb 7, 2017

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@slnode
Copy link

slnode commented Feb 7, 2017

Can one of the admins verify this patch?

2 similar comments
@slnode
Copy link

slnode commented Feb 7, 2017

Can one of the admins verify this patch?

@slnode
Copy link

slnode commented Feb 7, 2017

Can one of the admins verify this patch?

@zbarbuto
Copy link
Member Author

zbarbuto commented Feb 7, 2017

Added tests to prevent regressions as per request in #141. cc @superkhau

@superkhau
Copy link
Contributor

@zbarbuto TY for following up with a test to prevent regressions. 👍

@superkhau
Copy link
Contributor

@slnode test please

@zbarbuto
Copy link
Member Author

@superkhau Anything else I can do to help get this merged? Does someone else need to review?

@superkhau
Copy link
Contributor

@zbarbuto Nope, it's LGTM now. TY for the contribution. ;)

@superkhau superkhau merged commit dce16d9 into strongloop:master Feb 14, 2017
@superkhau superkhau self-assigned this Feb 14, 2017
@superkhau superkhau added this to the Sprint 29 milestone Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants