search

microsoft / BotFramework-Hubot

Hubot adapter for botframework
MIT License
111 stars 40 forks source link

Refactor the MS Teams middleware so that it does not have side-effects and can always be included. #8

Closed MatSFT closed 7 years ago

MatSFT commented 7 years ago

The usage is just including the hubot botframework adapter. this change automatically registers the MicrosoftTeamsMiddleware on the adapter so if a message comes in for the msteams channel, it is properly processed.

msftclas commented 7 years ago

@MattSFT, Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA. Thanks, Microsoft Pull Request Bot

billbliss commented 7 years ago

@maxpert this is a much cleaner way of using the MS Teams middleware or not than the way I did it with commenting/uncommenting. I was never a big fan of telling people to modify node_modules obtained via NPM in order to turn on a feature.

That said, it touches your code so I wanted you to look at it. It looks like a clean way of doing it to me.

maxpert commented 7 years ago

Image this line

MicrosoftTeamsMiddleware = require './msteams-middleware

not being there. In your target project (where you are installing the hubot and botframework adapter), there is a folder called scripts, as shown below:

image

This is usually the place where you will find logic to respond to scripts. In world of perfect pluggable middle-wares all the user should do require 'botframework-hubot/msteams-middleware, in his script file. Here is what I did in example.coffee: 

requre 'hubot-botframework/src/msteams-middleware'

module.exports = (robot) ->
    robot.hear /badger/i, (res) ->
        res.send "Badgers? BADGERS? WE DON'T NEED NO STINKIN BADGERS"

    robot.respond /open the (.*) doors/i, (res) ->
        console.log("Opening doors")
        doorType = res.match[1]
        if doorType is "pod bay"
            res.reply "I'm afraid I can't let you do that."
        else
            res.reply "Opening #{doorType} doors"

Before you say it I know src in middle looks ugly and that can be fixed. How do you feel about that? Giving user options to plug middlewares on his own will rather than us shipping everything?

Edit if you feel this is a better idea, you can remove the default import of msteams-middleware and remove the exports. I am fine either way but I am sure in future people will have their own adapter-middlewares to plugin for other platforms. So let's leave the choice of middlewares to the users rather than shoving it down their throats. If Hubot promotes the philosophy of plug on choice we should follow that for community.

MatSFT commented 7 years ago

@maxpert I update the PR with what we talked about. Let me know if there is anything else you think should be changed.