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 for multiple topics using condition parameter of FCM send API #262

Closed vibgy closed 7 years ago

vibgy commented 7 years ago

Aimed to solve this issue: https://github.com/ToothlessGear/node-gcm/issues/261

hypesystem commented 7 years ago

I think this looks pretty good :-)

You don't need to do the version bump, we will do that when we publish the update :-)

Would like to hear final thoughts from @eladnava

eladnava commented 7 years ago

@vibgy @hypesystem looks good! Feel free to merge. 😄

vibgy commented 7 years ago

Should I remove the version change?

vibgy commented 7 years ago

@eladnava @hypesystem - I dont have permissions to merge. Also, should I remove the version change?

eladnava commented 7 years ago

@vibgy Yes, please 👍 Don't worry, we'll merge.

vibgy commented 7 years ago

@eladnava - Reverted the version number change in package.json.. All tests still pass. Its ready to be merged.

eladnava commented 7 years ago

@vibgy Thanks so much for your contribution! Excellent job. 😄 @hypesystem Can I increment version and publish to npm?

hypesystem commented 7 years ago

@vibgy thanks for your effort on this :-)

@eladnava yeah! Remember to ensure that the CHANGELOG has all changes :)

eladnava commented 7 years ago

@hypesystem Changelog looks good. Published 0.14.4. 😄

hypesystem commented 7 years ago

:shipit: :star:

djvickx commented 5 years ago

@vibgy it only supports upto 5 topics using condition. How I can send to around 10 topics?

hypesystem commented 5 years ago

Hey @djvickx if would be great if you could create a new issue for your problem. If you include the code where you attempt to send to 10 topics and explain the problem you encounter that would give us a good starting point for investigating and helping you :smile:

djvickx commented 5 years ago

Hey @djvickx if would be great if you could create a new issue for your problem. If you include the code where you attempt to send to 10 topics and explain the problem you encounter that would give us a good starting point for investigating and helping you 😄

condition = "'TopicA' in topics || 'TopicB' in topics || 'TopicC' in topics || 'TopicD' in topics || 'TopicE' in topics || 'TopicF' in topics || 'TopicG' in topics "
            string serverKey = //SERVER_KEY

                WebRequest tRequest = WebRequest.Create("https://fcm.googleapis.com/fcm/send");
                tRequest.Method = "post";
                tRequest.ContentType = "application/json";
                var objNotification = new
                {
                    condition = condition,
                    priority = "high",
                    data = //NOTIFICATION DATA
                };

                WebResponse tResponse = tRequest.GetResponse();

As per firebase documentation, they don't allow more than 5 topics when using condition.

hypesystem commented 5 years ago

Hey @djvickx ! It would be much cleaner for us to deal with your issue if you create a separate ticket.

Click here: https://github.com/ToothlessGear/node-gcm/issues/new

Then fill in your question there, and we can take the discussion there.

Discussion here (on this Pull Request) should regard the Pull Request. Your question regards how to send to multiple topics, and is not really related to the code here. That's why I'm asking you to create a new issue :smile: I hope this makes sense!

djvickx commented 5 years ago

done.. Thanks