Restart Conversation from any point Functionality by srinaath · Pull Request #2089 · microsoft/BotFramework-Emulator

srinaath · 2020-02-29T01:05:30Z

Fixes #2039

This PR adds the capability to replay activities in a conversation. It allows multiple conversations being replayed at the same time.

coveralls · 2020-02-29T01:25:31Z

Coverage decreased (-0.4%) to 67.8% when pulling 723a8b2 on srravich/feature/2039-restart-conversation into 5ca8253 on master.

tonyanziano · 2020-03-02T18:22:14Z

  },

-  devtool: 'source-map',
+  devtool: 'eval-source-map',


Does this fix the Web Chat source maps and preserve the Emulator source maps? I tried to change it locally from devtool: sourcemap to using the SourceMapDevTool plugin from web chat and it managed to fix the Web Chat source maps, but downgraded the quality of the Emulator source maps to point to the transpiled code.

It does not Tony. So it still shows the transpiled code for webchat and debuggable code for emulator. So I was facing an issue with step through debugging on Chrome Dev tools failing when i use source-map. The moment I use eval-source-maps it does not break or Chrome dev tools does not seem to break when i step though and debug. Have u faced this issue before?

The issue I was facing was that the Web Chat sourcemaps were pointing to the wrong files (usually off by one file in the parent directory). So if in some Web Chat directory, there were files A, B, and C, in my debugger, trying to pull up file C would point to the sourcemap for file B.

Switching to the source map plugin fixed that for me, but also downgraded the sourcemaps of the Emulator code to point to the transpiled code instead of the TypeScript source.

I have reverted it to use just source-map. Seems like we can come up with a good solution for R9 that enables source maps for emulator as well as enable it for our shared libs (webchat, shared/app, shared/sdk)

tonyanziano · 2020-03-02T18:27:03Z

+    FROM_USER_ROLE: 'user',
+    FROM_BOT_ROLE: 'bot',


Consider changing these to USER_ROLE and BOT_ROLE because they are the same values used in the recipient property of activities, and then could be used elsewhere in the application without having to make new constants for TO_USER_ROLE and TO_BOT_ROLE

tonyanziano · 2020-03-02T18:38:51Z

+      const activities = await resp.json();
+      return activities;
+    } catch (ex) {
+      return [];


The pattern in the ConversationService is to return an HTTP response, so maybe consider returning the fetch and performing the resp.json() in the service-calling code.

Also, it might be a good idea to surface the exception in some way for debugging.

tonyanziano · 2020-03-02T20:28:55Z

  },

-  devtool: 'source-map',
+  devtool: 'eval-source-map',


The issue I was facing was that the Web Chat sourcemaps were pointing to the wrong files (usually off by one file in the parent directory). So if in some Web Chat directory, there were files A, B, and C, in my debugger, trying to pull up file C would point to the sourcemap for file B.

Switching to the source map plugin fixed that for me, but also downgraded the sourcemaps of the Emulator code to point to the transpiled code instead of the TypeScript source.

tonyanziano · 2020-03-02T20:37:57Z

+// import { EventEmitter } from 'events';
+
+// import { Activity } from 'botframework-schema';


tonyanziano · 2020-03-02T20:39:11Z

+export interface WebChatActivityChannel {
+  sendWcEvents: (args: ChannelPayload) => void;
+  getWebchatChannelSubscriber: () => Channel<ChannelPayload>;
+}
+
+export interface WebchatEventPayload {


align on capitalization pattern for Web Chat:

WebChatActivityChannel

WebchatEventPayload

is the C capitalized or not?

WebChat is what I'll go with. Reason being there are multiple usages like createWebChatStore etc written before. I'll probably stick to that pattern. Though, i think Webchat semantically makes more sense (https://github.com/microsoft/BotFramework-webchat)

tonyanziano · 2020-03-03T19:07:25Z

+}
+
+function createReplayActivitySniffer(documentId: string, meta: ReplayActivitySnifferProps = undefined) {
+  return ({ dispatch }) => next => async action => {


async not necessary

tonyanziano · 2020-03-03T19:08:05Z

+  user: User;
  speechKey?: string;
  speechRegion?: string;
-  user: User;


keep user here -- sorted alphabetically

tonyanziano · 2020-03-03T19:13:20Z

+        meta.conversationQueue.getNextActivityForPost,
+      ]);
+      if (postActivity) {
+        yield call(dispatchActivityToWebchat, dispatch, postActivity);


is there a reason you aren't using put here instead of calling dispatch?

Commented below

tonyanziano · 2020-03-03T20:30:04Z

+        ChatSagas.wcActivityChannel = createWebchatActivityChannel();
      } finally {
-        yield put(updatePendingSpeechTokenRetrieval(documentId, false));
+        yield fork(ChatSagas.handleReplayIfRequired, { documentId, action, dispatch, meta });


why does dispatch need to be passed in if we have access to put ?

So, this is dispatching on the webchat store. Put automatically dispatches to our emulator store. So, thats why I'm using call on the dispatch i get from the middleware

Ahhh, I didn't catch that. Sounds good to me 👍

tonyanziano · 2020-03-03T20:36:26Z

-      yield* throwErrorFromResponse('Error occurred while retrieving the web socket port', res);
+      throw new Error(
+        `Error occurred while retrieving the WebSocket server port: ${res.status}: ${res.statusText ||
+          'No status text'}`
+      );


change back to throwErrorFromResponse?

Nice catch.. Effects of resolving the Merge conflict

srinaath · 2020-03-04T04:10:44Z

+        meta.conversationQueue.getNextActivityForPost,
+      ]);
+      if (postActivity) {
+        yield call(dispatchActivityToWebchat, dispatch, postActivity);


Commented below

srinaath · 2020-03-04T04:11:02Z

+        ChatSagas.wcActivityChannel = createWebchatActivityChannel();
      } finally {
-        yield put(updatePendingSpeechTokenRetrieval(documentId, false));
+        yield fork(ChatSagas.handleReplayIfRequired, { documentId, action, dispatch, meta });


So, this is dispatching on the webchat store. Put automatically dispatches to our emulator store. So, thats why I'm using call on the dispatch i get from the middleware

srinaath · 2020-03-04T04:14:17Z

+  public static *watchForWcEvents() {
+    const wcEventChannel = ChatSagas.wcActivityChannel.getWebchatChannelSubscriber();
+    while (true) {
+      const { documentId, action, dispatch, meta }: ChannelPayload = yield take(wcEventChannel);


Dispatch I receive from webchat middleware

srinaath · 2020-03-04T04:15:32Z

-      yield* throwErrorFromResponse('Error occurred while retrieving the web socket port', res);
+      throw new Error(
+        `Error occurred while retrieving the WebSocket server port: ${res.status}: ${res.statusText ||
+          'No status text'}`
+      );


Nice catch.. Effects of resolving the Merge conflict

srinaath · 2020-03-04T04:36:12Z

+export interface WebChatActivityChannel {
+  sendWcEvents: (args: ChannelPayload) => void;
+  getWebchatChannelSubscriber: () => Channel<ChannelPayload>;
+}
+
+export interface WebchatEventPayload {


WebChat is what I'll go with. Reason being there are multiple usages like createWebChatStore etc written before. I'll probably stick to that pattern. Though, i think Webchat semantically makes more sense (https://github.com/microsoft/BotFramework-webchat)

srinaath · 2020-03-04T06:44:14Z

-  requireNewUserId: boolean = false
+  requireNewUserId: boolean = false,
+  activity: Activity = undefined,
+  createObjectUrl: Function = undefined


So the reason was I just thought accessing the window object directly inside chatSaga might not be a good idea. Rather injecting it as a dependacy would help us with unit testing as well.

srinaath · 2020-03-04T07:39:40Z

-  requireNewUserId: boolean = false
+  requireNewUserId: boolean = false,
+  activity: Activity = undefined,
+  createObjectUrl: Function = undefined


What do u think?

Signed-off-by: Srinaath Ravichandran <[email protected]> get activities for a conversation Signed-off-by: Srinaath Ravichandran <[email protected]> Return updated activity Signed-off-by: Srinaath Ravichandran <[email protected]> Adding declaration maps for inspection Signed-off-by: Srinaath Ravichandran <[email protected]> Conversation routes set for fetch activities Signed-off-by: Srinaath Ravichandran <[email protected]> Safe commit Signed-off-by: Srinaath Ravichandran <[email protected]> Queues updated Signed-off-by: Srinaath Ravichandran <[email protected]> Safe commit Signed-off-by: Srinaath Ravichandran <[email protected]> Safe saga commit Signed-off-by: Srinaath Ravichandran <[email protected]> Resart multiple times the same conversation working. Major landmark Signed-off-by: Srinaath Ravichandran <[email protected]> Stable replay Signed-off-by: Srinaath Ravichandran <[email protected]> Restart from specific activity working Signed-off-by: Srinaath Ravichandran <[email protected]> UI updates Signed-off-by: Srinaath Ravichandran <[email protected]> Safe saga flow Signed-off-by: Srinaath Ravichandran <[email protected]> Safe replay Signed-off-by: Srinaath Ravichandran <[email protected]> Forked post activity to let moving on to next activity Signed-off-by: Srinaath Ravichandran <[email protected]> Blocking webchat Signed-off-by: Srinaath Ravichandran <[email protected]> Simenatous conversations complete Signed-off-by: Srinaath Ravichandran <[email protected]> Error notficaition viewer completed Signed-off-by: Srinaath Ravichandran <[email protected]> Handling Dlspeech bot sniffer Signed-off-by: Srinaath Ravichandran <[email protected]> Conversation service spec completed Signed-off-by: Srinaath Ravichandran <[email protected]> Post activity test Signed-off-by: Srinaath Ravichandran <[email protected]> Mount conversation routes test Signed-off-by: Srinaath Ravichandran <[email protected]> Spec files updated Signed-off-by: Srinaath Ravichandran <[email protected]> Tests wrapped up for conversation queue Signed-off-by: Srinaath Ravichandran <[email protected]> Progressive response handling Signed-off-by: Srinaath Ravichandran <[email protected]> Stable commit to replay chat Signed-off-by: Srinaath Ravichandran <[email protected]> UI tests restored to normality Signed-off-by: Srinaath Ravichandran <[email protected]> Tests working for conversationQueue Signed-off-by: Srinaath Ravichandran <[email protected]> Chatsagas stable tests Signed-off-by: Srinaath Ravichandran <[email protected]> one more test working Signed-off-by: Srinaath Ravichandran <[email protected]> All tests passing Signed-off-by: Srinaath Ravichandran <[email protected]> Chat saga test wrapped up Signed-off-by: Srinaath Ravichandran <[email protected]> Restart conversation queue tests completed Signed-off-by: Srinaath Ravichandran <[email protected]> UI tests updated Signed-off-by: Srinaath Ravichandran <[email protected]>

Signed-off-by: Srinaath Ravichandran <[email protected]> More UI tests Signed-off-by: Srinaath Ravichandran <[email protected]>

Signed-off-by: Srinaath Ravichandran <[email protected]>

Renaming for consistent webchat captialized handling Renaming functions Signed-off-by: Srinaath Ravichandran <[email protected]>

Signed-off-by: Srinaath Ravichandran <[email protected]> More PR feedback handled Signed-off-by: Srinaath Ravichandran <[email protected]>

Signed-off-by: Srinaath Ravichandran <[email protected]>

tonyanziano

Looks good! Great job 😃

srinaath · 2020-03-05T19:16:25Z

Looks good! Great job 😃

thanks Tony

srinaath requested review from a-b-r-o-w-n, compulim, corinagum, cwhitten, tdurnford and tonyanziano as code owners February 29, 2020 01:05

tonyanziano reviewed Mar 2, 2020

View reviewed changes

tonyanziano suggested changes Mar 3, 2020

View reviewed changes

srinaath commented Mar 4, 2020

View reviewed changes

Srinaath Ravichandran added 10 commits March 4, 2020 17:36

More unit tests

4ba6b22

Signed-off-by: Srinaath Ravichandran <[email protected]> More UI tests Signed-off-by: Srinaath Ravichandran <[email protected]>

Tests Cleanup

e2a8684

Signed-off-by: Srinaath Ravichandran <[email protected]>

Changelog updated

488d16c

Signed-off-by: Srinaath Ravichandran <[email protected]>

Restart conversation options

e11217f

Signed-off-by: Srinaath Ravichandran <[email protected]>

Split button tests passed

8562dab

Signed-off-by: Srinaath Ravichandran <[email protected]>

PR feedback addressed

30e60b7

Renaming for consistent webchat captialized handling Renaming functions Signed-off-by: Srinaath Ravichandran <[email protected]>

Revertng webpack config

7bec918

Signed-off-by: Srinaath Ravichandran <[email protected]> More PR feedback handled Signed-off-by: Srinaath Ravichandran <[email protected]>

Renaming user role

3b2cc84

Signed-off-by: Srinaath Ravichandran <[email protected]>

Stable PR updates

6ae438e

Signed-off-by: Srinaath Ravichandran <[email protected]>

srinaath force-pushed the srravich/feature/2039-restart-conversation branch from fbbb974 to 6ae438e Compare March 5, 2020 01:37

Merge conflicts resolved

723a8b2

Signed-off-by: Srinaath Ravichandran <[email protected]>

srinaath requested a review from tonyanziano March 5, 2020 05:35

tonyanziano approved these changes Mar 5, 2020

View reviewed changes

srinaath merged commit 81cd3a9 into master Mar 5, 2020

srinaath deleted the srravich/feature/2039-restart-conversation branch March 7, 2020 21:13

cwhitten mentioned this pull request Mar 17, 2020

req.conversation.postActivityToUser is not a function #2109

Closed

srinaath changed the title ~~Srravich/feature/2039 restart conversation~~ Restart Conversation from any point Functionality May 20, 2020

		// import { EventEmitter } from 'events';

		// import { Activity } from 'botframework-schema';

Conversation

srinaath commented Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srinaath Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srinaath Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonyanziano left a comment

Choose a reason for hiding this comment

Uh oh!

srinaath commented Mar 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

srinaath commented Feb 29, 2020 •

edited

Loading

coveralls commented Feb 29, 2020 •

edited

Loading

srinaath Mar 4, 2020 •

edited

Loading

srinaath Mar 4, 2020 •

edited

Loading