apply SkillDialog updates from microsoft/botbuilder-dotnet#3474

Author: stevengum

libraries/botbuilder-dialogs/src/skillDialog.ts

@@ -6,16 +6,24 @@
  * Licensed under the MIT License.
  */
 
-import { Activity, ActivityTypes, BotFrameworkSkill, ConversationState, ConversationReference, StatePropertyAccessor, TurnContext } from 'botbuilder-core';
-import { Dialog, DialogTurnResult } from './dialog';
+import {
+    Activity,
+    ActivityTypes,
+    ConversationReference,
+    TurnContext
+} from 'botbuilder-core';
+import {
+    Dialog,
+    DialogInstance,
+    DialogReason,
+    DialogTurnResult
+} from './dialog';
 import { DialogContext } from './dialogContext';
 import { SkillDialogArgs } from './skillDialogArgs';
 import { SkillDialogOptions } from './skillDialogOptions';
 
 export class SkillDialog extends Dialog {
-    private readonly _activeSkillProperty: StatePropertyAccessor;
-    private readonly _conversationState: ConversationState; // Why is this restricted to ConversationState?
-    private readonly _dialogOptions: SkillDialogOptions;
+    protected dialogOptions: SkillDialogOptions;
 
     /**
      * A sample dialog that can wrap remote calls to a skill.
@@ -24,51 +32,36 @@ export class SkillDialog extends Dialog {
      * The options parameter in `beginDialog()` must be a `SkillDialogArgs` object with the initial parameters
      * for the dialog.
      * 
-     * @param SkillDialogOptions 
      * @param dialogOptions 
-     * @param ConversationState 
-     * @param conversationState 
+     * @param dialogId
      */
-    public constructor(dialogOptions: SkillDialogOptions, conversationState: ConversationState) {
-        super('SkillDialog'); // What about Dialog Id?
+    public constructor(dialogOptions: SkillDialogOptions, dialogId?: string) {
+        super(dialogId);
         if (!dialogOptions) {
             throw new TypeError('Missing dialogOptions parameter');
         }
-
-        if (!conversationState) {
-            throw new TypeError('Missing conversationState parameter');
-        }
-        
-        this._dialogOptions = dialogOptions;
-        this._conversationState = conversationState;
-
-        this._activeSkillProperty = conversationState.createProperty<BotFrameworkSkill>('botbuilder-dialogs.SkillDialog.ActiveSkillProperty');
+        this.dialogOptions = dialogOptions;
     }
 
     public async beginDialog(dc: DialogContext, options?: {}): Promise<DialogTurnResult> {
-        const dialogArgs = SkillDialog.validateBeginDialogOptions(options);
+        const dialogArgs = SkillDialog.validateBeginDialogArgs(options);
 
         await dc.context.sendTraceActivity(`${ this.id }.beginDialog()`, undefined, undefined, `Using activity of type: ${ dialogArgs.activity.type }`);
 
-        // Store Skill information for this dialog instance
-        await this._activeSkillProperty.set(dc.context, dialogArgs.skill);
-
         // Create deep clone of the original activity to avoid altering it before forwarding it.
         const clonedActivity = this.cloneActivity(dialogArgs.activity);
 
         // Apply conversation reference and common properties from incoming activity before sending.
         const skillActivity = TurnContext.applyConversationReference(clonedActivity, TurnContext.getConversationReference(dc.context.activity), true) as Activity;
 
         // Send the activity to the skill.
-        await this.sendToSkill(dc, skillActivity, dialogArgs.skill);
+        await this.sendToSkill(dc.context, skillActivity);
         return Dialog.EndOfTurn;
     }
 
     public async continueDialog(dc: DialogContext): Promise<DialogTurnResult> {
         await dc.context.sendTraceActivity(`${ this.id }.continueDialog()`, undefined, undefined, `ActivityType: ${ dc.context.activity.type }`);
 
-        // Retrieve the current skill information from ConversationState
-        var skillInfo = await this._activeSkillProperty.get(dc.context, null);
 
         // Handle EndOfConversation from the skill (this will be sent to the this dialog by the SkillHandler if received from the Skill)
         if (dc.context.activity.type === ActivityTypes.EndOfConversation) {
@@ -79,12 +72,28 @@ export class SkillDialog extends Dialog {
         // Forward only Message and Event activities to the skill
         if (dc.context.activity.type === ActivityTypes.Message || dc.context.activity.type === ActivityTypes.Event) {
             // Just forward to the remote skill
-            await this.sendToSkill(dc, dc.context.activity, skillInfo);
+            await this.sendToSkill(dc.context, dc.context.activity);
         }
 
         return Dialog.EndOfTurn;
     }
 
+    public async endDialog(context: TurnContext, instance: DialogInstance, reason: DialogReason): Promise<void> {
+        // Send of of conversation to the skill if the dialog has been cancelled. 
+        if (reason == DialogReason.cancelCalled || reason == DialogReason.replaceCalled) {
+            await context.sendTraceActivity(`${ this.id }.EndDialogAsync()`, undefined, undefined, `ActivityType: ${ context.activity.type }`);
+
+            const reference = TurnContext.getConversationReference(context.activity);
+            // Apply conversation reference and common properties from incoming activity before sending.
+            const activity = TurnContext.applyConversationReference({ type: ActivityTypes.EndOfConversation }, reference, true);
+            activity.channelData = context.activity.channelData;
+
+            await this.sendToSkill(context, activity as Activity);
+        }
+
+        await super.endDialog(context, instance, reason);
+    }
+
     /**
      * Clones the Activity entity.
      * @param activity Activity to clone.
@@ -93,18 +102,14 @@ export class SkillDialog extends Dialog {
         return Object.assign({} as Activity, activity);
     }
 
-    private static validateBeginDialogOptions(options: any): SkillDialogArgs {
+    private static validateBeginDialogArgs(options: any): SkillDialogArgs {
         if (!options) {
             throw new TypeError('Missing options parameter');
         }
 
         const dialogArgs = options as SkillDialogArgs;
 
-        if (dialogArgs.skill == undefined || dialogArgs.skill == null) {
-            throw new TypeError(`"skill" undefined or null in options`);
-        }
-
-        if (dialogArgs.activity === undefined || dialogArgs.activity === null) {
+        if (!dialogArgs.activity) {
             throw new TypeError(`"activity" is undefined or null in options.`);
         }
 
@@ -117,15 +122,16 @@ export class SkillDialog extends Dialog {
         return dialogArgs;
     }
 
-    private async sendToSkill(dc: DialogContext, activity: Activity, skillInfo: BotFrameworkSkill): Promise<void> {
+    private async sendToSkill(context: TurnContext, activity: Activity): Promise<void> {
+        // Create a conversationId to interact with the skill and send the activity
+        const skillConversationId = await this.dialogOptions.conversationIdFactory.createSkillConversationId(TurnContext.getConversationReference(activity) as ConversationReference);
+
         // Always save state before forwarding
         // (the dialog stack won't get updated with the skillDialog and things won't work if you don't)
-        await this._conversationState.saveChanges(dc.context, true);
-    
-        // Create a conversationId to interact with the skill and send the activity
-        const skillConversationId = await this._dialogOptions.conversationIdFactory.createSkillConversationId(TurnContext.getConversationReference(activity) as ConversationReference);
-        const response = await this._dialogOptions.skillClient.postActivity(this._dialogOptions.botId, skillInfo.appId, skillInfo.skillEndpoint, this._dialogOptions.skillHostEndpoint, skillConversationId, activity);
-    
+        await this.dialogOptions.conversationState.saveChanges(context, true);
+        const skillInfo = this.dialogOptions.skill;
+        const response = await this.dialogOptions.skillClient.postActivity(this.dialogOptions.botId, skillInfo.appId, skillInfo.skillEndpoint, skillInfo.skillEndpoint, skillConversationId, activity);
+
         // Inspect the skill response status
         if (!(response.status >= 200 && response.status <= 299)) {
             throw new Error(`Error invoking the skill id: "${ skillInfo.id }" at "${ skillInfo.skillEndpoint }" (status is ${ response.status }). \r\n ${ response.body }`);

libraries/botbuilder-dialogs/src/skillDialogArgs.ts

@@ -6,17 +6,12 @@
  * Licensed under the MIT License.
  */
 
-import { Activity, BotFrameworkSkill } from 'botbuilder-core';
+import { Activity } from 'botbuilder-core';
 
 /**
  * A class with dialog arguments for a SkillDialog.
  */
 export interface SkillDialogArgs {
-    /**
-     * The BotFrameworkSkill that the dialog will call.
-     */
-    skill: BotFrameworkSkill;
-
     /**
      * The Activity to send to the skill.
      */

libraries/botbuilder-dialogs/src/skillDialogOptions.ts

@@ -6,15 +6,41 @@
  * Licensed under the MIT License.
  */
 
-import { BotFrameworkClient, SkillConversationIdFactoryBase } from 'botbuilder-core';
+import {
+    BotFrameworkClient,
+    BotFrameworkSkill,
+    ConversationState,
+    SkillConversationIdFactoryBase
+} from 'botbuilder-core';
 
 export interface SkillDialogOptions {
-    
+    /**
+     * The the Microsoft app ID of the bot calling the skill.
+     */
     botId: string;
 
+    /**
+     * The BotFrameworkSkill that the dialog will call.
+     */
+    conversationIdFactory: SkillConversationIdFactoryBase;
+
+    /**
+     * The ConversationState to be used by the Dialog.
+     */
+    conversationState: ConversationState;
+
+    /**
+     * The BotFrameworkSkill the dialog will call. 
+     */
+    skill: BotFrameworkSkill;
+
+    /**
+     * The BotFrameworkClient used to call the remote skill.
+     */
     skillClient: BotFrameworkClient;
 
+    /**
+     * The callback Url for the skill host.
+     */
     skillHostEndpoint: string;
-
-    conversationIdFactory: SkillConversationIdFactoryBase;
 }