botkit-middleware-witai icon indicating copy to clipboard operation
botkit-middleware-witai copied to clipboard

adapt to new version of wit api, give configuration option for wit co…

Open ALBotStageSoftNaert opened this issue 7 years ago • 6 comments

I adapted the middleware to work with version 5.0.0 of node-wit. I added an optional config option so that you could specify a command to use instead of the bot identity. If no command was specified every request will be parsed by wit.

ALBotStageSoftNaert avatar Mar 26 '19 13:03 ALBotStageSoftNaert

CLA assistant check
All CLA requirements met.

msftclas avatar Mar 26 '19 13:03 msftclas

@ALBotStageSoftNaert This looks backward compatible but I want to make sure you agree. Is it?

Thanks for your quick response to my review!

benbrown avatar Mar 26 '19 15:03 benbrown

Also, it looks like the hears middleware may need a mild update? See this other PR -> https://github.com/howdyai/botkit-middleware-witai/pull/26

benbrown avatar Mar 26 '19 15:03 benbrown

@benbrown thanks for your fast review. I think that it won't be backwards compatible, node-wit changed the way they return intents in a different way. However they do provide the posibility to provide an option to include the apiversion. https://github.com/wit-ai/node-wit#changing-the-api-version. I can add an extra config option to make it backwards compatible in this manner.

ALBotStageSoftNaert avatar Mar 26 '19 15:03 ALBotStageSoftNaert

In that case the readme will need some updates and we'll have to bump the version number on npm.

Thanks!

benbrown avatar Mar 26 '19 15:03 benbrown

@benbrown I updated the discussed topics

  • configuration option api_version was added
  • the version nr in package.json has been placed on 2.0.0
  • the middleware.hears method has been updated, I checked the PR #26 and noticed that the improvement provided was to not have to loop over the tests array. I included this in the current code by using an include on the tests array.
 if (message.entities && message.entities.intent) {
            for (var i = 0; i < message.entities.intent.length; i++) {
                if(tests.includes(message.entities.intent[i].value) && message.entities.intent[i].confidence >= config.minimum_confidence){
                    return true;
                }
            }

I also noticed that the results from entities were taken instead of entities.intent. This is possible and would mean that all entities in wit can be used to get a match, not only the intents. However, I don't think this is desired behaviour, so it wasn't included in the changes.

  • the readme file has been updated

ALBotStageSoftNaert avatar Mar 27 '19 07:03 ALBotStageSoftNaert