From 729c2bfdb503f1979e529c5e7dd2e93ab44a5af7 Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 01:37:20 +0800 Subject: [PATCH 01/45] deeplink for lg --- .../pages/language-generation/code-editor.tsx | 7 +++-- .../src/pages/language-generation/index.tsx | 26 ++++++++++++++++--- .../pages/notifications/NotificationList.tsx | 8 +++--- .../client/src/pages/notifications/index.tsx | 14 +++++++++- .../client/src/pages/notifications/types.ts | 2 ++ .../pages/notifications/useNotifications.tsx | 8 +++--- 6 files changed, 53 insertions(+), 12 deletions(-) diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index 247e26a2a7..8012509617 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -8,6 +8,7 @@ import get from 'lodash/get'; import debounce from 'lodash/debounce'; import isEmpty from 'lodash/isEmpty'; import { CodeRange, LgFile } from '@bfc/shared'; +import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api'; import * as lgUtil from '../../utils/lgUtil'; @@ -15,10 +16,11 @@ interface CodeEditorProps { file: LgFile; onChange: (value: string) => void; codeRange?: Partial | null; + editorDidMount?: (editor: editor.IStandaloneCodeEditor) => void; } export default function CodeEditor(props: CodeEditorProps) { - const { file, codeRange } = props; + const { file, codeRange, editorDidMount } = props; const onChange = debounce(props.onChange, 500); const [diagnostics, setDiagnostics] = useState(get(file, 'diagnostics', [])); const [content, setContent] = useState(get(file, 'content', '')); @@ -53,7 +55,8 @@ export default function CodeEditor(props: CodeEditorProps) { lineDecorationsWidth: undefined, lineNumbersMinChars: false, }} - codeRange={codeRange || -1} + codeRange={codeRange} + editorDidMount={editorDidMount} errorMsg={errorMsg} value={content} onChange={_onChange} diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index 65fc47e74f..627470b256 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -10,6 +10,7 @@ import { Nav, INavLinkGroup, INavLink } from 'office-ui-fabric-react/lib/Nav'; import get from 'lodash/get'; import { RouteComponentProps } from '@reach/router'; import { CodeRange } from '@bfc/shared'; +import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api'; import { LoadingSpinner } from '../../components/LoadingSpinner'; import { OpenAlertModal, DialogStyle } from '../../components/Modal'; @@ -38,11 +39,23 @@ const LGPage: React.FC = props => { const [editMode, setEditMode] = useState(false); const [fileValid, setFileValid] = useState(true); const [codeRange, setCodeRange] = useState(null); + const [lgEditor, setLgEditor] = useState(null); + // const editorRef: MutableRefObject = useRef(null); + const hash = props.location ? props.location.hash : ''; const subPath = props['*']; const isRoot = subPath === ''; const activeDialog = dialogs.find(item => item.id === subPath); + useEffect(() => { + if (hash && lgEditor) { + const matched = hash.match(/line=(\d+)/g); + if (matched && matched.length > 0) { + lgEditor.revealLine(parseInt(matched[0].split('=')[1])); + } + } + }, [hash, lgEditor]); + // for now, one bot only have one lg file by default. all dialog share one lg // file. const lgFile = lgFiles.length ? lgFiles[0] : null; @@ -96,7 +109,7 @@ const LGPage: React.FC = props => { // fall back to the all-up page if we don't have an active dialog if (!isRoot && !activeDialog && dialogs.length) { - navigateTo('/language-generation'); + navigateTo('/language-generation/'); } }, [subPath, dialogs]); @@ -113,7 +126,7 @@ const LGPage: React.FC = props => { function onSelect(id) { if (id === '_all') { - navigateTo('/language-generation'); + navigateTo('/language-generation/'); } else { navigateTo(`language-generation/${id}`); } @@ -215,7 +228,14 @@ const LGPage: React.FC = props => {
{editMode ? ( }> - + { + setLgEditor(editor); + }} + /> ) : ( diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 15b573daf0..865ef520a1 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -11,6 +11,7 @@ import { notification, typeIcon, listRoot, icons } from './styles'; export interface INotificationListProps { items: INotification[]; + onItemInvoked: (item: INotification) => void; } const columns: IColumn[] = [ @@ -23,7 +24,7 @@ const columns: IColumn[] = [ minWidth: 30, maxWidth: 30, onRender: (item: INotification) => { - return ; + return ; }, }, { @@ -36,7 +37,7 @@ const columns: IColumn[] = [ isResizable: true, data: 'string', onRender: (item: INotification) => { - return {item.type}; + return {item.severity}; }, isPadded: true, }, @@ -71,7 +72,7 @@ const columns: IColumn[] = [ ]; export const NotificationList: React.FC = props => { - const { items } = props; + const { items, onItemInvoked } = props; return (
@@ -82,6 +83,7 @@ export const NotificationList: React.FC = props => { setKey="none" layoutMode={DetailsListLayoutMode.justified} isHeaderVisible={true} + onItemInvoked={onItemInvoked} />
); diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 84cbbeb375..753144f58d 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -11,15 +11,27 @@ import useNotifications from './useNotifications'; import { NotificationList } from './NotificationList'; import { NotificationHeader } from './NotificationHeader'; import { root } from './styles'; +import { INotification } from './types'; +import { navigateTo } from './../../utils'; +const navigations = { + lg: (item: INotification) => { + navigateTo(`/language-generation/#line=${item.diagnostic.Range.Start.Line || 0}`); + }, + lu: (item: INotification) => {}, + dialog: (item: INotification) => {}, +}; const Notifications: React.FC = () => { const [filter, setFilter] = useState(''); const { notifications, locations } = useNotifications(filter); + const handleItemInvoked = (item: INotification) => { + navigations[item.type](item); + }; return (
- +
); }; diff --git a/Composer/packages/client/src/pages/notifications/types.ts b/Composer/packages/client/src/pages/notifications/types.ts index d9a1528ce6..800b478801 100644 --- a/Composer/packages/client/src/pages/notifications/types.ts +++ b/Composer/packages/client/src/pages/notifications/types.ts @@ -2,7 +2,9 @@ // Licensed under the MIT License. export interface INotification { + severity: string; type: string; location: string; message: string; + diagnostic: any; } diff --git a/Composer/packages/client/src/pages/notifications/useNotifications.tsx b/Composer/packages/client/src/pages/notifications/useNotifications.tsx index b35d98d54a..04dcfc4533 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -21,14 +21,14 @@ export default function useNotifications(filter: string) { dialog.diagnostics.map(diagnostic => { const location = dialog.displayName; locations.add(location); - notifactions.push({ type: 'Error', location, message: diagnostic }); + notifactions.push({ type: 'dialog', location, message: diagnostic, severity: 'Error', diagnostic }); }); }); luFiles.forEach(lufile => { lufile.diagnostics.map(diagnostic => { const location = `${lufile.id}.lu`; locations.add(location); - notifactions.push({ type: 'Error', location, message: diagnostic.text }); + notifactions.push({ type: 'lu', location, message: diagnostic.text, severity: 'Error', diagnostic }); }); }); lgFiles.forEach(lgFiles => { @@ -36,9 +36,11 @@ export default function useNotifications(filter: string) { const location = `${lgFiles.id}.lg`; locations.add(location); notifactions.push({ - type: DiagnosticSeverity[diagnostic.Severity], + type: 'lg', + severity: DiagnosticSeverity[diagnostic.Severity], location, message: createSingleMessage(diagnostic), + diagnostic, }); }); }); From b956bd0e563b0cc87e01e1d455b5a5627fc8367c Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 01:56:45 +0800 Subject: [PATCH 02/45] make lu link to dialog --- .../client/src/pages/notifications/index.tsx | 8 ++++-- .../client/src/pages/notifications/types.ts | 1 + .../pages/notifications/useNotifications.tsx | 25 +++++++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 753144f58d..3a9bcf4dac 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -18,8 +18,12 @@ const navigations = { lg: (item: INotification) => { navigateTo(`/language-generation/#line=${item.diagnostic.Range.Start.Line || 0}`); }, - lu: (item: INotification) => {}, - dialog: (item: INotification) => {}, + lu: (item: INotification) => { + navigateTo(`/dialogs/${item.id}`); + }, + dialog: (item: INotification) => { + navigateTo(`/dialogs/${item.id}`); + }, }; const Notifications: React.FC = () => { const [filter, setFilter] = useState(''); diff --git a/Composer/packages/client/src/pages/notifications/types.ts b/Composer/packages/client/src/pages/notifications/types.ts index 800b478801..4689193a4d 100644 --- a/Composer/packages/client/src/pages/notifications/types.ts +++ b/Composer/packages/client/src/pages/notifications/types.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. export interface INotification { + id: string; severity: string; type: string; location: string; diff --git a/Composer/packages/client/src/pages/notifications/useNotifications.tsx b/Composer/packages/client/src/pages/notifications/useNotifications.tsx index 04dcfc4533..39e1431a23 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -21,19 +21,33 @@ export default function useNotifications(filter: string) { dialog.diagnostics.map(diagnostic => { const location = dialog.displayName; locations.add(location); - notifactions.push({ type: 'dialog', location, message: diagnostic, severity: 'Error', diagnostic }); + notifactions.push({ + type: 'dialog', + location, + message: diagnostic, + severity: 'Error', + diagnostic, + id: dialog.id, + }); }); }); luFiles.forEach(lufile => { lufile.diagnostics.map(diagnostic => { const location = `${lufile.id}.lu`; locations.add(location); - notifactions.push({ type: 'lu', location, message: diagnostic.text, severity: 'Error', diagnostic }); + notifactions.push({ + type: 'lu', + location, + message: diagnostic.text, + severity: 'Error', + diagnostic, + id: lufile.id, + }); }); }); - lgFiles.forEach(lgFiles => { - lgFiles.diagnostics.map(diagnostic => { - const location = `${lgFiles.id}.lg`; + lgFiles.forEach(lgFile => { + lgFile.diagnostics.map(diagnostic => { + const location = `${lgFile.id}.lg`; locations.add(location); notifactions.push({ type: 'lg', @@ -41,6 +55,7 @@ export default function useNotifications(filter: string) { location, message: createSingleMessage(diagnostic), diagnostic, + id: lgFile.id, }); }); }); From 49e49405e622a62e332b18a2b5ff5ca3b3be3f11 Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 14:46:10 +0800 Subject: [PATCH 03/45] make lu inline editor show error --- .../src/Form/fields/RecognizerField/index.tsx | 22 +++++++++++++------ .../server/src/models/bot/botProject.ts | 11 ---------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx index 36642be565..4ebba0be15 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import React, { useState, ReactElement, Suspense } from 'react'; +import React, { useState, ReactElement, Suspense, useEffect } from 'react'; import formatMessage from 'format-message'; import { FieldProps } from '@bfcomposer/react-jsonschema-form'; import { Dropdown, ResponsiveMode, IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; @@ -35,6 +35,17 @@ export const RecognizerField: React.FC> = props selectedFile && typeof props.formData === 'string' && props.formData.startsWith(selectedFile.id) ); + useEffect(() => { + if (selectedFile && selectedFile.diagnostics.length > 0) { + const msg = selectedFile.diagnostics.reduce((msg: string, diagnostic) => { + return (msg += `${diagnostic.text}\n`); + }, ''); + setErrorMsg(msg); + } else { + setErrorMsg(''); + } + }, [selectedFile]); + const handleChange = (_, option?: IDropdownOption): void => { if (option) { switch (option.key) { @@ -142,12 +153,9 @@ export const RecognizerField: React.FC> = props {() => { if (selectedFile && isLuFileSelected) { const updateLuFile = (newValue?: string): void => { - shellApi - .updateLuFile({ id: selectedFile.id, content: newValue }) - .then(() => setErrorMsg('')) - .catch(error => { - setErrorMsg(error); - }); + shellApi.updateLuFile({ id: selectedFile.id, content: newValue }).catch(error => { + setErrorMsg(error); + }); }; return ( diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index 7e682cc044..69f801f99b 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -3,7 +3,6 @@ import fs from 'fs'; -import isEqual from 'lodash/isEqual'; import { FileInfo, DialogInfo, LgFile, LuFile, getNewDesigner } from '@bfc/shared'; import { dialogIndexer, luIndexer, lgIndexer } from '@bfc/indexers'; @@ -267,16 +266,6 @@ export class BotProject { if (luFile === undefined) { throw new Error(`no such lu file ${id}`); } - let currentLufileParsedContentLUISJsonStructure = null; - try { - currentLufileParsedContentLUISJsonStructure = await luIndexer.parse(content); - } catch (error) { - throw new Error(`Update ${id}.lu Failed, ${error.text}`); - } - - const preLufileParsedContentLUISJsonStructure = luFile.parsedContent.LUISJsonStructure; - const isUpdate = !isEqual(currentLufileParsedContentLUISJsonStructure, preLufileParsedContentLUISJsonStructure); - if (!isUpdate) return this.luFiles; await this._updateFile(luFile.relativePath, content); await this.luPublisher.onFileChange(luFile.relativePath, FileUpdateType.UPDATE); From fd4fdb2f4442c86635aed3cca6c2405b239db1a1 Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 15:07:59 +0800 Subject: [PATCH 04/45] fix the lgeditor coderange error --- .../client/src/pages/language-generation/code-editor.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index 8012509617..a05360dca8 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -15,7 +15,7 @@ import * as lgUtil from '../../utils/lgUtil'; interface CodeEditorProps { file: LgFile; onChange: (value: string) => void; - codeRange?: Partial | null; + codeRange?: Partial | -1; editorDidMount?: (editor: editor.IStandaloneCodeEditor) => void; } From c58f800f22c279e185f0fd70e7030b49c55d96b0 Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 15:14:01 +0800 Subject: [PATCH 05/45] fix code range type --- .../client/src/pages/language-generation/code-editor.tsx | 2 +- .../packages/client/src/pages/language-generation/index.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index a05360dca8..6fbc6276cd 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -15,7 +15,7 @@ import * as lgUtil from '../../utils/lgUtil'; interface CodeEditorProps { file: LgFile; onChange: (value: string) => void; - codeRange?: Partial | -1; + codeRange?: Partial | undefined; editorDidMount?: (editor: editor.IStandaloneCodeEditor) => void; } diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index 627470b256..21efd5297d 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -38,7 +38,7 @@ const LGPage: React.FC = props => { const { lgFiles, dialogs } = state; const [editMode, setEditMode] = useState(false); const [fileValid, setFileValid] = useState(true); - const [codeRange, setCodeRange] = useState(null); + const [codeRange, setCodeRange] = useState(undefined); const [lgEditor, setLgEditor] = useState(null); // const editorRef: MutableRefObject = useRef(null); @@ -134,7 +134,7 @@ const LGPage: React.FC = props => { function onToggleEditMode() { setEditMode(!editMode); - setCodeRange(null); + setCodeRange(undefined); } async function onChange(newContent) { From 971376cada1c0b7d4d75b5bcd38c83f6efe611ed Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 15:45:36 +0800 Subject: [PATCH 06/45] fix test --- Composer/cypress/support/commands.ts | 1 + .../client/__tests__/components/notificationList.test.js | 6 +++--- Composer/packages/client/src/ShellApi.ts | 8 +++++++- .../client/src/pages/language-generation/index.tsx | 1 + .../src/Form/fields/RecognizerField/index.tsx | 1 + .../server/__tests__/models/bot/botProject.test.ts | 9 ++++++--- 6 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Composer/cypress/support/commands.ts b/Composer/cypress/support/commands.ts index de5939e41f..31a2204329 100644 --- a/Composer/cypress/support/commands.ts +++ b/Composer/cypress/support/commands.ts @@ -23,4 +23,5 @@ Cypress.Commands.add('withinEditor', (editorName, cb) => { Cypress.Commands.add('visitPage', page => { cy.findByTestId(`LeftNav-CommandBarButton${page}`).click(); + cy.wait(500); }); diff --git a/Composer/packages/client/__tests__/components/notificationList.test.js b/Composer/packages/client/__tests__/components/notificationList.test.js index 32fddf9733..ef312a2f8c 100644 --- a/Composer/packages/client/__tests__/components/notificationList.test.js +++ b/Composer/packages/client/__tests__/components/notificationList.test.js @@ -8,9 +8,9 @@ import { NotificationList } from '../../src/pages/notifications/NotificationList describe('', () => { const items = [ - { type: 'Error', location: 'test1', message: 'error1' }, - { type: 'Warning', location: 'test2', message: 'error2' }, - { type: 'Error', location: 'test3', message: 'error3' }, + { id: '1', severity: 'Error', type: 'dialog', location: 'test1', message: 'error1', diagnostic: '' }, + { id: '2', severity: 'Warning', type: 'lu', location: 'test2', message: 'error2', diagnostic: '' }, + { id: '3', severity: 'Error', type: 'lg', location: 'test3', message: 'error3', diagnostic: '' }, ]; it('should render the NotificationList', () => { const { container } = render(); diff --git a/Composer/packages/client/src/ShellApi.ts b/Composer/packages/client/src/ShellApi.ts index 391946b4ad..437d5a0956 100644 --- a/Composer/packages/client/src/ShellApi.ts +++ b/Composer/packages/client/src/ShellApi.ts @@ -5,6 +5,7 @@ import React, { useEffect, useContext, useMemo } from 'react'; import { ShellData } from '@bfc/shared'; import isEqual from 'lodash/isEqual'; import get from 'lodash/get'; +import { LGTemplate } from 'botbuilder-lg'; import { isExpression } from './utils'; import * as lgUtil from './utils/lgUtil'; @@ -132,8 +133,13 @@ export const ShellApi: React.FC = () => { if (id === undefined) throw new Error('must have a file id'); const file = lgFiles.find(file => file.id === id); if (!file) throw new Error(`lg file ${id} not found`); + let templates: LGTemplate[] = []; + try { + templates = lgUtil.parse(file.content); + } catch (error) { + console.error(error); + } - const templates = lgUtil.parse(file.content); const lines = file.content.split('\n'); return templates.map(t => { diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index 21efd5297d..4ec7288986 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -186,6 +186,7 @@ const LGPage: React.FC = props => { css={actionButton} onText={formatMessage('Edit mode')} offText={formatMessage('Edit mode')} + defaultChecked={false} checked={editMode} disabled={(!isRoot && editMode === false) || (codeRange === null && fileValid === false)} onChange={onToggleEditMode} diff --git a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx index 4ebba0be15..8f93863bc5 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx @@ -35,6 +35,7 @@ export const RecognizerField: React.FC> = props selectedFile && typeof props.formData === 'string' && props.formData.startsWith(selectedFile.id) ); + //make the inline editor show error message useEffect(() => { if (selectedFile && selectedFile.diagnostics.length > 0) { const msg = selectedFile.diagnostics.reduce((msg: string, diagnostic) => { diff --git a/Composer/packages/server/__tests__/models/bot/botProject.test.ts b/Composer/packages/server/__tests__/models/bot/botProject.test.ts index 22e3c99d4b..884f6066a6 100644 --- a/Composer/packages/server/__tests__/models/bot/botProject.test.ts +++ b/Composer/packages/server/__tests__/models/bot/botProject.test.ts @@ -228,11 +228,14 @@ describe('lu operation', () => { } }); - it('should throw error when lu content is invalid', async () => { + it('should update diagnostics when lu content is invalid', async () => { const id = 'root'; const content = 'hello \n hello3'; - - await expect(proj.updateLuFile(id, content)).rejects.toThrow(); + const luFiles = await proj.updateLuFile(id, content); + const result = luFiles.find(f => f.id === id); + if (result !== undefined) { + await expect(result.diagnostics.length).toBeGreaterThan(0); + } }); it('should delete lu file and update index', async () => { From f83a04305dd0c8a5c4d7079c30843ada4c926beb Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 27 Nov 2019 23:29:54 +0800 Subject: [PATCH 07/45] remove wait --- Composer/cypress/support/commands.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/Composer/cypress/support/commands.ts b/Composer/cypress/support/commands.ts index 31a2204329..de5939e41f 100644 --- a/Composer/cypress/support/commands.ts +++ b/Composer/cypress/support/commands.ts @@ -23,5 +23,4 @@ Cypress.Commands.add('withinEditor', (editorName, cb) => { Cypress.Commands.add('visitPage', page => { cy.findByTestId(`LeftNav-CommandBarButton${page}`).click(); - cy.wait(500); }); From 926ec368249e47845336308ea8e81a5218bf454a Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 28 Nov 2019 14:57:34 +0800 Subject: [PATCH 08/45] edit e2e azure pipelines --- azure-pipelines.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f0b2fa4364..94d3027357 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -5,7 +5,7 @@ pr: autoCancel: true branches: include: - - '*' + - "*" variables: YARN_CACHE_FOLDER: $(Pipeline.Workspace)/.yarn @@ -52,7 +52,7 @@ jobs: CYPRESS_SCREENSHOTS_FOLDER: $(Build.ArtifactStagingDirectory)/cypress/screenshots CYPRESS_VIDEOS_FOLDER: $(Build.ArtifactStagingDirectory)/cypress/videos TERM: xterm - COMPOSER_BOTS_FOLDER: $(Pipeline.Workspace)/Composer/TestBots + COMPOSER_BOTS_FOLDER: $(Pipeline.Workspace)/s/Composer/TestBots - task: PublishPipelineArtifact@1 condition: failed() continueOnError: true From 7252278200571e6e0eae1965ec750fd810d439d8 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 28 Nov 2019 15:04:31 +0800 Subject: [PATCH 09/45] update e2e dir --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 94d3027357..ee204162cb 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -52,7 +52,7 @@ jobs: CYPRESS_SCREENSHOTS_FOLDER: $(Build.ArtifactStagingDirectory)/cypress/screenshots CYPRESS_VIDEOS_FOLDER: $(Build.ArtifactStagingDirectory)/cypress/videos TERM: xterm - COMPOSER_BOTS_FOLDER: $(Pipeline.Workspace)/s/Composer/TestBots + COMPOSER_BOTS_FOLDER: $(System.DefaultWorkingDirectory)/Composer/TestBots - task: PublishPipelineArtifact@1 condition: failed() continueOnError: true From cedfecef2833f70300157d320413c7bd104e26fe Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 28 Nov 2019 21:22:58 +0800 Subject: [PATCH 10/45] fix some comments --- .../components/notificationList.test.js | 28 +++++++++++++++++-- .../pages/language-generation/code-editor.tsx | 8 +++--- .../src/pages/language-generation/index.tsx | 20 ++++--------- .../pages/notifications/NotificationList.tsx | 3 +- .../src/Form/fields/RecognizerField/index.tsx | 4 +-- 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/Composer/packages/client/__tests__/components/notificationList.test.js b/Composer/packages/client/__tests__/components/notificationList.test.js index ef312a2f8c..95cf15bae2 100644 --- a/Composer/packages/client/__tests__/components/notificationList.test.js +++ b/Composer/packages/client/__tests__/components/notificationList.test.js @@ -3,14 +3,36 @@ import * as React from 'react'; import { render } from 'react-testing-library'; +import { formatMessage } from 'format-message'; import { NotificationList } from '../../src/pages/notifications/NotificationList'; describe('', () => { const items = [ - { id: '1', severity: 'Error', type: 'dialog', location: 'test1', message: 'error1', diagnostic: '' }, - { id: '2', severity: 'Warning', type: 'lu', location: 'test2', message: 'error2', diagnostic: '' }, - { id: '3', severity: 'Error', type: 'lg', location: 'test3', message: 'error3', diagnostic: '' }, + { + id: 'Main.dialog', + severity: formatMessage('Error'), + type: 'dialog', + location: formatMessage('test1'), + message: formatMessage('error1'), + diagnostic: '', + }, + { + id: 'Main.lu', + severity: formatMessage('Warning'), + type: 'lu', + location: formatMessage('test2'), + message: formatMessage('error2'), + diagnostic: '', + }, + { + id: 'common.lg', + severity: formatMessage('Error'), + type: 'lg', + location: formatMessage('test3'), + message: formatMessage('error3'), + diagnostic: '', + }, ]; it('should render the NotificationList', () => { const { container } = render(); diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index 6fbc6276cd..3a24e0ab59 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -15,12 +15,12 @@ import * as lgUtil from '../../utils/lgUtil'; interface CodeEditorProps { file: LgFile; onChange: (value: string) => void; - codeRange?: Partial | undefined; - editorDidMount?: (editor: editor.IStandaloneCodeEditor) => void; + codeRange?: Partial; + onMount?: (editor: editor.IStandaloneCodeEditor) => void; } export default function CodeEditor(props: CodeEditorProps) { - const { file, codeRange, editorDidMount } = props; + const { file, codeRange, onMount } = props; const onChange = debounce(props.onChange, 500); const [diagnostics, setDiagnostics] = useState(get(file, 'diagnostics', [])); const [content, setContent] = useState(get(file, 'content', '')); @@ -56,7 +56,7 @@ export default function CodeEditor(props: CodeEditorProps) { lineNumbersMinChars: false, }} codeRange={codeRange} - editorDidMount={editorDidMount} + editorDidMount={onMount} errorMsg={errorMsg} value={content} onChange={_onChange} diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index 4ec7288986..56ea9c348a 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -38,9 +38,8 @@ const LGPage: React.FC = props => { const { lgFiles, dialogs } = state; const [editMode, setEditMode] = useState(false); const [fileValid, setFileValid] = useState(true); - const [codeRange, setCodeRange] = useState(undefined); + const [codeRange, setCodeRange] = useState(); const [lgEditor, setLgEditor] = useState(null); - // const editorRef: MutableRefObject = useRef(null); const hash = props.location ? props.location.hash : ''; const subPath = props['*']; @@ -49,9 +48,9 @@ const LGPage: React.FC = props => { useEffect(() => { if (hash && lgEditor) { - const matched = hash.match(/line=(\d+)/g); - if (matched && matched.length > 0) { - lgEditor.revealLine(parseInt(matched[0].split('=')[1])); + const match = /line=(\d+)/g.exec(hash); + if (match) { + lgEditor.revealLine(+match[1]); } } }, [hash, lgEditor]); @@ -188,7 +187,7 @@ const LGPage: React.FC = props => { offText={formatMessage('Edit mode')} defaultChecked={false} checked={editMode} - disabled={(!isRoot && editMode === false) || (codeRange === null && fileValid === false)} + disabled={(!isRoot && editMode === false) || (!codeRange && fileValid === false)} onChange={onToggleEditMode} />
@@ -229,14 +228,7 @@ const LGPage: React.FC = props => {
{editMode ? ( }> - { - setLgEditor(editor); - }} - /> + ) : ( diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 865ef520a1..2bff13e781 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -24,7 +24,8 @@ const columns: IColumn[] = [ minWidth: 30, maxWidth: 30, onRender: (item: INotification) => { - return ; + const icon = icons[item.severity]; + return ; }, }, { diff --git a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx index 8f93863bc5..4cd2564923 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx @@ -154,9 +154,7 @@ export const RecognizerField: React.FC> = props {() => { if (selectedFile && isLuFileSelected) { const updateLuFile = (newValue?: string): void => { - shellApi.updateLuFile({ id: selectedFile.id, content: newValue }).catch(error => { - setErrorMsg(error); - }); + shellApi.updateLuFile({ id: selectedFile.id, content: newValue }).catch(setErrorMsg); }; return ( From 27762f0f3d18424ceb0bd32b0764230acd354bef Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 29 Nov 2019 12:45:45 +0800 Subject: [PATCH 11/45] fix unit test --- .../client/__tests__/components/notificationList.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/__tests__/components/notificationList.test.js b/Composer/packages/client/__tests__/components/notificationList.test.js index 95cf15bae2..347e8e7453 100644 --- a/Composer/packages/client/__tests__/components/notificationList.test.js +++ b/Composer/packages/client/__tests__/components/notificationList.test.js @@ -3,7 +3,7 @@ import * as React from 'react'; import { render } from 'react-testing-library'; -import { formatMessage } from 'format-message'; +import formatMessage from 'format-message'; import { NotificationList } from '../../src/pages/notifications/NotificationList'; From 216f243d6806364d12c190d9d7c658635f4d3c92 Mon Sep 17 00:00:00 2001 From: leilzh Date: Mon, 2 Dec 2019 19:41:49 +0800 Subject: [PATCH 12/45] add dialog check --- Composer/packages/client/src/ShellApi.ts | 9 +- .../pages/language-generation/code-editor.tsx | 12 ++- .../src/pages/language-generation/index.tsx | 3 +- .../pages/language-generation/table-view.tsx | 9 +- .../pages/notifications/useNotifications.tsx | 10 +- .../client/src/store/reducer/index.ts | 2 +- Composer/packages/client/src/utils/lgUtil.ts | 40 +------- .../packages/lib/indexers/src/diagnostic.ts | 71 +++++++++++++++ .../lib/indexers/src/dialogIndexer.ts | 22 ++++- Composer/packages/lib/indexers/src/index.ts | 3 +- .../packages/lib/indexers/src/lgIndexer.ts | 91 +++++++++++-------- Composer/packages/lib/indexers/src/type.ts | 33 +------ Composer/packages/lib/package.json | 2 +- Composer/packages/lib/shared/package.json | 1 + .../packages/lib/shared/src/types/shell.ts | 81 +---------------- .../server/src/models/bot/botProject.ts | 3 +- .../language-generation/package.json | 2 +- 17 files changed, 185 insertions(+), 209 deletions(-) create mode 100644 Composer/packages/lib/indexers/src/diagnostic.ts diff --git a/Composer/packages/client/src/ShellApi.ts b/Composer/packages/client/src/ShellApi.ts index 437d5a0956..b30ae8a640 100644 --- a/Composer/packages/client/src/ShellApi.ts +++ b/Composer/packages/client/src/ShellApi.ts @@ -6,6 +6,7 @@ import { ShellData } from '@bfc/shared'; import isEqual from 'lodash/isEqual'; import get from 'lodash/get'; import { LGTemplate } from 'botbuilder-lg'; +import { lgIndexer } from '@bfc/indexers'; import { isExpression } from './utils'; import * as lgUtil from './utils/lgUtil'; @@ -134,11 +135,7 @@ export const ShellApi: React.FC = () => { const file = lgFiles.find(file => file.id === id); if (!file) throw new Error(`lg file ${id} not found`); let templates: LGTemplate[] = []; - try { - templates = lgUtil.parse(file.content); - } catch (error) { - console.error(error); - } + templates = lgIndexer.parse(file.content); const lines = file.content.split('\n'); @@ -184,7 +181,7 @@ export const ShellApi: React.FC = () => { }); const content = lgUtil.updateTemplate(file.content, templateName, template); - return lgUtil.checkLgContent(content); + return lgUtil.checkLgContent(content, id); } function copyLgTemplateHandler({ id, fromTemplateName, toTemplateName }, event) { diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index 317220bd61..072b806aeb 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -7,13 +7,15 @@ import { LgEditor, LGOption } from '@bfc/code-editor'; import get from 'lodash/get'; import debounce from 'lodash/debounce'; import isEmpty from 'lodash/isEmpty'; -import { Diagnostic } from 'botbuilder-lg'; import { LgFile } from '@bfc/shared'; import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api'; +import { lgIndexer, Diagnostic } from '@bfc/indexers'; import { StoreContext } from '../../store'; import * as lgUtil from '../../utils/lgUtil'; +const { check, isValid, combineMessage } = lgIndexer; + interface CodeEditorProps { file: LgFile; template: lgUtil.Template | null; @@ -42,8 +44,8 @@ export default function CodeEditor(props: CodeEditorProps) { }, [fileId, template]); useEffect(() => { - const isInvalid = !lgUtil.isValid(diagnostics); - const text = isInvalid ? lgUtil.combineMessage(diagnostics) : ''; + const isInvalid = !isValid(diagnostics); + const text = isInvalid ? combineMessage(diagnostics) : ''; setErrorMsg(text); }, [diagnostics]); @@ -97,13 +99,13 @@ export default function CodeEditor(props: CodeEditorProps) { Parameters: get(template, 'Parameters'), Body: value, }); - diagnostics = lgUtil.check(newContent); + diagnostics = check(newContent, fileId); updateLgTemplate(value); } catch (error) { setErrorMsg(error.message); } } else { - diagnostics = lgUtil.check(value); + diagnostics = check(value, fileId); updateLgFile(value); } setDiagnostics(diagnostics); diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index 4e7fbcad13..ed9fe65d5c 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -10,6 +10,7 @@ import { Nav, INavLinkGroup, INavLink } from 'office-ui-fabric-react/lib/Nav'; import { LGTemplate } from 'botbuilder-lg'; import { RouteComponentProps } from '@reach/router'; import get from 'lodash/get'; +import { lgIndexer } from '@bfc/indexers'; import { LoadingSpinner } from '../../components/LoadingSpinner'; import { StoreContext } from '../../store'; @@ -110,7 +111,7 @@ const LGPage: React.FC = props => { useEffect(() => { const errorFiles = lgFiles.filter(file => { - return lgUtil.isValid(file.diagnostics) === false; + return lgIndexer.isValid(file.diagnostics) === false; }); const hasError = errorFiles.length !== 0; setFileValid(hasError === false); diff --git a/Composer/packages/client/src/pages/language-generation/table-view.tsx b/Composer/packages/client/src/pages/language-generation/table-view.tsx index b6ec6e59a4..9778dcf68f 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -18,6 +18,7 @@ import { NeutralColors, FontSizes } from '@uifabric/fluent-theme'; import { DialogInfo, LgFile } from '@bfc/shared'; import { LGTemplate } from 'botbuilder-lg'; +import { lgIndexer } from '@bfc/indexers'; import { StoreContext } from '../../store'; import * as lgUtil from '../../utils/lgUtil'; import { navigateTo } from '../../utils'; @@ -42,13 +43,7 @@ const TableView: React.FC = props => { useEffect(() => { if (isEmpty(lgFile)) return; let allTemplates: LGTemplate[] = []; - try { - allTemplates = lgUtil.parse(lgFile.content); - // mute lg file invalid cause page crash, setState is async, this component may render at first - } catch (error) { - console.error(error); - } - + allTemplates = lgIndexer.parse(lgFile.content); if (!activeDialog) { setTemplates(allTemplates); } else { diff --git a/Composer/packages/client/src/pages/notifications/useNotifications.tsx b/Composer/packages/client/src/pages/notifications/useNotifications.tsx index 39e1431a23..994026ee21 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -2,9 +2,9 @@ // Licensed under the MIT License. import { useContext, useMemo } from 'react'; +import { lgIndexer } from '@bfc/indexers'; import { StoreContext } from '../../store'; -import { createSingleMessage } from '../../utils/lgUtil'; import { INotification } from './types'; @@ -24,8 +24,8 @@ export default function useNotifications(filter: string) { notifactions.push({ type: 'dialog', location, - message: diagnostic, - severity: 'Error', + message: diagnostic.message, + severity: DiagnosticSeverity[diagnostic.severity], diagnostic, id: dialog.id, }); @@ -51,9 +51,9 @@ export default function useNotifications(filter: string) { locations.add(location); notifactions.push({ type: 'lg', - severity: DiagnosticSeverity[diagnostic.Severity], + severity: DiagnosticSeverity[diagnostic.severity], location, - message: createSingleMessage(diagnostic), + message: lgIndexer.createSingleMessage(diagnostic), diagnostic, id: lgFile.id, }); diff --git a/Composer/packages/client/src/store/reducer/index.ts b/Composer/packages/client/src/store/reducer/index.ts index 40f7fd0890..1d5441163c 100644 --- a/Composer/packages/client/src/store/reducer/index.ts +++ b/Composer/packages/client/src/store/reducer/index.ts @@ -71,7 +71,7 @@ const removeRecentProject: ReducerFunc = (state, { path }) => { const updateDialog: ReducerFunc = (state, { id, content }) => { state.dialogs = state.dialogs.map(dialog => { if (dialog.id === id) { - const result = dialogIndexer.parse(content); + const result = dialogIndexer.parse(dialog.id, content); return { ...dialog, ...result }; } return dialog; diff --git a/Composer/packages/client/src/utils/lgUtil.ts b/Composer/packages/client/src/utils/lgUtil.ts index 6f3e47618d..64afa0e36d 100644 --- a/Composer/packages/client/src/utils/lgUtil.ts +++ b/Composer/packages/client/src/utils/lgUtil.ts @@ -7,49 +7,19 @@ * */ -import { LGParser, StaticChecker, DiagnosticSeverity, ImportResolver, Diagnostic, LGTemplate } from 'botbuilder-lg'; -import get from 'lodash/get'; - -const lgStaticChecker = new StaticChecker(); - -const lgImportResolver = ImportResolver.fileResolver; +import { LGParser, LGTemplate } from 'botbuilder-lg'; +import { lgIndexer } from '@bfc/indexers'; +const { check, isValid, combineMessage, parse } = lgIndexer; export interface Template { Name: string; Parameters?: string[]; Body: string; } -export function isValid(diagnostics: Diagnostic[]): boolean { - return diagnostics.every(d => d.Severity !== DiagnosticSeverity.Error); -} - -export function check(content: string, id = ''): Diagnostic[] { - return lgStaticChecker.checkText(content, id, lgImportResolver); -} - -export function parse(content: string, id = ''): LGTemplate[] { - const resource = LGParser.parse(content, id); - return get(resource, 'Templates', []); -} - -export function createSingleMessage(diagnostic: Diagnostic): string { - const { Start, End } = diagnostic.Range; - const position = `line ${Start.Line}:${Start.Character} - line ${End.Line}:${End.Character}`; - - return `${position} \n ${diagnostic.Message}\n`; -} - -export function combineMessage(diagnostics: Diagnostic[]): string { - return diagnostics.reduce((msg, d) => { - msg += createSingleMessage(d); - return msg; - }, ''); -} - -export function checkLgContent(content: string) { +export function checkLgContent(content: string, id: string) { // check lg content, make up error message - const diagnostics = check(content); + const diagnostics = check(content, id); if (isValid(diagnostics) === false) { const errorMsg = combineMessage(diagnostics); throw new Error(errorMsg); diff --git a/Composer/packages/lib/indexers/src/diagnostic.ts b/Composer/packages/lib/indexers/src/diagnostic.ts new file mode 100644 index 0000000000..da9946f9d6 --- /dev/null +++ b/Composer/packages/lib/indexers/src/diagnostic.ts @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +export class Range { + start: Position; + end: Position; + + constructor(start: Position, end: Position) { + this.start = start; + this.end = end; + } +} + +export class Position { + line: number; + character: number; + + constructor(line: number, character: number) { + this.line = line; + this.character = character; + } +} + +export enum DiagnosticSeverity { + Error = 0, + Warning = 1, + Information = 2, + Hint = 3, +} + +export class Diagnostic { + /** + * Error + * Warning + * Information + * Hint + */ + severity: DiagnosticSeverity; + + /** + * human-readable message + */ + message: string; + + /** + * source is used to indentify the source of this error + * ie, resource id or file name + */ + source: string; + + /** + * path and range are to help locate the error, + * path is used for json or any structured content + * range is used for text-based content + */ + range?: Range; + path?: string; + + /* + * code is a machine readable idenfier to classify error + * and allow further or alternative intepretation of this error + * for example CA2001 + */ + code?: string; + + constructor(message: string, source: string, severity?: DiagnosticSeverity) { + this.message = message; + this.source = source; + this.severity = severity ? severity : DiagnosticSeverity.Error; + } +} diff --git a/Composer/packages/lib/indexers/src/dialogIndexer.ts b/Composer/packages/lib/indexers/src/dialogIndexer.ts index ec235de7ae..b30c8191e0 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -8,6 +8,7 @@ import { ITrigger, DialogInfo, FileInfo } from './type'; import { DialogChecker } from './utils/dialogChecker'; import { JsonWalk, VisitorFunc } from './utils/jsonWalk'; import { getBaseName } from './utils/help'; +import { Diagnostic } from './diagnostic'; // find out all lg templates given dialog function ExtractLgTemplates(dialog): string[] { const templates: string[] = []; @@ -52,6 +53,7 @@ function ExtractLgTemplates(dialog): string[] { JsonWalk('$', dialog, visitor); return uniq(templates); } + // find out all lu intents given dialog function ExtractLuIntents(dialog): string[] { const intents: string[] = []; @@ -71,6 +73,7 @@ function ExtractLuIntents(dialog): string[] { JsonWalk('$', dialog, visitor); return uniq(intents); } + // find out all triggers given dialog function ExtractTriggers(dialog): ITrigger[] { const trigers: ITrigger[] = []; @@ -106,6 +109,7 @@ function ExtractTriggers(dialog): ITrigger[] { JsonWalk('$', dialog, visitor); return trigers; } + // find out all referred dialog function ExtractReferredDialogs(dialog): string[] { const dialogs: string[] = []; @@ -125,6 +129,7 @@ function ExtractReferredDialogs(dialog): string[] { JsonWalk('$', dialog, visitor); return uniq(dialogs); } + // check all fields function CheckFields(dialog): string[] { const errors: string[] = []; @@ -155,12 +160,22 @@ function CheckFields(dialog): string[] { JsonWalk('$', dialog, visitor); return errors; } -function parse(content) { + +function validate(id: string, content): Diagnostic[] { + try { + const errors = CheckFields(content); + return errors.map(x => new Diagnostic(x, id)); + } catch (error) { + return [new Diagnostic(error.message, id)]; + } +} + +function parse(id: string, content) { const luFile = typeof content.recognizer === 'string' ? content.recognizer : ''; const lgFile = typeof content.generator === 'string' ? content.generator : ''; return { content, - diagnostics: CheckFields(content), + diagnostics: validate(id, content), referredDialogs: ExtractReferredDialogs(content), lgTemplates: ExtractLgTemplates(content), luIntents: ExtractLuIntents(content), @@ -169,6 +184,7 @@ function parse(content) { triggers: ExtractTriggers(content), }; } + function index(files: FileInfo[], botName: string): DialogInfo[] { const dialogs: DialogInfo[] = []; if (files.length !== 0) { @@ -184,7 +200,7 @@ function index(files: FileInfo[], botName: string): DialogInfo[] { displayName: isRoot ? `${botName}.Main` : id, content: dialogJson, relativePath: file.relativePath, - ...parse(dialogJson), + ...parse(id, dialogJson), }; dialogs.push(dialog); } diff --git a/Composer/packages/lib/indexers/src/index.ts b/Composer/packages/lib/indexers/src/index.ts index 5a51568712..07cad3f7c3 100644 --- a/Composer/packages/lib/indexers/src/index.ts +++ b/Composer/packages/lib/indexers/src/index.ts @@ -3,4 +3,5 @@ export * from './dialogIndexer'; export * from './lgIndexer'; -export * from './luIndexer'; +export * from './type'; +export * from './diagnostic'; diff --git a/Composer/packages/lib/indexers/src/lgIndexer.ts b/Composer/packages/lib/indexers/src/lgIndexer.ts index dea21f2d54..35dba74719 100644 --- a/Composer/packages/lib/indexers/src/lgIndexer.ts +++ b/Composer/packages/lib/indexers/src/lgIndexer.ts @@ -1,68 +1,86 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { LGParser, StaticChecker, DiagnosticSeverity, Diagnostic } from 'botbuilder-lg'; -import get from 'lodash/get'; +import { + LGParser, + StaticChecker, + Diagnostic as LGDiagnostic, + LGTemplate as LgTemplate, + ImportResolver, +} from 'botbuilder-lg'; -import { FileInfo, LgFile, LgTemplate } from './type'; +import { FileInfo, LgFile } from './type'; import { getBaseName } from './utils/help'; +import { Diagnostic, DiagnosticSeverity, Position, Range } from './diagnostic'; const lgStaticChecker = new StaticChecker(); +// NOTE: LGDiagnostic is defined in PascalCase which should be corrected +function convertLGDiagnostic(d: LGDiagnostic, source: string): Diagnostic { + const result = new Diagnostic(d.Message, source, d.Severity); + + const start: Position = new Position(d.Range.Start.Line, d.Range.Start.Character); + const end: Position = new Position(d.Range.End.Line, d.Range.End.Character); + result.range = new Range(start, end); + + return result; +} + +function check(content: string, id: string, path?: string): Diagnostic[] { + const lgImportResolver = ImportResolver.fileResolver; + let diagnostics: LGDiagnostic[] = []; + if (path) { + diagnostics = lgStaticChecker.checkText(content, path); + } else { + diagnostics = lgStaticChecker.checkText(content, path, lgImportResolver); + } + return diagnostics.map((d: LGDiagnostic) => { + return convertLGDiagnostic(d, id); + }); +} + +function parse(content: string, id?: string): LgTemplate[] { + const resource = LGParser.parse(content, id); + return resource.Templates; +} + function index(files: FileInfo[]): LgFile[] { if (files.length === 0) return []; const lgFiles: LgFile[] = []; for (const file of files) { - if (file.name.endsWith('.lg')) { - const diagnostics = lgStaticChecker.checkText(file.content, file.path); + const { name, relativePath, content } = file; + if (name.endsWith('.lg')) { + const id = getBaseName(name, '.lg'); + const diagnostics = check(content, id); let templates: LgTemplate[] = []; try { templates = parse(file.content, ''); } catch (err) { - console.error(err); + diagnostics.push(new Diagnostic(err.message, id, DiagnosticSeverity.Error)); } - lgFiles.push({ - id: getBaseName(file.name, '.lg'), - relativePath: file.relativePath, - content: file.content, - templates, - diagnostics, - }); + lgFiles.push({ id, relativePath, content, templates, diagnostics }); } } return lgFiles; } function isValid(diagnostics: Diagnostic[]): boolean { - return diagnostics.every(d => d.Severity !== DiagnosticSeverity.Error); -} - -function check(content: string, path: string): Diagnostic[] { - return lgStaticChecker.checkText(content, path); + return diagnostics.every(d => d.severity !== DiagnosticSeverity.Error); } -function parse(content: string, id: string): LgTemplate[] { - const resource = LGParser.parse(content, id); - const templates = resource.Templates.map(t => { - return { - Name: t.Name, - Body: t.Body, - Parameters: t.Parameters, - Range: { - startLineNumber: get(t, 'ParseTree._start._line', 0), - endLineNumber: get(t, 'ParseTree._stop._line', 0), - }, - }; - }); - return templates; +function createSingleMessage(d: Diagnostic): string { + let msg = `${d.message}\n`; + if (d.range) { + const { start, end } = d.range; + const position = `line ${start.line}:${start.character} - line ${end.line}:${end.character}`; + msg += `${position} \n ${msg}`; + } + return msg; } function combineMessage(diagnostics: Diagnostic[]): string { return diagnostics.reduce((msg, d) => { - const { Start, End } = d.Range; - const position = `line ${Start.Line}:${Start.Character} - line ${End.Line}:${End.Character}`; - - msg += `${position} \n ${d.Message}\n`; + msg += createSingleMessage(d); return msg; }, ''); } @@ -72,5 +90,6 @@ export const lgIndexer = { parse, check, isValid, + createSingleMessage, combineMessage, }; diff --git a/Composer/packages/lib/indexers/src/type.ts b/Composer/packages/lib/indexers/src/type.ts index 8a6a270a9e..2179999be9 100644 --- a/Composer/packages/lib/indexers/src/type.ts +++ b/Composer/packages/lib/indexers/src/type.ts @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { Diagnostic as LGDiagnostic } from 'botbuilder-lg'; +import { LGTemplate as LgTemplate } from 'botbuilder-lg'; + +import { Diagnostic } from './diagnostic'; export interface FileInfo { name: string; @@ -19,7 +21,7 @@ export interface ITrigger { export interface DialogInfo { content: any; - diagnostics: string[]; + diagnostics: Diagnostic[]; displayName: string; id: string; isRoot: boolean; @@ -32,19 +34,6 @@ export interface DialogInfo { triggers: ITrigger[]; } -export interface EditorSchema { - content?: { - fieldTemplateOverrides?: any; - SDKOverrides?: any; - }; -} - -export interface BotSchemas { - editor: EditorSchema; - sdk?: any; - diagnostics?: any[]; -} - export interface Intent { name: string; } @@ -76,18 +65,6 @@ export interface LgFile { id: string; relativePath: string; content: string; - diagnostics: LGDiagnostic[]; + diagnostics: Diagnostic[]; templates: LgTemplate[]; } - -export interface CodeRange { - startLineNumber: number; - endLineNumber: number; -} - -export interface LgTemplate { - Name: string; - Body: string; - Parameters: string[]; - Range: CodeRange; -} diff --git a/Composer/packages/lib/package.json b/Composer/packages/lib/package.json index 6e6604e56c..dd08d62b9b 100644 --- a/Composer/packages/lib/package.json +++ b/Composer/packages/lib/package.json @@ -10,7 +10,7 @@ "build:code-editor": "cd code-editor && yarn build", "build:shared": "cd shared && yarn build", "build:indexers": "cd indexers && yarn build", - "build:all": "concurrently --kill-others-on-fail \"yarn:build:code-editor\" \"yarn:build:shared\" \"yarn:build:indexers\"" + "build:all": "concurrently --kill-others-on-fail \"yarn:build:code-editor\" \"yarn:build:indexers\" && yarn build:shared" }, "author": "", "license": "ISC" diff --git a/Composer/packages/lib/shared/package.json b/Composer/packages/lib/shared/package.json index 73e55bf6e9..0468b585c0 100644 --- a/Composer/packages/lib/shared/package.json +++ b/Composer/packages/lib/shared/package.json @@ -31,6 +31,7 @@ "@babel/plugin-transform-runtime": "^7.4.4", "@babel/preset-env": "^7.4.5", "@babel/preset-react": "^7.0.0", + "@bfc/indexers": "*", "@types/jest": "^24.0.11", "@types/nanoid": "^2.1.0", "@types/react": "16.9.0", diff --git a/Composer/packages/lib/shared/src/types/shell.ts b/Composer/packages/lib/shared/src/types/shell.ts index 10087b4165..9bc618cb10 100644 --- a/Composer/packages/lib/shared/src/types/shell.ts +++ b/Composer/packages/lib/shared/src/types/shell.ts @@ -1,38 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { Diagnostic as LGDiagnostic } from 'botbuilder-lg'; +import { LGTemplate as LgTemplate } from 'botbuilder-lg'; +import { DialogInfo, LgFile, LuFile } from '@bfc/indexers'; -import { MicrosoftAdaptiveDialog } from './sdk'; - -export interface FileInfo { - name: string; - content: string; - path: string; - relativePath: string; -} - -export interface ITrigger { - id: string; - displayName: string; - type: string; - isIntent: boolean; -} - -export interface DialogInfo { - content: MicrosoftAdaptiveDialog; - diagnostics: string[]; - displayName: string; - id: string; - isRoot: boolean; - lgFile: string; - lgTemplates: string[]; - luFile: string; - luIntents: string[]; - referredDialogs: string[]; - relativePath: string; - triggers: ITrigger[]; -} +export * from '@bfc/indexers/lib/type'; export interface EditorSchema { content?: { @@ -47,53 +19,6 @@ export interface BotSchemas { diagnostics?: any[]; } -export interface Intent { - name: string; -} - -export interface Utterance { - intent: string; - text: string; -} - -export interface LuDiagnostic { - text: string; -} - -export interface LuFile { - id: string; - relativePath: string; - content: string; - parsedContent: { - LUISJsonStructure: { - intents: Intent[]; - utterances: Utterance[]; - }; - }; - diagnostics: LuDiagnostic[]; - [key: string]: any; -} - -export interface LgFile { - id: string; - relativePath: string; - content: string; - diagnostics: LGDiagnostic[]; - templates: LgTemplate[]; -} - -export interface CodeRange { - startLineNumber: number; - endLineNumber: number; -} - -export interface LgTemplate { - Name: string; - Body: string; - Parameters: string[]; - Range: CodeRange; -} - export interface ShellData { botName: string; currentDialog: DialogInfo; diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index 69f801f99b..c55d7d8e55 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -4,7 +4,8 @@ import fs from 'fs'; import { FileInfo, DialogInfo, LgFile, LuFile, getNewDesigner } from '@bfc/shared'; -import { dialogIndexer, luIndexer, lgIndexer } from '@bfc/indexers'; +import { dialogIndexer, lgIndexer } from '@bfc/indexers'; +import { luIndexer } from '@bfc/indexers/lib/luIndexer'; import { Path } from '../../utility/path'; import { copyDir } from '../../utility/storage'; diff --git a/Composer/packages/tools/language-servers/language-generation/package.json b/Composer/packages/tools/language-servers/language-generation/package.json index 10e073ed36..02bb3c386a 100644 --- a/Composer/packages/tools/language-servers/language-generation/package.json +++ b/Composer/packages/tools/language-servers/language-generation/package.json @@ -24,7 +24,7 @@ "jest": "24.0.0", "rimraf": "^2.6.3", "ts-jest": "^24.1.0", - "ts-node": "^8.3.0", + "ts-node": "^8.4.1", "typescript": "^3.6.3", "vscode-ws-jsonrpc": "^0.1.1", "ws": "^7.2.0" From b86b979e68a8c83d55a8a3f70002e982ebe2c862 Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 01:18:07 +0800 Subject: [PATCH 13/45] add dialog error jump --- .../client/src/pages/notifications/index.tsx | 20 +++++++++- .../pages/notifications/useNotifications.tsx | 2 +- .../lib/indexers/src/dialogIndexer.ts | 21 +++++----- .../lib/indexers/src/utils/dialogChecker.ts | 38 ++++++++++++------- .../lib/indexers/src/utils/jsonWalk.ts | 2 +- 5 files changed, 53 insertions(+), 30 deletions(-) diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 3a9bcf4dac..b8d9dd7067 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -16,13 +16,29 @@ import { navigateTo } from './../../utils'; const navigations = { lg: (item: INotification) => { - navigateTo(`/language-generation/#line=${item.diagnostic.Range.Start.Line || 0}`); + navigateTo(`/language-generation/#line=${item.diagnostic.range.start.line || 0}`); }, lu: (item: INotification) => { navigateTo(`/dialogs/${item.id}`); }, dialog: (item: INotification) => { - navigateTo(`/dialogs/${item.id}`); + //path is like main.trigers[0].actions[0] + //uri = id?selected=triggers[0]&focused=triggers[0].actions[0] + const path = item.diagnostic.path; + let uri = `/dialogs/${item.id}`; + if (path) { + const matchTriggers = /triggers\[(\d+)\]/g.exec(path); + const matchActions = /actions\[(\d+)\]/g.exec(path); + const trigger = matchTriggers ? `triggers[${+matchTriggers[1]}]` : ''; + const action = matchActions ? `actions[${+matchActions[1]}]` : ''; + if (trigger) { + uri += `?selected=${trigger}`; + if (action) { + uri += `&focused=${trigger}.${action}`; + } + } + } + navigateTo(uri); }, }; const Notifications: React.FC = () => { diff --git a/Composer/packages/client/src/pages/notifications/useNotifications.tsx b/Composer/packages/client/src/pages/notifications/useNotifications.tsx index 994026ee21..8fedb8745b 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -19,7 +19,7 @@ export default function useNotifications(filter: string) { const locations = new Set(); dialogs.forEach(dialog => { dialog.diagnostics.map(diagnostic => { - const location = dialog.displayName; + const location = `${dialog.id}.dialog`; locations.add(location); notifactions.push({ type: 'dialog', diff --git a/Composer/packages/lib/indexers/src/dialogIndexer.ts b/Composer/packages/lib/indexers/src/dialogIndexer.ts index b30c8191e0..c22911a792 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -131,8 +131,8 @@ function ExtractReferredDialogs(dialog): string[] { } // check all fields -function CheckFields(dialog): string[] { - const errors: string[] = []; +function CheckFields(dialog, id: string): Diagnostic[] { + const errors: Diagnostic[] = []; /** * * @param path , jsonPath string @@ -144,12 +144,7 @@ function CheckFields(dialog): string[] { if (has(value, '$type') && has(DialogChecker, value.$type)) { const matchedCheckers = DialogChecker[value.$type]; matchedCheckers.forEach(checker => { - const checkRes = checker.apply(null, [ - { - path, - value, - }, - ]); + const checkRes = checker.apply(null, [{ path, value }]); if (checkRes) { Array.isArray(checkRes) ? errors.push(...checkRes) : errors.push(checkRes); } @@ -157,14 +152,16 @@ function CheckFields(dialog): string[] { } return false; }; - JsonWalk('$', dialog, visitor); - return errors; + JsonWalk(id, dialog, visitor); + return errors.map(e => { + e.source = id; + return e; + }); } function validate(id: string, content): Diagnostic[] { try { - const errors = CheckFields(content); - return errors.map(x => new Diagnostic(x, id)); + return CheckFields(content, id); } catch (error) { return [new Diagnostic(error.message, id)]; } diff --git a/Composer/packages/lib/indexers/src/utils/dialogChecker.ts b/Composer/packages/lib/indexers/src/utils/dialogChecker.ts index 654bde729b..c91d15df8b 100644 --- a/Composer/packages/lib/indexers/src/utils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/utils/dialogChecker.ts @@ -4,24 +4,33 @@ import get from 'lodash/get'; import { ExpressionEngine } from 'botbuilder-expression-parser'; +import { Diagnostic } from './../diagnostic'; + const ExpressionParser = new ExpressionEngine(); interface CheckerFunc { - (node: { path: string; value: any }): string; // error msg + (node: { path: string; value: any }): Diagnostic | null; // error msg } function IsExpression(name: string): CheckerFunc { return node => { - const exp = get(node.value, name); - if (!exp) return `In ${node.path}: ${node.value.$type}: ${name} is missing or empty`; - let message = ''; - try { - ExpressionParser.parse(exp); - } catch (error) { - message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; + const exp = get(node.value, name); + if (!exp) { + message = `In ${node.path}: ${node.value.$type}: ${name} is missing or empty`; + } else { + try { + ExpressionParser.parse(exp); + } catch (error) { + message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; + } } - return message; + if (message) { + const diagnostic = new Diagnostic(message, ''); + diagnostic.path = node.path; + return diagnostic; + } + return null; }; } @@ -34,9 +43,7 @@ enum EditArrayChangeTypes { } // eslint-disable-next-line @typescript-eslint/no-explicit-any -function EditArrayValueChecker(node: { path: string; value: any }): string { - let message = ''; - +function EditArrayValueChecker(node: { path: string; value: any }): Diagnostic | null { const changeType = get(node.value, 'changeType'); // when push and remove, value is required @@ -45,11 +52,14 @@ function EditArrayValueChecker(node: { path: string; value: any }): string { try { ExpressionParser.parse(exp); } catch (error) { - message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; + const message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; + const diagnostic = new Diagnostic(message, ''); + diagnostic.path = node.path; + return diagnostic; } } - return message; + return null; } /** diff --git a/Composer/packages/lib/indexers/src/utils/jsonWalk.ts b/Composer/packages/lib/indexers/src/utils/jsonWalk.ts index 9e356aacfd..d644a144ef 100644 --- a/Composer/packages/lib/indexers/src/utils/jsonWalk.ts +++ b/Composer/packages/lib/indexers/src/utils/jsonWalk.ts @@ -25,7 +25,7 @@ export const JsonWalk = (path: string, value: any, visitor: VisitorFunc) => { // extract array if (Array.isArray(value)) { value.forEach((child, index) => { - JsonWalk(`${path}[:${index}]`, child, visitor); + JsonWalk(`${path}[${index}]`, child, visitor); }); // extract object From b0b72b328a541dca2b84352e66c04581b26132cc Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 02:01:50 +0800 Subject: [PATCH 14/45] fix lg templete type error --- .../packages/lib/indexers/src/lgIndexer.ts | 19 ++++++++++--------- Composer/packages/lib/indexers/src/type.ts | 8 ++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/Composer/packages/lib/indexers/src/lgIndexer.ts b/Composer/packages/lib/indexers/src/lgIndexer.ts index 35dba74719..02ebaea1f1 100644 --- a/Composer/packages/lib/indexers/src/lgIndexer.ts +++ b/Composer/packages/lib/indexers/src/lgIndexer.ts @@ -1,15 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { - LGParser, - StaticChecker, - Diagnostic as LGDiagnostic, - LGTemplate as LgTemplate, - ImportResolver, -} from 'botbuilder-lg'; +import { LGParser, StaticChecker, Diagnostic as LGDiagnostic, ImportResolver } from 'botbuilder-lg'; -import { FileInfo, LgFile } from './type'; +import { FileInfo, LgFile, LgTemplate } from './type'; import { getBaseName } from './utils/help'; import { Diagnostic, DiagnosticSeverity, Position, Range } from './diagnostic'; @@ -41,7 +35,14 @@ function check(content: string, id: string, path?: string): Diagnostic[] { function parse(content: string, id?: string): LgTemplate[] { const resource = LGParser.parse(content, id); - return resource.Templates; + const templates = resource.Templates.map(t => { + return { + Name: t.Name, + Body: t.Body, + Parameters: t.Parameters, + }; + }); + return templates; } function index(files: FileInfo[]): LgFile[] { diff --git a/Composer/packages/lib/indexers/src/type.ts b/Composer/packages/lib/indexers/src/type.ts index 2179999be9..389566a90e 100644 --- a/Composer/packages/lib/indexers/src/type.ts +++ b/Composer/packages/lib/indexers/src/type.ts @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { LGTemplate as LgTemplate } from 'botbuilder-lg'; - import { Diagnostic } from './diagnostic'; export interface FileInfo { @@ -61,6 +59,12 @@ export interface LuFile { [key: string]: any; } +export interface LgTemplate { + Name: string; + Body: string; + Parameters: string[]; +} + export interface LgFile { id: string; relativePath: string; From 4dadf1cb02e27f6078d15331ba1c4390a1f0318b Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 02:10:50 +0800 Subject: [PATCH 15/45] fix lint --- .../client/src/pages/language-generation/table-view.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/src/pages/language-generation/table-view.tsx b/Composer/packages/client/src/pages/language-generation/table-view.tsx index 9778dcf68f..9bb82e3cfc 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -17,8 +17,8 @@ import formatMessage from 'format-message'; import { NeutralColors, FontSizes } from '@uifabric/fluent-theme'; import { DialogInfo, LgFile } from '@bfc/shared'; import { LGTemplate } from 'botbuilder-lg'; - import { lgIndexer } from '@bfc/indexers'; + import { StoreContext } from '../../store'; import * as lgUtil from '../../utils/lgUtil'; import { navigateTo } from '../../utils'; From 460b944fedc642f6ccb068a5442912d4c259594a Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 14:15:19 +0800 Subject: [PATCH 16/45] change the build order --- Composer/packages/lib/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/lib/package.json b/Composer/packages/lib/package.json index ce957b0aa9..9f86a13046 100644 --- a/Composer/packages/lib/package.json +++ b/Composer/packages/lib/package.json @@ -10,7 +10,7 @@ "build:code-editor": "cd code-editor && yarn build", "build:shared": "cd shared && yarn build", "build:indexers": "cd indexers && yarn build", - "build:all": "yarn build:shared && concurrently --kill-others-on-fail \"yarn:build:code-editor\" \"yarn:build:indexers\"" + "build:all": "yarn build:indexers && yarn build:shared && yarn build:code-editor" }, "author": "", "license": "ISC" From aba5ce60ab551a1a0713631133eb2b23b671d9e1 Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 15:55:59 +0800 Subject: [PATCH 17/45] fix conflict --- Composer/packages/lib/indexers/package.json | 1 - Composer/packages/lib/indexers/src/dialogIndexer.ts | 10 ++++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Composer/packages/lib/indexers/package.json b/Composer/packages/lib/indexers/package.json index 67184bfa1b..497ecdb788 100644 --- a/Composer/packages/lib/indexers/package.json +++ b/Composer/packages/lib/indexers/package.json @@ -28,7 +28,6 @@ "ts-jest": "^24.1.0" }, "dependencies": { - "@bfc/shared": "*", "botbuilder-expression-parser": "^4.5.11", "botbuilder-lg": "https://botbuilder.myget.org/F/botbuilder-declarative/npm/botbuilder-lg/-/4.7.0-preview2.tgz", "lodash": "^4.17.15", diff --git a/Composer/packages/lib/indexers/src/dialogIndexer.ts b/Composer/packages/lib/indexers/src/dialogIndexer.ts index a0244be654..c22911a792 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -3,7 +3,6 @@ import has from 'lodash/has'; import uniq from 'lodash/uniq'; -import { extractLgTemplateRefs } from '@bfc/shared'; import { ITrigger, DialogInfo, FileInfo } from './type'; import { DialogChecker } from './utils/dialogChecker'; @@ -39,7 +38,14 @@ function ExtractLgTemplates(dialog): string[] { return true; } targets.forEach(target => { - templates.push(...extractLgTemplateRefs(target).map(x => x.name)); + // match a template name match a temlate func e.g. `showDate()` + // eslint-disable-next-line security/detect-unsafe-regex + const reg = /\[([A-Za-z_][-\w]+)(\(.*\))?\]/g; + let matchResult; + while ((matchResult = reg.exec(target)) !== null) { + const templateName = matchResult[1]; + templates.push(templateName); + } }); } return false; From ae2ee213eaa6eda72ed601c9f59c960bbb42ee9f Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 18:56:34 +0800 Subject: [PATCH 18/45] fix some type --- Composer/packages/client/src/ShellApi.ts | 19 +--------- .../pages/language-generation/table-view.tsx | 2 +- .../lib/indexers/__test__/utils/help.test.ts | 0 .../indexers/__test__/utils}/jsonWalk.test.ts | 0 .../packages/server/src/utility/jsonWalk.ts | 37 ------------------- 5 files changed, 2 insertions(+), 56 deletions(-) create mode 100644 Composer/packages/lib/indexers/__test__/utils/help.test.ts rename Composer/packages/{server/__tests__/utility => lib/indexers/__test__/utils}/jsonWalk.test.ts (100%) delete mode 100644 Composer/packages/server/src/utility/jsonWalk.ts diff --git a/Composer/packages/client/src/ShellApi.ts b/Composer/packages/client/src/ShellApi.ts index b30ae8a640..ae14703ae9 100644 --- a/Composer/packages/client/src/ShellApi.ts +++ b/Composer/packages/client/src/ShellApi.ts @@ -5,7 +5,6 @@ import React, { useEffect, useContext, useMemo } from 'react'; import { ShellData } from '@bfc/shared'; import isEqual from 'lodash/isEqual'; import get from 'lodash/get'; -import { LGTemplate } from 'botbuilder-lg'; import { lgIndexer } from '@bfc/indexers'; import { isExpression } from './utils'; @@ -134,23 +133,7 @@ export const ShellApi: React.FC = () => { if (id === undefined) throw new Error('must have a file id'); const file = lgFiles.find(file => file.id === id); if (!file) throw new Error(`lg file ${id} not found`); - let templates: LGTemplate[] = []; - templates = lgIndexer.parse(file.content); - - const lines = file.content.split('\n'); - - return templates.map(t => { - const [start, end] = getTemplateBodyRange(t); - const body = lines.slice(start - 1, end).join('\n'); - - return { Name: t.Name, Parameters: t.Parameters, Body: body }; - }); - } - - function getTemplateBodyRange(template) { - const startLineNumber = template.ParseTree._start.line + 1; - const endLineNumber = template.ParseTree._stop.line; - return [startLineNumber, endLineNumber]; + return lgIndexer.parse(file.content); } /** diff --git a/Composer/packages/client/src/pages/language-generation/table-view.tsx b/Composer/packages/client/src/pages/language-generation/table-view.tsx index 9bb82e3cfc..a943d46f35 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -43,7 +43,7 @@ const TableView: React.FC = props => { useEffect(() => { if (isEmpty(lgFile)) return; let allTemplates: LGTemplate[] = []; - allTemplates = lgIndexer.parse(lgFile.content); + allTemplates = lgIndexer.parse(lgFile.content) as LGTemplate[]; if (!activeDialog) { setTemplates(allTemplates); } else { diff --git a/Composer/packages/lib/indexers/__test__/utils/help.test.ts b/Composer/packages/lib/indexers/__test__/utils/help.test.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/Composer/packages/server/__tests__/utility/jsonWalk.test.ts b/Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts similarity index 100% rename from Composer/packages/server/__tests__/utility/jsonWalk.test.ts rename to Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts diff --git a/Composer/packages/server/src/utility/jsonWalk.ts b/Composer/packages/server/src/utility/jsonWalk.ts deleted file mode 100644 index 9e356aacfd..0000000000 --- a/Composer/packages/server/src/utility/jsonWalk.ts +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -/** - * visitor function used by JsonWalk - * @param path jsonPath string - * @param value current node value - * @return boolean, true to stop walk deep - */ -export interface VisitorFunc { - (path: string, value: any): boolean; -} - -/** - * - * @param path jsonPath string - * @param value current node value - * @param visitor - */ - -export const JsonWalk = (path: string, value: any, visitor: VisitorFunc) => { - const stop = visitor(path, value); - if (stop === true) return; - - // extract array - if (Array.isArray(value)) { - value.forEach((child, index) => { - JsonWalk(`${path}[:${index}]`, child, visitor); - }); - - // extract object - } else if (typeof value === 'object' && value) { - Object.keys(value).forEach(key => { - JsonWalk(`${path}.${key}`, value[key], visitor); - }); - } -}; From b9b2ef10da06087fff7874e5a0c4566a1fc3f9dd Mon Sep 17 00:00:00 2001 From: leilzh Date: Tue, 3 Dec 2019 23:26:51 +0800 Subject: [PATCH 19/45] disable start button if bot has error --- .../packages/client/src/TestController.tsx | 31 ++++++++++++++++--- .../pages/notifications/useNotifications.tsx | 2 +- Composer/packages/client/src/styles.ts | 13 +++++++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index 5c0575fb51..f3a7024e50 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -4,21 +4,24 @@ /** @jsx jsx */ import { jsx } from '@emotion/core'; import React, { useState, useRef, Fragment, useContext, useEffect, useCallback } from 'react'; -import { ActionButton, PrimaryButton, DefaultButton } from 'office-ui-fabric-react/lib/Button'; +import { ActionButton, PrimaryButton, DefaultButton, IconButton } from 'office-ui-fabric-react/lib/Button'; import { Spinner, SpinnerSize } from 'office-ui-fabric-react/lib/Spinner'; import { Callout } from 'office-ui-fabric-react/lib/Callout'; import { Stack } from 'office-ui-fabric-react/lib/Stack'; import formatMessage from 'format-message'; import { DialogInfo } from '@bfc/shared'; +import { DiagnosticSeverity } from '@bfc/indexers'; import settingsStorage from './utils/dialogSettingStorage'; import { StoreContext } from './store'; -import { bot, botButton, calloutLabel, calloutDescription, calloutContainer } from './styles'; +import { bot, botButton, calloutLabel, calloutDescription, calloutContainer, errorButton, errorCount } from './styles'; import { BotStatus, LuisConfig, Text } from './constants'; import { PublishLuisDialog } from './publishDialog'; import { OpenAlertModal, DialogStyle } from './components/Modal'; import { isAbsHosted } from './utils/envUtil'; import { getReferredFiles } from './utils/luUtil'; +import useNotifications from './pages/notifications/useNotifications'; +import { navigateTo } from './utils'; const openInEmulator = (url, authSettings: { MicrosoftAppId: string; MicrosoftAppPassword: string }) => { // this creates a temporary hidden iframe to fire off the bfemulator protocol @@ -46,11 +49,12 @@ export const TestController: React.FC = () => { const [error, setError] = useState({ title: '', message: '' }); const [luisPublishSucceed, setLuisPublishSucceed] = useState(true); const botActionRef = useRef(null); + const { notifications } = useNotifications(); const { botEndpoint, botName, botStatus, dialogs, toStartBot, luFiles, settings } = state; const { connectBot, reloadBot, onboardingAddCoachMarkRef, publishLuis, startBot } = actions; const connected = botStatus === BotStatus.connected; - const addRef = useCallback(startBot => onboardingAddCoachMarkRef({ startBot }), []); + const showError = notifications.filter(n => n.severity === 'Error').length > 0; useEffect(() => { toStartBot && handleClick(); @@ -141,6 +145,10 @@ export const TestController: React.FC = () => { } } + function handleErrorButtonClick() { + navigateTo('/notifications/'); + } + return (
@@ -171,14 +179,27 @@ export const TestController: React.FC = () => { labelPosition="left" /> )} - +
+ {showError && ( + + {notifications.length} + + + )} - +
Date: Tue, 3 Dec 2019 23:30:57 +0800 Subject: [PATCH 20/45] hide the emulater button --- Composer/packages/client/src/TestController.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index f3a7024e50..b1378df582 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -10,8 +10,8 @@ import { Callout } from 'office-ui-fabric-react/lib/Callout'; import { Stack } from 'office-ui-fabric-react/lib/Stack'; import formatMessage from 'format-message'; import { DialogInfo } from '@bfc/shared'; - import { DiagnosticSeverity } from '@bfc/indexers'; + import settingsStorage from './utils/dialogSettingStorage'; import { StoreContext } from './store'; import { bot, botButton, calloutLabel, calloutDescription, calloutContainer, errorButton, errorCount } from './styles'; @@ -152,7 +152,7 @@ export const TestController: React.FC = () => { return (
- {connected && fetchState === STATE.SUCCESS && ( + {connected && showError && fetchState === STATE.SUCCESS && ( Date: Wed, 4 Dec 2019 00:06:02 +0800 Subject: [PATCH 21/45] fix e2e test and lg page crash --- Composer/cypress/integration/NotificationPage.spec.ts | 2 +- Composer/packages/client/src/TestController.tsx | 1 - .../packages/client/src/pages/language-generation/index.tsx | 4 +++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index 0f98c58149..8ed2345703 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -20,7 +20,7 @@ context('Notification Page', () => { cy.visitPage('Notifications'); cy.get('[data-testid="notifications-table-view"]').within(() => { - cy.findByText('common.lg').should('exist'); + cy.findAllByText('common.lg').should('exist'); }); }); }); diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index b1378df582..b326941841 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -10,7 +10,6 @@ import { Callout } from 'office-ui-fabric-react/lib/Callout'; import { Stack } from 'office-ui-fabric-react/lib/Stack'; import formatMessage from 'format-message'; import { DialogInfo } from '@bfc/shared'; -import { DiagnosticSeverity } from '@bfc/indexers'; import settingsStorage from './utils/dialogSettingStorage'; import { StoreContext } from './store'; diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index ed9fe65d5c..da1334f1f5 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -35,7 +35,9 @@ const CodeEditor = React.lazy(() => import('./code-editor')); const LGPage: React.FC = props => { const { state } = useContext(StoreContext); const { lgFiles, dialogs } = state; - const [editMode, setEditMode] = useState(false); + const [editMode, setEditMode] = useState( + lgFiles.filter(file => lgIndexer.isValid(file.diagnostics) === false).length > 0 + ); const [fileValid, setFileValid] = useState(true); const [inlineTemplate, setInlineTemplate] = useState(null); const [line, setLine] = useState(0); From 4243ea531d613c50fba53cdac8d03027c06b808c Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 4 Dec 2019 00:26:38 +0800 Subject: [PATCH 22/45] add some e2e test and unit test --- .../integration/NotificationPage.spec.ts | 18 +++++++++++++++--- .../packages/client/src/TestController.tsx | 2 +- .../lib/indexers/__test__/utils/help.test.ts | 19 +++++++++++++++++++ .../indexers/__test__/utils/jsonWalk.test.ts | 6 +++--- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index 8ed2345703..5fac7bc750 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -10,9 +10,6 @@ context('Notification Page', () => { it('can show lg syntax error ', () => { cy.visitPage('Bot Responses'); - // left nav tree - cy.contains('TodoSample.Main'); - cy.contains('All'); cy.get('.toggleEditMode button').click(); cy.get('textarea').type('test lg syntax error'); @@ -23,4 +20,19 @@ context('Notification Page', () => { cy.findAllByText('common.lg').should('exist'); }); }); + + it('can show lu syntax error ', () => { + cy.visitPage('User Input'); + + cy.get('.dialogNavTree button[title="__TestTodoSample.Main"]').click({ multiple: true }); + + cy.get('.toggleEditMode button').click(); + cy.get('textarea').type('test lu syntax error'); + + cy.visitPage('Notifications'); + + cy.get('[data-testid="notifications-table-view"]').within(() => { + cy.findAllByText('Main.lu').should('exist'); + }); + }); }); diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index b326941841..37dc18da61 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -151,7 +151,7 @@ export const TestController: React.FC = () => { return (
- {connected && showError && fetchState === STATE.SUCCESS && ( + {connected && !showError && fetchState === STATE.SUCCESS && ( { + it('should get corrent base name', () => { + const n1 = getBaseName('b.c', '.c'); + const n2 = getBaseName('b.c'); + const n3 = getBaseName('b'); + const n4 = getBaseName('a.b.c'); + const n5 = getBaseName('a.b.c', 'd'); + expect(n1).toBe('b'); + expect(n2).toBe('b'); + expect(n3).toBe('b'); + expect(n4).toBe('a.b'); + expect(n5).toBe(''); + }); +}); diff --git a/Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts b/Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts index 5655eeb50c..6d59c60864 100644 --- a/Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts +++ b/Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { JsonWalk, VisitorFunc } from '../../../server/src/utility/jsonWalk'; +import { JsonWalk, VisitorFunc } from '../../src/utils/jsonWalk'; const data = { firstName: 'John', @@ -34,7 +34,7 @@ describe('run json walk', () => { JsonWalk('$', data, visitor); const lastPath = visitedPath.pop(); expect(visitedPath.length).toBe(14); - expect(lastPath).toBe('$.phoneNumbers[:1].number'); + expect(lastPath).toBe('$.phoneNumbers[1].number'); }); it('if visitor stop, its children should not be visited', () => { @@ -49,6 +49,6 @@ describe('run json walk', () => { }; JsonWalk('$', data, visitor); expect(visitedPath.length).toBe(8); - expect(visitedPath.indexOf('$.phoneNumbers[:1].number')).toBe(-1); + expect(visitedPath.indexOf('$.phoneNumbers[1].number')).toBe(-1); }); }); From 2533c5620e707fa3cf1fb26c8c95175db0542dd4 Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 4 Dec 2019 15:45:56 +0800 Subject: [PATCH 23/45] fix some comment --- Composer/package.json | 2 +- Composer/packages/client/src/App.tsx | 4 ++-- .../client/src/pages/language-generation/index.tsx | 4 ++-- Composer/packages/client/src/styles.ts | 7 +++++-- .../language-generation/package.json | 2 +- Composer/yarn.lock | 13 ++++--------- 6 files changed, 15 insertions(+), 17 deletions(-) diff --git a/Composer/package.json b/Composer/package.json index 619741f985..e1dab441f4 100644 --- a/Composer/package.json +++ b/Composer/package.json @@ -102,7 +102,7 @@ "react-testing-library": "^6.0.2", "rimraf": "^2.6.3", "ts-loader": "^6.2.1", - "typescript": "3.6.4", + "typescript": "3.7.2", "wsrun": "^3.6.4" }, "dependencies": { diff --git a/Composer/packages/client/src/App.tsx b/Composer/packages/client/src/App.tsx index ee7d8f6ef3..b0a1be423b 100644 --- a/Composer/packages/client/src/App.tsx +++ b/Composer/packages/client/src/App.tsx @@ -51,14 +51,14 @@ const topLinks = (botLoaded: boolean) => { disabled: true, // will delete }, { - to: 'language-generation/', + to: '/language-generation', iconName: 'Robot', labelName: formatMessage('Bot Responses'), exact: false, disabled: !botLoaded, }, { - to: 'language-understanding/', + to: '/language-understanding', iconName: 'People', labelName: formatMessage('User Input'), exact: false, diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index da1334f1f5..d00d207477 100644 --- a/Composer/packages/client/src/pages/language-generation/index.tsx +++ b/Composer/packages/client/src/pages/language-generation/index.tsx @@ -107,7 +107,7 @@ const LGPage: React.FC = props => { // fall back to the all-up page if we don't have an active dialog if (!isRoot && !activeDialog && dialogs.length) { - navigateTo('/language-generation/'); + navigateTo('/language-generation'); } }, [subPath, dialogs]); @@ -124,7 +124,7 @@ const LGPage: React.FC = props => { function onSelect(id) { if (id === '_all') { - navigateTo('/language-generation/'); + navigateTo('/language-generation'); } else { navigateTo(`language-generation/${id}`); } diff --git a/Composer/packages/client/src/styles.ts b/Composer/packages/client/src/styles.ts index 3fbc4be174..d6dfd937fd 100644 --- a/Composer/packages/client/src/styles.ts +++ b/Composer/packages/client/src/styles.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import { css } from '@emotion/core'; -import { NeutralColors, FontSizes } from '@uifabric/fluent-theme'; +import { NeutralColors, FontSizes, SharedColors } from '@uifabric/fluent-theme'; import { FontWeights } from 'office-ui-fabric-react/lib/Styling'; export const main = css` @@ -85,7 +85,10 @@ export const botButton = css` `; export const errorButton = css` - color: #a80000; + color: ${SharedColors.red20}; + &:hover { + color: ${SharedColors.red20}; + } `; export const errorCount = css` diff --git a/Composer/packages/tools/language-servers/language-generation/package.json b/Composer/packages/tools/language-servers/language-generation/package.json index 02bb3c386a..0d820c7ee8 100644 --- a/Composer/packages/tools/language-servers/language-generation/package.json +++ b/Composer/packages/tools/language-servers/language-generation/package.json @@ -25,7 +25,7 @@ "rimraf": "^2.6.3", "ts-jest": "^24.1.0", "ts-node": "^8.4.1", - "typescript": "^3.6.3", + "typescript": "^3.7.2", "vscode-ws-jsonrpc": "^0.1.1", "ws": "^7.2.0" } diff --git a/Composer/yarn.lock b/Composer/yarn.lock index 84024269f1..a65e348837 100644 --- a/Composer/yarn.lock +++ b/Composer/yarn.lock @@ -15826,15 +15826,10 @@ typedarray@^0.0.6: resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" integrity sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c= -typescript@3.6.4: - version "3.6.4" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.6.4.tgz#b18752bb3792bc1a0281335f7f6ebf1bbfc5b91d" - integrity sha512-unoCll1+l+YK4i4F8f22TaNVPRHcD9PA3yCuZ8g5e0qGqlVlJ/8FSateOLLSagn+Yg5+ZwuPkL8LFUc0Jcvksg== - -typescript@^3.6.3: - version "3.6.3" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.6.3.tgz#fea942fabb20f7e1ca7164ff626f1a9f3f70b4da" - integrity sha512-N7bceJL1CtRQ2RiG0AQME13ksR7DiuQh/QehubYcghzv20tnh+MQnQIuJddTmsbqYj+dztchykemz0zFzlvdQw== +typescript@3.7.2, typescript@^3.7.2: + version "3.7.2" + resolved "https://botbuilder.myget.org/F/botbuilder-declarative/npm/typescript/-/typescript-3.7.2.tgz#27e489b95fa5909445e9fef5ee48d81697ad18fb" + integrity sha1-J+SJuV+lkJRF6f717kjYFpetGPs= ua-parser-js@^0.7.18: version "0.7.19" From 5174faef4177f4cdef8e35f170d44a1f177dfbaa Mon Sep 17 00:00:00 2001 From: leilzh Date: Wed, 4 Dec 2019 20:33:10 +0800 Subject: [PATCH 24/45] move lgutil from shared to indexers --- .../__test__}/lgUtils/models/LgMetaData.test.ts | 0 .../__test__}/lgUtils/models/LgTemplateRef.test.ts | 0 .../lgUtils/parsers/parseLgParamString.test.ts | 0 .../lgUtils/parsers/parseLgTemplateName.test.ts | 0 .../lgUtils/parsers/parseLgTemplateRef.test.ts | 0 Composer/packages/lib/indexers/src/dialogIndexer.ts | 10 ++-------- Composer/packages/lib/indexers/src/index.ts | 1 + .../lib/{shared => indexers}/src/lgUtils/index.ts | 3 --- .../src/lgUtils/models/LgMetaData.ts | 0 .../src/lgUtils/models/LgTemplateRef.ts | 0 .../src/lgUtils/models/stringTypes.ts | 0 .../src/lgUtils/parsers/lgPatterns.ts | 0 .../src/lgUtils/parsers/parseLgParamString.ts | 0 .../src/lgUtils/parsers/parseLgTemplateName.ts | 0 .../src/lgUtils/parsers/parseLgTemplateRef.ts | 0 .../src/lgUtils/stringBuilders/buildLgParamString.ts | 0 .../src/lgUtils/stringBuilders/buildLgTemplateName.ts | 0 .../lgUtils/stringBuilders/buildLgTemplateRefString.ts | 0 Composer/packages/lib/shared/src/index.ts | 2 +- Composer/packages/lib/shared/tsconfig.json | 2 +- .../server/__tests__/models/bot/botProject.test.ts | 6 +++++- .../language-servers/language-generation/package.json | 2 +- 22 files changed, 11 insertions(+), 15 deletions(-) rename Composer/packages/lib/{shared/__tests__ => indexers/__test__}/lgUtils/models/LgMetaData.test.ts (100%) rename Composer/packages/lib/{shared/__tests__ => indexers/__test__}/lgUtils/models/LgTemplateRef.test.ts (100%) rename Composer/packages/lib/{shared/__tests__ => indexers/__test__}/lgUtils/parsers/parseLgParamString.test.ts (100%) rename Composer/packages/lib/{shared/__tests__ => indexers/__test__}/lgUtils/parsers/parseLgTemplateName.test.ts (100%) rename Composer/packages/lib/{shared/__tests__ => indexers/__test__}/lgUtils/parsers/parseLgTemplateRef.test.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/index.ts (71%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/models/LgMetaData.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/models/LgTemplateRef.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/models/stringTypes.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/parsers/lgPatterns.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/parsers/parseLgParamString.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/parsers/parseLgTemplateName.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/parsers/parseLgTemplateRef.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/stringBuilders/buildLgParamString.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/stringBuilders/buildLgTemplateName.ts (100%) rename Composer/packages/lib/{shared => indexers}/src/lgUtils/stringBuilders/buildLgTemplateRefString.ts (100%) diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/models/LgMetaData.test.ts b/Composer/packages/lib/indexers/__test__/lgUtils/models/LgMetaData.test.ts similarity index 100% rename from Composer/packages/lib/shared/__tests__/lgUtils/models/LgMetaData.test.ts rename to Composer/packages/lib/indexers/__test__/lgUtils/models/LgMetaData.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/models/LgTemplateRef.test.ts b/Composer/packages/lib/indexers/__test__/lgUtils/models/LgTemplateRef.test.ts similarity index 100% rename from Composer/packages/lib/shared/__tests__/lgUtils/models/LgTemplateRef.test.ts rename to Composer/packages/lib/indexers/__test__/lgUtils/models/LgTemplateRef.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgParamString.test.ts b/Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgParamString.test.ts similarity index 100% rename from Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgParamString.test.ts rename to Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgParamString.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgTemplateName.test.ts b/Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateName.test.ts similarity index 100% rename from Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgTemplateName.test.ts rename to Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateName.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgTemplateRef.test.ts b/Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateRef.test.ts similarity index 100% rename from Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgTemplateRef.test.ts rename to Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateRef.test.ts diff --git a/Composer/packages/lib/indexers/src/dialogIndexer.ts b/Composer/packages/lib/indexers/src/dialogIndexer.ts index c22911a792..91c4b4893e 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -9,6 +9,7 @@ import { DialogChecker } from './utils/dialogChecker'; import { JsonWalk, VisitorFunc } from './utils/jsonWalk'; import { getBaseName } from './utils/help'; import { Diagnostic } from './diagnostic'; +import { extractLgTemplateRefs } from './lgUtils/parsers/parseLgTemplateRef'; // find out all lg templates given dialog function ExtractLgTemplates(dialog): string[] { const templates: string[] = []; @@ -38,14 +39,7 @@ function ExtractLgTemplates(dialog): string[] { return true; } targets.forEach(target => { - // match a template name match a temlate func e.g. `showDate()` - // eslint-disable-next-line security/detect-unsafe-regex - const reg = /\[([A-Za-z_][-\w]+)(\(.*\))?\]/g; - let matchResult; - while ((matchResult = reg.exec(target)) !== null) { - const templateName = matchResult[1]; - templates.push(templateName); - } + templates.push(...extractLgTemplateRefs(target).map(x => x.name)); }); } return false; diff --git a/Composer/packages/lib/indexers/src/index.ts b/Composer/packages/lib/indexers/src/index.ts index 07cad3f7c3..62d3e6a040 100644 --- a/Composer/packages/lib/indexers/src/index.ts +++ b/Composer/packages/lib/indexers/src/index.ts @@ -5,3 +5,4 @@ export * from './dialogIndexer'; export * from './lgIndexer'; export * from './type'; export * from './diagnostic'; +export * from './lgUtils'; diff --git a/Composer/packages/lib/shared/src/lgUtils/index.ts b/Composer/packages/lib/indexers/src/lgUtils/index.ts similarity index 71% rename from Composer/packages/lib/shared/src/lgUtils/index.ts rename to Composer/packages/lib/indexers/src/lgUtils/index.ts index 34aa3f9dde..4d467a438d 100644 --- a/Composer/packages/lib/shared/src/lgUtils/index.ts +++ b/Composer/packages/lib/indexers/src/lgUtils/index.ts @@ -4,6 +4,3 @@ /** models */ export { default as LgMetaData } from './models/LgMetaData'; export { default as LgTemplateRef } from './models/LgTemplateRef'; - -/** parsers */ -export { extractLgTemplateRefs } from './parsers/parseLgTemplateRef'; diff --git a/Composer/packages/lib/shared/src/lgUtils/models/LgMetaData.ts b/Composer/packages/lib/indexers/src/lgUtils/models/LgMetaData.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/models/LgMetaData.ts rename to Composer/packages/lib/indexers/src/lgUtils/models/LgMetaData.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/models/LgTemplateRef.ts b/Composer/packages/lib/indexers/src/lgUtils/models/LgTemplateRef.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/models/LgTemplateRef.ts rename to Composer/packages/lib/indexers/src/lgUtils/models/LgTemplateRef.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/models/stringTypes.ts b/Composer/packages/lib/indexers/src/lgUtils/models/stringTypes.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/models/stringTypes.ts rename to Composer/packages/lib/indexers/src/lgUtils/models/stringTypes.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/parsers/lgPatterns.ts b/Composer/packages/lib/indexers/src/lgUtils/parsers/lgPatterns.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/parsers/lgPatterns.ts rename to Composer/packages/lib/indexers/src/lgUtils/parsers/lgPatterns.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/parsers/parseLgParamString.ts b/Composer/packages/lib/indexers/src/lgUtils/parsers/parseLgParamString.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/parsers/parseLgParamString.ts rename to Composer/packages/lib/indexers/src/lgUtils/parsers/parseLgParamString.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/parsers/parseLgTemplateName.ts b/Composer/packages/lib/indexers/src/lgUtils/parsers/parseLgTemplateName.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/parsers/parseLgTemplateName.ts rename to Composer/packages/lib/indexers/src/lgUtils/parsers/parseLgTemplateName.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/parsers/parseLgTemplateRef.ts b/Composer/packages/lib/indexers/src/lgUtils/parsers/parseLgTemplateRef.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/parsers/parseLgTemplateRef.ts rename to Composer/packages/lib/indexers/src/lgUtils/parsers/parseLgTemplateRef.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/stringBuilders/buildLgParamString.ts b/Composer/packages/lib/indexers/src/lgUtils/stringBuilders/buildLgParamString.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/stringBuilders/buildLgParamString.ts rename to Composer/packages/lib/indexers/src/lgUtils/stringBuilders/buildLgParamString.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/stringBuilders/buildLgTemplateName.ts b/Composer/packages/lib/indexers/src/lgUtils/stringBuilders/buildLgTemplateName.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/stringBuilders/buildLgTemplateName.ts rename to Composer/packages/lib/indexers/src/lgUtils/stringBuilders/buildLgTemplateName.ts diff --git a/Composer/packages/lib/shared/src/lgUtils/stringBuilders/buildLgTemplateRefString.ts b/Composer/packages/lib/indexers/src/lgUtils/stringBuilders/buildLgTemplateRefString.ts similarity index 100% rename from Composer/packages/lib/shared/src/lgUtils/stringBuilders/buildLgTemplateRefString.ts rename to Composer/packages/lib/indexers/src/lgUtils/stringBuilders/buildLgTemplateRefString.ts diff --git a/Composer/packages/lib/shared/src/index.ts b/Composer/packages/lib/shared/src/index.ts index 0c0d75b91e..2f496d4a9b 100644 --- a/Composer/packages/lib/shared/src/index.ts +++ b/Composer/packages/lib/shared/src/index.ts @@ -12,4 +12,4 @@ export * from './promptTabs'; export * from './appschema'; export * from './types'; export * from './constant'; -export * from './lgUtils'; +export { LgMetaData, LgTemplateRef } from '@bfc/indexers'; diff --git a/Composer/packages/lib/shared/tsconfig.json b/Composer/packages/lib/shared/tsconfig.json index bdcff025b4..238c431e66 100644 --- a/Composer/packages/lib/shared/tsconfig.json +++ b/Composer/packages/lib/shared/tsconfig.json @@ -3,5 +3,5 @@ "compilerOptions": { "outDir": "lib" }, - "include": ["./src/**/*", "./__tests__/**/*"] + "include": ["./src/**/*", "./__tests__/**/*", "../indexers/__test__/lgUtils"] } diff --git a/Composer/packages/server/__tests__/models/bot/botProject.test.ts b/Composer/packages/server/__tests__/models/bot/botProject.test.ts index eb18e449af..f6409be184 100644 --- a/Composer/packages/server/__tests__/models/bot/botProject.test.ts +++ b/Composer/packages/server/__tests__/models/bot/botProject.test.ts @@ -238,7 +238,11 @@ describe('lu operation', () => { it('should update diagnostics when lu content is invalid', async () => { const id = 'root'; - const content = 'hello \n hello3'; + let content = '## hello \n - hello'; + await proj.createLuFile(id, content); + + content = 'hello \n hello3'; + const luFiles = await proj.updateLuFile(id, content); const result = luFiles.find(f => f.id === id); expect(result?.diagnostics?.length).toBeGreaterThan(0); diff --git a/Composer/packages/tools/language-servers/language-generation/package.json b/Composer/packages/tools/language-servers/language-generation/package.json index 20652e362e..fdd32946fb 100644 --- a/Composer/packages/tools/language-servers/language-generation/package.json +++ b/Composer/packages/tools/language-servers/language-generation/package.json @@ -26,7 +26,7 @@ "rimraf": "^2.6.3", "ts-jest": "^24.1.0", "ts-node": "^8.4.1", - "typescript": "^3.7.2", + "typescript": "3.7.2", "vscode-ws-jsonrpc": "^0.1.1", "ws": "^7.2.0" } From afc35813aef02a0264711e07dd1046a85c94ad6e Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 01:56:02 +0800 Subject: [PATCH 25/45] add e2e test --- .../integration/NotificationPage.spec.ts | 52 +++++++++++++++++-- .../pages/notifications/NotificationList.tsx | 8 +-- .../client/src/pages/notifications/styles.ts | 5 ++ Composer/yarn.lock | 11 ---- 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index 5fac7bc750..d84b9c9abd 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -11,14 +11,21 @@ context('Notification Page', () => { it('can show lg syntax error ', () => { cy.visitPage('Bot Responses'); - cy.get('.toggleEditMode button').click(); + cy.get('.toggleEditMode button').as('switchButton'); + cy.get('@switchButton').click(); cy.get('textarea').type('test lg syntax error'); cy.visitPage('Notifications'); cy.get('[data-testid="notifications-table-view"]').within(() => { - cy.findAllByText('common.lg').should('exist'); + cy.findAllByText('common.lg') + .should('exist') + .first() + .dblclick(); }); + + cy.findAllByText('Bot Responses').should('exist'); + cy.get('@switchButton').should('be.disabled'); }); it('can show lu syntax error ', () => { @@ -32,7 +39,46 @@ context('Notification Page', () => { cy.visitPage('Notifications'); cy.get('[data-testid="notifications-table-view"]').within(() => { - cy.findAllByText('Main.lu').should('exist'); + cy.findAllByText('Main.lu') + .should('exist') + .first() + .dblclick(); + }); + + cy.findAllByText('__TestTodoSample.Main').should('exist'); + }); + + it('can show dialog expression error ', () => { + cy.visitPage('Design Flow'); + + cy.findByTestId('ProjectTree').within(() => { + cy.findByText('Greeting').click(); + }); + + cy.withinEditor('VisualEditor', () => { + cy.findAllByTestId('StepGroupAdd') + .first() + .click(); + cy.findByText('Create a condition') + .click() + .findByText('Branch: if/else') + .click(); + cy.wait(100); }); + + cy.withinEditor('FormEditor', () => { + cy.get('.ObjectItem input').type('()'); + }); + + cy.visitPage('Notifications'); + + cy.get('[data-testid="notifications-table-view"]').within(() => { + cy.findAllByText('Main.dialog') + .should('exist') + .first() + .dblclick(); + }); + + cy.findAllByText('Branch: if/else').should('exist'); }); }); diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 2bff13e781..7c0a8f20fb 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -7,7 +7,7 @@ import { DetailsList, DetailsListLayoutMode, SelectionMode, IColumn } from 'offi import { FontIcon } from 'office-ui-fabric-react/lib/Icon'; import { INotification } from './types'; -import { notification, typeIcon, listRoot, icons } from './styles'; +import { notification, typeIcon, listRoot, icons, columnItemText } from './styles'; export interface INotificationListProps { items: INotification[]; @@ -38,7 +38,7 @@ const columns: IColumn[] = [ isResizable: true, data: 'string', onRender: (item: INotification) => { - return {item.severity}; + return {item.severity}; }, isPadded: true, }, @@ -51,7 +51,7 @@ const columns: IColumn[] = [ isResizable: true, data: 'string', onRender: (item: INotification) => { - return {item.location}; + return {item.location}; }, isPadded: true, }, @@ -66,7 +66,7 @@ const columns: IColumn[] = [ isMultiline: true, data: 'string', onRender: (item: INotification) => { - return {item.message}; + return {item.message}; }, isPadded: true, }, diff --git a/Composer/packages/client/src/pages/notifications/styles.ts b/Composer/packages/client/src/pages/notifications/styles.ts index 980f728ff5..d1d9dbf467 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -42,6 +42,7 @@ export const typeIcon = icon => css` background: ${icon.background}; line-height: 24px; color: ${icon.color}; + cursor: pointer; `; export const notificationHeader = css` @@ -59,6 +60,10 @@ export const notificationHeaderText = css` font-weight: bold; `; +export const columnItemText = css` + cursor: pointer; +`; + export const root = css` display: flex; height: 100%; diff --git a/Composer/yarn.lock b/Composer/yarn.lock index b673b6a2d5..4b19d40cd5 100644 --- a/Composer/yarn.lock +++ b/Composer/yarn.lock @@ -15933,17 +15933,6 @@ ts-loader@^6.2.1: micromatch "^4.0.0" semver "^6.0.0" -ts-node@^8.3.0: - version "8.5.4" - resolved "https://registry.yarnpkg.com/ts-node/-/ts-node-8.5.4.tgz#a152add11fa19c221d0b48962c210cf467262ab2" - integrity sha512-izbVCRV68EasEPQ8MSIGBNK9dc/4sYJJKYA+IarMQct1RtEot6Xp0bXuClsbUSnKpg50ho+aOAx8en5c+y4OFw== - dependencies: - arg "^4.1.0" - diff "^4.0.1" - make-error "^1.1.1" - source-map-support "^0.5.6" - yn "^3.0.0" - ts-node@^8.4.1: version "8.4.1" resolved "https://registry.yarnpkg.com/ts-node/-/ts-node-8.4.1.tgz#270b0dba16e8723c9fa4f9b4775d3810fd994b4f" From 6d90ee7def1400a79524279b2911fb350c9defe3 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 02:39:19 +0800 Subject: [PATCH 26/45] use indexer types --- .../packages/client/src/TestController.tsx | 2 +- .../ProjectTree/TriggerCreationModal.tsx | 2 +- .../src/components/ProjectTree/index.tsx | 2 +- .../pages/language-generation/code-editor.tsx | 2 +- .../pages/language-generation/table-view.tsx | 6 ++++-- .../packages/client/src/store/action/dialog.ts | 2 +- Composer/packages/client/src/store/types.ts | 3 ++- .../packages/client/src/utils/dialogUtil.ts | 2 +- Composer/packages/client/src/utils/luUtil.ts | 2 +- .../obiformeditor/demo/src/index.tsx | 18 ++++++++++++++---- .../extensions/obiformeditor/package.json | 3 ++- .../fields/RecognizerField/InlineLuEditor.tsx | 2 +- .../src/Form/fields/RecognizerField/index.tsx | 3 ++- .../src/Form/widgets/IntentWidget.tsx | 3 ++- .../packages/lib/shared/src/types/shell.ts | 2 -- .../__tests__/models/bot/botProject.test.ts | 3 ++- .../server/src/models/bot/botProject.ts | 4 ++-- .../server/src/models/bot/luPublisher.ts | 2 +- 18 files changed, 39 insertions(+), 24 deletions(-) diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index 37dc18da61..463c775294 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -9,7 +9,7 @@ import { Spinner, SpinnerSize } from 'office-ui-fabric-react/lib/Spinner'; import { Callout } from 'office-ui-fabric-react/lib/Callout'; import { Stack } from 'office-ui-fabric-react/lib/Stack'; import formatMessage from 'format-message'; -import { DialogInfo } from '@bfc/shared'; +import { DialogInfo } from '@bfc/indexers'; import settingsStorage from './utils/dialogSettingStorage'; import { StoreContext } from './store'; diff --git a/Composer/packages/client/src/components/ProjectTree/TriggerCreationModal.tsx b/Composer/packages/client/src/components/ProjectTree/TriggerCreationModal.tsx index 71c9075202..323a36a202 100644 --- a/Composer/packages/client/src/components/ProjectTree/TriggerCreationModal.tsx +++ b/Composer/packages/client/src/components/ProjectTree/TriggerCreationModal.tsx @@ -11,7 +11,7 @@ import { Stack } from 'office-ui-fabric-react/lib/Stack'; import { IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; import { Dropdown } from 'office-ui-fabric-react/lib/Dropdown'; import get from 'lodash/get'; -import { DialogInfo } from '@bfc/shared'; +import { DialogInfo } from '@bfc/indexers'; import { addNewTrigger, diff --git a/Composer/packages/client/src/components/ProjectTree/index.tsx b/Composer/packages/client/src/components/ProjectTree/index.tsx index 358ededa04..01fd56c392 100644 --- a/Composer/packages/client/src/components/ProjectTree/index.tsx +++ b/Composer/packages/client/src/components/ProjectTree/index.tsx @@ -16,7 +16,7 @@ import { SearchBox } from 'office-ui-fabric-react/lib/SearchBox'; import { IIconProps } from 'office-ui-fabric-react/lib/Icon'; import cloneDeep from 'lodash/cloneDeep'; import formatMessage from 'format-message'; -import { DialogInfo, ITrigger } from '@bfc/shared'; +import { DialogInfo, ITrigger } from '@bfc/indexers'; import { StoreContext } from '../../store'; import { createSelectedPath, getFriendlyName } from '../../utils'; diff --git a/Composer/packages/client/src/pages/language-generation/code-editor.tsx b/Composer/packages/client/src/pages/language-generation/code-editor.tsx index 072b806aeb..2242dd8cb8 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -7,7 +7,7 @@ import { LgEditor, LGOption } from '@bfc/code-editor'; import get from 'lodash/get'; import debounce from 'lodash/debounce'; import isEmpty from 'lodash/isEmpty'; -import { LgFile } from '@bfc/shared'; +import { LgFile } from '@bfc/indexers'; import { editor } from '@bfcomposer/monaco-editor/esm/vs/editor/editor.api'; import { lgIndexer, Diagnostic } from '@bfc/indexers'; diff --git a/Composer/packages/client/src/pages/language-generation/table-view.tsx b/Composer/packages/client/src/pages/language-generation/table-view.tsx index a943d46f35..eebd8bc141 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -15,7 +15,7 @@ import { ScrollablePane, ScrollbarVisibility } from 'office-ui-fabric-react/lib/ import { Sticky, StickyPositionType } from 'office-ui-fabric-react/lib/Sticky'; import formatMessage from 'format-message'; import { NeutralColors, FontSizes } from '@uifabric/fluent-theme'; -import { DialogInfo, LgFile } from '@bfc/shared'; +import { DialogInfo, LgFile } from '@bfc/indexers'; import { LGTemplate } from 'botbuilder-lg'; import { lgIndexer } from '@bfc/indexers'; @@ -43,7 +43,9 @@ const TableView: React.FC = props => { useEffect(() => { if (isEmpty(lgFile)) return; let allTemplates: LGTemplate[] = []; - allTemplates = lgIndexer.parse(lgFile.content) as LGTemplate[]; + if (lgIndexer.isValid(lgFile.diagnostics) === true) { + allTemplates = lgIndexer.parse(lgFile.content) as LGTemplate[]; + } if (!activeDialog) { setTemplates(allTemplates); } else { diff --git a/Composer/packages/client/src/store/action/dialog.ts b/Composer/packages/client/src/store/action/dialog.ts index 9809e26c9d..bcd86e3c52 100644 --- a/Composer/packages/client/src/store/action/dialog.ts +++ b/Composer/packages/client/src/store/action/dialog.ts @@ -3,7 +3,7 @@ import clonedeep from 'lodash/cloneDeep'; import reject from 'lodash/reject'; -import { DialogInfo } from '@bfc/shared'; +import { DialogInfo } from '@bfc/indexers'; import debounce from 'lodash/debounce'; import { ActionCreator, State } from '../types'; diff --git a/Composer/packages/client/src/store/types.ts b/Composer/packages/client/src/store/types.ts index aef9416b44..50f1cfc33b 100644 --- a/Composer/packages/client/src/store/types.ts +++ b/Composer/packages/client/src/store/types.ts @@ -4,7 +4,8 @@ // TODO: remove this once we can expand the types /* eslint-disable @typescript-eslint/no-explicit-any */ import React from 'react'; -import { PromptTab, DialogInfo, BotSchemas, LgFile, LuFile, ProjectTemplate } from '@bfc/shared'; +import { PromptTab, BotSchemas, ProjectTemplate } from '@bfc/shared'; +import { DialogInfo, LgFile, LuFile } from '@bfc/indexers'; import { CreationFlowStatus, BotStatus } from '../constants'; diff --git a/Composer/packages/client/src/utils/dialogUtil.ts b/Composer/packages/client/src/utils/dialogUtil.ts index 5fb27a7898..588588b283 100644 --- a/Composer/packages/client/src/utils/dialogUtil.ts +++ b/Composer/packages/client/src/utils/dialogUtil.ts @@ -7,7 +7,7 @@ import set from 'lodash/set'; import cloneDeep from 'lodash/cloneDeep'; import { ExpressionEngine } from 'botbuilder-expression-parser'; import { IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; -import { DialogInfo } from '@bfc/shared'; +import { DialogInfo } from '@bfc/indexers'; import { getFocusPath } from './navigation'; import { upperCaseName } from './fileUtil'; diff --git a/Composer/packages/client/src/utils/luUtil.ts b/Composer/packages/client/src/utils/luUtil.ts index 923d0b8210..d0c993165b 100644 --- a/Composer/packages/client/src/utils/luUtil.ts +++ b/Composer/packages/client/src/utils/luUtil.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { LuFile, DialogInfo, LuDiagnostic } from '@bfc/shared'; +import { LuFile, DialogInfo, LuDiagnostic } from '@bfc/indexers'; export function getReferredFiles(luFiles: LuFile[], dialogs: DialogInfo[]) { return luFiles.filter(file => { diff --git a/Composer/packages/extensions/obiformeditor/demo/src/index.tsx b/Composer/packages/extensions/obiformeditor/demo/src/index.tsx index 8dee3657fe..ad2a535b16 100644 --- a/Composer/packages/extensions/obiformeditor/demo/src/index.tsx +++ b/Composer/packages/extensions/obiformeditor/demo/src/index.tsx @@ -6,7 +6,8 @@ import debounce from 'lodash/debounce'; import nanoid from 'nanoid'; import { initializeIcons } from '@uifabric/icons'; import { ExpressionEngine } from 'botbuilder-expression-parser'; -import { seedNewDialog, LuFile, DialogInfo, ShellApi } from '@bfc/shared'; +import { seedNewDialog, ShellApi } from '@bfc/shared'; +import { LuFile, DialogInfo } from '@bfc/indexers'; import Example from '../../src'; import { buildDialogOptions } from '../../src/Form/utils'; @@ -89,7 +90,10 @@ const luFiles: LuFile[] = [ parsedContent: { LUISJsonStructure: { intents: [{ name: 'FirstHello' }, { name: 'FirstGoodBye' }], - utterances: [{ intent: 'FirstHello', text: 'Hi' }, { intent: 'FirstGoodBye', text: 'Goodbye' }], + utterances: [ + { intent: 'FirstHello', text: 'Hi' }, + { intent: 'FirstGoodBye', text: 'Goodbye' }, + ], }, }, }, @@ -100,7 +104,10 @@ const luFiles: LuFile[] = [ parsedContent: { LUISJsonStructure: { intents: [{ name: 'SecondHello' }, { name: 'SecondGoodBye' }], - utterances: [{ intent: 'SecondHello', text: 'Good morning' }, { intent: 'SecondGoodBye', text: 'See ya!' }], + utterances: [ + { intent: 'SecondHello', text: 'Good morning' }, + { intent: 'SecondGoodBye', text: 'See ya!' }, + ], }, }, }, @@ -111,7 +118,10 @@ const luFiles: LuFile[] = [ parsedContent: { LUISJsonStructure: { intents: [{ name: 'ThirdHello' }, { name: 'ThirdGoodbye' }], - utterances: [{ intent: 'ThirdHello', text: 'Hello' }, { intent: 'ThirdGoodbye', text: 'Later' }], + utterances: [ + { intent: 'ThirdHello', text: 'Hello' }, + { intent: 'ThirdGoodbye', text: 'Later' }, + ], }, }, }, diff --git a/Composer/packages/extensions/obiformeditor/package.json b/Composer/packages/extensions/obiformeditor/package.json index 455cc095d3..f282ebb250 100644 --- a/Composer/packages/extensions/obiformeditor/package.json +++ b/Composer/packages/extensions/obiformeditor/package.json @@ -29,6 +29,7 @@ }, "dependencies": { "@bfc/code-editor": "*", + "@bfc/indexers": "*", "@bfc/shared": "*", "@bfcomposer/react-jsonschema-form": "1.6.5", "@emotion/core": "^10.0.17", @@ -91,4 +92,4 @@ "keywords": [ "react-component" ] -} +} \ No newline at end of file diff --git a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/InlineLuEditor.tsx b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/InlineLuEditor.tsx index 92aff48cd7..d6d628e2c4 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/InlineLuEditor.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/InlineLuEditor.tsx @@ -3,7 +3,7 @@ import React, { useState } from 'react'; import { LuEditor } from '@bfc/code-editor'; -import { LuFile } from '@bfc/shared'; +import { LuFile } from '@bfc/indexers'; interface InlineLuEditorProps { file: LuFile; diff --git a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx index 4cd2564923..49b589bc00 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx @@ -6,7 +6,8 @@ import formatMessage from 'format-message'; import { FieldProps } from '@bfcomposer/react-jsonschema-form'; import { Dropdown, ResponsiveMode, IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; import { Spinner, SpinnerSize } from 'office-ui-fabric-react/lib/Spinner'; -import { MicrosoftIRecognizer, LuFile } from '@bfc/shared'; +import { MicrosoftIRecognizer } from '@bfc/shared'; +import { LuFile } from '@bfc/indexers'; import { BaseField } from '../BaseField'; import { LoadingSpinner } from '../../../LoadingSpinner'; diff --git a/Composer/packages/extensions/obiformeditor/src/Form/widgets/IntentWidget.tsx b/Composer/packages/extensions/obiformeditor/src/Form/widgets/IntentWidget.tsx index 2b6645f936..5565dfc2f5 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/widgets/IntentWidget.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/widgets/IntentWidget.tsx @@ -5,7 +5,8 @@ import React from 'react'; import { Dropdown, ResponsiveMode, IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; import get from 'lodash/get'; import formatMessage from 'format-message'; -import { LuFile, DialogInfo, RegexRecognizer } from '@bfc/shared'; +import { RegexRecognizer } from '@bfc/shared'; +import { LuFile, DialogInfo } from '@bfc/indexers'; import { BFDWidgetProps, FormContext } from '../types'; diff --git a/Composer/packages/lib/shared/src/types/shell.ts b/Composer/packages/lib/shared/src/types/shell.ts index 9bc618cb10..0e46cc0975 100644 --- a/Composer/packages/lib/shared/src/types/shell.ts +++ b/Composer/packages/lib/shared/src/types/shell.ts @@ -4,8 +4,6 @@ import { LGTemplate as LgTemplate } from 'botbuilder-lg'; import { DialogInfo, LgFile, LuFile } from '@bfc/indexers'; -export * from '@bfc/indexers/lib/type'; - export interface EditorSchema { content?: { fieldTemplateOverrides?: any; diff --git a/Composer/packages/server/__tests__/models/bot/botProject.test.ts b/Composer/packages/server/__tests__/models/bot/botProject.test.ts index f6409be184..a1855b738d 100644 --- a/Composer/packages/server/__tests__/models/bot/botProject.test.ts +++ b/Composer/packages/server/__tests__/models/bot/botProject.test.ts @@ -4,7 +4,8 @@ import fs from 'fs'; import rimraf from 'rimraf'; -import { seedNewDialog, DialogInfo } from '@bfc/shared'; +import { seedNewDialog } from '@bfc/shared'; +import { DialogInfo } from '@bfc/indexers'; import { Path } from '../../../src/utility/path'; import { BotProject } from '../../../src/models/bot/botProject'; diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index 5d62c6624a..56e59f2cc3 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -3,8 +3,8 @@ import fs from 'fs'; -import { FileInfo, DialogInfo, LgFile, LuFile, getNewDesigner } from '@bfc/shared'; -import { dialogIndexer, lgIndexer } from '@bfc/indexers'; +import { getNewDesigner } from '@bfc/shared'; +import { FileInfo, DialogInfo, LgFile, LuFile, dialogIndexer, lgIndexer } from '@bfc/indexers'; import { luIndexer } from '@bfc/indexers/lib/luIndexer'; import { Path } from '../../utility/path'; diff --git a/Composer/packages/server/src/models/bot/luPublisher.ts b/Composer/packages/server/src/models/bot/luPublisher.ts index 4aa6baf774..625a7f2f32 100644 --- a/Composer/packages/server/src/models/bot/luPublisher.ts +++ b/Composer/packages/server/src/models/bot/luPublisher.ts @@ -3,7 +3,7 @@ import isEqual from 'lodash/isEqual'; import { runBuild } from 'lubuild'; -import { LuFile } from '@bfc/shared'; +import { LuFile } from '@bfc/indexers'; import { Path } from './../../utility/path'; import { IFileStorage } from './../storage/interface'; From 5a027ef6a2d4d9a70e6603bf550717f5aa3b7c95 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 19:33:41 +0800 Subject: [PATCH 27/45] add schema to check expression field --- .../client/src/pages/notifications/index.tsx | 2 +- .../client/src/store/reducer/index.ts | 6 +- .../lib/indexers/src/dialogIndexer.ts | 17 +++--- .../{utils => dialogUtils}/dialogChecker.ts | 25 ++------- .../extractExpressionDefinitions.ts | 56 +++++++++++++++++++ .../lib/indexers/src/dialogUtils/index.ts | 6 ++ .../lib/indexers/src/dialogUtils/types.ts | 48 ++++++++++++++++ Composer/packages/lib/indexers/src/index.ts | 1 + .../server/src/models/bot/botProject.ts | 20 ++++++- 9 files changed, 147 insertions(+), 34 deletions(-) rename Composer/packages/lib/indexers/src/{utils => dialogUtils}/dialogChecker.ts (63%) create mode 100644 Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts create mode 100644 Composer/packages/lib/indexers/src/dialogUtils/index.ts create mode 100644 Composer/packages/lib/indexers/src/dialogUtils/types.ts diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index b8d9dd7067..23e2f4b757 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -16,7 +16,7 @@ import { navigateTo } from './../../utils'; const navigations = { lg: (item: INotification) => { - navigateTo(`/language-generation/#line=${item.diagnostic.range.start.line || 0}`); + navigateTo(`/language-generation/#line=${item.diagnostic.range?.start.line || 0}`); }, lu: (item: INotification) => { navigateTo(`/dialogs/${item.id}`); diff --git a/Composer/packages/client/src/store/reducer/index.ts b/Composer/packages/client/src/store/reducer/index.ts index 0281d08df0..b982168e35 100644 --- a/Composer/packages/client/src/store/reducer/index.ts +++ b/Composer/packages/client/src/store/reducer/index.ts @@ -3,7 +3,7 @@ import get from 'lodash/get'; import set from 'lodash/set'; -import { dialogIndexer } from '@bfc/indexers/lib/dialogIndexer'; +import { dialogIndexer, extractExpressionDefinitions, ValidateFields } from '@bfc/indexers'; import { SensitiveProperties } from '@bfc/shared'; import { ActionTypes, FileTypes } from '../../constants'; @@ -71,7 +71,9 @@ const removeRecentProject: ReducerFunc = (state, { path }) => { const updateDialog: ReducerFunc = (state, { id, content }) => { state.dialogs = state.dialogs.map(dialog => { if (dialog.id === id) { - const result = dialogIndexer.parse(dialog.id, content); + state.schemas.sdk.content.definitions; + const expressions = extractExpressionDefinitions(state.schemas.sdk, 'content.definitions'); + const result = dialogIndexer.parse(dialog.id, content, new ValidateFields(expressions)); return { ...dialog, ...result }; } return dialog; diff --git a/Composer/packages/lib/indexers/src/dialogIndexer.ts b/Composer/packages/lib/indexers/src/dialogIndexer.ts index 91c4b4893e..d84fb10efd 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -4,8 +4,8 @@ import has from 'lodash/has'; import uniq from 'lodash/uniq'; +import { ValidateFields } from './dialogUtils/types'; import { ITrigger, DialogInfo, FileInfo } from './type'; -import { DialogChecker } from './utils/dialogChecker'; import { JsonWalk, VisitorFunc } from './utils/jsonWalk'; import { getBaseName } from './utils/help'; import { Diagnostic } from './diagnostic'; @@ -125,7 +125,7 @@ function ExtractReferredDialogs(dialog): string[] { } // check all fields -function CheckFields(dialog, id: string): Diagnostic[] { +function CheckFields(dialog, id: string, validateFields: ValidateFields): Diagnostic[] { const errors: Diagnostic[] = []; /** * @@ -135,6 +135,7 @@ function CheckFields(dialog, id: string): Diagnostic[] { * */ const visitor: VisitorFunc = (path: string, value: any): boolean => { // it's a valid schema dialog node. + const DialogChecker = validateFields.getDialogChecker(); if (has(value, '$type') && has(DialogChecker, value.$type)) { const matchedCheckers = DialogChecker[value.$type]; matchedCheckers.forEach(checker => { @@ -153,20 +154,20 @@ function CheckFields(dialog, id: string): Diagnostic[] { }); } -function validate(id: string, content): Diagnostic[] { +function validate(id: string, content, validateFields: ValidateFields): Diagnostic[] { try { - return CheckFields(content, id); + return CheckFields(content, id, validateFields); } catch (error) { return [new Diagnostic(error.message, id)]; } } -function parse(id: string, content) { +function parse(id: string, content: any, validateFields: ValidateFields) { const luFile = typeof content.recognizer === 'string' ? content.recognizer : ''; const lgFile = typeof content.generator === 'string' ? content.generator : ''; return { content, - diagnostics: validate(id, content), + diagnostics: validate(id, content, validateFields), referredDialogs: ExtractReferredDialogs(content), lgTemplates: ExtractLgTemplates(content), luIntents: ExtractLuIntents(content), @@ -176,7 +177,7 @@ function parse(id: string, content) { }; } -function index(files: FileInfo[], botName: string): DialogInfo[] { +function index(files: FileInfo[], botName: string, validateFields: ValidateFields): DialogInfo[] { const dialogs: DialogInfo[] = []; if (files.length !== 0) { for (const file of files) { @@ -191,7 +192,7 @@ function index(files: FileInfo[], botName: string): DialogInfo[] { displayName: isRoot ? `${botName}.Main` : id, content: dialogJson, relativePath: file.relativePath, - ...parse(id, dialogJson), + ...parse(id, dialogJson, validateFields), }; dialogs.push(dialog); } diff --git a/Composer/packages/lib/indexers/src/utils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts similarity index 63% rename from Composer/packages/lib/indexers/src/utils/dialogChecker.ts rename to Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index c91d15df8b..4e4fa23859 100644 --- a/Composer/packages/lib/indexers/src/utils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -4,15 +4,13 @@ import get from 'lodash/get'; import { ExpressionEngine } from 'botbuilder-expression-parser'; -import { Diagnostic } from './../diagnostic'; +import { Diagnostic } from '../diagnostic'; -const ExpressionParser = new ExpressionEngine(); +import { CheckerFunc } from './types'; -interface CheckerFunc { - (node: { path: string; value: any }): Diagnostic | null; // error msg -} +const ExpressionParser = new ExpressionEngine(); -function IsExpression(name: string): CheckerFunc { +export function IsExpression(name: string): CheckerFunc { return node => { let message = ''; const exp = get(node.value, name); @@ -43,7 +41,7 @@ enum EditArrayChangeTypes { } // eslint-disable-next-line @typescript-eslint/no-explicit-any -function EditArrayValueChecker(node: { path: string; value: any }): Diagnostic | null { +export function EditArrayValueChecker(node: { path: string; value: any }): Diagnostic | null { const changeType = get(node.value, 'changeType'); // when push and remove, value is required @@ -61,16 +59,3 @@ function EditArrayValueChecker(node: { path: string; value: any }): Diagnostic | return null; } - -/** - * Dialog Validation Rules - */ -// TODO: check field by schema. -export const DialogChecker: { [key: string]: CheckerFunc[] } = { - 'Microsoft.IfCondition': [IsExpression('condition')], - 'Microsoft.SwitchCondition': [IsExpression('condition')], - 'Microsoft.SetProperty': [IsExpression('property'), IsExpression('value')], - 'Microsoft.ForeachPage': [IsExpression('itemsProperty')], - 'Microsoft.Foreach': [IsExpression('itemsProperty')], - 'Microsoft.EditArray': [IsExpression('itemsProperty'), EditArrayValueChecker], -}; diff --git a/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts new file mode 100644 index 0000000000..a66dda5bf4 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +import has from 'lodash/has'; +import get from 'lodash/get'; + +import { VisitorFunc, JsonWalk } from '../utils/jsonWalk'; + +import { ISearchTarget, ISearchResult } from './types'; + +const defaultExpressionSearchTargets = [ + { + type: '$role', + value: 'expression', + }, +]; + +function findAllProperties(definition: any, searchTarget: ISearchTarget[]): string[] { + let properties: string[] = []; + + const visitor: VisitorFunc = (path: string, value: any): boolean => { + const result = searchTarget.reduce((result: string[], target) => { + if (has(value, target.type) && value[target.type] === target.value) { + const parents = path.split('.'); + result.push(parents[parents.length - 1]); + } + return result; + }, []); + properties = [...properties, ...result]; + return false; + }; + JsonWalk('$', definition, visitor); + return properties; +} + +/** + * @param schema schema. + * @param definitionPath The path of the definition in schema. + * @param searchTargets + * @returns all types. + */ +export function extractExpressionDefinitions( + schema: any, + definitionPath: string, + searchTargets?: ISearchTarget[] +): ISearchResult { + if (!schema) return {}; + const definitions = get(schema, definitionPath); + const result = Object.keys(definitions).reduce((result: ISearchResult, key: string) => { + const properties = findAllProperties(definitions[key], searchTargets || defaultExpressionSearchTargets); + if (properties.length) { + result[key] = properties; + } + return result; + }, {}); + return result; +} diff --git a/Composer/packages/lib/indexers/src/dialogUtils/index.ts b/Composer/packages/lib/indexers/src/dialogUtils/index.ts new file mode 100644 index 0000000000..4a6214ba96 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/index.ts @@ -0,0 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +export * from './dialogChecker'; +export * from './extractExpressionDefinitions'; +export * from './types'; diff --git a/Composer/packages/lib/indexers/src/dialogUtils/types.ts b/Composer/packages/lib/indexers/src/dialogUtils/types.ts new file mode 100644 index 0000000000..ee15609315 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/types.ts @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import keys from 'lodash/keys'; + +import { Diagnostic } from '../diagnostic'; + +import { IsExpression } from './dialogChecker'; + +export interface ISearchTarget { + type: string; + value: string; +} + +export interface IDefinitions { + [key: string]: any; +} + +export interface ISearchResult { + [key: string]: string[]; +} + +export interface CheckerFunc { + (node: { path: string; value: any }): Diagnostic | null; // error msg +} + +export class ValidateFields { + constructor(expressions: ISearchResult) { + this.expressions = expressions; + } + + public expressions: ISearchResult; + + getDialogChecker(): { [key: string]: CheckerFunc[] } { + const result = {}; + keys(this.expressions).map((key: string) => { + const temp = this.expressions[key].map((property: string) => { + return IsExpression(property); + }); + if (result[key]) { + result[key] = { ...result[key], ...temp }; + } else { + result[key] = temp; + } + }); + return result; + } +} diff --git a/Composer/packages/lib/indexers/src/index.ts b/Composer/packages/lib/indexers/src/index.ts index 62d3e6a040..7ca8969ba8 100644 --- a/Composer/packages/lib/indexers/src/index.ts +++ b/Composer/packages/lib/indexers/src/index.ts @@ -6,3 +6,4 @@ export * from './lgIndexer'; export * from './type'; export * from './diagnostic'; export * from './lgUtils'; +export * from './dialogUtils'; diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index 56e59f2cc3..af9a8adb2d 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -4,7 +4,16 @@ import fs from 'fs'; import { getNewDesigner } from '@bfc/shared'; -import { FileInfo, DialogInfo, LgFile, LuFile, dialogIndexer, lgIndexer } from '@bfc/indexers'; +import { + FileInfo, + DialogInfo, + LgFile, + LuFile, + dialogIndexer, + lgIndexer, + extractExpressionDefinitions, + ValidateFields, +} from '@bfc/indexers'; import { luIndexer } from '@bfc/indexers/lib/luIndexer'; import { Path } from '../../utility/path'; @@ -66,7 +75,7 @@ export class BotProject { public index = async () => { this.files = await this._getFiles(); this.settings = await this.getEnvSettings(this.environment.getDefaultSlot(), false); - this.dialogs = dialogIndexer.index(this.files, this.name); + this.dialogs = this.indexDialog(); this.lgFiles = lgIndexer.index(this.files); this.luFiles = (await luIndexer.index(this.files)) as LuFile[]; // ludown parser is async await this._checkProjectStructure(); @@ -420,13 +429,18 @@ export class BotProject { await this.reindex(relativePath); }; + private indexDialog() { + const expressions = extractExpressionDefinitions(this.getSchemas().sdk.content, 'definitions'); + return dialogIndexer.index(this.files, this.name, new ValidateFields(expressions)); + } + // re index according to file change in a certain path private reindex = async (filePath: string) => { const fileExtension = Path.extname(filePath); // only call the specific indexer to re-index switch (fileExtension) { case '.dialog': - this.dialogs = dialogIndexer.index(this.files, this.name); + this.dialogs = this.indexDialog(); break; case '.lg': this.lgFiles = lgIndexer.index(this.files); From be2c128928efe704d855722427586c38876ba4b0 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 19:56:22 +0800 Subject: [PATCH 28/45] make empty expression as waining --- .../packages/lib/indexers/src/dialogUtils/dialogChecker.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index 4e4fa23859..5a9d910a66 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -5,6 +5,7 @@ import get from 'lodash/get'; import { ExpressionEngine } from 'botbuilder-expression-parser'; import { Diagnostic } from '../diagnostic'; +import { DiagnosticSeverity } from './../diagnostic'; import { CheckerFunc } from './types'; @@ -13,8 +14,10 @@ const ExpressionParser = new ExpressionEngine(); export function IsExpression(name: string): CheckerFunc { return node => { let message = ''; + let severity = DiagnosticSeverity.Error; const exp = get(node.value, name); if (!exp) { + severity = DiagnosticSeverity.Warning; message = `In ${node.path}: ${node.value.$type}: ${name} is missing or empty`; } else { try { @@ -24,7 +27,7 @@ export function IsExpression(name: string): CheckerFunc { } } if (message) { - const diagnostic = new Diagnostic(message, ''); + const diagnostic = new Diagnostic(message, '', severity); diagnostic.path = node.path; return diagnostic; } From 9ebf451d15b531498967cc5db0325804ad73fcde Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 21:05:40 +0800 Subject: [PATCH 29/45] fix lint and e2e test --- .../packages/client/src/TestController.tsx | 20 ++++++++++--------- .../indexers/src/dialogUtils/dialogChecker.ts | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index 463c775294..279774e783 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -9,7 +9,7 @@ import { Spinner, SpinnerSize } from 'office-ui-fabric-react/lib/Spinner'; import { Callout } from 'office-ui-fabric-react/lib/Callout'; import { Stack } from 'office-ui-fabric-react/lib/Stack'; import formatMessage from 'format-message'; -import { DialogInfo } from '@bfc/indexers'; +import { DialogInfo, DiagnosticSeverity, Diagnostic } from '@bfc/indexers'; import settingsStorage from './utils/dialogSettingStorage'; import { StoreContext } from './store'; @@ -53,7 +53,8 @@ export const TestController: React.FC = () => { const { connectBot, reloadBot, onboardingAddCoachMarkRef, publishLuis, startBot } = actions; const connected = botStatus === BotStatus.connected; const addRef = useCallback(startBot => onboardingAddCoachMarkRef({ startBot }), []); - const showError = notifications.filter(n => n.severity === 'Error').length > 0; + const errorLength = notifications.filter(n => n.severity === 'Error').length; + const showError = errorLength > 0; useEffect(() => { toStartBot && handleClick(); @@ -72,16 +73,17 @@ export const TestController: React.FC = () => { } async function handleClick() { - const dialogErrors = dialogs.reduce((result, dialog) => { - if (dialog.diagnostics.length !== 0) { - return result.concat([dialog]); + const diagnostics = dialogs.reduce((result, dialog) => { + const errors = dialog.diagnostics.filter(n => n.severity === DiagnosticSeverity.Error); + if (errors.length !== 0) { + return result.concat(errors); } return result; }, []); - if (dialogErrors.length !== 0) { + if (diagnostics.length !== 0) { const title = `StaticValidationError`; - const subTitle = dialogErrors.reduce((msg, dialog) => { - msg += `\n In ${dialog.id}.dialog: \n ${dialog.diagnostics.join('\n')} \n`; + const subTitle = diagnostics.reduce((msg, diagnostic) => { + msg += `${diagnostic.message} \n`; return msg; }, ''); @@ -181,7 +183,7 @@ export const TestController: React.FC = () => {
{showError && ( - {notifications.length} + {errorLength} Date: Thu, 5 Dec 2019 21:14:34 +0800 Subject: [PATCH 30/45] fix test --- Composer/packages/client/src/TestController.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index 279774e783..3a682c4a89 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -9,7 +9,7 @@ import { Spinner, SpinnerSize } from 'office-ui-fabric-react/lib/Spinner'; import { Callout } from 'office-ui-fabric-react/lib/Callout'; import { Stack } from 'office-ui-fabric-react/lib/Stack'; import formatMessage from 'format-message'; -import { DialogInfo, DiagnosticSeverity, Diagnostic } from '@bfc/indexers'; +import { DiagnosticSeverity, Diagnostic } from '@bfc/indexers'; import settingsStorage from './utils/dialogSettingStorage'; import { StoreContext } from './store'; From a2641474aaa4d51304dde1a132f98c10f58878b2 Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 21:54:19 +0800 Subject: [PATCH 31/45] add some unit test --- .../extractExpressionDefinitions.test.ts | 41 +++++++++++++++++++ .../lgUtils/models/LgMetaData.test.ts | 0 .../lgUtils/models/LgTemplateRef.test.ts | 0 .../parsers/parseLgParamString.test.ts | 0 .../parsers/parseLgTemplateName.test.ts | 0 .../parsers/parseLgTemplateRef.test.ts | 0 .../utils/help.test.ts | 0 .../utils/jsonWalk.test.ts | 0 .../extractExpressionDefinitions.ts | 1 + 9 files changed, 42 insertions(+) create mode 100644 Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts rename Composer/packages/lib/indexers/{__test__ => __tests__}/lgUtils/models/LgMetaData.test.ts (100%) rename Composer/packages/lib/indexers/{__test__ => __tests__}/lgUtils/models/LgTemplateRef.test.ts (100%) rename Composer/packages/lib/indexers/{__test__ => __tests__}/lgUtils/parsers/parseLgParamString.test.ts (100%) rename Composer/packages/lib/indexers/{__test__ => __tests__}/lgUtils/parsers/parseLgTemplateName.test.ts (100%) rename Composer/packages/lib/indexers/{__test__ => __tests__}/lgUtils/parsers/parseLgTemplateRef.test.ts (100%) rename Composer/packages/lib/indexers/{__test__ => __tests__}/utils/help.test.ts (100%) rename Composer/packages/lib/indexers/{__test__ => __tests__}/utils/jsonWalk.test.ts (100%) diff --git a/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts b/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts new file mode 100644 index 0000000000..97acea0243 --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License + +import { extractExpressionDefinitions } from '../../src'; + +describe('extractExpressionDefinitions', () => { + it('should return all expressions properties', () => { + const schema = { + definitions: { + 'Ms.type1': { + properties: { + $type: { + title: '$type', + }, + $copy: { + title: '$copy', + }, + condition: { + $role: 'expression', + }, + items: { + $role: 'expression', + }, + }, + }, + 'Ms.type2': { + properties: { + $type: { + title: '$type', + }, + }, + }, + }, + }; + expect(extractExpressionDefinitions(schema, 'definitions')).toEqual({ 'Ms.type1': ['condition', 'items'] }); + expect(extractExpressionDefinitions(schema, '')).toEqual({}); + expect(extractExpressionDefinitions(schema, 'definitions', [{ type: 'title', value: '$copy' }])).toEqual({ + 'Ms.type1': ['$copy'], + }); + }); +}); diff --git a/Composer/packages/lib/indexers/__test__/lgUtils/models/LgMetaData.test.ts b/Composer/packages/lib/indexers/__tests__/lgUtils/models/LgMetaData.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/lgUtils/models/LgMetaData.test.ts rename to Composer/packages/lib/indexers/__tests__/lgUtils/models/LgMetaData.test.ts diff --git a/Composer/packages/lib/indexers/__test__/lgUtils/models/LgTemplateRef.test.ts b/Composer/packages/lib/indexers/__tests__/lgUtils/models/LgTemplateRef.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/lgUtils/models/LgTemplateRef.test.ts rename to Composer/packages/lib/indexers/__tests__/lgUtils/models/LgTemplateRef.test.ts diff --git a/Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgParamString.test.ts b/Composer/packages/lib/indexers/__tests__/lgUtils/parsers/parseLgParamString.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgParamString.test.ts rename to Composer/packages/lib/indexers/__tests__/lgUtils/parsers/parseLgParamString.test.ts diff --git a/Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateName.test.ts b/Composer/packages/lib/indexers/__tests__/lgUtils/parsers/parseLgTemplateName.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateName.test.ts rename to Composer/packages/lib/indexers/__tests__/lgUtils/parsers/parseLgTemplateName.test.ts diff --git a/Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateRef.test.ts b/Composer/packages/lib/indexers/__tests__/lgUtils/parsers/parseLgTemplateRef.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/lgUtils/parsers/parseLgTemplateRef.test.ts rename to Composer/packages/lib/indexers/__tests__/lgUtils/parsers/parseLgTemplateRef.test.ts diff --git a/Composer/packages/lib/indexers/__test__/utils/help.test.ts b/Composer/packages/lib/indexers/__tests__/utils/help.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/utils/help.test.ts rename to Composer/packages/lib/indexers/__tests__/utils/help.test.ts diff --git a/Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts b/Composer/packages/lib/indexers/__tests__/utils/jsonWalk.test.ts similarity index 100% rename from Composer/packages/lib/indexers/__test__/utils/jsonWalk.test.ts rename to Composer/packages/lib/indexers/__tests__/utils/jsonWalk.test.ts diff --git a/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts index a66dda5bf4..a86e26fbe0 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts @@ -45,6 +45,7 @@ export function extractExpressionDefinitions( ): ISearchResult { if (!schema) return {}; const definitions = get(schema, definitionPath); + if (!definitions) return {}; const result = Object.keys(definitions).reduce((result: ISearchResult, key: string) => { const properties = findAllProperties(definitions[key], searchTargets || defaultExpressionSearchTargets); if (properties.length) { From 7f8e23313a8af5854b6eae2dfdcce45ae60a269f Mon Sep 17 00:00:00 2001 From: leilzh Date: Thu, 5 Dec 2019 23:10:31 +0800 Subject: [PATCH 32/45] fix a dialog nav bug --- Composer/cypress/integration/NotificationPage.spec.ts | 2 +- Composer/packages/client/src/pages/notifications/index.tsx | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index d84b9c9abd..06b4725aed 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -63,7 +63,7 @@ context('Notification Page', () => { .click() .findByText('Branch: if/else') .click(); - cy.wait(100); + cy.wait(1000); }); cy.withinEditor('FormEditor', () => { diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 23e2f4b757..47bc26bc86 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -28,7 +28,12 @@ const navigations = { let uri = `/dialogs/${item.id}`; if (path) { const matchTriggers = /triggers\[(\d+)\]/g.exec(path); - const matchActions = /actions\[(\d+)\]/g.exec(path); + const actionPatt = /actions\[(\d+)\]/g; + let temp: RegExpExecArray | null = null; + let matchActions: RegExpExecArray | null = null; + while ((temp = actionPatt.exec(path)) !== null) { + matchActions = temp; + } const trigger = matchTriggers ? `triggers[${+matchTriggers[1]}]` : ''; const action = matchActions ? `actions[${+matchActions[1]}]` : ''; if (trigger) { From 940e37a868af14abdb044df1a52138be9466a171 Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Dec 2019 15:10:55 +0800 Subject: [PATCH 33/45] fix some ui comments --- .../packages/client/src/TestController.tsx | 2 +- .../notifications/NotificationHeader.tsx | 16 +++++-------- .../pages/notifications/NotificationList.tsx | 14 +++++++---- .../client/src/pages/notifications/index.tsx | 4 ++-- .../client/src/pages/notifications/styles.ts | 20 +++++----------- .../client/src/pages/notifications/types.ts | 2 ++ .../pages/notifications/useNotifications.tsx | 23 +++++++------------ .../packages/client/src/utils/dialogUtil.ts | 10 ++++++++ .../indexers/src/dialogUtils/dialogChecker.ts | 6 ++--- 9 files changed, 48 insertions(+), 49 deletions(-) diff --git a/Composer/packages/client/src/TestController.tsx b/Composer/packages/client/src/TestController.tsx index 3a682c4a89..735a0fa9f8 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -48,7 +48,7 @@ export const TestController: React.FC = () => { const [error, setError] = useState({ title: '', message: '' }); const [luisPublishSucceed, setLuisPublishSucceed] = useState(true); const botActionRef = useRef(null); - const { notifications } = useNotifications(); + const notifications = useNotifications(); const { botEndpoint, botName, botStatus, dialogs, toStartBot, luFiles, settings } = state; const { connectBot, reloadBot, onboardingAddCoachMarkRef, publishLuis, startBot } = actions; const connected = botStatus === BotStatus.connected; diff --git a/Composer/packages/client/src/pages/notifications/NotificationHeader.tsx b/Composer/packages/client/src/pages/notifications/NotificationHeader.tsx index 92c43f21fe..5287df0a5c 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationHeader.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationHeader.tsx @@ -5,15 +5,15 @@ import { jsx } from '@emotion/core'; import { Dropdown, IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; import formatMessage from 'format-message'; -import { useMemo } from 'react'; +import { DiagnosticSeverity } from './types'; import { notificationHeader, notificationHeaderText, dropdownStyles } from './styles'; -const createOptions = (items: string[]): IDropdownOption[] => { +const createOptions = (): IDropdownOption[] => { const defaultOptions: IDropdownOption[] = [ - { key: formatMessage('Show All Locations'), text: formatMessage('All'), data: '', isSelected: true }, + { key: formatMessage('Show All Notifications'), text: formatMessage('All'), data: '', isSelected: true }, ]; - items.forEach(item => { + DiagnosticSeverity.forEach(item => { return defaultOptions.push({ key: item, text: item, data: item }); }); return defaultOptions; @@ -21,14 +21,10 @@ const createOptions = (items: string[]): IDropdownOption[] => { export interface INotificationHeader { onChange: (text: string) => void; - items: string[]; } export const NotificationHeader: React.FC = props => { - const { onChange, items } = props; - const options = useMemo(() => { - return createOptions(items); - }, [items]); + const { onChange } = props; return (
@@ -37,7 +33,7 @@ export const NotificationHeader: React.FC = props => { onChange={(event, option) => { if (option) onChange(option.data); }} - options={options} + options={createOptions()} styles={dropdownStyles} data-testid="notifications-dropdown" /> diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 7c0a8f20fb..1db5f6f739 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -7,7 +7,7 @@ import { DetailsList, DetailsListLayoutMode, SelectionMode, IColumn } from 'offi import { FontIcon } from 'office-ui-fabric-react/lib/Icon'; import { INotification } from './types'; -import { notification, typeIcon, listRoot, icons, columnItemText } from './styles'; +import { notification, typeIcon, listRoot, icons } from './styles'; export interface INotificationListProps { items: INotification[]; @@ -31,6 +31,7 @@ const columns: IColumn[] = [ { key: 'Notification Type', name: 'Type', + className: notification.columnCell, fieldName: 'type', minWidth: 70, maxWidth: 90, @@ -38,26 +39,28 @@ const columns: IColumn[] = [ isResizable: true, data: 'string', onRender: (item: INotification) => { - return {item.severity}; + return {item.severity}; }, isPadded: true, }, { key: 'Notification Location', name: 'Location', + className: notification.columnCell, fieldName: 'location', minWidth: 70, maxWidth: 90, isResizable: true, data: 'string', onRender: (item: INotification) => { - return {item.location}; + return {item.location}; }, isPadded: true, }, { key: 'Notification Detail', name: 'Message', + className: notification.columnCell, fieldName: 'message', minWidth: 70, maxWidth: 90, @@ -66,7 +69,7 @@ const columns: IColumn[] = [ isMultiline: true, data: 'string', onRender: (item: INotification) => { - return {item.message}; + return {item.message}; }, isPadded: true, }, @@ -85,6 +88,9 @@ export const NotificationList: React.FC = props => { layoutMode={DetailsListLayoutMode.justified} isHeaderVisible={true} onItemInvoked={onItemInvoked} + onRenderItemColumn={() => { + return
test
; + }} />
); diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 47bc26bc86..a62b4c9e72 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -48,14 +48,14 @@ const navigations = { }; const Notifications: React.FC = () => { const [filter, setFilter] = useState(''); - const { notifications, locations } = useNotifications(filter); + const notifications = useNotifications(filter); const handleItemInvoked = (item: INotification) => { navigations[item.type](item); }; return (
- +
); diff --git a/Composer/packages/client/src/pages/notifications/styles.ts b/Composer/packages/client/src/pages/notifications/styles.ts index d1d9dbf467..72cf81732e 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -17,16 +17,12 @@ export const notification = mergeStyleSets({ }, typeIconCell: { textAlign: 'center', - selectors: { - '&:before': { - content: '.', - display: 'inline-block', - verticalAlign: 'middle', - height: '100%', - width: '0px', - visibility: 'hidden', - }, - }, + cursor: 'pointer', + padding: 0, + }, + columnCell: { + cursor: 'pointer', + padding: 0, }, }); @@ -60,10 +56,6 @@ export const notificationHeaderText = css` font-weight: bold; `; -export const columnItemText = css` - cursor: pointer; -`; - export const root = css` display: flex; height: 100%; diff --git a/Composer/packages/client/src/pages/notifications/types.ts b/Composer/packages/client/src/pages/notifications/types.ts index 4689193a4d..bce5eef0a0 100644 --- a/Composer/packages/client/src/pages/notifications/types.ts +++ b/Composer/packages/client/src/pages/notifications/types.ts @@ -9,3 +9,5 @@ export interface INotification { message: string; diagnostic: any; } + +export const DiagnosticSeverity = ['Error', 'Warning']; //'Information', 'Hint' diff --git a/Composer/packages/client/src/pages/notifications/useNotifications.tsx b/Composer/packages/client/src/pages/notifications/useNotifications.tsx index c77b11ce27..4d4b46e0ec 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -5,10 +5,9 @@ import { useContext, useMemo } from 'react'; import { lgIndexer } from '@bfc/indexers'; import { StoreContext } from '../../store'; +import { replaceDialogDiagnosticLabel } from '../../utils'; -import { INotification } from './types'; - -const DiagnosticSeverity = ['Error', 'Warning']; //'Information', 'Hint' +import { INotification, DiagnosticSeverity } from './types'; export default function useNotifications(filter?: string) { const { state } = useContext(StoreContext); @@ -16,16 +15,14 @@ export default function useNotifications(filter?: string) { const memoized = useMemo(() => { const notifactions: INotification[] = []; - const locations = new Set(); dialogs.forEach(dialog => { dialog.diagnostics.map(diagnostic => { const location = `${dialog.id}.dialog`; - locations.add(location); notifactions.push({ type: 'dialog', location, - message: diagnostic.message, - severity: DiagnosticSeverity[diagnostic.severity], + message: `In ${replaceDialogDiagnosticLabel(diagnostic.path)} ${diagnostic.message}`, + severity: DiagnosticSeverity[diagnostic.severity] || '', diagnostic, id: dialog.id, }); @@ -34,7 +31,6 @@ export default function useNotifications(filter?: string) { luFiles.forEach(lufile => { lufile.diagnostics.map(diagnostic => { const location = `${lufile.id}.lu`; - locations.add(location); notifactions.push({ type: 'lu', location, @@ -48,10 +44,9 @@ export default function useNotifications(filter?: string) { lgFiles.forEach(lgFile => { lgFile.diagnostics.map(diagnostic => { const location = `${lgFile.id}.lg`; - locations.add(location); notifactions.push({ type: 'lg', - severity: DiagnosticSeverity[diagnostic.severity], + severity: DiagnosticSeverity[diagnostic.severity] || '', location, message: lgIndexer.createSingleMessage(diagnostic), diagnostic, @@ -59,12 +54,10 @@ export default function useNotifications(filter?: string) { }); }); }); - return { notifactions, locations: Array.from(locations) }; + return notifactions; }, [dialogs, luFiles, lgFiles]); - const notifications: INotification[] = !filter - ? memoized.notifactions - : memoized.notifactions.filter(x => x.location === filter); + const notifications: INotification[] = !filter ? memoized : memoized.filter(x => x.severity === filter); - return { notifications, locations: memoized.locations }; + return notifications; } diff --git a/Composer/packages/client/src/utils/dialogUtil.ts b/Composer/packages/client/src/utils/dialogUtil.ts index 588588b283..b1c01b9c99 100644 --- a/Composer/packages/client/src/utils/dialogUtil.ts +++ b/Composer/packages/client/src/utils/dialogUtil.ts @@ -11,6 +11,7 @@ import { DialogInfo } from '@bfc/indexers'; import { getFocusPath } from './navigation'; import { upperCaseName } from './fileUtil'; +import { title } from './../pages/about/styles'; const ExpressionParser = new ExpressionEngine(); @@ -289,3 +290,12 @@ export function getSelected(focused: string): string { if (!focused) return ''; return focused.split('.')[0]; } + +export function replaceDialogDiagnosticLabel(path?: string): string { + if (!path) return ''; + let list = path.split(': '); + list = list.map(item => { + return ConceptLabels[item]?.title || item; + }); + return list.join(': '); +} diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index 323f251fef..35e653ec2f 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -18,17 +18,17 @@ export function IsExpression(name: string): CheckerFunc { const exp = get(node.value, name); if (!exp) { severity = DiagnosticSeverity.Warning; - message = `In ${node.path}: ${node.value.$type}: ${name} is missing or empty`; + message = `is missing or empty`; } else { try { ExpressionParser.parse(exp); } catch (error) { - message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; + message = `must be an expression`; } } if (message) { const diagnostic = new Diagnostic(message, '', severity); - diagnostic.path = node.path; + diagnostic.path = `${node.path}: ${node.value.$type}: ${name}`; return diagnostic; } return null; From c65df3a304bf16ab7f28ec4b4d83733691b92ad5 Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Dec 2019 15:35:58 +0800 Subject: [PATCH 34/45] remove some code --- Composer/packages/client/src/utils/dialogUtil.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/Composer/packages/client/src/utils/dialogUtil.ts b/Composer/packages/client/src/utils/dialogUtil.ts index b1c01b9c99..d82214f6ec 100644 --- a/Composer/packages/client/src/utils/dialogUtil.ts +++ b/Composer/packages/client/src/utils/dialogUtil.ts @@ -11,7 +11,6 @@ import { DialogInfo } from '@bfc/indexers'; import { getFocusPath } from './navigation'; import { upperCaseName } from './fileUtil'; -import { title } from './../pages/about/styles'; const ExpressionParser = new ExpressionEngine(); From bac053f45754569f5a0c6475b9c74a407c31919d Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Dec 2019 15:51:34 +0800 Subject: [PATCH 35/45] fix unit test --- .../client/__tests__/components/notificationHeader.test.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Composer/packages/client/__tests__/components/notificationHeader.test.js b/Composer/packages/client/__tests__/components/notificationHeader.test.js index 182299bdd8..3664a47724 100644 --- a/Composer/packages/client/__tests__/components/notificationHeader.test.js +++ b/Composer/packages/client/__tests__/components/notificationHeader.test.js @@ -7,17 +7,16 @@ import { fireEvent, render } from 'react-testing-library'; import { NotificationHeader } from '../../src/pages/notifications/NotificationHeader'; describe('', () => { - const items = ['test1', 'test2', 'test3']; it('should render the NotificationHeader', () => { const mockOnChange = jest.fn(() => null); - const { container } = render(); + const { container } = render(); expect(container).toHaveTextContent('Notifications'); expect(container).toHaveTextContent('All'); const dropdown = container.querySelector('[data-testid="notifications-dropdown"]'); fireEvent.click(dropdown); const test = document.querySelector('.ms-Dropdown-callout'); - expect(test).toHaveTextContent('test1'); - expect(test).toHaveTextContent('test2'); + expect(test).toHaveTextContent('Error'); + expect(test).toHaveTextContent('Warning'); }); }); From cd3c00693954ec419f8604cc150f9b98332beead Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Dec 2019 18:37:57 +0800 Subject: [PATCH 36/45] add empty check --- .../client/src/pages/notifications/styles.ts | 2 - .../client/src/store/reducer/index.ts | 6 +- .../extractExpressionDefinitions.test.ts | 26 +++++-- .../lib/indexers/src/dialogIndexer.ts | 33 ++++---- .../indexers/src/dialogUtils/dialogChecker.ts | 55 ++++---------- .../extractExpressionDefinitions.ts | 76 +++++++++---------- .../lib/indexers/src/dialogUtils/types.ts | 36 ++------- .../server/src/models/bot/botProject.ts | 14 +--- 8 files changed, 99 insertions(+), 149 deletions(-) diff --git a/Composer/packages/client/src/pages/notifications/styles.ts b/Composer/packages/client/src/pages/notifications/styles.ts index 72cf81732e..34264552f3 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -18,11 +18,9 @@ export const notification = mergeStyleSets({ typeIconCell: { textAlign: 'center', cursor: 'pointer', - padding: 0, }, columnCell: { cursor: 'pointer', - padding: 0, }, }); diff --git a/Composer/packages/client/src/store/reducer/index.ts b/Composer/packages/client/src/store/reducer/index.ts index b982168e35..8ddd5dbe20 100644 --- a/Composer/packages/client/src/store/reducer/index.ts +++ b/Composer/packages/client/src/store/reducer/index.ts @@ -3,7 +3,7 @@ import get from 'lodash/get'; import set from 'lodash/set'; -import { dialogIndexer, extractExpressionDefinitions, ValidateFields } from '@bfc/indexers'; +import { dialogIndexer } from '@bfc/indexers'; import { SensitiveProperties } from '@bfc/shared'; import { ActionTypes, FileTypes } from '../../constants'; @@ -71,9 +71,7 @@ const removeRecentProject: ReducerFunc = (state, { path }) => { const updateDialog: ReducerFunc = (state, { id, content }) => { state.dialogs = state.dialogs.map(dialog => { if (dialog.id === id) { - state.schemas.sdk.content.definitions; - const expressions = extractExpressionDefinitions(state.schemas.sdk, 'content.definitions'); - const result = dialogIndexer.parse(dialog.id, content, new ValidateFields(expressions)); + const result = dialogIndexer.parse(dialog.id, content, state.schemas.sdk.content); return { ...dialog, ...result }; } return dialog; diff --git a/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts b/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts index 97acea0243..68fd2a05be 100644 --- a/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts +++ b/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts @@ -1,13 +1,19 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License -import { extractExpressionDefinitions } from '../../src'; +import { getExpressionProperties } from '../../src'; describe('extractExpressionDefinitions', () => { it('should return all expressions properties', () => { const schema = { definitions: { 'Ms.type1': { + anyOf: [ + { + title: 'Type', + required: ['condition'], + }, + ], properties: { $type: { title: '$type', @@ -28,14 +34,24 @@ describe('extractExpressionDefinitions', () => { $type: { title: '$type', }, + items: { + $role: 'expression', + }, }, }, }, }; - expect(extractExpressionDefinitions(schema, 'definitions')).toEqual({ 'Ms.type1': ['condition', 'items'] }); - expect(extractExpressionDefinitions(schema, '')).toEqual({}); - expect(extractExpressionDefinitions(schema, 'definitions', [{ type: 'title', value: '$copy' }])).toEqual({ - 'Ms.type1': ['$copy'], + expect(getExpressionProperties(schema)).toEqual({ + 'Ms.type1': { + properties: ['condition', 'items'], + requiredTypes: { + condition: true, + }, + }, + 'Ms.type2': { + properties: ['items'], + requiredTypes: {}, + }, }); }); }); diff --git a/Composer/packages/lib/indexers/src/dialogIndexer.ts b/Composer/packages/lib/indexers/src/dialogIndexer.ts index d84fb10efd..62e495bef0 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -4,12 +4,13 @@ import has from 'lodash/has'; import uniq from 'lodash/uniq'; -import { ValidateFields } from './dialogUtils/types'; import { ITrigger, DialogInfo, FileInfo } from './type'; import { JsonWalk, VisitorFunc } from './utils/jsonWalk'; import { getBaseName } from './utils/help'; import { Diagnostic } from './diagnostic'; import { extractLgTemplateRefs } from './lgUtils/parsers/parseLgTemplateRef'; +import { getExpressionProperties } from './dialogUtils/extractExpressionDefinitions'; +import { IsExpression } from './dialogUtils'; // find out all lg templates given dialog function ExtractLgTemplates(dialog): string[] { const templates: string[] = []; @@ -125,8 +126,9 @@ function ExtractReferredDialogs(dialog): string[] { } // check all fields -function CheckFields(dialog, id: string, validateFields: ValidateFields): Diagnostic[] { +function CheckFields(dialog, id: string, schema: any): Diagnostic[] { const errors: Diagnostic[] = []; + const expressionProperties = getExpressionProperties(schema); /** * * @param path , jsonPath string @@ -135,15 +137,12 @@ function CheckFields(dialog, id: string, validateFields: ValidateFields): Diagno * */ const visitor: VisitorFunc = (path: string, value: any): boolean => { // it's a valid schema dialog node. - const DialogChecker = validateFields.getDialogChecker(); - if (has(value, '$type') && has(DialogChecker, value.$type)) { - const matchedCheckers = DialogChecker[value.$type]; - matchedCheckers.forEach(checker => { - const checkRes = checker.apply(null, [{ path, value }]); - if (checkRes) { - Array.isArray(checkRes) ? errors.push(...checkRes) : errors.push(checkRes); - } - }); + if (has(value, '$type') && has(expressionProperties, value.$type)) { + const diagnostics = IsExpression(path, value, { ...expressionProperties[value.$type] }); + + if (diagnostics) { + errors.push(...diagnostics); + } } return false; }; @@ -154,20 +153,20 @@ function CheckFields(dialog, id: string, validateFields: ValidateFields): Diagno }); } -function validate(id: string, content, validateFields: ValidateFields): Diagnostic[] { +function validate(id: string, content, schema: any): Diagnostic[] { try { - return CheckFields(content, id, validateFields); + return CheckFields(content, id, schema); } catch (error) { return [new Diagnostic(error.message, id)]; } } -function parse(id: string, content: any, validateFields: ValidateFields) { +function parse(id: string, content: any, schema: any) { const luFile = typeof content.recognizer === 'string' ? content.recognizer : ''; const lgFile = typeof content.generator === 'string' ? content.generator : ''; return { content, - diagnostics: validate(id, content, validateFields), + diagnostics: validate(id, content, schema), referredDialogs: ExtractReferredDialogs(content), lgTemplates: ExtractLgTemplates(content), luIntents: ExtractLuIntents(content), @@ -177,7 +176,7 @@ function parse(id: string, content: any, validateFields: ValidateFields) { }; } -function index(files: FileInfo[], botName: string, validateFields: ValidateFields): DialogInfo[] { +function index(files: FileInfo[], botName: string, schema: any): DialogInfo[] { const dialogs: DialogInfo[] = []; if (files.length !== 0) { for (const file of files) { @@ -192,7 +191,7 @@ function index(files: FileInfo[], botName: string, validateFields: ValidateField displayName: isRoot ? `${botName}.Main` : id, content: dialogJson, relativePath: file.relativePath, - ...parse(id, dialogJson, validateFields), + ...parse(id, dialogJson, schema), }; dialogs.push(dialog); } diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index 35e653ec2f..0e330db51c 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -6,18 +6,20 @@ import { ExpressionEngine } from 'botbuilder-expression-parser'; import { Diagnostic } from '../diagnostic'; -import { DiagnosticSeverity } from './../diagnostic'; import { CheckerFunc } from './types'; const ExpressionParser = new ExpressionEngine(); -export function IsExpression(name: string): CheckerFunc { - return node => { - let message = ''; - let severity = DiagnosticSeverity.Error; - const exp = get(node.value, name); - if (!exp) { - severity = DiagnosticSeverity.Warning; +export const IsExpression: CheckerFunc = ( + path, + value, + optional: { properties: string[]; requiredTypes: { [key: string]: boolean } } +) => { + let message = ''; + const { properties, requiredTypes } = optional; + return properties.reduce((result: Diagnostic[], property) => { + const exp = get(value, property); + if (!exp && requiredTypes[property]) { message = `is missing or empty`; } else { try { @@ -27,38 +29,11 @@ export function IsExpression(name: string): CheckerFunc { } } if (message) { - const diagnostic = new Diagnostic(message, '', severity); - diagnostic.path = `${node.path}: ${node.value.$type}: ${name}`; - return diagnostic; - } - return null; - }; -} - -enum EditArrayChangeTypes { - Push, - Pop, - Take, - Remove, - Clear, -} - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export function EditArrayValueChecker(node: { path: string; value: any }): Diagnostic | null { - const changeType = get(node.value, 'changeType'); - - // when push and remove, value is required - if (changeType === EditArrayChangeTypes.Push || changeType === EditArrayChangeTypes.Remove) { - const exp = get(node.value, 'value'); - try { - ExpressionParser.parse(exp); - } catch (error) { - const message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; const diagnostic = new Diagnostic(message, ''); - diagnostic.path = node.path; - return diagnostic; + diagnostic.path = `${path}: ${value.$type}: ${property}`; + result.push(diagnostic); } - } - + return result; + }, []); return null; -} +}; diff --git a/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts index a86e26fbe0..a2d41dd960 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts @@ -1,57 +1,53 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. import has from 'lodash/has'; -import get from 'lodash/get'; +import keys from 'lodash/keys'; import { VisitorFunc, JsonWalk } from '../utils/jsonWalk'; -import { ISearchTarget, ISearchResult } from './types'; +import { IDefinition, IExpressionProperties } from './types'; -const defaultExpressionSearchTargets = [ - { - type: '$role', - value: 'expression', - }, -]; - -function findAllProperties(definition: any, searchTarget: ISearchTarget[]): string[] { - let properties: string[] = []; +function findAllProperties(obj: any, searchTarget: (value: any) => boolean): string[] { + const properties: string[] = []; const visitor: VisitorFunc = (path: string, value: any): boolean => { - const result = searchTarget.reduce((result: string[], target) => { - if (has(value, target.type) && value[target.type] === target.value) { - const parents = path.split('.'); - result.push(parents[parents.length - 1]); - } - return result; - }, []); - properties = [...properties, ...result]; + if (searchTarget(value)) { + const parents = path.split('.'); + properties.push(parents[parents.length - 1]); + } return false; }; - JsonWalk('$', definition, visitor); + JsonWalk('$', obj, visitor); return properties; } -/** - * @param schema schema. - * @param definitionPath The path of the definition in schema. - * @param searchTargets - * @returns all types. - */ -export function extractExpressionDefinitions( - schema: any, - definitionPath: string, - searchTargets?: ISearchTarget[] -): ISearchResult { - if (!schema) return {}; - const definitions = get(schema, definitionPath); - if (!definitions) return {}; - const result = Object.keys(definitions).reduce((result: ISearchResult, key: string) => { - const properties = findAllProperties(definitions[key], searchTargets || defaultExpressionSearchTargets); +function findAllRequiredType(definition: IDefinition): { [key: string]: boolean } { + const types = definition.anyOf?.filter(x => x.title === 'Type'); + if (types && types.length) { + return types[0].required.reduce((result: { [key: string]: boolean }, t: string) => { + result[t] = true; + return result; + }, {}); + } + return {}; +} + +export function getExpressionProperties(schema: any): IExpressionProperties { + const definitions = schema.definitions; + + const expressionProperties: IExpressionProperties = {}; + + keys(definitions).forEach((key: string) => { + const definition = definitions[key]; + const requiredTypes = findAllRequiredType(definition); + const properties = findAllProperties(definition.properties, value => { + return has(value, '$role') && value.$role === 'expression'; + }); + if (properties.length) { - result[key] = properties; + expressionProperties[key] = { properties, requiredTypes }; } - return result; - }, {}); - return result; + }); + + return expressionProperties; } diff --git a/Composer/packages/lib/indexers/src/dialogUtils/types.ts b/Composer/packages/lib/indexers/src/dialogUtils/types.ts index ee15609315..6f77b02165 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/types.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/types.ts @@ -1,18 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import keys from 'lodash/keys'; - import { Diagnostic } from '../diagnostic'; -import { IsExpression } from './dialogChecker'; - export interface ISearchTarget { type: string; value: string; } -export interface IDefinitions { +export interface IDefinition { [key: string]: any; } @@ -20,29 +16,11 @@ export interface ISearchResult { [key: string]: string[]; } -export interface CheckerFunc { - (node: { path: string; value: any }): Diagnostic | null; // error msg +export interface IExpressionProperties { + [key: string]: { + properties: string[]; + requiredTypes: { [key: string]: boolean }; + }; } -export class ValidateFields { - constructor(expressions: ISearchResult) { - this.expressions = expressions; - } - - public expressions: ISearchResult; - - getDialogChecker(): { [key: string]: CheckerFunc[] } { - const result = {}; - keys(this.expressions).map((key: string) => { - const temp = this.expressions[key].map((property: string) => { - return IsExpression(property); - }); - if (result[key]) { - result[key] = { ...result[key], ...temp }; - } else { - result[key] = temp; - } - }); - return result; - } -} +export type CheckerFunc = (path: string, value: any, optional?: any) => Diagnostic[] | null; // error msg diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index af9a8adb2d..e41ed15983 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -4,16 +4,7 @@ import fs from 'fs'; import { getNewDesigner } from '@bfc/shared'; -import { - FileInfo, - DialogInfo, - LgFile, - LuFile, - dialogIndexer, - lgIndexer, - extractExpressionDefinitions, - ValidateFields, -} from '@bfc/indexers'; +import { FileInfo, DialogInfo, LgFile, LuFile, dialogIndexer, lgIndexer } from '@bfc/indexers'; import { luIndexer } from '@bfc/indexers/lib/luIndexer'; import { Path } from '../../utility/path'; @@ -430,8 +421,7 @@ export class BotProject { }; private indexDialog() { - const expressions = extractExpressionDefinitions(this.getSchemas().sdk.content, 'definitions'); - return dialogIndexer.index(this.files, this.name, new ValidateFields(expressions)); + return dialogIndexer.index(this.files, this.name, this.getSchemas().sdk.content); } // re index according to file change in a certain path From 02c9f2ff94c07493619ad0f15c01885a0fd3fe76 Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Dec 2019 19:00:35 +0800 Subject: [PATCH 37/45] make notifications single click --- .../pages/notifications/NotificationList.tsx | 30 ++++++++++++++----- .../client/src/pages/notifications/index.tsx | 4 +-- .../client/src/pages/notifications/styles.ts | 1 + 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 1db5f6f739..78b8177329 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -3,7 +3,14 @@ /** @jsx jsx */ import { jsx } from '@emotion/core'; -import { DetailsList, DetailsListLayoutMode, SelectionMode, IColumn } from 'office-ui-fabric-react/lib/DetailsList'; +import { + DetailsList, + DetailsListLayoutMode, + SelectionMode, + IColumn, + CheckboxVisibility, +} from 'office-ui-fabric-react/lib/DetailsList'; +import { Selection } from 'office-ui-fabric-react/lib/DetailsList'; import { FontIcon } from 'office-ui-fabric-react/lib/Icon'; import { INotification } from './types'; @@ -11,7 +18,7 @@ import { notification, typeIcon, listRoot, icons } from './styles'; export interface INotificationListProps { items: INotification[]; - onItemInvoked: (item: INotification) => void; + onItemClick: (item: INotification) => void; } const columns: IColumn[] = [ @@ -76,21 +83,28 @@ const columns: IColumn[] = [ ]; export const NotificationList: React.FC = props => { - const { items, onItemInvoked } = props; + const { items, onItemClick } = props; + + const selection = new Selection({ + onSelectionChanged: () => { + const item = selection.getSelection()[0] as INotification; + // selected item will be cleaned when folder path changed file will be undefine + // when no item selected. + onItemClick(item); + }, + }); return (
{ - return
test
; - }} + checkboxVisibility={CheckboxVisibility.hidden} />
); diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index a62b4c9e72..416a3cd03e 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -49,14 +49,14 @@ const navigations = { const Notifications: React.FC = () => { const [filter, setFilter] = useState(''); const notifications = useNotifications(filter); - const handleItemInvoked = (item: INotification) => { + const handleItemClick = (item: INotification) => { navigations[item.type](item); }; return (
- +
); }; diff --git a/Composer/packages/client/src/pages/notifications/styles.ts b/Composer/packages/client/src/pages/notifications/styles.ts index 34264552f3..1a5f5877d3 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -61,5 +61,6 @@ export const root = css` `; export const listRoot = css` + position: relative; overflow-y: auto; `; From 73e51deaae5e9c41d7097461c0c9c94d107c579e Mon Sep 17 00:00:00 2001 From: leilzh Date: Fri, 6 Dec 2019 19:03:30 +0800 Subject: [PATCH 38/45] clean the tsconfig in shared --- Composer/packages/lib/shared/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/lib/shared/tsconfig.json b/Composer/packages/lib/shared/tsconfig.json index 238c431e66..bdcff025b4 100644 --- a/Composer/packages/lib/shared/tsconfig.json +++ b/Composer/packages/lib/shared/tsconfig.json @@ -3,5 +3,5 @@ "compilerOptions": { "outDir": "lib" }, - "include": ["./src/**/*", "./__tests__/**/*", "../indexers/__test__/lgUtils"] + "include": ["./src/**/*", "./__tests__/**/*"] } From 0e4665ab697fea8e84ee5a5de1068230417883f4 Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Fri, 6 Dec 2019 23:27:51 +0800 Subject: [PATCH 39/45] fix list header --- .../pages/notifications/NotificationList.tsx | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 78b8177329..5ea5c8f539 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -10,7 +10,10 @@ import { IColumn, CheckboxVisibility, } from 'office-ui-fabric-react/lib/DetailsList'; +import { Sticky, StickyPositionType } from 'office-ui-fabric-react/lib/Sticky'; +import { TooltipHost } from 'office-ui-fabric-react/lib/Tooltip'; import { Selection } from 'office-ui-fabric-react/lib/DetailsList'; +import { ScrollablePane, ScrollbarVisibility } from 'office-ui-fabric-react/lib/ScrollablePane'; import { FontIcon } from 'office-ui-fabric-react/lib/Icon'; import { INotification } from './types'; @@ -82,30 +85,44 @@ const columns: IColumn[] = [ }, ]; +function onRenderDetailsHeader(props, defaultRender) { + return ( + + {defaultRender({ + ...props, + onRenderColumnHeaderTooltip: tooltipHostProps => , + })} + + ); +} + export const NotificationList: React.FC = props => { const { items, onItemClick } = props; const selection = new Selection({ onSelectionChanged: () => { - const item = selection.getSelection()[0] as INotification; - // selected item will be cleaned when folder path changed file will be undefine - // when no item selected. - onItemClick(item); + const items = selection.getSelection(); + if (items.length) { + onItemClick(items[0] as INotification); + } }, }); return (
- + + +
); }; From cecc4c20d608a7d41e7e863a89774862d762aed8 Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Fri, 6 Dec 2019 23:37:10 +0800 Subject: [PATCH 40/45] update the style --- Composer/packages/client/src/pages/notifications/styles.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/Composer/packages/client/src/pages/notifications/styles.ts b/Composer/packages/client/src/pages/notifications/styles.ts index 1a5f5877d3..ed61560b4d 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -63,4 +63,5 @@ export const root = css` export const listRoot = css` position: relative; overflow-y: auto; + flex-grow: 1; `; From f1b731917ca3f00346a5403a7f16367a76c37135 Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Fri, 6 Dec 2019 23:49:13 +0800 Subject: [PATCH 41/45] update the e2e test --- .../integration/NotificationPage.spec.ts | 72 +++++++++---------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index 06b4725aed..d69c035f8c 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -8,45 +8,45 @@ context('Notification Page', () => { cy.visitPage('Notifications'); }); - it('can show lg syntax error ', () => { - cy.visitPage('Bot Responses'); + // it('can show lg syntax error ', () => { + // cy.visitPage('Bot Responses'); - cy.get('.toggleEditMode button').as('switchButton'); - cy.get('@switchButton').click(); - cy.get('textarea').type('test lg syntax error'); + // cy.get('.toggleEditMode button').as('switchButton'); + // cy.get('@switchButton').click(); + // cy.get('textarea').type('test lg syntax error'); - cy.visitPage('Notifications'); + // cy.visitPage('Notifications'); - cy.get('[data-testid="notifications-table-view"]').within(() => { - cy.findAllByText('common.lg') - .should('exist') - .first() - .dblclick(); - }); + // cy.get('[data-testid="notifications-table-view"]').within(() => { + // cy.findAllByText('common.lg') + // .should('exist') + // .first() + // .click(); + // }); - cy.findAllByText('Bot Responses').should('exist'); - cy.get('@switchButton').should('be.disabled'); - }); + // cy.findAllByText('Bot Responses').should('exist'); + // cy.get('@switchButton').should('be.disabled'); + // }); - it('can show lu syntax error ', () => { - cy.visitPage('User Input'); + // it('can show lu syntax error ', () => { + // cy.visitPage('User Input'); - cy.get('.dialogNavTree button[title="__TestTodoSample.Main"]').click({ multiple: true }); + // cy.get('.dialogNavTree button[title="__TestTodoSample.Main"]').click({ multiple: true }); - cy.get('.toggleEditMode button').click(); - cy.get('textarea').type('test lu syntax error'); + // cy.get('.toggleEditMode button').click(); + // cy.get('textarea').type('test lu syntax error'); - cy.visitPage('Notifications'); + // cy.visitPage('Notifications'); - cy.get('[data-testid="notifications-table-view"]').within(() => { - cy.findAllByText('Main.lu') - .should('exist') - .first() - .dblclick(); - }); + // cy.get('[data-testid="notifications-table-view"]').within(() => { + // cy.findAllByText('Main.lu') + // .should('exist') + // .first() + // .click(); + // }); - cy.findAllByText('__TestTodoSample.Main').should('exist'); - }); + // cy.findAllByText('__TestTodoSample.Main').should('exist'); + // }); it('can show dialog expression error ', () => { cy.visitPage('Design Flow'); @@ -56,17 +56,11 @@ context('Notification Page', () => { }); cy.withinEditor('VisualEditor', () => { - cy.findAllByTestId('StepGroupAdd') - .first() - .click(); - cy.findByText('Create a condition') - .click() - .findByText('Branch: if/else') - .click(); - cy.wait(1000); + cy.findByText('Greeting').should('exist'); }); cy.withinEditor('FormEditor', () => { + cy.findByText('Condition').should('exist'); cy.get('.ObjectItem input').type('()'); }); @@ -76,9 +70,9 @@ context('Notification Page', () => { cy.findAllByText('Main.dialog') .should('exist') .first() - .dblclick(); + .click(); }); - cy.findAllByText('Branch: if/else').should('exist'); + cy.findAllByText('Greeting').should('exist'); }); }); From f58310350312c6817baf67642117bce73ca29203 Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Sat, 7 Dec 2019 00:38:33 +0800 Subject: [PATCH 42/45] fix some conflict --- .../integration/NotificationPage.spec.ts | 58 +++++++++---------- .../visual-designer/src/editors/ObiEditor.tsx | 3 +- .../src/store/NodeRendererContext.ts | 3 +- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index d69c035f8c..99835edfaa 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -8,45 +8,45 @@ context('Notification Page', () => { cy.visitPage('Notifications'); }); - // it('can show lg syntax error ', () => { - // cy.visitPage('Bot Responses'); + it('can show lg syntax error ', () => { + cy.visitPage('Bot Responses'); - // cy.get('.toggleEditMode button').as('switchButton'); - // cy.get('@switchButton').click(); - // cy.get('textarea').type('test lg syntax error'); + cy.get('.toggleEditMode button').as('switchButton'); + cy.get('@switchButton').click(); + cy.get('textarea').type('test lg syntax error'); - // cy.visitPage('Notifications'); + cy.visitPage('Notifications'); - // cy.get('[data-testid="notifications-table-view"]').within(() => { - // cy.findAllByText('common.lg') - // .should('exist') - // .first() - // .click(); - // }); + cy.get('[data-testid="notifications-table-view"]').within(() => { + cy.findAllByText('common.lg') + .should('exist') + .first() + .click(); + }); - // cy.findAllByText('Bot Responses').should('exist'); - // cy.get('@switchButton').should('be.disabled'); - // }); + cy.findAllByText('Bot Responses').should('exist'); + cy.get('@switchButton').should('be.disabled'); + }); - // it('can show lu syntax error ', () => { - // cy.visitPage('User Input'); + it('can show lu syntax error ', () => { + cy.visitPage('User Input'); - // cy.get('.dialogNavTree button[title="__TestTodoSample.Main"]').click({ multiple: true }); + cy.get('.dialogNavTree button[title="__TestTodoSample.Main"]').click({ multiple: true }); - // cy.get('.toggleEditMode button').click(); - // cy.get('textarea').type('test lu syntax error'); + cy.get('.toggleEditMode button').click(); + cy.get('textarea').type('test lu syntax error'); - // cy.visitPage('Notifications'); + cy.visitPage('Notifications'); - // cy.get('[data-testid="notifications-table-view"]').within(() => { - // cy.findAllByText('Main.lu') - // .should('exist') - // .first() - // .click(); - // }); + cy.get('[data-testid="notifications-table-view"]').within(() => { + cy.findAllByText('Main.lu') + .should('exist') + .first() + .click(); + }); - // cy.findAllByText('__TestTodoSample.Main').should('exist'); - // }); + cy.findAllByText('__TestTodoSample.Main').should('exist'); + }); it('can show dialog expression error ', () => { cy.visitPage('Design Flow'); diff --git a/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx b/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx index f0248d351a..4d5e68a192 100644 --- a/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx +++ b/Composer/packages/extensions/visual-designer/src/editors/ObiEditor.tsx @@ -5,7 +5,8 @@ import { jsx } from '@emotion/core'; import { useContext, FC, useEffect, useState, useRef } from 'react'; import { MarqueeSelection, Selection } from 'office-ui-fabric-react/lib/MarqueeSelection'; -import { deleteAction, deleteActions, LgTemplateRef, LgMetaData, LgFile } from '@bfc/shared'; +import { deleteAction, deleteActions, LgTemplateRef, LgMetaData } from '@bfc/shared'; +import { LgFile } from '@bfc/indexers'; import { NodeEventTypes } from '../constants/NodeEventTypes'; import { KeyboardCommandTypes, KeyboardPrimaryTypes } from '../constants/KeyboardCommandTypes'; diff --git a/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts b/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts index a2a8afa93f..b63ecc70ad 100644 --- a/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts +++ b/Composer/packages/extensions/visual-designer/src/store/NodeRendererContext.ts @@ -2,7 +2,8 @@ // Licensed under the MIT License. import React from 'react'; -import { ShellApi, LgFile } from '@bfc/shared'; +import { ShellApi } from '@bfc/shared'; +import { LgFile } from '@bfc/indexers'; type ShellApiFuncs = 'copyLgTemplate' | 'removeLgTemplate' | 'removeLgTemplates' | 'updateLgTemplate'; From 0b59aba68b7ffc0135910ee8eaa0aa167060e595 Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Sat, 7 Dec 2019 03:09:27 +0800 Subject: [PATCH 43/45] implement Pagination --- .../src/components/Pagination/index.tsx | 65 +++++++++++++++++++ .../client/src/components/Pagination/style.ts | 24 +++++++ .../pages/notifications/NotificationList.tsx | 44 +++++++++---- .../client/src/pages/notifications/styles.ts | 11 ++++ 4 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 Composer/packages/client/src/components/Pagination/index.tsx create mode 100644 Composer/packages/client/src/components/Pagination/style.ts diff --git a/Composer/packages/client/src/components/Pagination/index.tsx b/Composer/packages/client/src/components/Pagination/index.tsx new file mode 100644 index 0000000000..ee3448e033 --- /dev/null +++ b/Composer/packages/client/src/components/Pagination/index.tsx @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** @jsx jsx */ +import { jsx } from '@emotion/core'; +import { DefaultButton } from 'office-ui-fabric-react/lib/Button'; +import { Dropdown, IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown'; +import { useState, useEffect } from 'react'; + +import { container, dropdownStyles, text } from './style'; + +export interface IPaginationProps { + pageCount: number; + onChange: (page: number) => void; +} + +const createDropdownOption = (pageCount: number) => { + const options: IDropdownOption[] = []; + for (let i = 1; i <= pageCount; i++) { + options.push({ key: 'page' + i, text: '' + i }); + } + return options; +}; + +export const Pagination: React.FC = props => { + const [index, setIndex] = useState(0); + const { pageCount, onChange } = props; + + useEffect(() => { + setIndex(0); + }, [pageCount]); + + const handlePageSelected = (event: React.FormEvent, item?: IDropdownOption, index?: number) => { + setIndex(index || 0); + onChange(index || 0); + }; + + const hanglePreviousClick = () => { + const current = index - 1; + setIndex(current); + onChange(current + 1); + }; + + const hangleNextClick = () => { + const current = index + 1; + setIndex(current); + onChange(current + 1); + }; + + return ( +
+ + Page + + of {pageCount} + +
+ ); +}; diff --git a/Composer/packages/client/src/components/Pagination/style.ts b/Composer/packages/client/src/components/Pagination/style.ts new file mode 100644 index 0000000000..166287c7ed --- /dev/null +++ b/Composer/packages/client/src/components/Pagination/style.ts @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { css } from '@emotion/core'; +import { IDropdownStyles } from 'office-ui-fabric-react/lib/Dropdown'; + +export const dropdownStyles: Partial = { + dropdown: { width: 80 }, +}; + +export const container = css` + display: flex; + width: 400px; + height: 45px; + padding-top: 5px; + line-height: 30px; + background-color: transparent; + justify-content: space-around; +`; + +export const text = css` + font-size: 12px; + cursor: default; +`; diff --git a/Composer/packages/client/src/pages/notifications/NotificationList.tsx b/Composer/packages/client/src/pages/notifications/NotificationList.tsx index 5ea5c8f539..43088b5c07 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -15,15 +15,20 @@ import { TooltipHost } from 'office-ui-fabric-react/lib/Tooltip'; import { Selection } from 'office-ui-fabric-react/lib/DetailsList'; import { ScrollablePane, ScrollbarVisibility } from 'office-ui-fabric-react/lib/ScrollablePane'; import { FontIcon } from 'office-ui-fabric-react/lib/Icon'; +import { useMemo, useState } from 'react'; + +import { Pagination } from '../../components/Pagination'; import { INotification } from './types'; -import { notification, typeIcon, listRoot, icons } from './styles'; +import { notification, typeIcon, listRoot, icons, tableView, detailList } from './styles'; export interface INotificationListProps { items: INotification[]; onItemClick: (item: INotification) => void; } +const itemCount = 10; + const columns: IColumn[] = [ { key: 'Icon', @@ -98,6 +103,11 @@ function onRenderDetailsHeader(props, defaultRender) { export const NotificationList: React.FC = props => { const { items, onItemClick } = props; + const [pageIndex, setPageIndex] = useState(1); + + const pageCount: number = useMemo(() => { + return Math.ceil(items.length / itemCount) || 1; + }, [items]); const selection = new Selection({ onSelectionChanged: () => { @@ -108,21 +118,27 @@ export const NotificationList: React.FC = props => { }, }); + const showItems = items.slice((pageIndex - 1) * itemCount, pageIndex * itemCount); + return (
- - - +
+ + + +
+
); }; diff --git a/Composer/packages/client/src/pages/notifications/styles.ts b/Composer/packages/client/src/pages/notifications/styles.ts index ed61560b4d..156d3afc29 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -64,4 +64,15 @@ export const listRoot = css` position: relative; overflow-y: auto; flex-grow: 1; + display: flex; + flex-direction: column; +`; + +export const tableView = css` + position: relative; + flex-grow: 1; +`; + +export const detailList = css` + overflow-x: hidden; `; From 33a43860e85b18c74c85f6ad91fa17c2bdb26f4d Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Sat, 7 Dec 2019 03:16:27 +0800 Subject: [PATCH 44/45] fix pagination select from dropdown error --- Composer/packages/client/src/components/Pagination/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Composer/packages/client/src/components/Pagination/index.tsx b/Composer/packages/client/src/components/Pagination/index.tsx index ee3448e033..4b64396ba7 100644 --- a/Composer/packages/client/src/components/Pagination/index.tsx +++ b/Composer/packages/client/src/components/Pagination/index.tsx @@ -32,7 +32,7 @@ export const Pagination: React.FC = props => { const handlePageSelected = (event: React.FormEvent, item?: IDropdownOption, index?: number) => { setIndex(index || 0); - onChange(index || 0); + onChange(index ? index + 1 : 1); }; const hanglePreviousClick = () => { From 7ad612428647bc22d6fd1bfec3350519f2e5253d Mon Sep 17 00:00:00 2001 From: Leilei Zhang Date: Sat, 7 Dec 2019 13:10:18 +0800 Subject: [PATCH 45/45] fix expression function error --- .../packages/lib/indexers/src/dialogUtils/dialogChecker.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts index 0e330db51c..d610553c83 100644 --- a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -3,6 +3,7 @@ import get from 'lodash/get'; import { ExpressionEngine } from 'botbuilder-expression-parser'; +import formatMessage from 'format-message'; import { Diagnostic } from '../diagnostic'; @@ -15,17 +16,17 @@ export const IsExpression: CheckerFunc = ( value, optional: { properties: string[]; requiredTypes: { [key: string]: boolean } } ) => { - let message = ''; const { properties, requiredTypes } = optional; return properties.reduce((result: Diagnostic[], property) => { + let message = ''; const exp = get(value, property); if (!exp && requiredTypes[property]) { - message = `is missing or empty`; + message = formatMessage(`is missing or empty`); } else { try { ExpressionParser.parse(exp); } catch (error) { - message = `must be an expression`; + message = formatMessage(`must be an expression`); } } if (message) {