Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

rasa documentation supports a single response with both a text type a… #112

Merged

Conversation

nicholasbulka
Copy link
Contributor

@nicholasbulka nicholasbulka commented Jul 16, 2019

rasa documentation supports a single response with both a text type and a button type. This commit creates two chat messages for any entry in the messages array that has both a text and button component.

from https://github.com/RasaHQ/rasa-demo/blob/master/domain.yml

    - text: "How is this conversation going?"
      buttons:
        - title: "👍"
          payload: "/feedback{\"feedback_value\":\"positive\"}"
        - title: "👎"
          payload: "/feedback{\"feedback_value\":\"negative\"}"

Fixes #111

…nd a button type. This commit creates two chat messages for any entry in the messages array that has both a text and button component
@hotzenklotz hotzenklotz self-requested a review July 17, 2019 11:12
Copy link
Member

@hotzenklotz hotzenklotz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicholasbulka Thank you very much for this PR. This is a very welcome contribution. Indeed, we overlooked buttons with included text attributes, when we built this component.

May I suggest some changes:

  1. Do image and attachments support the text attribute as well? Perhaps you would like to handle them as well.
  2. Stylewise, the originalMessages object is never used again and therefore is not necessary. Further, the whole .map(message: RasaMessage) => ... is overkill if you never return anything. Perhaps a simple for ... loop or .forEach will do.
  3. Unfortunately, CircleCI builds are not enabled for forked PRs. We have a few checks in place, that I would encourage you to run locally, which otherwise would fail the build after merging: Flow Type Checking yarn flow and Code Styleing with yarn pretty.

Thanks for the contribution. Let me know if there is anything else I can help you with.

@hotzenklotz hotzenklotz added the enhancement New feature or request label Jul 19, 2019
@hotzenklotz hotzenklotz merged commit ff6caa7 into scalableminds:master Jul 29, 2019
@hotzenklotz
Copy link
Member

Thanks for the pull request. I will make some of the suggested style changes myself and release a new version.

@nicholasbulka
Copy link
Contributor Author

Hey I considered your feedback and I appreciate the merge. I tried to implement what might be a better PR than what I originally posted. I've been pressed for time with releasing and whatnot, but here's what I ended up with. in

`async parseMessages(RasaMessages: Array) {
const validMessageTypes = ["text", "image", "buttons", "attachment"];

let expandedMessages = [];

RasaMessages.filter((message: RasaMessage) =>
  validMessageTypes.some(type => type in message),
).forEach(function(message){

  if(message.hasOwnProperty("text")) {
    const textMessage = {...message}
    delete textMessage.buttons;
    delete textMessage.image;
    delete textMessage.attachment;

    expandedMessages.push(textMessage);
  }

  if(message.hasOwnProperty("buttons")) {

    const buttonsMessage = {...message}
    delete buttonsMessage.text;
    delete buttonsMessage.image;
    delete buttonsMessage.attachment;

    expandedMessages.push(buttonsMessage);
  }

  if(message.hasOwnProperty("image")) {

    const imageMessage = {...message}
    delete imageMessage.text;
    delete imageMessage.buttons;
    delete imageMessage.attachment;

    expandedMessages.push(imageMessage);
  }

  if(message.hasOwnProperty("attachment")) {

    const attachmentMessage = {...message}
    delete attachmentMessage.text;
    delete attachmentMessage.buttons;
    delete attachmentMessage.image;

    expandedMessages.push(attachmentMessage);
  }
});`

@hotzenklotz hotzenklotz mentioned this pull request Jul 29, 2019
@nicholasbulka nicholasbulka deleted the bifurcate_text_and_buttons branch July 29, 2019 13:45
@hotzenklotz
Copy link
Member

Hi @nicholasbulka Your changes went live as version 0.10.1. Thank you very much. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I want to contribute a pull request but I'm getting 403 when pushing to a feature branch
2 participants