FCM device groups

[baskin]

Hi Team,
I wasn't able to figure out how to send an FCM notification to a device group. Any help would be greatly appreciated? FWIW, the underlying library used (https://github.com/ToothlessGear/node-gcm) supports sending to device groups.

Thanks,

[baskin]

The relevant snippets can be found

• here
• and from node-gcm lib can be found here

Since we're explicitly using the key registrationTokens there seems no way to publish to a topic or to a device group.

[alex-friedl]

@baskin I briefly looked at the snippet from node-gcm and the FCM documentation.

From the docs it seems that notification_key_name and registrationIds must be part of the payload. Is that the change we need to implement on our end? Allowing to send notificationKey and registrationIds (as seen in https://github.com/ToothlessGear/node-gcm/blob/05ebebd581379eed5927288d494ed91ddf42115c/lib/sender.js#L234) with the request payload?

[baskin]

As the node-gcm documentation https://github.com/ToothlessGear/node-gcm#recipients says, notificatoinKey is deprecated. A better option would be to support the more ubiquitous to option -- this will add support for a single registration token, notification key, or topic. I would also suggest to support the condition parameter which allows sending to multiple topics using a condition.

So with these 3 recipient options

• registrationTokens: already supported
• to (a single registration token, notification key, or topic.)
• condition (multiple topics)

node-pushnotification will end up support the entire gamut of FCM recipient options.

[alex-friedl]

@baskin thank you for the clarification.
From what I see in the documentation you provided, am I correct in my assumption that one only uses one of the three options exclusively, but never a combination in one send request?
So I either .send(message, {registrationTokens}, ...), .send(message, {to}, ...) or .send(message, {condition}, ...) but never .send(message, {registrationTokens, condition}, ...) or .send(message, {registrationTokens, to}, ...) ?

[baskin]

Correct.

[alex-friedl]

Hi @baskin

I implemented a solution that allows to pass on custom to or condition recipients with the data payload. These take precedence over any FCM device tokens that may be present in the list of registration ids: https://github.com/appfeel/node-pushnotifications/tree/dev/fcm-recipients#send-to-custom-recipients-device-groups-or-topics

Could you test it with the https://github.com/appfeel/node-pushnotifications/tree/dev/fcm-recipients branch and let me know if it works as expected for you?

Thank you!

[baskin]

Thanks @alex-friedl

Quick thought. The current API semantics are sth like push.send(recipients, data), where recipients can be registrationTokens for gcm currently. Would it possible to support the new types of recipients by passing them as the first recipients argument instead of clubbing them into the second data argument.

IMHO it would be cleaner from an API usability standpoint. What do you think?

[alex-friedl]

Hi @baskin,

I agree that this may be preferable upon first glance and I looked into at first as well.
But that would require to somehow distinguish between regular device tokens and arbitrary to or condition parameters in

node-pushnotifications/src/push-notifications.js

Line 53 in 16a5ab2

} else if (regId.length > 64) {

. I didn't see a good solution to achieve this.
Neither would it be possible to know if to or condition is the desired recipient, because the regId list is only an array containing device tokens for all possible devices.

The idea of my current API proposal is to specifically override the default behaviour for GCM only, without the need to introduce any breaking changes.

Does that make sense? Or do you see an alternative solution that would work?

[alex-friedl]

@baskin any update from your end? Did you get around to testing it? Does the proposed API work for you?

[baskin]

The idea of my current API proposal is to specifically override the default behaviour for GCM only, without the need to introduce any breaking changes.
Does that make sense? Or do you see an alternative solution that would work?

This makes sense. Thanks @alex-friedl

Sorry. Didn't get a chance to test so far.

I'm not using this library directly but via the library notifme-sdk. I've asked the author of that library to take a look too. Pl allow me a few days.

[alex-friedl]

@baskin Judging from notifme/notifme-sdk#69 (comment) the suggested solution should work for you? I will merge and release this change.
Let me know if you encounter any issues or need any further changes.