feat: show error message in form editor. (#1737)

* show validation message in form editor * lg diagnostic * add formErrors to formContext * use correct formatMessage api * pass test * when file invalid, dont go to table list view * add detail error message for expression * condition errMessage & expression errorMessage display * botbuilder-expression-parser --> botframework-expressions * add back template range * fix unit test * T->t
microsoft · Dec 10, 2019 · 6ea46cd · 6ea46cd
1 parent 4180fed
commit 6ea46cd
Show file tree

Hide file tree

Showing 17 changed files with 82 additions and 93 deletions.
diff --git a/Composer/packages/client/package.json b/Composer/packages/client/package.json
@@ -24,6 +24,7 @@
     "@reach/router": "^1.2.1",
     "axios": "^0.18.0",
     "botbuilder-lg": "4.7.0-preview.93464",
+    "botframework-expressions": "4.7.0-preview.93464",
     "format-message": "^6.2.3",
     "immer": "^2.1.4",
     "jwt-decode": "^2.2.0",

diff --git a/Composer/packages/client/src/pages/language-generation/index.tsx b/Composer/packages/client/src/pages/language-generation/index.tsx
@@ -98,7 +98,7 @@ const LGPage: React.FC<RouteComponentProps> = props => {
 
   useEffect(() => {
     // dialog lg templates is part of commong.lg. By restricting edit in root view, user would aware that the changes they made may affect other dialogs.
-    if (!isRoot) {
+    if (!isRoot && fileValid) {
       setEditMode(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
@@ -16,8 +16,9 @@ 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/indexers';
-import { LGTemplate } from 'botbuilder-lg';
+import { LGTemplate, LGParser } from 'botbuilder-lg';
 import { lgIndexer } from '@bfc/indexers';
+import get from 'lodash/get';
 
 import { StoreContext } from '../../store';
 import * as lgUtil from '../../utils/lgUtil';
@@ -44,7 +45,8 @@ const TableView: React.FC<TableViewProps> = props => {
     if (isEmpty(lgFile)) return;
     let allTemplates: LGTemplate[] = [];
     if (lgIndexer.isValid(lgFile.diagnostics) === true) {
-      allTemplates = lgIndexer.parse(lgFile.content) as LGTemplate[];
+      const resource = LGParser.parse(lgFile.content, '');
+      allTemplates = get(resource, 'templates', []);
     }
     if (!activeDialog) {
       setTemplates(allTemplates);

diff --git a/Composer/packages/client/src/utils/dialogUtil.ts b/Composer/packages/client/src/utils/dialogUtil.ts
@@ -5,7 +5,7 @@ import { ConceptLabels, DialogGroup, SDKTypes, dialogGroups, seedNewDialog } fro
 import get from 'lodash/get';
 import set from 'lodash/set';
 import cloneDeep from 'lodash/cloneDeep';
-import { ExpressionEngine } from 'botbuilder-expression-parser';
+import { ExpressionEngine } from 'botframework-expressions';
 import { IDropdownOption } from 'office-ui-fabric-react/lib/Dropdown';
 import { DialogInfo } from '@bfc/indexers';
 

diff --git a/Composer/packages/extensions/obiformeditor/demo/src/index.tsx b/Composer/packages/extensions/obiformeditor/demo/src/index.tsx
@@ -5,7 +5,7 @@ import { PrimaryButton, DefaultButton, DirectionalHint } from 'office-ui-fabric-
 import debounce from 'lodash/debounce';
 import nanoid from 'nanoid';
 import { initializeIcons } from '@uifabric/icons';
-import { ExpressionEngine } from 'botbuilder-expression-parser';
+import { ExpressionEngine } from 'botframework-expressions';
 import { seedNewDialog, ShellApi } from '@bfc/shared';
 import { LuFile, DialogInfo } from '@bfc/indexers';
 

diff --git a/Composer/packages/extensions/obiformeditor/package.json b/Composer/packages/extensions/obiformeditor/package.json
@@ -1,6 +1,5 @@
 {
   "name": "@bfc/obiformeditor",
-  "license": "MIT",
   "version": "1.0.0",
   "description": "obieditortest React component",
   "engines": {
@@ -62,7 +61,7 @@
     "@types/react": "16.9.0",
     "@types/react-dom": "16.9.0",
     "autoprefixer": "^9.5.1",
-    "botbuilder-expression-parser": "^4.5.11",
+    "botframework-expressions": "4.7.0-preview.93464",
     "codemirror": "^5.44.0",
     "copyfiles": "^2.1.0",
     "css-loader": "^2.1.1",
@@ -92,4 +91,4 @@
   "keywords": [
     "react-component"
   ]
-}
+}
diff --git a/Composer/packages/extensions/obiformeditor/src/Form/types.ts b/Composer/packages/extensions/obiformeditor/src/Form/types.ts
@@ -12,6 +12,7 @@ export interface FormContext
   dialogOptions: { value: string; label: string }[];
   dialogId?: string;
   isRoot: boolean;
+  formErrors: any;
 }
 
 interface EnumOption {

diff --git a/Composer/packages/extensions/obiformeditor/src/Form/widgets/ExpressionWidget.tsx b/Composer/packages/extensions/obiformeditor/src/Form/widgets/ExpressionWidget.tsx
@@ -6,6 +6,7 @@ import { TextField, ITextFieldProps, ITextFieldStyles } from 'office-ui-fabric-r
 import { MessageBar, MessageBarType } from 'office-ui-fabric-react/lib/MessageBar';
 import { JSONSchema6 } from 'json-schema';
 import formatMessage from 'format-message';
+import get from 'lodash/get';
 
 import { FormContext } from '../types';
 import { EditableField } from '../fields/EditableField';
@@ -25,8 +26,9 @@ interface ExpresionWidgetProps extends ITextFieldProps {
   options?: any;
 }
 
-const getDefaultErrorMessage = () => {
-  return formatMessage.rich('Invalid expression syntax. Refer to the syntax documentation<a>here</a>', {
+const getDefaultErrorMessage = errMessage => {
+  return formatMessage.rich('{errMessage}. Refer to the syntax documentation<a>here</a>', {
+    errMessage,
     a: ({ children }) => (
       <a
         key="a"
@@ -41,39 +43,13 @@ const getDefaultErrorMessage = () => {
 };
 
 export const ExpressionWidget: React.FC<ExpresionWidgetProps> = props => {
-  const {
-    rawErrors,
-    formContext,
-    schema,
-    id,
-    label,
-    editable,
-    hiddenErrMessage,
-    onValidate,
-    options = {},
-    ...rest
-  } = props;
-  const { shellApi } = formContext;
+  const { formContext, schema, id, label, editable, hiddenErrMessage, onValidate, options = {}, ...rest } = props;
   const { description } = schema;
   const { hideLabel } = options;
+  const name = props.id?.split('_')[props.id?.split('_').length - 1];
 
-  const onGetErrorMessage = async (value: string): Promise<JSX.Element | string> => {
-    if (!value) {
-      if (hiddenErrMessage) {
-        onValidate && onValidate();
-      }
-      return '';
-    }
-
-    const isValid = await shellApi.validateExpression(value);
-
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    let errMessage: string | any[] = '';
-    if (!isValid) {
-      errMessage = getDefaultErrorMessage();
-    } else if (rawErrors && rawErrors.length > 0) {
-      errMessage = rawErrors[0];
-    }
+  const onGetErrorMessage = (): JSX.Element | string => {
+    const errMessage = name && get(formContext, ['formErrors', name], '');
 
     const messageBar = errMessage ? (
       <MessageBar
@@ -83,7 +59,7 @@ export const ExpressionWidget: React.FC<ExpresionWidgetProps> = props => {
         truncated
         overflowButtonAriaLabel={formatMessage('See more')}
       >
-        {errMessage}
+        {getDefaultErrorMessage(`${label} ${errMessage}`)}
       </MessageBar>
     ) : (
       ''

diff --git a/Composer/packages/extensions/obiformeditor/src/Form/widgets/LgEditorWidget.tsx b/Composer/packages/extensions/obiformeditor/src/Form/widgets/LgEditorWidget.tsx
@@ -45,18 +45,14 @@ interface LgEditorWidgetProps {
 
 export const LgEditorWidget: React.FC<LgEditorWidgetProps> = props => {
   const { formContext, name, value, height = 250 } = props;
-  const [errorMsg, setErrorMsg] = useState('');
   const lgName = new LgMetaData(name, formContext.dialogId || '').toString();
   const lgFileId = formContext.currentDialog.lgFile || 'common';
   const lgFile = formContext.lgFiles && formContext.lgFiles.find(file => file.id === lgFileId);
 
   const updateLgTemplate = useMemo(
     () =>
       debounce((body: string) => {
-        formContext.shellApi
-          .updateLgTemplate(lgFileId, lgName, body)
-          .then(() => setErrorMsg(''))
-          .catch(error => setErrorMsg(error));
+        formContext.shellApi.updateLgTemplate(lgFileId, lgName, body).catch(() => {});
       }, 500),
     [lgName, lgFileId]
   );
@@ -68,8 +64,25 @@ export const LgEditorWidget: React.FC<LgEditorWidgetProps> = props => {
     })) || {
     name: lgName,
     body: getInitialTemplate(name, value),
+    range: {
+      startLineNumber: 0,
+      endLineNumber: 2,
+    },
   };
 
+  const diagnostic =
+    lgFile &&
+    lgFile.diagnostics.find(d => {
+      return (
+        d.range &&
+        d.range.start.line >= template.range.startLineNumber &&
+        d.range.end.line <= template.range.endLineNumber
+      );
+    });
+
+  const errorMsg = diagnostic
+    ? diagnostic.message.split('error message: ')[diagnostic.message.split('error message: ').length - 1]
+    : '';
   const [localValue, setLocalValue] = useState(template.body);
   const lgOption = {
     inline: true,

diff --git a/Composer/packages/extensions/obiformeditor/src/FormEditor.tsx b/Composer/packages/extensions/obiformeditor/src/FormEditor.tsx
@@ -3,13 +3,14 @@
 
 /** @jsx jsx */
 import { Global, jsx } from '@emotion/core';
-import React, { useState } from 'react';
+import React, { useState, useMemo } from 'react';
 import { Dropdown } from 'office-ui-fabric-react/lib/Dropdown';
 import { JSONSchema6Definition, JSONSchema6 } from 'json-schema';
 import merge from 'lodash/merge';
 import get from 'lodash/get';
 import isEqual from 'lodash/isEqual';
 import { appschema, ShellData, ShellApi } from '@bfc/shared';
+import { Diagnostic } from '@bfc/indexers';
 
 import Form from './Form';
 import { uiSchema } from './schema/uischema';
@@ -33,6 +34,25 @@ export const FormEditor: React.FunctionComponent<FormEditorProps> = props => {
   const [localData, setLocalData] = useState(data);
   const type = getType(localData);
 
+  const formErrors = useMemo(() => {
+    if (props.currentDialog && props.currentDialog.diagnostics) {
+      const currentPath = props.focusPath.replace('#', '');
+      const diagnostics = get(props.currentDialog, 'diagnostics', [] as Diagnostic[]);
+
+      return diagnostics.reduce((errors, d) => {
+        const [dPath, dType, dProp] = d.path?.split('#') || [];
+
+        if (dPath === currentPath && dType === type && dProp) {
+          errors[dProp] = d.message;
+        }
+
+        return errors;
+      }, {});
+    }
+
+    return {};
+  }, [props.currentDialog]);
+
   if (!type) {
     return (
       <div>
@@ -112,6 +132,7 @@ export const FormEditor: React.FunctionComponent<FormEditorProps> = props => {
           focusedEvent: props.focusedEvent,
           focusedSteps: props.focusedSteps,
           focusedTab: props.focusedTab,
+          formErrors,
         }}
         idPrefix={props.focusPath}
       >

diff --git a/Composer/packages/lib/code-editor/src/RichEditor.tsx b/Composer/packages/lib/code-editor/src/RichEditor.tsx
@@ -51,16 +51,14 @@ export function RichEditor(props: RichEditorProps) {
     }
   }, [editor]);
 
-  const errorHelp = formatMessage.rich(
-    'This text has errors in the syntax. Refer to the syntax documentation<a>here</a>.',
-    {
-      a: ({ children }) => (
-        <a key="a" href={helpURL} target="_blank" rel="noopener noreferrer">
-          {children}
-        </a>
-      ),
-    }
-  );
+  const errorHelp = formatMessage.rich('{errorMsg}. Refer to the syntax documentation<a>here</a>.', {
+    errorMsg,
+    a: ({ children }) => (
+      <a key="a" href={helpURL} target="_blank" rel="noopener noreferrer">
+        {children}
+      </a>
+    ),
+  });
 
   const getHeight = () => {
     if (height === null || height === undefined) {

diff --git a/Composer/packages/lib/indexers/package.json b/Composer/packages/lib/indexers/package.json
@@ -28,8 +28,8 @@
     "ts-jest": "^24.1.0"
   },
   "dependencies": {
-    "botbuilder-expression-parser": "^4.5.11",
     "botbuilder-lg": "4.7.0-preview.93464",
+    "botframework-expressions": "4.7.0-preview.93464",
     "lodash": "^4.17.15",
     "ludown": "^1.3.4"
   }

diff --git a/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts b/Composer/packages/lib/indexers/src/dialogUtils/dialogChecker.ts
@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 import get from 'lodash/get';
-import { ExpressionEngine } from 'botbuilder-expression-parser';
+import { ExpressionEngine } from 'botframework-expressions';
 import formatMessage from 'format-message';
 
 import { Diagnostic } from '../diagnostic';
@@ -26,7 +26,7 @@ export const IsExpression: CheckerFunc = (
       try {
         ExpressionParser.parse(exp);
       } catch (error) {
-        message = formatMessage(`must be an expression`);
+        message = `${formatMessage('must be an expression:')} ${error})`;
       }
     }
     if (message) {

diff --git a/Composer/packages/lib/indexers/src/lgIndexer.ts b/Composer/packages/lib/indexers/src/lgIndexer.ts
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import { LGParser, StaticChecker, Diagnostic as LGDiagnostic, ImportResolver } from 'botbuilder-lg';
+import get from 'lodash/get';
 
 import { FileInfo, LgFile, LgTemplate } from './type';
 import { getBaseName } from './utils/help';
@@ -40,6 +41,10 @@ function parse(content: string, id?: string): LgTemplate[] {
       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;

diff --git a/Composer/packages/lib/indexers/src/type.ts b/Composer/packages/lib/indexers/src/type.ts
@@ -58,11 +58,16 @@ export interface LuFile {
   diagnostics: LuDiagnostic[];
   [key: string]: any;
 }
+export interface CodeRange {
+  startLineNumber: number;
+  endLineNumber: number;
+}
 
 export interface LgTemplate {
   name: string;
   body: string;
   parameters: string[];
+  range: CodeRange;
 }
 
 export interface LgFile {

diff --git a/Composer/packages/tools/language-servers/language-generation/src/builtinFunctionsMap.ts b/Composer/packages/tools/language-servers/language-generation/src/builtinFunctionsMap.ts
@@ -6,7 +6,7 @@
  * Licensed under the MIT License.
  */
 
-import { ReturnType } from 'botbuilder-expression';
+import { ReturnType } from 'botframework-expressions';
 export class FunctionEntity {
   public constructor(params: string[], returntype: ReturnType, introduction: string) {
     this.Params = params;