This repository was archived by the owner on Jan 15, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 85
chore: [Internal dev branch PR] Telephony: integration tests and CommandDialog interruption model first pass #1066
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,14 @@ | |
| // Licensed under the MIT License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Text; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using AdaptiveExpressions.Properties; | ||
| using Microsoft.Bot.Builder; | ||
| using Microsoft.Bot.Builder.Dialogs; | ||
| using Microsoft.Bot.Builder.Dialogs.Adaptive; | ||
| using Microsoft.Bot.Schema; | ||
| using Newtonsoft.Json; | ||
|
|
||
| namespace Microsoft.Bot.Components.Telephony.Common | ||
| { | ||
|
|
@@ -23,13 +23,22 @@ namespace Microsoft.Bot.Components.Telephony.Common | |
| /// <typeparam name="T">Type of data stored in the <see <see cref="CommandValue{T}"/>.</typeparam> | ||
| public class CommandDialog<T> : Dialog | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets intteruption policy. | ||
| /// </summary> | ||
| /// <value> | ||
| /// Bool or expression which evalutes to bool. | ||
| /// </value> | ||
| [JsonProperty("allowInterruptions")] | ||
| public BoolExpression AllowInterruptions { get; set; } = true; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the name of the command. | ||
| /// </summary> | ||
| /// <value> | ||
| /// <see cref="StringExpression"/>. | ||
| /// </value> | ||
| protected StringExpression Name { get; set; } | ||
| protected StringExpression CommandName { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the data payload of the command. | ||
|
|
@@ -42,8 +51,7 @@ public class CommandDialog<T> : Dialog | |
| public override async Task<DialogTurnResult> BeginDialogAsync(DialogContext dc, object options = null, CancellationToken cancellationToken = default) | ||
| { | ||
| // TODO: check name not null / expression has value | ||
|
|
||
| var startRecordingActivity = CreateCommandActivity<T>(dc.Context, this.Data, this.Name.GetValue(dc.State)); | ||
| var startRecordingActivity = CreateCommandActivity<T>(dc.Context, this.Data, this.CommandName.GetValue(dc.State)); | ||
|
|
||
| var response = await dc.Context.SendActivityAsync(startRecordingActivity, cancellationToken).ConfigureAwait(false); | ||
|
|
||
|
|
@@ -54,30 +62,51 @@ public override async Task<DialogTurnResult> BeginDialogAsync(DialogContext dc, | |
|
|
||
| public override async Task<DialogTurnResult> ContinueDialogAsync(DialogContext dc, CancellationToken cancellationToken = default) | ||
| { | ||
| // check activity | ||
| var activity = dc.Context.Activity; | ||
|
|
||
| // If command result, handle it | ||
| // We are expecting a command result with the same name as our current CommandName. | ||
| if (activity.Type == ActivityTypes.CommandResult | ||
| && activity.Name == this.Name.GetValue(dc.State)) | ||
| && activity.Name == this.CommandName.GetValue(dc.State)) | ||
| { | ||
| // TODO: correlate command id before handling it. | ||
|
|
||
| var commandResult = TelephonyExtensions.GetCommandResultValue(activity); | ||
|
|
||
| if (commandResult.Error != null) | ||
| if (commandResult?.Error != null) | ||
| { | ||
| throw new ErrorResponseException($"{commandResult.Error.Code}: {commandResult.Error.Message}"); | ||
| } | ||
|
|
||
| return await dc.EndDialogAsync(cancellationToken: cancellationToken).ConfigureAwait(false); | ||
| } | ||
|
|
||
| // for now, end turn and keep waiting | ||
| // TODO: Carlos add interruption model | ||
| // This activity was not the command result we were expecting. Mark as waiting and end the turn. | ||
| return new DialogTurnResult(DialogTurnStatus.Waiting); | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
| protected override async Task<bool> OnPreBubbleEventAsync(DialogContext dc, DialogEvent e, CancellationToken cancellationToken) | ||
| { | ||
| if (e.Name == DialogEvents.ActivityReceived && dc.Context.Activity.Type == ActivityTypes.Message) | ||
| { | ||
| // Ask parent to perform recognition | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that the recognition block is before the interruption check. Will recognize utterance lead to activities being processed?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are just emitting the event but if we return false in this method, then interruptions will not block this flow no matter the recognition. |
||
| await dc.Parent.EmitEventAsync(AdaptiveEvents.RecognizeUtterance, value: dc.Context.Activity, bubble: false, cancellationToken: cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // Should we allow interruptions | ||
| var canInterrupt = true; | ||
| if (this.AllowInterruptions != null) | ||
| { | ||
| var (allowInterruptions, error) = this.AllowInterruptions.TryGetValue(dc.State); | ||
| canInterrupt = error == null && allowInterruptions; | ||
| } | ||
|
|
||
| // Stop bubbling if interruptions ar NOT allowed | ||
| return !canInterrupt; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private static Activity CreateCommandActivity<TValue>(ITurnContext turnContext, T data, string name) | ||
| { | ||
| if (turnContext == null) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
39 changes: 39 additions & 0 deletions
39
....Components.Telephony.Tests/Integration/RecordingTests/Recording_BaseScenario.test.dialog
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| { | ||
| "$schema": "../../../tests.schema", | ||
| "$kind": "Microsoft.AdaptiveDialog", | ||
| "id": "Recording_BaseScenario.test", | ||
| "triggers": [ | ||
| { | ||
| "$kind": "Microsoft.OnBeginDialog", | ||
| "actions": [ | ||
| { | ||
| "$kind": "Microsoft.Telephony.StartRecording", | ||
| "allowInterruptions": "=coalesce(settings.allowInterruptions, true)" | ||
| }, | ||
| { | ||
| "$kind": "Microsoft.SendActivity", | ||
| "activity": "Started recording!" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "$kind": "Microsoft.OnIntent", | ||
| "intent": "HelpIntent", | ||
| "actions": [ | ||
| { | ||
| "$kind": "Microsoft.SendActivity", | ||
| "activity": "On help intent handler" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "recognizer": { | ||
| "$kind": "Microsoft.RegexRecognizer", | ||
| "intents": [ | ||
| { | ||
| "intent": "HelpIntent", | ||
| "pattern": "help" | ||
| } | ||
| ] | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this TODO. Each command will decide if it needs correlation or is fire-and-forget.