ChoiceInput\ConfirmInput shows TypeError: Cannot read property 'getValue' of undefined in Telegram channel

[Hejazz]

Versions

bot framework v 4.9.2. nodejs 12.18.0 botbuilder-dialogs-adaptive@4.9.2-preview OS - Windows Channel Telegram

Describe the bug

I have tried to build an adaptive dialog based on the sample here : https://github.com/microsoft/BotBuilder-Samples/tree/master/experimental/adaptive-dialog/javascript_nodejs/06.todo-bot

ConfirmInput code:

new ConfirmInput().configure({
    property: new StringExpression('turn.addTodo.cancelConfirmation'),
    prompt: new ActivityTemplate('${ConfirmCancellation()}'),
    // Allow interruptions is an expression. So you can write any expression to determine if an interruption should be allowed.
    // In this case, we will disallow interruptions since this is a cancellation confirmation.
    allowInterruptions: new BoolExpression(false),
    // Controls the number of times user is prompted for this input.
    maxTurnCount: new NumberExpression(1),
    // Default value to use if we have hit the MaxTurnCount
    defaultValue: new BoolExpression('=false'),
    // You can refer to properties of this input via %propertyName notation.
    // The default response is sent if we have prompted the user for MaxTurnCount number of times
    // and if a default value is assumed for the property.
    defaultValueResponse: new ActivityTemplate("Sorry, I do not recognize '${this.value}'. I'm going with '${%DefaultValue}' for now to be safe.")
}),

ChoiceInput code:

new ChoiceInput().configure({
    id: 'questionID',
    property: new StringExpression('turn.userAnswer'),
    prompt: new ActivityTemplate('${Question()}'),
    style: new EnumExpression(ListStyle.auto),
    choices: new ArrayExpression(this.getChoices(this.taskProperties.choices)),
    alwaysPrompt: new BoolExpression(true),
    unrecognizedPrompt: new ActivityTemplate('${errUseButtons()}'),
    maxTurnCount: new IntExpression(1),
    allowInterruptions: new BoolExpression(true),
    outputFormat: new EnumExpression(ChoiceOutputFormat.index)
}),

Everything is working on a WebChat and Emulator, but when I try to use Telegram channel, I get the following error : [onTurnError] unhandled error: TypeError: Cannot read property 'getValue' of undefined

Expected behavior

If I am correct Choices are supported in Telegram channel , so I expect see prompt with choices in Telegram.

[bug]

[Hejazz]

I have investigated the issue and found possible problem in choiceInput.ts file.

protected async onRenderPrompt(dc: DialogContext, state: InputState): Promise<Partial<Activity>> {
        // Determine locale
        let locale: string = dc.context.activity.locale || this.**defaultLocale**.getValue(dc.state);
        if (!locale || !ChoiceInput.defaultChoiceOptions.hasOwnProperty(locale)) {
            locale = 'en-us';
        }

the problem appears when defaultLocale is not set in ChoiceInput configuration AND in Telegram channel the context.activity.locale is not set. So onRenderPrompt() it throws an error.

In WebChat channel the context.activity.locale has a value = en-us , that's why there is no error there.

So, I guess, defaultLocale should be a mandatory and I think the code also should be fixed, because there is no sense to check for (!locale) on the next line if it throws an error before that.

[v-kydela]

@Hejazz - I have reproduced this issue on my end. It seems to me like the intention was to make en-us a "default default" locale, as though it could be used as a fallback if no default locale was set. Why do you think defaultLocale should be mandatory?

@cleemullins - Do you want me to fix this? If so, do you think defaultLocale should be mandatory?

[Hejazz]

@v-kydela Hi, thank you for your reply. I think it should be mandatory just because it throws unhandled an error. But anyway, I think it will be better if you check that this.defaultLocale is not Undefined before trying to get its value.

[v-kydela]

@Hejazz - Are you saying you only think it should be mandatory if the type error isn't fixed?

[Hejazz]

@v-kydela Even after you fix it, I would suggest to update the docs and mention that it is suggested to have defaultLocale filled in, otherwise it will be en-us.

[v-kydela]

@Hejazz - Okay that sounds like a good idea, but it's different from making defaultLocale mandatory.

[cleemullins]

I would align this with what the WebChat side is doing. That seems the cleanest. Alternativy, having a default Local is reasonable, and barring that, a clean error message seems resonable.

[carlosscastro]

Hello all, great discussion here. I've been doing an end to end survey of multilanguage flows (channel, webchat, sdk, composer, etc). I'm doing some work in c# with a slightly different approach to account for the e2e scenario. We'll hold off the PR merge until end of next week to make sure things are aligned, and the end to end plan makes sense.

[carlosscastro]

Pr for this should be out by Wednesday 8/12

[lauren-mills]

@carlosscastro - did the PR for this go out?

[carlosscastro]

Unfortunately not yet. Good news is that Kyle and I went thoroughly through his changes and they seem aligned with the direction we are going to. Steven has some comments on the code specifics which I think are very fair and we'd like to have addressed prior to merging. I've asked @stevengum to work with @v-kydela on the comments before we get this in. Steven / Kyle, let me know if you prefer to meet and go over the PR live if that helps.

[v-kydela]

@carlosscastro - After Steven reviewed the PR, I replied to his comments and made some changes. Can you tell me what comments haven't been addressed?

@stevengum - Would you like to set up a meeting?

[tsuwandy]

@v-kydela , @carlosscastro, any update?

[v-kydela]

@tsuwandy - The PR is still waiting for a review from any of its 7 reviewers. I tagged Carlos and Steven on the PR itself on Friday

[tdurnford]

@carlosscastro Have you had a chance to review the PR associated with this issue?