appfeel / node-pushnotifications

Push notifications for GCM, APNS, MPNS, AMZ (automatic detection from device token)
MIT License
534 stars 123 forks source link

Deprecated FCM APIs, add FCM method (HTTP v1 new API) #194

Open banshiAnton opened 8 months ago

banshiAnton commented 8 months ago

Cause: GCM method which use old firebase api (will be deprecated )

Usage:

const PushNotifications = require('./lib')

const settings = {
  fcm: {
    appName: 'MyApp',
    serviceAccountKey: require('../firebase-project-service-account-key.json')
  },
  useFCMMethodInsteadOfGCM: true
}

const pushNotifications = new PushNotifications(settings)

const tokens = ['e..Gwso:APA91.......7r910HljzGUVS_f...kbyIFk2sK6......D2s6XZWn2E21x']

const notifications = {
  collapseKey: Math.random().toString().replace('0.', ''),
  priority: 'high',
  sound: 'default',
  title: 'Title 1',
  body: 'Body 2',
  // titleLocKey: 'GREETING',
  // titleLocArgs: ['Smith', 'M'],
  // fcm_notification: {
  //   title: 'Title 1',
  //   body: 'Body 2',
  //   sound: 'default',
  //   default_vibrate_timings: true,
  // },
  // alert: {
  //   title: 'Title 2',
  //   body: 'Body 2'
  // },
  custom: {
    frined_id: 54657,
    list_id: 'N7jSif1INyZkA7r910HljzGUVS'
  }
}

pushNotifications.send(tokens, notifications, (error, result) => {
  if (error) {
    console.log('[error]', error)
    throw error
  } else {
    console.log('[result]', result, result.at(0))
  }
})

Fcm object:

{
     "data": {
          "frined_id": "54657",
          "list_id": "N7jSif1INyZkA7r910HljzGUVS"
     },
     "android": {
          "collapse_key": "5658586678087056",
          "priority": "high",
          "notification": {
               "title": "Title 1",
               "body": "Body 2",
               "sound": "default"
          },
          "ttl": 2419200000
     },
     "apns": {
          "headers": {
               "apns-expiration": "1697456586",
               "apns-collapse-id": "5658586678087056"
          },
          "payload": {
               "aps": {
                    "sound": "default",
                    "alert": {
                         "title": "Title 1",
                         "body": "Body 2"
                    }
               }
          }
     },
     "tokens": [
          "e..Gwso:APA91.......7r910HljzGUVS_f...kbyIFk2sK6......D2s6XZWn2E21x"
     ]
}
infuzz commented 5 months ago

HI ! Any plan to merge this great PR in the base repo ?

miqmago commented 5 months ago

@banshiAnton first of all sorry for the delay on the review and many thanks for your contribution. I think it can help to evolve the repo to better integration with FCM/GCM.

I've made some comments on your code that would help to make this repo better mantainable.

If possible, could you please add the corresponding documentation for the new options to the Readme.md, and perhaps include the examples from this pull request? It would be really helpful!

banshiAnton commented 4 months ago

Hi @miqmago i have updated README.md but i can't see any comments for code

I've made some comments on your code that would help to make this repo better mantainable.

miqmago commented 4 months ago

Hi @banshiAnton it's quite strange you can't see my comments... Don't appear to you on this thread? I can se them just few messages before this one, as a review... Also if you open https://github.com/appfeel/node-pushnotifications/pull/194/files?diff=split&w=1 I can see my comments there.

I've added a ping on one of them to see if you receive a message on the first one. Just let me know if it works, I will ping on every comment then.

miqmago commented 4 months ago

Ok @banshiAnton I've just noticed that I had to submit my review... 🤦🏼‍♂️🤦🏼‍♂️ You should see it now.

infuzz commented 2 months ago

Hello, I just want to thank you for all your great work, as the deprecation of the old http fcm is fast approaching. I hope you have enough time to finish the new major version. Thanks again.

miqmago commented 2 months ago

From my side seems all fine. @alex-friedl could you please confirm if is it ok for you? @banshiAnton could you please solve package conflicts?

alex-friedl commented 1 month ago

@miqmago @banshiAnton Generally looks good to me as well. @banshiAnton When you get to rebasing the branch, could you also take a look at the open conversations in the PR? :)

Hossman333 commented 1 month ago

Thanks everyone for your great work here! This will be good to have so that we aren't left in a broken state in June.

nsakaimbo commented 1 month ago

@banshiAnton Nice work! I believe you're missing an export reference in the file lib/push_notifications.js. It currently looks like:

module.exports = PN;
module.exports.WEB = _constants.WEB_METHOD;
module.exports.WNS = _constants.WNS_METHOD;
module.exports.ADM = _constants.ADM_METHOD;
module.exports.GCM = _constants.GCM_METHOD;
module.exports.APN = _constants.APN_METHOD;

You will need to add module.exports.FCM there as well.

miqmago commented 1 month ago

@banshiAnton are you available to make pull from master, resolve conflicts and push again to your branch? This would help with merging this important feature.

nsakaimbo commented 1 month ago

@miqmago 👋🏽 I left a couple of comments with some important bug fixes for this PR. My review comment is still marked as "pending" so unsure if @banshiAnton is able to see it. Specifically:

ehaynes99 commented 3 weeks ago

In the linked issue in node-gcm, the OP assumes that since it was deprecated on June 20, 2023, it will be discontinued on June 20, 2024, but Google doesn't actually give a day, only a month. It could as easily be June 1, so we're 3 weeks away.

Since we haven't heard from @banshiAnton since January, I forked the fork, updated it with current master, and addressed the feedback by @nsakaimbo

I have opened a PR here: https://github.com/appfeel/node-pushnotifications/pull/205

Just to be clear, I'm not trying to steal credit in any way, and I don't pretend to have much domain knowledge nor a full grasp of all of the preceding changes. But my company depends on this library and will be in serious trouble without the update.

pushchris commented 1 week ago

Deadline is fast approaching on this. When is this going to be merged and is there anything the rest of us can do to help expedite?

Arpanexe commented 4 days ago

Any plan to merge this PR?