remixz / messenger-bot

A Node client for the Facebook Messenger Platform
MIT License
1.09k stars 213 forks source link

Add possibility of sending message with tag #89

Closed baudev closed 6 years ago

baudev commented 6 years ago

Add the function sendMessageWithTag(recipient, payload, tag, cb) which allows sending messages with the specified tag.

Example of usage :


bot.sendMessageWithTag(payload.sender.id, { "text": "message_test"}, "NON_PROMOTIONAL_SUBSCRIPTION", (error, body) => {
    // ...
});
eXeDK commented 6 years ago

Please fix tests @baudev

baudev commented 6 years ago

Test passed on Node v6 ! https://travis-ci.org/remixz/messenger-bot/jobs/385714367

eXeDK commented 6 years ago

What about Node4? I have not setup these pipelines so I'm not sure I can make changes to those. Maybe @remixz can.

baudev commented 6 years ago

The mentioned errors in Node4 don't seem to be related to my MR.

EDIT : Sorry for closing the MR. Mistake on my part.

baudev commented 6 years ago

The function could be used as following finally :

bot.sendMessage(payload.sender.id, {"text": "message_test"},  (error, body) => {
    // ...
}, 'MESSAGE_TAG', 'NON_PROMOTIONAL_SUBSCRIPTION');
baudev commented 6 years ago

This issue's comment https://github.com/istanbuljs/nyc/issues/865#issuecomment-398435383 explains that node 4.x is no more supported with the latest versions of nyc. This is confirmed by the following commit: https://github.com/istanbuljs/nyc/commit/19b7d2130556b939bdd2fb69c2a35d09fe83f61e

We have two options:

I prefer the last solution: easier and more secure. Then it requires that @remixz remove the node 4 test in his Travis account.

What do you think about @eXeDK ?

baudev commented 6 years ago

I tried everything. I think that specifying the tap version doesn't work due to the Travis cache. Anyway. @remixz should remove the Node4 test or maybe @eXeDK have the rights of editing/remove/add tests ?

eXeDK commented 6 years ago

I think I have some access to the travis-ci setup as well. I'll try and remove the Node4

eXeDK commented 6 years ago

I'll merge this and try and fix the tests afterwards @baudev