FLOIP / flow-spec

7 stars 6 forks source link

SelectOne and SelectMany (Multiple choice) blocks: Need an independent way to determine the choice selected, separate from the exit configuration #53

Closed markboots closed 2 years ago

markboots commented 3 years ago

Short version of the problem:

SelectOne and SelectMany require the contact to pick a choice(s) from the set of choices, which determines the output value of the block. How a contact's raw response is mapped to a choice could depend on the language and mode. Currently the spec is lacking a way to specify this mapping, other than through exit test configurations. This is a problem because someone might want to create a SelectOne block that has a single exit for all valid responses (or a different combo of exits), but still needs to specify how a raw response maps to a choice selection.

Example:

Consider a SelectOne with 3 choices: What's your favorite ice cream?

The value of the block (available via block.value, and within expressions in future blocks) needs to be set to one of the above. The exit tests below suggest how that might be determined from the contact input (block.response) based on the current language and mode:

"exits": [
    {
      "uuid": "95fd672c-92e9-4352-b761-7008b27cbe26",
      "test":
        "OR(
          AND(flow.mode = 'IVR', block.response = 7), 
          AND(flow.mode != 'IVR', 
            OR(block.response = 1, 
              AND(flow.language = 'eng', OR(lower(block.response) = 'chocolate'), lower(block.response) = 'c'), lower(block.response) = '1')), 
              AND(flow.language = 'fre', OR(lower(block.response) = 'chocolat', lower(block.response) = 'c', lower(block.response) = '1'))
            )
          )
        )",
      "label": "b0f6d3ec-b9ec-4761-b280-6777d965deab",
      "name": "chocolate",
    },
    {
      "uuid": "9fab760c-a680-4e40-83b7-9b3f8c66ccdb",
      "test":
        "OR(
          AND(flow.mode = 'IVR', block.response = 8), 
          AND(flow.mode != 'IVR', 
            OR( 
              AND(flow.language = 'eng', OR(lower(block.response) = 'vanilla'), lower(block.response) = 'v'), lower(block.response) = '2')), 
              AND(flow.language = 'fre', OR(lower(block.response) = 'vanille', lower(block.response) = 'v', lower(block.response) = '2'))
            )
          )
        )",
      "label": "b75fa302-8ff7-4f49-bf26-8f915e807222",
      "name": "vanilla",
    },
    {
      "uuid": "d99d43ec-6f0a-42b4-97f9-aa1c50ddebe0",
      "test": 
        "OR(
          AND(flow.mode = 'IVR', block.response = 9), 
          AND(flow.mode != 'IVR', 
            OR(
              AND(flow.language = 'eng', OR(lower(block.response) = 'strawberry'), lower(block.response) = 's'), lower(block.response) = '3')), 
              AND(flow.language = 'fre', OR(lower(block.response) = 'fraise', lower(block.response) = 'f', lower(block.response) = '3'))
            )
          )
        )",
      "label": "22619b04-b06d-483e-af83-ee3ba9c8c867",
      "name": "strawberry",
    }
    {
      "uuid": "78012084-b811-4177-88ea-5de5d3eba57d",
      "default": true,
      "label": "10a11345-9575-4e4a-bf61-0e04758626e7",
      "name": "Default",
    },
  ]

However, the problem here is that these are exit tests: they determine the exit used, not the block.value. Exits are also configurable -- one could choose to create a different combination of exits, and we would still need to resolve the selected choice.

Proposed solution

  1. Expand the choices array in the config for SelectOne and SelectMany. Currently it is a mapping from choice keys to the resource that presents these choices to a contact on different languages and modes:
  "config": {
    "prompt": "42095857-6782-425d-809b-4226c4d53d4d",
    "choices": {
      "chocolate": "b0f6d3ec-b9ec-4761-b280-6777d965deab",
      "vanilla": "b75fa302-8ff7-4f49-bf26-8f915e807222",
      "strawberry": "22619b04-b06d-483e-af83-ee3ba9c8c867"
    }
  }

This could be updated to have a name (choice key), resource, and test for each choice. An expression test here would be used to define the choice name that is put into block.value based on the contact's response.

{
  "type": "MobilePrimitives.SelectOneResponse",
  "name": "favorite_ice_cream",
  "label": "Favorite Ice Cream",
  "config": {
    "prompt": "42095857-6782-425d-809b-4226c4d53d4d",
    "choices": [
      {
        "name": "Chocolate",
        "resource": "b0f6d3ec-b9ec-4761-b280-6777d965deab",
        "test": "OR(
            AND(flow.mode = 'IVR', block.response = 7), 
            AND(flow.mode != 'IVR', 
              OR(
                AND(flow.language = 'eng', OR(lower(block.response) = 'chocolate'), lower(block.response) = 'c', lower(block.response) = '1'), 
                AND(flow.language = 'fre', OR(lower(block.response) = 'chocolat'), lower(block.response) = 'c', lower(block.response) = '1'))
              )
            )
          )"
      },
      {
        "name": "Vanilla",
        "resource": "b75fa302-8ff7-4f49-bf26-8f915e807222",
        "test": "OR(
            AND(flow.mode = 'IVR', block.response = 8), 
            AND(flow.mode != 'IVR', 
              OR(
                AND(flow.language = 'eng', OR(lower(block.response) = 'vanilla'), lower(block.response) = 'v', lower(block.response) = '1'), 
                AND(flow.language = 'fre', OR(lower(block.response) = 'vanille'), lower(block.response) = 'v', lower(block.response) = '1'))
              )
            )
          )"
      },
      {
        "name": "Strawberry",
        "resource": "22619b04-b06d-483e-af83-ee3ba9c8c867",
        "test": "OR(
            AND(flow.mode = 'IVR', block.response = 9), 
            AND(flow.mode != 'IVR', 
              OR(
                AND(flow.language = 'eng', OR(lower(block.response) = 'strawberry'), lower(block.response) = 's', lower(block.response) = '3'), 
                AND(flow.language = 'fre', OR(lower(block.response) = 'fraise'), lower(block.response) = 'f', lower(block.response) = '3'))
              )
            )
          )"
      }
    ]
  },
  "exits": [
    {
      "uuid": "95fd672c-92e9-4352-b761-7008b27cbe26",
      "test": "OR(block.value = Chocolate, block.value = Vanilla, block.value = Strawberry)",
      "label": "35f4f314-2ea1-49fa-99ec-36fab034b5b5",
      "name": "1",
    },
    {
      "uuid": "78012084-b811-4177-88ea-5de5d3eba57d",
      "default": true,
      "label": "10a11345-9575-4e4a-bf61-0e04758626e7",
      "name": "Default",
    },
  ]
}
  1. Expressions would support the following. Assuming the name of this block was "favorite_ice_cream", the following expressions would evaluate as:
    • flow.favorite_ice_cream.value // "strawberry" (name of the choice selected on the block.)

New fields to add support for on expressions:

markboots commented 3 years ago

Hi @bzabos @AlteaDown @seifertk, how does Flow Runner deal with this right now: getting the block value (selected choice) from the contact's response?

AlteaDown commented 3 years ago

I feel uncomfortable with this change as I understand it (having test blocks in the block.exits, as well as in block.config.choices), as this means we will have test in 2 places, so there are 2 sources of truth, which means an increase in complexity, which means we are much more likely to encounter bugs.

Right now, when you you make a selection, your value of your interaction is not the exit you took, the interaction.value is the label of the selected exit.

This makes it so the interaction.value (choice) determines the block.exit used.

The interaction.value is submitted to the runner as the selected value, (e.g. clipboard determines that "option_a" was clicked on the UI, and then on clicking Next, the app submits that the value of the block is "option_a", so then interaction.value is set to "option_a")

AlteaDown commented 3 years ago

As an example, I've attached a FlowSession that has been generated by Clipboard on Android, and it contains a MobilePrimitives.SelectOneResponse block, that has been used in an interaction. simpleFloipExample.zip

AlteaDown commented 3 years ago

For {block}.exits, I don't see a need for a translated label resource. A numeric index would be fine (I'm not sure of a case where it's used right now, so it seems like a rather useless field to me).

AlteaDown commented 3 years ago

There are other blocks with rather useless block exit labels as well, which can be seen in the example JSON I posted above, (such as the block exit with the id 29841127-3247-4610-8000-d2601adaa881, which contains a resource with a value of 1).

markboots commented 3 years ago

Hi @AlteaDown , thanks for the reply. Can you help me understand this better:

Right now, when you you make a selection, your value of your interaction is not the exit you took, the interaction.value is the label of the selected exit. This makes it so the interaction.value (choice) determines the block.exit used.

In an example like this one (3 choices, a single exit for all valid responses): Screen Shot 2021-06-26 at 9 25 33 PM Screen Shot 2021-06-26 at 8 40 35 PM

How does the flow specify the selected choice based on the raw user input? (In Clipboard, we can control the raw user input because they are clicking a button. In IVR and SMS, multiple different kinds of raw input would map to a choice selection, and this could depend on the language.

Outcome we need: how to specify that "7" in IVR, "3" in SMS, "strawberry" (in EN SMS), or "fraise" (in FR SMS) would all represent the "strawberry" choice -- even when we don't have a unique exit for Strawberry.

My understanding (if I got you correctly) is that in Clipboard, only the choice name (e.g.: "strawberry") can be sent as a response, since we control the prompt UI. That explains why we didn't hit this issue earlier, but are identifying it now (when IVR and SMS can have free-form input, that we need to map first to choices... Before/independently of deciding on exits.)

There are essentially 2 sequential steps we need:

and these are independent of each other. Using tests in the categorization step has a couple benefits:

markboots commented 3 years ago

For {block}.exits, I don't see a need for a translated label resource. A numeric index would be fine (I'm not sure of a case where it's used right now, so it seems like a rather useless field to me).

You'd be happy to see https://github.com/FLOIP/flow-spec/issues/55 then :) Agreed that they don't need to be translated resources, although a text label instead of a number is helpful for clarity on Flow Builder UIs (ie: the flowchart view).

AlteaDown commented 3 years ago

Okay, that makes more sense now. The new explanation/images helped clarify that quite a bit.

A thought. The types that would be returned, compared to what the exits has for tests wouldn't align well without some code between them. So what if instead of this:

"exits": [
    {
      "uuid": "95fd672c-92e9-4352-b761-7008b27cbe26",
      "test": "OR(block.value = Chocolate, block.value = Vanilla, block.value = Strawberry)",
      "label": "35f4f314-2ea1-49fa-99ec-36fab034b5b5",
      "name": "1",
    },
    {
      "uuid": "78012084-b811-4177-88ea-5de5d3eba57d",
      "default": true,
      "label": "10a11345-9575-4e4a-bf61-0e04758626e7",
      "name": "Default",
    },
  ]

We did something like:

{
  "exits": [
    {
      "uuid": "95fd672c-92e9-4352-b761-7008b27cbe26",
      "test": "OR(CONTAINS(block.config.chocolate, block.value), CONTAINS(block.config.vanilla, block.value), CONTAINS(block.config.strawberry, block.value))",
      "label": "35f4f314-2ea1-49fa-99ec-36fab034b5b5",
      "name": "1"
    },
    {
      "uuid": "78012084-b811-4177-88ea-5de5d3eba57d",
      "default": true,
      "label": "10a11345-9575-4e4a-bf61-0e04758626e7",
      "name": "Default"
    }
  ]
}

Or we could make a way to run sub expressions using something like this:

{
  "exits": [
    {
      "uuid": "95fd672c-92e9-4352-b761-7008b27cbe26",
      "test": "OR(EXPR(block.config.chocolate.test), EXPR(block.config.vanilla.test), EXPR(block.config.strawberry.test))",
      "label": "35f4f314-2ea1-49fa-99ec-36fab034b5b5",
      "name": "1"
    },
    {
      "uuid": "78012084-b811-4177-88ea-5de5d3eba57d",
      "default": true,
      "label": "10a11345-9575-4e4a-bf61-0e04758626e7",
      "name": "Default"
    }
  ]
}
AlteaDown commented 3 years ago

Okay, after chatting with Mark, we are on the same page, we want the resolution of a choice to be handled internally in flow-runner, likely in BasePrompt.{set value()}, so that we DO NOT depend on each person who implements flowrunner to do this resolution, to have to write their own code that checks that "c" is a valid response for choosing "Chocolate", so resolve "c" to "Chocolate" then in the prompt, set the prompt.value to "Chocolate".

Instead, we want to be able to submit "c", by setting prompt.value = "c", which should internally set the blockInteraction.value to "Chocolate"

seifertk commented 3 years ago

Looks like #1 of the proposed solution has been addressed, so I'll just comment on #2: We could structure the value as an object like so:

flow: {
  favorite_ice_cream: {
    __value__: "strawberry",
    value: "strawberry",
    response: "3",
    exit: "strawberry"
    }
}

The evaluator will handle any arbitrary nesting of objects, so this can expand for future needs. If accessed as @flow.favorite_ice_cream it will just print strawberry. That way, common use cases are still easy while still allowing for more complex behavior.

markboots commented 3 years ago

@seifertk, that seems usable, flexible, and compact -- nice! Anything we should consider here for interoperability/convertibility with RapidPro expressions?

The important bit I was thinking about was test expressions while inside a block (ie: @block.value and @block.response). Any concerns with those? (I don't think @block.exit is possible because you haven't gone through and selected an exit yet at the point you'd be using @block.whatever).