Botbuilder-dotnet: [Adaptive] `getProperty` prebuilt function or dynamic index does not work

Created on 17 Oct 2019  路  11Comments  路  Source: microsoft/botbuilder-dotnet

@boydc2014 @luhan2017

Our Todo bot sample uses this approach and this does not work in master.

The goal is that we need this -

user.lists = {
    "todo": [
        "one", 
        "two"
    ],
    "shopping": [],
    "generic": []
}

The following C# code does not work -

```C#
var rootDialog = new AdaptiveDialog(nameof(AdaptiveDialog))
{
Generator = new TemplateEngineLanguageGenerator(),
Triggers = new List()
{
new OnConversationUpdateActivity()
{
Actions = WelcomeUserAction()
},
new OnIntent() {
Intent = "GetUserProfile",
Actions = new List

() {
new InitProperty() {
Property = "user.lists",
Type = "object"
},
new SetProperty() {
Property = "$listName",
Value = "'todo'"
},
new InitProperty() {
// Alternatively getProperty(user.lists, $listName) does not work either
Property = "user.lists[$listName]",
Type = "array"
},
new EditArray() {
// Alternatively getProperty(user.lists, $listName) does not work either
ItemsProperty = "user.lists[$listName]",
Value = "'one'",
ChangeType = EditArray.ArrayChangeType.Push
},
new SendActivity() {
Activity = new ActivityTemplate("I have '{join(user.lists[$listName], ', ')}'")
}
}
}
}
};

Expected: 
I have 'one'
Actual: 
  Exception caught : Error occurs when evaluating expression $join(user.lists[$listName], ', '): user.lists[$listName] evaluates to  which is not a list.

```

4.6 P1

Most helpful comment

I verified in Tom's branch, it is working now.

All 11 comments

Currently dynamic property is not support in "InitProperty" and "SetProperty". Maybe we can open a DCR to discuss it?

Currently dynamic property is not support in "InitProperty" and "SetProperty". Maybe we can open a DCR to discuss it?

@Danieladu is this treated as an expression in SDK?

This seems like a regression since the sample probably was this way. The LUIS Todo bot sample.. What am I missing?

This seems like a regression since the sample probably was this way. The LUIS Todo bot sample.. What am I missing?

Yes, the property is reverted from expression to string, by this commit. https://github.com/microsoft/botbuilder-dotnet/commit/fc900079

I think we should bring it back, I will prepare a PR on this. @vishwacsena, you can double check with Tom is there a specific reason for the Property to be a string instead of expression.

Yes please prep a PR. @tomlm can you provide context into the change?

@vishwacsena , let's make it all consistent. currently all the following properties are string, Let's make it all expressions. I will prepare a pr on this.

  • InitProperty->Property
  • SetProperty->Property
  • DeleteProperty->Property
  • Foreach->itemsProperty
  • ForeachPage->itemsProperty, pageSize

I vaguely remember Tom falling about Property being a memory path and not an expression but that gets in the way of dynamic properties like in this case. @tomlm - thoughts on how we should address this? You ok with moving these back to expression?

2773

I verified the scenario of todoluis bot which is provided by Vishwac at the beginning of the dialog. it is still not working. @vishwacsena you can help double check on this.
@tomlm, if the sample is wrong, please let us know the correct way to use that.

Based on my investigations, I think the problem might be

  1. in ObjectPath, bot the SetPathValue and GetPathValue and RemovePathValue should handle the "bracketPath", should we add a shared ResolvePath function?
  2. bracketPath can only resolve path within the current object scope, when I use dialog.name[user.name], it is still not working correctly.

@cwhitten , this is an advanced usage for the SetProperty, I think we can remove the 4.6 tag in composer, because I think it is not a blocking for Ignite, agree on this?

2818 PR is out

I verified in Tom's branch, it is working now.

Was this page helpful?
0 / 5 - 0 ratings