search

ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 208 forks source link

Support send to multiple topics #261

Closed vibgy closed 7 years ago

vibgy commented 7 years ago

FCM has introduced a way to send notification to multiple topics using the "condition" parameter in send API. Ref: https://firebase.google.com/docs/cloud-messaging/http-server-ref

eladnava commented 7 years ago

@vibgy Excellent PR!

However, could it be better to introduce a brand new recipient key condition instead of piggybacking on the topic parameter? @hypesystem

vibgy commented 7 years ago

@eladnava Thank you. Let me know.. I was also debating if I should introduce "condition" or not. If you guys want me to change it, I'm happy to do so. But today is a busy day for me. I can do it tomorrow.

hypesystem commented 7 years ago

Cool that this has been introduced! I think we should use the "condition" parameter.

This might complicate the interface for v1 or force us to rethink some things. (See and discuss at #238)

hypesystem commented 7 years ago

I should clarify: let's use "condition" as it is closer to the underlyibg interface :)

Don't forget to add yourself as contributor in the package :)

vibgy commented 7 years ago

Sure.. I'll update the PR. Thank you @hypesystem !

vibgy commented 7 years ago

I updated the PR. Please review. Thanks you @hypesystem @eladnava ..

eladnava commented 7 years ago

PR has been merged. Thanks for your hard work!