-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update SkillDialog to handle all activity types (e.g. set DeliveryMode for Invokes) #2161
Conversation
Pull Request Test Coverage Report for Build 127831
💛 - Coveralls |
// If the SkillConversationIdFactoryBase implementation doesn't support createSkillConversationIdWithOptions(), | ||
// use createSkillConversationId() instead. | ||
skillConversationId = await this.dialogOptions.conversationIdFactory.createSkillConversationId(TurnContext.getConversationReference(activity) as ConversationReference); | ||
if (activity.type === ActivityTypes.Invoke) { |
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.
having to switch expectReplies on for an invoke seems a little odd.
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.
if the dialog is interacting with a skill that only works with invokes, this makes sure that EoC replies are buffered with the invokeResponse and the dialog ends when an EoC comes back.
Here is an example on how this would look on a skill for teams:
protected override async Task<TaskModuleResponse> OnTeamsTaskModuleSubmitAsync(ITurnContext<IInvokeActivity> turnContext, TaskModuleRequest taskModuleRequest, CancellationToken cancellationToken)
{
var reply = MessageFactory.Text("OnTeamsTaskModuleSubmitAsync Value: " + JsonConvert.SerializeObject(taskModuleRequest));
await turnContext.SendActivityAsync(reply, cancellationToken);
// Send End of conversation at the end.
var activity = new Activity(ActivityTypes.EndOfConversation) { Value = taskModuleRequest.Data, Locale = ((Activity)turnContext.Activity).Locale };
await turnContext.SendActivityAsync(activity, cancellationToken);
return new TaskModuleResponse
{
Task = new TaskModuleMessageResponse
{
Value = "Thanks!",
},
};
}
@@ -843,8 +843,18 @@ export class BotFrameworkAdapter extends BotAdapter implements ConnectorClientBu | |||
context.turnState.set(BotCallbackHandlerKey, logic); | |||
await this.runMiddleware(context, logic); | |||
|
|||
// Retrieve cached invoke response. | |||
if (request.type === ActivityTypes.Invoke) { |
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.
and this change presumably for consistency with the other place this condition occurs...
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.
Yes, these changes are ported from C# with exactly the same spirit (and tests) in mind. That said, I will add some more complex unit tests tomorrow to cover the interactions between the BFAdapter and ActivityHandler, and the two with SkillDialog.
Fixes #2139
Specific Changes
StatusCodes.MULTIPLE_CHOICES = 300
SkillDialog.onValidateActivity()
SkillDialog
now forwards allActivityTypes
to the Skill.BotFrameworkAdapter.createConnectorClientWithIdentity()
BotFrameworkAdapter.createConnectorClientWithIdentity()
BotFrameworkAdapter.processActivity()
when deliveryMode === 'expectReplies', type isInvoke
and reply activities have typeinvokeResponse