Charca / bootbot

Facebook Messenger Bot Framework for Node.js
MIT License
974 stars 253 forks source link

Proposal to support multi page bots #137

Open ntgx opened 6 years ago

ntgx commented 6 years ago

Had the same issue as #125 and this is my attempt for a fix. I've been testing it for the past few days and everything's working so far. All tests pass and it doesn't create any breaking changes for people using it for one page. Let me know what you think about the approach :)

Anyone who wants to support multiple pages just needs to pass an object instead of a string to the access token option in the following format and they're good to go

accessToken: {
    'PAGE_ONE_ID': 'PAGE_ONE_ACCESS_TOKEN',
    'PAGE_TWO_ID': 'PAGE_TWO_ACCESS_TOKEN',
 }

This lets you handle messages from both pages exactly like you used to and if you want to send different messages to different pages you can just check the value on chat.pageId first

bot.on('message', (payload, chat) => {
  const text = payload.message.text;
  chat.say(`Echo: ${text}`);
});

Even though all the methods on the chat object stay the same you have to pass pageId for some methods on bot if you plan to support multiple pages:

bot.setGreetingText(greeting, pageId);
bot.setGetStartedButton(action, pageId);
bot.setPersistentMenu(menu, null, pageId);

The functions that require accessToken in BootBot.js just call _getAccessToken(pageId) which looks like

_getAccessToken(pageId) {
    return typeof this.accessToken === 'string' ? this.accessToken : this.accessToken[pageId];
}
mraaroncruz commented 6 years ago

Thanks @ntgx. It looks nice and like you put some time into it. I hope @Charca or I get some time to look at it fully in the next days. This is much requested feature and first class support would be nice (instead of my handleFacebookData solution).

Charca commented 6 years ago

Thanks @ntgx! PR looks good to me at a first glance, I'm not a huge fan of passing around yet another param to basically every method on the API, but I understand the need for doing so.

I'll have to do some more testing before merging this, but this is looking great. Thanks a lot!

mahmed0715 commented 6 years ago

+1, My suggestion is: we can implement a useAccessToken method or something like that just before we send a response, so when we find a message is for a pageid, we call to that method like bot.useAccessToken()? in implemention we should have a map like [{pageId: accessToken}] .

You know we dont need accessToken when we set webhook and any other express things when starting the bot server.. so we can omit accessToken at startup, this feature will be very much helpful in a multipage app where a new page can be added or removed on the fly.. I am running this situation right now. any kind of help is appreciated.. Thanks @ntgx thanks for this, and can you do something like this quickly?

mahmed0715 commented 6 years ago

Please check my request @Charca

mahmed0715 commented 6 years ago

actually we dont need to do any change as we just use the accessToken that is set to the bot instance, like ${this.accessToken} so when we need to send a response from different page, we just set bot.accessToken = "whateverAccessTokenWeHaveforOtherPage"; and send the response.. thats it.. how is it @ntgx @Charca ? please let me know..

Charca commented 6 years ago

@mahmed0715 I don't think overwriting the accessToken value on the fly is a good idea. You can end up with a weird state when you try to use the accessToken of page A to reply to a message coming from page B. I think using a map of page IDs and access tokens like in this PR is the way to go. What don't you like about @ntgx's solution?

mahmed0715 commented 6 years ago

yeah you are right @Charca , but i think overwriting in right place could be good if possible. need to check if I can get things done, otherwise that solution is okay. I will overwrite exactly before call to fb api..will see..

ghost commented 6 years ago

Ran into an error while running this code. When pressing 'Get Started' bots would send back a number of messages equal to the number of times the for loop ran. Is it possible to set the get started button for multiple pages?

for(let i =0; i < Object.keys(accessTokens).length;i++){ bot.setGetStartedButton((payload, chat) => { chat.getUserProfile().then((user)=>{ //Code was here }); }); },ActivePages[i]); }

ntgx commented 5 years ago

Hey @DevinODowd are you still having this problem? I do something very similar and it's working fine for me

AkramLazkanee commented 4 years ago

Dears @ntgx & @Charca, did this pull request approved and merged into master? And if not is there any estimated time to be approved ?