space10-community / conversational-form

Turning web forms into conversations
https://space10-community.github.io/conversational-form/
MIT License
3.8k stars 775 forks source link

Conditionals: If an answer is a prefix of another answer, both branches will be activated #327

Open buesing opened 5 years ago

buesing commented 5 years ago

I created a simplified example with the following tags (formless, but I assume that makes no difference):

        {
          tag: "input",
          type: "radio",
          name: "doneness",
          "cf-questions": "How do you like your steak?",
          "cf-label": "Medium Rare",
          value: "Medium Rare",
        }, {
          tag: "input",
          type: "radio",
          name: "doneness",
          "cf-questions": "How do you like your steak?",
          "cf-label": "Medium",
          value: "Medium",
        }, {
          tag: "input",
          type: "radio",
          name: "doneness",
          "cf-questions": "How do you like your steak?",
          "cf-label": "Well done",
          value: "Well done",
        }, {
          tag: "cf-robot-message",
          "cf-questions": "Yummy!",
          "cf-conditional-doneness": "Medium Rare",
          name: "response-medium-rare",
        }, {
          tag: "cf-robot-message",
          "cf-questions": "Hmmmm.",
          "cf-conditional-doneness": "Medium",
          name: "response-medium",
        }, {
          tag: "cf-robot-message",
          "cf-questions": "Gross!",
          "cf-conditional-doneness": "Well done",
          name: "response-medium-rare",
        },

If you select "Medium Rare", you get both the response for "Medium Rare" and "Medium". It seems there is some kind of regexing going on but I couldn't find the exact reason in the code. Is it possible this block https://github.com/space10-community/conversational-form/blob/336257955d6638ccfe33293ef65e313ff9acf4ec/src/scripts/cf/form-tags/Tag.ts#L231 is reading the conditional as a regex when it should be a string?

jenssogaard commented 5 years ago

@buesing thank you for pointing this out. I'll take a look.

jenssogaard commented 5 years ago

@buesing I see the problem. Basically we are creating a new Regexp(_conditional) which comes out as /Medium/ which causes the match. This is not ideal and patching it seems likely to wreak some havoc on the experience for some. The quickfix would be to use regex like ^Medium$ to avoid a half match.

This is where we push the new Regexp. We should ideally only do regex when it is a regex and string when it's not.