search

dialogflow / dialogflow-fulfillment-nodejs

Dialogflow agent fulfillment library supporting v1&v2, 8 platforms, and text, card, image, suggestion, custom responses
Apache License 2.0
598 stars 281 forks source link

Field fulfillment provided in the request is not stored in agent properties #7

Open Cmbdurand opened 6 years ago

Cmbdurand commented 6 years ago

The Use Case I have designed a poem server which responds to requests from DialogFlow. When the webhook is fired, it sends a simpleResponse containing the poem audio & text and then the pre-canned responses prepared in the intent by Dialogflow. These pre-canned responses are sent in the request and accessible in the field request.body.result.fulfillment_

My suggestion add the field fulfillment to the agent_v1 object

How ? In file v1-agent.js, add the following lines to the function processRequest() processRequest() { ...

  /**
   * Dialogflow fulfillment included in the request or null if no value
   * https://dialogflow.com/docs/fulfillment
   * @type {string}
   */
  this.agent.fulfillment = this.agent.request_.body.result.fulfillment;
  debug(`Input fulfillment: ${JSON.stringify(this.agent.fulfillment)}`);

...

Tests ? I haven't managed to figure out were to add the associated unitary tests associated to this modification. All I can say is that with this patch I works like a charm and I can have access to agent.fulfillment information in my code.

agent-v2 equivalent ? I have looked in detail yet which modification is required but I'll have a look later when I use version 2

Next steps ? I you agree, I can send a pull request for this modification. Looking forward to receiving feedback on this issue and congratulations for this great module!

mattcarrollcode commented 6 years ago

Thanks for the feedback. I'll consider this a feature request for the next release.

I'd want to make sure this works across v1 and v2 and ingests the fulfillment messages properly to what the library uses internally (i.e. text, cards, custom payload classes), as well as tests covering this. if you're up for that magnitude of a pull request I'd welcome the contribution, otherwise I'll implement this in the coming weeks.