parse-community / parse-server-push-adapter

A push notification adapter for Parse Server
https://parseplatform.org
MIT License
85 stars 100 forks source link

API Compatibility #246

Closed charles-ramos closed 1 month ago

charles-ramos commented 1 month ago

New Pull Request Checklist

Issue Description

Adding API Compatibility, so he user can now send both 'notification' and 'data' as objects for push notifications for Android.

Additionally, adding compatibility with nested objects, so, JSON is now accepted and the same behaviors from GCM are now replicable for FCM push notifications.

Approach

TODOs before merging

parse-github-assistant[bot] commented 1 month ago

Thanks for opening this pull request!

mman commented 1 month ago

@charles-ramos I honestly do not have any opinion on this PR. Is this PR bringing in some previous functionality that was working before and is now broken?

jimnor0xF commented 1 month ago

You can already use notification currently so it's unclear to me what this adds.

charles-ramos commented 1 month ago

Hey @mman and @jimnor0xF, the idea is to keep everything compatible with the current code (GCM) that uses "data" for sending push notifications for Android. Right now, in case I want to send push notifications to everyone (android and iOS), I must use both "data" and "notification". With the PR I raised, it will be possible to keep using "data" for both.

mman commented 1 month ago

@charles-ramos Are you using Parse Android SDK to receive the notifications? If you do could you post a snippet of your broadcast receiver that handles receiving the push?

jimnor0xF commented 1 month ago

@charles-ramos Are you sure you are using the Parse Android SDK for push notifications?

Back when we were using the Parse Android SDK we only used the data key. When we moved to using the native Firebase Cloud Messaging SDK on the client side, we started using the notification key. The notification key with that SDK is used for notifications that should pop up in the system tray. data is reserved for background (silent) notifications.

charles-ramos commented 1 month ago

Hi @mman and @jimnor0xF, I'm testing with the Javascript SDK.

Parse.Push.send({
  channels: [ "Giants", "Mets" ],
  data: {
    alert: "The Giants won against the Mets 2-3."
  }
})
.then(function() {
  // Push was successful
}, function(error) {
  // Handle error
});

With this PR, you can both send "notification" and "data" will work. Without that, the "data" will not work for FCM.

Also, it will ensure that those who use the Parse Dashboard will be able to send push from there. It sends a notification as an "alert".

mtrezza commented 1 month ago

@charles-ramos Could you please resolve the conflicts?

charles-ramos commented 1 month ago

With the latest release (version 6.2.0), everything covered by this PR is already working, so it's no longer required to proceed with it. Thank you everyone.

mtrezza commented 1 month ago

Thanks for investigating