Bot Change Detection #2044

Stevenic · 2020-04-15T23:10:06Z

Description

These changes add logic to automatically detect that a bots dialogs have changed since a persisted dialog stack was started. A hash code is stored with each persisted container dialog that is then checked on each subsequent execution.

When a change is detected a new 'dialogChanged' event is raised which the bot can optionally handle to deal with the change in some custom way. If that event isn't handled and error will be thrown which will trigger the bots standard error handling logic.

note: WaterfallDialog does not currently support change detection as there's no great way to do this. You can check for the number of steps to change but that's about it.

Specific Changes

Added new DialogSetchangeHash property.
Added new ContainerDialog.checkForChanges() method.
Updated ComponentDialog to call new method.
Updated AdaptiveDialog to call new method.

Testing

Added unit tests to cover the new feature.

…heir changed

coveralls · 2020-04-15T23:16:45Z

Pull Request Test Coverage Report for Build 125410

47 of 51 (92.16%) changed or added relevant lines in 7 files are covered.
9 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.008%) to 79.786%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
libraries/botbuilder-dialogs-adaptive/src/adaptiveDialog.ts	18	20	90.0%
libraries/botbuilder-dialogs/src/componentDialog.ts	2	4	50.0%

Files with Coverage Reduction	New Missed Lines	%
libraries/botframework-streaming/src/webSocket/webSocketTransport.ts	2	80.65%
libraries/botframework-streaming/src/webSocket/webSocketServer.ts	7	66.67%

Totals
Change from base Build 125266:	0.008%
Covered Lines:	12351
Relevant Lines:	14769

💛 - Coveralls

stevengum

LGTM

Leaving the approval to those more familiar with Adaptive Dialogs and its requirements.

stevengum · 2020-04-16T00:46:31Z

libraries/botbuilder-dialogs/tests/componentDialog.test.js

+                assert(results.status === DialogTurnStatus.complete, `results.status should be 'complete' not ${ results.status }`);
+                assert(results.result === undefined, `results.result should be undefined, not ${ results.result }`);


To avoid having to add descriptive error messages, this could be simplified to

Suggested change

assert(results.status === DialogTurnStatus.complete, `results.status should be 'complete' not ${ results.status }`);

assert(results.result === undefined, `results.result should be undefined, not ${ results.result }`);

assert.strictEqual(results.status, DialogTurnStatus.complete);

assert.strictEqual(results.result, undefined);

tomlm · 2020-04-16T15:02:14Z

libraries/botbuilder-dialogs-adaptive/src/adaptiveDialog.ts

@@ -143,6 +143,8 @@ export class AdaptiveDialog<O extends object = {}> extends DialogContainer<O> {
    public async beginDialog(dc: DialogContext, options?: O): Promise<DialogTurnResult> {
        this.onPushScopedServices(dc.context);
        try {
+            await this.checkForChanges(dc);


await this.checkForChanges(dc); [](start = 8, length = 35)

Should checking for changes be built into the dialog context itself? It's the thing that is orchestrating switching between dialogs isn't it? #Closed

tomlm

tomlm · 2020-04-24T16:05:53Z

libraries/botbuilder-dialogs/src/dialogEvents.ts

@@ -11,5 +11,6 @@ export class DialogEvents {
    static readonly repromptDialog = "repromptDialog";
    static readonly cancelDialog = "cancelDialog";
    static readonly activityReceived = "activityReceived";
+    static readonly versionChanged = "dialogChanged";


dialogChanged [](start = 38, length = 13)

name doesn't match variable name? #Closed

tomlm · 2020-04-24T16:57:44Z

libraries/botbuilder-dialogs/src/waterfallDialog.ts

+        // Simply return the id + number of steps to help detect when new steps have
+        // been added to a given waterfall.
+        return `${this.id}:${this.steps.length}`;
+    }


I wonder if this should be a method.

This requires that the langauge supports virtual properties. For example, WaterfallDialog derives from Dialog and so needs to "override" the base version because it's a property. #Resolved

tomlm

🕐

libraries/botbuilder-dialogs-adaptive/src/adaptiveDialog.ts

tomlm · 2020-04-24T22:49:52Z

libraries/botbuilder-dialogs/src/dialogContainer.ts

+     */
+    protected async checkForVersionChange(dc: DialogContext): Promise<void> {
+        const current = dc.activeDialog.version;
+        dc.activeDialog.version = this.dialogs.getVersion();


this.dialogs.getVersion(); [](start = 33, length = 27)

Instead of this call put this into virtual GetInternalVersion()

tomlm

… schema

Added logic to automatically detect container dialogs that have had t…

eeee829

…heir changed

Stevenic requested review from chon219, stevengum, tomlm and carlosscastro April 15, 2020 23:10

Stevenic mentioned this pull request Apr 15, 2020

[PORT] Bot Change Detection microsoft/botbuilder-dotnet#3749

Closed

stevengum reviewed Apr 16, 2020

View reviewed changes

tomlm reviewed Apr 16, 2020

View reviewed changes

Stevenic added 2 commits April 21, 2020 14:10

Added dedicated version property to dialogs.

f8834d0

Merge branch 'master' into stevenic/detect-bot-changes

5dcb4bc

tomlm approved these changes Apr 24, 2020

View reviewed changes

tomlm reviewed Apr 24, 2020

View reviewed changes

tomlm suggested changes Apr 24, 2020

View reviewed changes

tomlm reviewed Apr 24, 2020

View reviewed changes

libraries/botbuilder-dialogs-adaptive/src/adaptiveDialog.ts Show resolved Hide resolved

Updated code based on comments

c7dd69a

tomlm reviewed Apr 24, 2020

View reviewed changes

Stevenic added 2 commits April 25, 2020 11:10

Updates to match parity with C#

7e2a1bc

Merge branch 'master' into stevenic/detect-bot-changes

f58659a

tomlm approved these changes Apr 26, 2020

View reviewed changes

Stevenic added 2 commits April 27, 2020 14:12

Updated adaptive to detect changes made to trigger expressions and/or…

e18b16b

… schema

Merge branch 'master' into stevenic/detect-bot-changes

62127d8

Stevenic merged commit d1df6cb into master Apr 27, 2020

Stevenic deleted the stevenic/detect-bot-changes branch April 27, 2020 21:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bot Change Detection #2044

Bot Change Detection #2044

Stevenic commented Apr 15, 2020

coveralls commented Apr 15, 2020 •

edited

Loading

stevengum left a comment

stevengum Apr 16, 2020

tomlm Apr 16, 2020 •

edited

Loading

tomlm left a comment

tomlm Apr 24, 2020 •

edited

Loading

tomlm Apr 24, 2020 •

edited

Loading

tomlm left a comment

tomlm Apr 24, 2020

tomlm left a comment

		assert(results.status === DialogTurnStatus.complete, `results.status should be 'complete' not ${ results.status }`);
		assert(results.result === undefined, `results.result should be undefined, not ${ results.result }`);

Bot Change Detection #2044

Bot Change Detection #2044

Conversation

Stevenic commented Apr 15, 2020

Description

Specific Changes

Testing

coveralls commented Apr 15, 2020 • edited Loading

Pull Request Test Coverage Report for Build 125410

💛 - Coveralls

stevengum left a comment

Choose a reason for hiding this comment

stevengum Apr 16, 2020

Choose a reason for hiding this comment

tomlm Apr 16, 2020 • edited Loading

Choose a reason for hiding this comment

tomlm left a comment

Choose a reason for hiding this comment

tomlm Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

tomlm Apr 24, 2020 • edited Loading

Choose a reason for hiding this comment

tomlm left a comment

Choose a reason for hiding this comment

tomlm Apr 24, 2020

Choose a reason for hiding this comment

tomlm left a comment

Choose a reason for hiding this comment

coveralls commented Apr 15, 2020 •

edited

Loading

tomlm Apr 16, 2020 •

edited

Loading

tomlm Apr 24, 2020 •

edited

Loading

tomlm Apr 24, 2020 •

edited

Loading