diff --git a/Composer/cypress/integration/NotificationPage.spec.ts b/Composer/cypress/integration/NotificationPage.spec.ts index 0f98c58149..99835edfaa 100644 --- a/Composer/cypress/integration/NotificationPage.spec.ts +++ b/Composer/cypress/integration/NotificationPage.spec.ts @@ -10,17 +10,69 @@ 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('.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.findByText('common.lg').should('exist'); + cy.findAllByText('common.lg') + .should('exist') + .first() + .click(); + }); + + cy.findAllByText('Bot Responses').should('exist'); + cy.get('@switchButton').should('be.disabled'); + }); + + 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') + .first() + .click(); }); + + 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.findByText('Greeting').should('exist'); + }); + + cy.withinEditor('FormEditor', () => { + cy.findByText('Condition').should('exist'); + cy.get('.ObjectItem input').type('()'); + }); + + cy.visitPage('Notifications'); + + cy.get('[data-testid="notifications-table-view"]').within(() => { + cy.findAllByText('Main.dialog') + .should('exist') + .first() + .click(); + }); + + cy.findAllByText('Greeting').should('exist'); }); }); 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'); }); }); diff --git a/Composer/packages/client/__tests__/components/notificationList.test.js b/Composer/packages/client/__tests__/components/notificationList.test.js index 32fddf9733..347e8e7453 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 = [ - { type: 'Error', location: 'test1', message: 'error1' }, - { type: 'Warning', location: 'test2', message: 'error2' }, - { type: 'Error', location: 'test3', message: 'error3' }, + { + 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/App.tsx b/Composer/packages/client/src/App.tsx index 5e3d50304f..001e598d90 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/ShellApi.ts b/Composer/packages/client/src/ShellApi.ts index ad661bfd24..c5a1991e6e 100644 --- a/Composer/packages/client/src/ShellApi.ts +++ b/Composer/packages/client/src/ShellApi.ts @@ -154,7 +154,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/TestController.tsx b/Composer/packages/client/src/TestController.tsx index 5c0575fb51..735a0fa9f8 100644 --- a/Composer/packages/client/src/TestController.tsx +++ b/Composer/packages/client/src/TestController.tsx @@ -4,21 +4,23 @@ /** @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, Diagnostic } 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 +48,13 @@ 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 errorLength = notifications.filter(n => n.severity === 'Error').length; + const showError = errorLength > 0; useEffect(() => { toStartBot && handleClick(); @@ -69,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; }, ''); @@ -141,10 +146,14 @@ export const TestController: React.FC = () => { } } + function handleErrorButtonClick() { + navigateTo('/notifications/'); + } + return (
- {connected && fetchState === STATE.SUCCESS && ( + {connected && !showError && fetchState === STATE.SUCCESS && ( { labelPosition="left" /> )} - +
+ {showError && ( + + {errorLength} + + + )} - +
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 ? index + 1 : 1); + }; + + 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/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 9692cf4c8f..6531b9daa2 100644 --- a/Composer/packages/client/src/pages/language-generation/code-editor.tsx +++ b/Composer/packages/client/src/pages/language-generation/code-editor.tsx @@ -7,15 +7,19 @@ 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 { LgFile } from '@bfc/indexers'; +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; + line: number; } // lsp server port should be same with composer/server port. @@ -24,10 +28,12 @@ const lspServerPath = '/lg-language-server'; export default function CodeEditor(props: CodeEditorProps) { const { actions } = useContext(StoreContext); - const { file, template } = props; + const { file, template, line } = props; const [diagnostics, setDiagnostics] = useState(get(file, 'diagnostics', [])); const [content, setContent] = useState(''); const [errorMsg, setErrorMsg] = useState(''); + const [lgEditor, setLgEditor] = useState(null); + const fileId = file && file.id; const inlineMode = !!template; useEffect(() => { @@ -38,11 +44,17 @@ 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]); + useEffect(() => { + if (lgEditor) { + lgEditor.revealLine(line); + } + }, [lgEditor]); + const updateLgTemplate = useMemo( () => debounce((body: string) => { @@ -88,13 +100,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); @@ -120,6 +132,7 @@ export default function CodeEditor(props: CodeEditorProps) { lineNumbersMinChars: false, }} hidePlaceholder={inlineMode} + editorDidMount={setLgEditor} value={content} errorMsg={errorMsg} lgOption={lgOption} diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx index 66b5d9e91e..d8cd74ff51 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'; @@ -34,14 +35,25 @@ 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); + const hash = props.location ? props.location.hash : ''; const subPath = props['*']; const isRoot = subPath === ''; const activeDialog = dialogs.find(item => item.id === subPath); + useEffect(() => { + if (hash) { + const match = /line=(\d+)/g.exec(hash); + if (match) setLine(+match[1]); + } + }, [hash]); + // for now, one bot only have one lg file by default. all dialog share one lg // file. const lgFile = lgFiles.length ? lgFiles[0] : null; @@ -98,7 +110,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); @@ -149,6 +161,7 @@ const LGPage: React.FC = props => { css={actionButton} onText={formatMessage('Edit mode')} offText={formatMessage('Edit mode')} + defaultChecked={false} checked={editMode} disabled={(!isRoot && editMode === false) || (fileValid === false && editMode === true)} onChange={onToggleEditMode} @@ -191,7 +204,7 @@ const LGPage: React.FC = props => {
{editMode ? ( }> - + ) : ( 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 5403572fdc..da1b557b85 100644 --- a/Composer/packages/client/src/pages/language-generation/table-view.tsx +++ b/Composer/packages/client/src/pages/language-generation/table-view.tsx @@ -15,8 +15,9 @@ 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'; import { StoreContext } from '../../store'; import * as lgUtil from '../../utils/lgUtil'; @@ -42,13 +43,9 @@ 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); + 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/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 15b573daf0..43088b5c07 100644 --- a/Composer/packages/client/src/pages/notifications/NotificationList.tsx +++ b/Composer/packages/client/src/pages/notifications/NotificationList.tsx @@ -3,16 +3,32 @@ /** @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 { 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 { 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', @@ -23,12 +39,14 @@ const columns: IColumn[] = [ minWidth: 30, maxWidth: 30, onRender: (item: INotification) => { - return ; + const icon = icons[item.severity]; + return ; }, }, { key: 'Notification Type', name: 'Type', + className: notification.columnCell, fieldName: 'type', minWidth: 70, maxWidth: 90, @@ -36,13 +54,14 @@ const columns: IColumn[] = [ isResizable: true, data: 'string', onRender: (item: INotification) => { - return {item.type}; + return {item.severity}; }, isPadded: true, }, { key: 'Notification Location', name: 'Location', + className: notification.columnCell, fieldName: 'location', minWidth: 70, maxWidth: 90, @@ -56,6 +75,7 @@ const columns: IColumn[] = [ { key: 'Notification Detail', name: 'Message', + className: notification.columnCell, fieldName: 'message', minWidth: 70, maxWidth: 90, @@ -70,19 +90,55 @@ const columns: IColumn[] = [ }, ]; +function onRenderDetailsHeader(props, defaultRender) { + return ( + + {defaultRender({ + ...props, + onRenderColumnHeaderTooltip: tooltipHostProps => , + })} + + ); +} + export const NotificationList: React.FC = props => { - const { items } = 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: () => { + const items = selection.getSelection(); + if (items.length) { + onItemClick(items[0] as INotification); + } + }, + }); + + const showItems = items.slice((pageIndex - 1) * itemCount, pageIndex * itemCount); return (
- +
+ + + +
+
); }; diff --git a/Composer/packages/client/src/pages/notifications/index.tsx b/Composer/packages/client/src/pages/notifications/index.tsx index 84cbbeb375..416a3cd03e 100644 --- a/Composer/packages/client/src/pages/notifications/index.tsx +++ b/Composer/packages/client/src/pages/notifications/index.tsx @@ -11,15 +11,52 @@ 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) => { + navigateTo(`/dialogs/${item.id}`); + }, + dialog: (item: INotification) => { + //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 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) { + uri += `?selected=${trigger}`; + if (action) { + uri += `&focused=${trigger}.${action}`; + } + } + } + navigateTo(uri); + }, +}; const Notifications: React.FC = () => { const [filter, setFilter] = useState(''); - const { notifications, locations } = useNotifications(filter); + const notifications = useNotifications(filter); + 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 980f728ff5..156d3afc29 100644 --- a/Composer/packages/client/src/pages/notifications/styles.ts +++ b/Composer/packages/client/src/pages/notifications/styles.ts @@ -17,16 +17,10 @@ export const notification = mergeStyleSets({ }, typeIconCell: { textAlign: 'center', - selectors: { - '&:before': { - content: '.', - display: 'inline-block', - verticalAlign: 'middle', - height: '100%', - width: '0px', - visibility: 'hidden', - }, - }, + cursor: 'pointer', + }, + columnCell: { + cursor: 'pointer', }, }); @@ -42,6 +36,7 @@ export const typeIcon = icon => css` background: ${icon.background}; line-height: 24px; color: ${icon.color}; + cursor: pointer; `; export const notificationHeader = css` @@ -66,5 +61,18 @@ export const root = css` `; 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; `; diff --git a/Composer/packages/client/src/pages/notifications/types.ts b/Composer/packages/client/src/pages/notifications/types.ts index d9a1528ce6..bce5eef0a0 100644 --- a/Composer/packages/client/src/pages/notifications/types.ts +++ b/Composer/packages/client/src/pages/notifications/types.ts @@ -2,7 +2,12 @@ // Licensed under the MIT License. export interface INotification { + id: string; + severity: string; type: string; location: string; 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 0004803f2e..4d4b46e0ec 100644 --- a/Composer/packages/client/src/pages/notifications/useNotifications.tsx +++ b/Composer/packages/client/src/pages/notifications/useNotifications.tsx @@ -2,52 +2,62 @@ // 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 { replaceDialogDiagnosticLabel } from '../../utils'; -import { INotification } from './types'; +import { INotification, DiagnosticSeverity } from './types'; -const DiagnosticSeverity = ['Error', 'Warning']; //'Information', 'Hint' - -export default function useNotifications(filter: string) { +export default function useNotifications(filter?: string) { const { state } = useContext(StoreContext); const { dialogs, luFiles, lgFiles } = state; const memoized = useMemo(() => { const notifactions: INotification[] = []; - const locations = new Set(); dialogs.forEach(dialog => { dialog.diagnostics.map(diagnostic => { - const location = dialog.displayName; - locations.add(location); - notifactions.push({ type: 'Error', location, message: diagnostic }); + const location = `${dialog.id}.dialog`; + notifactions.push({ + type: 'dialog', + location, + message: `In ${replaceDialogDiagnosticLabel(diagnostic.path)} ${diagnostic.message}`, + severity: DiagnosticSeverity[diagnostic.severity] || '', + diagnostic, + id: dialog.id, + }); }); }); 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, + id: lufile.id, + }); }); }); - lgFiles.forEach(lgFiles => { - lgFiles.diagnostics.map(diagnostic => { - const location = `${lgFiles.id}.lg`; - locations.add(location); + lgFiles.forEach(lgFile => { + lgFile.diagnostics.map(diagnostic => { + const location = `${lgFile.id}.lg`; notifactions.push({ - type: DiagnosticSeverity[diagnostic.severity], + type: 'lg', + severity: DiagnosticSeverity[diagnostic.severity] || '', location, - message: createSingleMessage(diagnostic), + message: lgIndexer.createSingleMessage(diagnostic), + diagnostic, + id: lgFile.id, }); }); }); - 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/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/reducer/index.ts b/Composer/packages/client/src/store/reducer/index.ts index 7de2dbf2b3..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 } from '@bfc/indexers/lib/dialogIndexer'; +import { dialogIndexer } from '@bfc/indexers'; import { SensitiveProperties } from '@bfc/shared'; import { ActionTypes, FileTypes } from '../../constants'; @@ -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, state.schemas.sdk.content); return { ...dialog, ...result }; } return dialog; 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/styles.ts b/Composer/packages/client/src/styles.ts index e039299be3..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` @@ -81,7 +81,21 @@ export const bot = css` `; export const botButton = css` - margin-left: 15px; + margin-left: 5px; +`; + +export const errorButton = css` + color: ${SharedColors.red20}; + &:hover { + color: ${SharedColors.red20}; + } +`; + +export const errorCount = css` + height: 32px; + line-height: 32px; + font-size 16px; + cursor: pointer; `; export const calloutLabel = css` diff --git a/Composer/packages/client/src/utils/dialogUtil.ts b/Composer/packages/client/src/utils/dialogUtil.ts index 5fb27a7898..d82214f6ec 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'; @@ -289,3 +289,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/client/src/utils/lgUtil.ts b/Composer/packages/client/src/utils/lgUtil.ts index 86cae3bf92..34e71a13a0 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/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 75138c68a8..8c537a6458 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'; 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 f8f343ae48..7d81a1ad68 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, useEffect } 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 36642be565..49b589bc00 100644 --- a/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx +++ b/Composer/packages/extensions/obiformeditor/src/Form/fields/RecognizerField/index.tsx @@ -1,12 +1,13 @@ // 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'; 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'; @@ -35,6 +36,18 @@ 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) => { + return (msg += `${diagnostic.text}\n`); + }, ''); + setErrorMsg(msg); + } else { + setErrorMsg(''); + } + }, [selectedFile]); + const handleChange = (_, option?: IDropdownOption): void => { if (option) { switch (option.key) { @@ -142,12 +155,7 @@ 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(setErrorMsg); }; return ( 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/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'; 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..68fd2a05be --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/dialogUtils/extractExpressionDefinitions.test.ts @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License + +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', + }, + $copy: { + title: '$copy', + }, + condition: { + $role: 'expression', + }, + items: { + $role: 'expression', + }, + }, + }, + 'Ms.type2': { + properties: { + $type: { + title: '$type', + }, + items: { + $role: 'expression', + }, + }, + }, + }, + }; + expect(getExpressionProperties(schema)).toEqual({ + 'Ms.type1': { + properties: ['condition', 'items'], + requiredTypes: { + condition: true, + }, + }, + 'Ms.type2': { + properties: ['items'], + requiredTypes: {}, + }, + }); + }); +}); diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/models/LgMetaData.test.ts b/Composer/packages/lib/indexers/__tests__/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/__tests__/lgUtils/models/LgMetaData.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/models/LgTemplateRef.test.ts b/Composer/packages/lib/indexers/__tests__/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/__tests__/lgUtils/models/LgTemplateRef.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgParamString.test.ts b/Composer/packages/lib/indexers/__tests__/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/__tests__/lgUtils/parsers/parseLgParamString.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgTemplateName.test.ts b/Composer/packages/lib/indexers/__tests__/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/__tests__/lgUtils/parsers/parseLgTemplateName.test.ts diff --git a/Composer/packages/lib/shared/__tests__/lgUtils/parsers/parseLgTemplateRef.test.ts b/Composer/packages/lib/indexers/__tests__/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/__tests__/lgUtils/parsers/parseLgTemplateRef.test.ts diff --git a/Composer/packages/lib/indexers/__tests__/utils/help.test.ts b/Composer/packages/lib/indexers/__tests__/utils/help.test.ts new file mode 100644 index 0000000000..06bdf585f6 --- /dev/null +++ b/Composer/packages/lib/indexers/__tests__/utils/help.test.ts @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { getBaseName } from '../../src/utils/help'; + +describe('get base name', () => { + 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/server/__tests__/utility/jsonWalk.test.ts b/Composer/packages/lib/indexers/__tests__/utils/jsonWalk.test.ts similarity index 85% rename from Composer/packages/server/__tests__/utility/jsonWalk.test.ts rename to Composer/packages/lib/indexers/__tests__/utils/jsonWalk.test.ts index 5655eeb50c..6d59c60864 100644 --- a/Composer/packages/server/__tests__/utility/jsonWalk.test.ts +++ b/Composer/packages/lib/indexers/__tests__/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); }); }); diff --git a/Composer/packages/lib/indexers/package.json b/Composer/packages/lib/indexers/package.json index 8fcdfa7ef7..d9383d70f9 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": "4.7.0-preview.93464", "lodash": "^4.17.15", 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 fb7aa816a6..62e495bef0 100644 --- a/Composer/packages/lib/indexers/src/dialogIndexer.ts +++ b/Composer/packages/lib/indexers/src/dialogIndexer.ts @@ -3,12 +3,14 @@ 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'; 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[] = []; @@ -46,6 +48,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[] = []; @@ -65,6 +68,7 @@ function ExtractLuIntents(dialog): string[] { JsonWalk('$', dialog, visitor); return uniq(intents); } + // find out all triggers given dialog function ExtractTriggers(dialog): ITrigger[] { const trigers: ITrigger[] = []; @@ -100,6 +104,7 @@ function ExtractTriggers(dialog): ITrigger[] { JsonWalk('$', dialog, visitor); return trigers; } + // find out all referred dialog function ExtractReferredDialogs(dialog): string[] { const dialogs: string[] = []; @@ -119,9 +124,11 @@ function ExtractReferredDialogs(dialog): string[] { JsonWalk('$', dialog, visitor); return uniq(dialogs); } + // check all fields -function CheckFields(dialog): string[] { - const errors: string[] = []; +function CheckFields(dialog, id: string, schema: any): Diagnostic[] { + const errors: Diagnostic[] = []; + const expressionProperties = getExpressionProperties(schema); /** * * @param path , jsonPath string @@ -130,31 +137,36 @@ function CheckFields(dialog): string[] { * */ const visitor: VisitorFunc = (path: string, value: any): boolean => { // it's a valid schema dialog node. - 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; }; - JsonWalk('$', dialog, visitor); - return errors; + JsonWalk(id, dialog, visitor); + return errors.map(e => { + e.source = id; + return e; + }); } -function parse(content) { + +function validate(id: string, content, schema: any): Diagnostic[] { + try { + return CheckFields(content, id, schema); + } catch (error) { + return [new Diagnostic(error.message, id)]; + } +} + +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: CheckFields(content), + diagnostics: validate(id, content, schema), referredDialogs: ExtractReferredDialogs(content), lgTemplates: ExtractLgTemplates(content), luIntents: ExtractLuIntents(content), @@ -163,7 +175,8 @@ function parse(content) { triggers: ExtractTriggers(content), }; } -function index(files: FileInfo[], botName: string): DialogInfo[] { + +function index(files: FileInfo[], botName: string, schema: any): DialogInfo[] { const dialogs: DialogInfo[] = []; if (files.length !== 0) { for (const file of files) { @@ -178,7 +191,7 @@ function index(files: FileInfo[], botName: string): DialogInfo[] { displayName: isRoot ? `${botName}.Main` : id, content: dialogJson, relativePath: file.relativePath, - ...parse(dialogJson), + ...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 new file mode 100644 index 0000000000..d610553c83 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import get from 'lodash/get'; +import { ExpressionEngine } from 'botbuilder-expression-parser'; +import formatMessage from 'format-message'; + +import { Diagnostic } from '../diagnostic'; + +import { CheckerFunc } from './types'; + +const ExpressionParser = new ExpressionEngine(); + +export const IsExpression: CheckerFunc = ( + path, + value, + optional: { properties: string[]; requiredTypes: { [key: string]: boolean } } +) => { + const { properties, requiredTypes } = optional; + return properties.reduce((result: Diagnostic[], property) => { + let message = ''; + const exp = get(value, property); + if (!exp && requiredTypes[property]) { + message = formatMessage(`is missing or empty`); + } else { + try { + ExpressionParser.parse(exp); + } catch (error) { + message = formatMessage(`must be an expression`); + } + } + if (message) { + const diagnostic = new Diagnostic(message, ''); + 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 new file mode 100644 index 0000000000..a2d41dd960 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/extractExpressionDefinitions.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +import has from 'lodash/has'; +import keys from 'lodash/keys'; + +import { VisitorFunc, JsonWalk } from '../utils/jsonWalk'; + +import { IDefinition, IExpressionProperties } from './types'; + +function findAllProperties(obj: any, searchTarget: (value: any) => boolean): string[] { + const properties: string[] = []; + + const visitor: VisitorFunc = (path: string, value: any): boolean => { + if (searchTarget(value)) { + const parents = path.split('.'); + properties.push(parents[parents.length - 1]); + } + return false; + }; + JsonWalk('$', obj, visitor); + return properties; +} + +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) { + expressionProperties[key] = { properties, requiredTypes }; + } + }); + + return expressionProperties; +} 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..6f77b02165 --- /dev/null +++ b/Composer/packages/lib/indexers/src/dialogUtils/types.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Diagnostic } from '../diagnostic'; + +export interface ISearchTarget { + type: string; + value: string; +} + +export interface IDefinition { + [key: string]: any; +} + +export interface ISearchResult { + [key: string]: string[]; +} + +export interface IExpressionProperties { + [key: string]: { + properties: string[]; + requiredTypes: { [key: string]: boolean }; + }; +} + +export type CheckerFunc = (path: string, value: any, optional?: any) => Diagnostic[] | null; // error msg diff --git a/Composer/packages/lib/indexers/src/index.ts b/Composer/packages/lib/indexers/src/index.ts index 5a51568712..7ca8969ba8 100644 --- a/Composer/packages/lib/indexers/src/index.ts +++ b/Composer/packages/lib/indexers/src/index.ts @@ -3,4 +3,7 @@ export * from './dialogIndexer'; export * from './lgIndexer'; -export * from './luIndexer'; +export * from './type'; +export * from './diagnostic'; +export * from './lgUtils'; +export * from './dialogUtils'; diff --git a/Composer/packages/lib/indexers/src/lgIndexer.ts b/Composer/packages/lib/indexers/src/lgIndexer.ts index c82c3aaf42..1c6fe50d25 100644 --- a/Composer/packages/lib/indexers/src/lgIndexer.ts +++ b/Composer/packages/lib/indexers/src/lgIndexer.ts @@ -1,68 +1,90 @@ // 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, ImportResolver } from 'botbuilder-lg'; import { FileInfo, LgFile, LgTemplate } from './type'; import { getBaseName } from './utils/help'; +import { Diagnostic, DiagnosticSeverity, Position, Range } from './diagnostic'; const lgStaticChecker = new StaticChecker(); -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); - let templates: LgTemplate[] = []; - try { - templates = parse(file.content, ''); - } catch (err) { - console.error(err); - } - lgFiles.push({ - id: getBaseName(file.name, '.lg'), - relativePath: file.relativePath, - content: file.content, - templates, - diagnostics, - }); - } - } - return lgFiles; -} +// 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); -function isValid(diagnostics: Diagnostic[]): boolean { - return diagnostics.every(d => d.severity !== DiagnosticSeverity.Error); + 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, path: string): Diagnostic[] { - return lgStaticChecker.checkText(content, path); +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[] { +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 combineMessage(diagnostics: Diagnostic[]): string { - return diagnostics.reduce((msg, d) => { +function isValid(diagnostics: Diagnostic[]): boolean { + return diagnostics.every(d => d.severity !== DiagnosticSeverity.Error); +} + +function index(files: FileInfo[]): LgFile[] { + if (files.length === 0) return []; + const lgFiles: LgFile[] = []; + for (const file of files) { + const { name, relativePath, content } = file; + if (name.endsWith('.lg')) { + const id = getBaseName(name, '.lg'); + const diagnostics = check(content, id); + let templates: LgTemplate[] = []; + if (isValid(diagnostics)) { + try { + templates = parse(file.content, ''); + } catch (err) { + diagnostics.push(new Diagnostic(err.message, id, DiagnosticSeverity.Error)); + } + } + + lgFiles.push({ id, relativePath, content, templates, diagnostics }); + } + } + return lgFiles; +} + +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; +} - msg += `${position} \n ${d.message}\n`; +function combineMessage(diagnostics: Diagnostic[]): string { + return diagnostics.reduce((msg, d) => { + msg += createSingleMessage(d); return msg; }, ''); } @@ -72,5 +94,6 @@ export const lgIndexer = { parse, check, isValid, + createSingleMessage, combineMessage, }; 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/indexers/src/type.ts b/Composer/packages/lib/indexers/src/type.ts index b4191f2b90..1d0c0cb508 100644 --- a/Composer/packages/lib/indexers/src/type.ts +++ b/Composer/packages/lib/indexers/src/type.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { Diagnostic as LGDiagnostic } from 'botbuilder-lg'; +import { Diagnostic } from './diagnostic'; export interface FileInfo { name: string; @@ -19,7 +19,7 @@ export interface ITrigger { export interface DialogInfo { content: any; - diagnostics: string[]; + diagnostics: Diagnostic[]; displayName: string; id: string; isRoot: boolean; @@ -32,19 +32,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; } @@ -72,22 +59,16 @@ export interface LuFile { [key: string]: any; } +export interface LgTemplate { + name: string; + body: string; + parameters: string[]; +} + 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/indexers/src/utils/dialogChecker.ts b/Composer/packages/lib/indexers/src/utils/dialogChecker.ts deleted file mode 100644 index 654bde729b..0000000000 --- a/Composer/packages/lib/indexers/src/utils/dialogChecker.ts +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import get from 'lodash/get'; -import { ExpressionEngine } from 'botbuilder-expression-parser'; - -const ExpressionParser = new ExpressionEngine(); - -interface CheckerFunc { - (node: { path: string; value: any }): string; // 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`; - } - return message; - }; -} - -enum EditArrayChangeTypes { - Push, - Pop, - Take, - Remove, - Clear, -} - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function EditArrayValueChecker(node: { path: string; value: any }): string { - let message = ''; - - 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) { - message = `In ${node.path}: ${node.value.$type}: ${name} must be an expression`; - } - } - - return message; -} - -/** - * 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/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 diff --git a/Composer/packages/lib/package.json b/Composer/packages/lib/package.json index 413c3725cb..39bb9da15f 100644 --- a/Composer/packages/lib/package.json +++ b/Composer/packages/lib/package.json @@ -11,7 +11,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" 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/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/src/types/shell.ts b/Composer/packages/lib/shared/src/types/shell.ts index e0f1a426c3..4bff8bdbb8 100644 --- a/Composer/packages/lib/shared/src/types/shell.ts +++ b/Composer/packages/lib/shared/src/types/shell.ts @@ -1,38 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { Diagnostic as LGDiagnostic } from 'botbuilder-lg'; - -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[]; -} +import { LGTemplate as LgTemplate } from 'botbuilder-lg'; +import { DialogInfo, LgFile, LuFile } from '@bfc/indexers'; export interface EditorSchema { content?: { @@ -47,53 +17,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/__tests__/models/bot/botProject.test.ts b/Composer/packages/server/__tests__/models/bot/botProject.test.ts index 4a3de65282..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'; @@ -236,14 +237,16 @@ describe('lu operation', () => { expect(result?.content).toEqual(content); }); - it('should throw error when lu content is invalid', async () => { + it('should update diagnostics when lu content is invalid', async () => { const id = 'root'; let content = '## hello \n - hello'; await proj.createLuFile(id, content); 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); + expect(result?.diagnostics?.length).toBeGreaterThan(0); }); it('should delete lu file and update index', async () => { diff --git a/Composer/packages/server/src/models/bot/botProject.ts b/Composer/packages/server/src/models/bot/botProject.ts index 541256d949..e41ed15983 100644 --- a/Composer/packages/server/src/models/bot/botProject.ts +++ b/Composer/packages/server/src/models/bot/botProject.ts @@ -3,9 +3,9 @@ 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'; +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'; import { copyDir } from '../../utility/storage'; @@ -66,7 +66,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(); @@ -267,16 +267,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); @@ -430,13 +420,17 @@ export class BotProject { await this.reindex(relativePath); }; + private indexDialog() { + return dialogIndexer.index(this.files, this.name, this.getSchemas().sdk.content); + } + // 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); 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'; 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); - }); - } -};