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 206 forks source link

Support explicit topic target in send() #153

Closed bastienleonard closed 8 years ago

bastienleonard commented 8 years ago

Following issue #146:

This is a quick fix for sending to a topic with send():

sender.send(
    message,
    { topic: '/topics/global' }, // can also be { registrationIds: [...] },
    { retries: 2 },
    callback
);

If it looks good enough, I'll add support for the same parameters for sendNoRetry().

hypesystem commented 8 years ago

Seeing as you are changing the possible contents of the second parameter of send, I think it would be fitting to rename it registrationIds -> recipient (likewise, originalRegIds -> originalRecipient). This makes clearer what the variable contains.

It looks good, and it would be nice to support this in sendNoRetry.

Remember to add tests for sending to topics, update the README, and add yourself to the contributors in package.json.

bastienleonard commented 8 years ago
bastienleonard commented 8 years ago
bastienleonard commented 8 years ago
bastienleonard commented 8 years ago

That's all for today. I just realized that I should probably add support for notificationKey.

hypesystem commented 8 years ago

Looks really good! I had a few comments. Other than that I see the following things missing:

Thank you so much for your time :)

bastienleonard commented 8 years ago
bastienleonard commented 8 years ago
bastienleonard commented 8 years ago

(Will update the readme and contributors list after work, if you're OK with the changes)

hypesystem commented 8 years ago

Looks good :) Let me know when you've updated README and contributors, and I will merge :)

bastienleonard commented 8 years ago
hypesystem commented 8 years ago

Looks good :) I think the explicit recipient-type is the best model until we make a breaking change, so we should encourage that in the README.

hypesystem commented 8 years ago

Just now released in 0.12.0