fix: Fix dialog redirect/refresh behavior

Composer/packages/client/src/pages/design/DesignPage.tsx

@@ -42,7 +42,6 @@ import {
   breadcrumbState,
   visualEditorSelectionState,
   focusPathState,
-  designPageLocationState,
   showAddSkillDialogModalState,
   skillsState,
   actionsSeedState,
@@ -118,7 +117,6 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
   const breadcrumb = useRecoilValue(breadcrumbState);
   const visualEditorSelection = useRecoilValue(visualEditorSelectionState);
   const focusPath = useRecoilValue(focusPathState);
-  const designPageLocation = useRecoilValue(designPageLocationState);
   const showCreateDialogModal = useRecoilValue(showCreateDialogModalState);
   const showAddSkillDialogModal = useRecoilValue(showAddSkillDialogModalState);
   const { undo, redo, canRedo, canUndo, commitChanges, clearUndo } = useRecoilValue(undoFunctionState);
@@ -161,12 +159,13 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
   const shellForPropertyEditor = useShell('PropertyEditor');
   const triggerApi = useTriggerApi(shell.api);
   const { createTrigger } = shell.api;
+
   useEffect(() => {
     const currentDialog = dialogs.find(({ id }) => id === dialogId);
     if (currentDialog) {
       setCurrentDialog(currentDialog);
     }
-    const rootDialog = dialogs.find(({ isRoot }) => isRoot === true);
+    const rootDialog = dialogs.find(({ isRoot }) => isRoot);
     if (!currentDialog && rootDialog) {
       const { search } = location || {};
       navigateTo(`/bot/${projectId}/dialogs/${rootDialog.id}${search}`);
@@ -195,13 +194,6 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
     });
   }, [dialogs]);
 
-  useEffect(() => {
-    const index = currentDialog.triggers.findIndex(({ type }) => type === SDKKinds.OnBeginDialog);
-    if (index >= 0 && !designPageLocation.selected && designPageLocation.projectId === projectId) {
-      selectTo(createSelectedPath(index));
-    }
-  }, [currentDialog?.id]);
-
   useEffect(() => {
     if (location && props.dialogId && props.projectId) {
       const { dialogId, projectId } = props;
@@ -211,7 +203,7 @@ const DesignPage: React.FC<RouteComponentProps<{ dialogId: string; projectId: st
         projectId: projectId,
         selected: params.get('selected') ?? '',
         focused: params.get('focused') ?? '',
-        breadcrumb: location.state ? location.state.breadcrumb || [] : [],
+        breadcrumb: location.state?.breadcrumb || [],
         promptTab: getTabFromFragment(),
       });
       /* eslint-disable no-underscore-dangle */

Composer/packages/client/src/recoilModel/dispatchers/__tests__/navigation.test.tsx

@@ -3,13 +3,27 @@
 
 import { useRecoilValue } from 'recoil';
 import { act } from '@bfc/test-utils/lib/hooks';
+import { SDKKinds } from '@bfc/shared/src/types';
 
 import { navigationDispatcher } from '../navigation';
 import { renderRecoilHook } from '../../../../__tests__/testUtils';
-import { focusPathState, breadcrumbState, designPageLocationState, projectIdState } from '../../atoms/botState';
+import {
+  focusPathState,
+  breadcrumbState,
+  designPageLocationState,
+  projectIdState,
+  dialogsState,
+} from '../../atoms/botState';
 import { dispatcherState } from '../../../recoilModel/DispatcherWrapper';
-import { navigateTo, checkUrl, updateBreadcrumb, getUrlSearch, BreadcrumbUpdateType } from '../../../utils/navigation';
-import { getSelected } from '../../../utils/dialogUtil';
+import {
+  convertPathToUrl,
+  navigateTo,
+  checkUrl,
+  updateBreadcrumb,
+  getUrlSearch,
+  BreadcrumbUpdateType,
+} from '../../../utils/navigation';
+import { createSelectedPath, getSelected } from '../../../utils/dialogUtil';
 import { BreadcrumbItem } from '../../../recoilModel/types';
 
 jest.mock('../../../utils/navigation');
@@ -20,6 +34,8 @@ const mockNavigateTo = navigateTo as jest.Mock<void>;
 const mockGetSelected = getSelected as jest.Mock<string>;
 const mockUpdateBreadcrumb = updateBreadcrumb as jest.Mock<BreadcrumbItem[]>;
 const mockGetUrlSearch = getUrlSearch as jest.Mock<string>;
+const mockConvertPathToUrl = convertPathToUrl as jest.Mock<string>;
+const mockCreateSelectedPath = createSelectedPath as jest.Mock<string>;
 
 const PROJECT_ID = '12345.678';
 
@@ -33,6 +49,8 @@ describe('navigation dispatcher', () => {
     mockCheckUrl.mockClear();
     mockNavigateTo.mockClear();
     mockUpdateBreadcrumb.mockReturnValue([]);
+    mockConvertPathToUrl.mockClear();
+    mockCreateSelectedPath.mockClear();
 
     mockCheckUrl.mockReturnValue(false);
 
@@ -42,8 +60,10 @@ describe('navigation dispatcher', () => {
       const designPageLocation = useRecoilValue(designPageLocationState);
       const projectId = useRecoilValue(projectIdState);
       const currentDispatcher = useRecoilValue(dispatcherState);
+      const dialogs = useRecoilValue(dialogsState);
 
       return {
+        dialogs,
         focusPath,
         breadcrumb,
         designPageLocation,
@@ -66,6 +86,10 @@ describe('navigation dispatcher', () => {
           },
         },
         { recoilState: projectIdState, initialValue: PROJECT_ID },
+        {
+          recoilState: dialogsState,
+          initialValue: [{ id: 'newDialogId', triggers: [{ type: SDKKinds.OnBeginDialog }] }],
+        },
       ],
       dispatcher: {
         recoilState: dispatcherState,
@@ -158,10 +182,23 @@ describe('navigation dispatcher', () => {
 
   describe('navTo', () => {
     it('navigates to a destination', async () => {
+      mockConvertPathToUrl.mockReturnValue(`/bot/${PROJECT_ID}/dialogs/dialogId`);
       await act(async () => {
         await dispatcher.navTo('dialogId', []);
       });
       expectNavTo(`/bot/${PROJECT_ID}/dialogs/dialogId`);
+      expect(mockConvertPathToUrl).toBeCalledWith(PROJECT_ID, 'dialogId', undefined);
+    });
+
+    it('redirects to the begin dialog trigger', async () => {
+      mockConvertPathToUrl.mockReturnValue(`/bot/${PROJECT_ID}/dialogs/newDialogId?selection=triggers[0]`);
+      mockCreateSelectedPath.mockReturnValue('triggers[0]');
+      await act(async () => {
+        await dispatcher.navTo('newDialogId', []);
+      });
+      expectNavTo(`/bot/${PROJECT_ID}/dialogs/newDialogId?selection=triggers[0]`);
+      expect(mockConvertPathToUrl).toBeCalledWith(PROJECT_ID, 'newDialogId', 'triggers[0]');
+      expect(mockCreateSelectedPath).toBeCalledWith(0);
     });
 
     it("doesn't navigate to a destination where we already are", async () => {
@@ -182,10 +219,12 @@ describe('navigation dispatcher', () => {
     });
 
     it('navigates to a default URL with selected path', async () => {
+      mockConvertPathToUrl.mockReturnValue(`/bot/${PROJECT_ID}/dialogs/dialogId?selected=selection`);
       await act(async () => {
         await dispatcher.selectTo('selection');
       });
       expectNavTo(`/bot/${PROJECT_ID}/dialogs/dialogId?selected=selection`);
+      expect(mockConvertPathToUrl).toBeCalledWith(PROJECT_ID, 'dialogId', 'selection');
     });
 
     it("doesn't go anywhere if we're already there", async () => {

Composer/packages/client/src/recoilModel/dispatchers/navigation.ts

@@ -4,12 +4,25 @@
 
 //TODO: refactor the router to use one-way data flow
 import { useRecoilCallback, CallbackInterface } from 'recoil';
-import { PromptTab } from '@bfc/shared';
+import { PromptTab, SDKKinds } from '@bfc/shared';
 
-import { getSelected } from './../../utils/dialogUtil';
+import { createSelectedPath, getSelected } from './../../utils/dialogUtil';
 import { BreadcrumbItem } from './../../recoilModel/types';
-import { focusPathState, breadcrumbState, designPageLocationState, projectIdState } from './../atoms/botState';
-import { updateBreadcrumb, navigateTo, checkUrl, getUrlSearch, BreadcrumbUpdateType } from './../../utils/navigation';
+import {
+  breadcrumbState,
+  designPageLocationState,
+  dialogsState,
+  focusPathState,
+  projectIdState,
+} from './../atoms/botState';
+import {
+  BreadcrumbUpdateType,
+  checkUrl,
+  convertPathToUrl,
+  getUrlSearch,
+  navigateTo,
+  updateBreadcrumb,
+} from './../../utils/navigation';
 
 export const navigationDispatcher = () => {
   const setDesignPageLocation = useRecoilCallback(
@@ -46,7 +59,21 @@ export const navigationDispatcher = () => {
     ({ snapshot }: CallbackInterface) => async (dialogId: string, breadcrumb: BreadcrumbItem[] = []) => {
       const projectId = await snapshot.getPromise(projectIdState);
       const designPageLocation = await snapshot.getPromise(designPageLocationState);
-      const currentUri = `/bot/${projectId}/dialogs/${dialogId}`;
+      const dialogs = await snapshot.getPromise(dialogsState);
+      const currentDialog = dialogs.find(({ id }) => id === dialogId);
+
+      let path;
+      if (dialogId !== designPageLocation.dialogId) {
+        // Redirect to Microsoft.OnBeginDialog trigger if it exists on the dialog
+        const beginDialogIndex = currentDialog?.triggers.findIndex(({ type }) => type === SDKKinds.OnBeginDialog);
+        if (typeof beginDialogIndex !== 'undefined') {
+          path = createSelectedPath(beginDialogIndex);
+        }
+        breadcrumb.push({ dialogId, selected: '', focused: '' });
+      }
+
+      const currentUri = convertPathToUrl(projectId, dialogId, path);
+
       if (checkUrl(currentUri, designPageLocation)) return;
       //if dialog change we should flush some debounced functions
 
@@ -65,9 +92,7 @@ export const navigationDispatcher = () => {
     if (!dialogId) dialogId = 'Main';
     if (!projectId) projectId = currentProjectId;
 
-    let currentUri = `/bot/${projectId}/dialogs/${dialogId}`;
-
-    currentUri = `${currentUri}?selected=${selectPath}`;
+    const currentUri = convertPathToUrl(projectId, dialogId, selectPath);
 
     if (checkUrl(currentUri, designPageLocation)) return;
     navigateTo(currentUri, { state: { breadcrumb: updateBreadcrumb(breadcrumb, BreadcrumbUpdateType.Selected) } });

Composer/packages/client/src/utils/navigation.ts

@@ -82,7 +82,7 @@ interface NavigationState {
 }
 
 export function convertPathToUrl(projectId: string, dialogId: string, path?: string): string {
-  //path is like main.trigers[0].actions[0]
+  //path is like main.triggers[0].actions[0]
   //uri = id?selected=triggers[0]&focused=triggers[0].actions[0]
 
   let uri = `/bot/${projectId}/dialogs/${dialogId}`;