Skip to content

Conversation

hiranya911
Copy link
Contributor

FCM Webpush API now supports additional parameters as defined in https://developer.mozilla.org/en-US/docs/Web/API/notification/Notification

Related to firebase/firebase-admin-node#305

/**
* Sets any arbitrary data that should be associated with the notification.
*
* @param data A json-serializable object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/json/JSON/ :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


@Key("notification")
private final WebpushNotification notification;
private final Map<String, Object> notification;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don't quite follow where this private fields are ever read. Can you elaborate?

I am asking this because I liked using WebpushNotification here better but I can see reason why the Map would be more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these get serialized into JSON. We now need a Map because the notification now allows arbitrary key-value pairs (via putCustomData()).

Copy link
Contributor Author

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@schmidt-sebastian made the suggested change.


@Key("notification")
private final WebpushNotification notification;
private final Map<String, Object> notification;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these get serialized into JSON. We now need a Map because the notification now allows arbitrary key-value pairs (via putCustomData()).

/**
* Sets any arbitrary data that should be associated with the notification.
*
* @param data A json-serializable object.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for enlightening me!

@hiranya911 hiranya911 merged commit 78c6198 into master Jul 30, 2018
@hiranya911 hiranya911 deleted the hkj-fcm-webpush branch July 30, 2018 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants