fix: add skill show error if manifest url can not be accessed

Composer/packages/client/__tests__/components/skill.test.tsx

@@ -7,9 +7,13 @@ import { Skill } from '@bfc/shared';
 
 import Skills from '../../src/pages/skills';
 import SkillList from '../../src/pages/skills/skill-list';
-import SkillForm from '../../src/pages/skills/skill-form';
+import CreateSkillModal from '../../src/components/SkillForm/CreateSkillModal';
 import { renderWithStore } from '../testUtils';
 
+jest.mock('../../src/components/SkillForm/CreateSkillModal/validateManifestUrl', () => ({
+  validateManifestUrl: () => {},
+}));
+
 const items: Skill[] = [
   {
     manifestUrl: 'https://yuesuemailskill0207-gjvga67.azurewebsites.net/manifest/manifest-1.0.json',
@@ -50,14 +54,18 @@ describe('Skill page', () => {
 
 describe('<SkillList />', () => {
   it('should render the SkillList', () => {
-    const { container } = render(<SkillList skills={items} projectId="test-project" onEdit={jest.fn()} />);
+    const { container } = render(
+      <SkillList skills={items} projectId="test-project" onEdit={jest.fn()} onDelete={jest.fn()} />
+    );
     expect(container).toHaveTextContent('Email Skill');
     expect(container).toHaveTextContent('Point Of Interest Skill');
   });
 
   it('can edit the skill', () => {
     const onEdit = jest.fn();
-    const { getAllByTestId } = render(<SkillList skills={items} projectId="test-project" onEdit={onEdit} />);
+    const { getAllByTestId } = render(
+      <SkillList skills={items} projectId="test-project" onEdit={onEdit} onDelete={jest.fn()} />
+    );
 
     const editBtns = getAllByTestId('EditSkill');
     editBtns.forEach((btn, i) => {
@@ -69,12 +77,20 @@ describe('<SkillList />', () => {
 
 describe('<SkillForm />', () => {
   it('should render the skill form, and do update', () => {
+    jest.useFakeTimers();
     const onSubmit = jest.fn(formData => {
       expect(formData.manifestUrl).toBe('http://AwesomeSkill');
     });
     const onDismiss = jest.fn(() => {});
     const { getByLabelText, getByText } = render(
-      <SkillForm skills={items} editIndex={0} onSubmit={onSubmit} onDismiss={onDismiss} isOpen />
+      <CreateSkillModal
+        skills={items}
+        editIndex={0}
+        projectId={'243245'}
+        onSubmit={onSubmit}
+        onDismiss={onDismiss}
+        isOpen
+      />
     );
 
     const urlInput = getByLabelText('Manifest url');
@@ -83,6 +99,6 @@ describe('<SkillForm />', () => {
 
     const submitButton = getByText('Confirm');
     fireEvent.click(submitButton);
-    expect(onSubmit).toBeCalled();
+    expect(onSubmit).not.toBeCalled();
   });
 });

Composer/packages/client/src/components/SkillForm/CreateSkillModal/index.tsx

@@ -0,0 +1,212 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+/** @jsx jsx */
+import has from 'lodash/has';
+import { jsx } from '@emotion/core';
+import React, { useState, FormEvent, useEffect, useCallback, useRef } from 'react';
+import formatMessage from 'format-message';
+import { PrimaryButton, DefaultButton } from 'office-ui-fabric-react/lib/Button';
+import { Stack, StackItem } from 'office-ui-fabric-react/lib/Stack';
+import { TextField } from 'office-ui-fabric-react/lib/TextField';
+import { assignDefined, Skill } from '@bfc/shared';
+import debounce from 'lodash/debounce';
+
+import { DialogWrapper } from '../../DialogWrapper';
+import { DialogTypes } from '../../DialogWrapper/styles';
+import { addSkillDialog } from '../../../constants';
+import { ISkillFormData, ISkillFormDataErrors, SkillUrlRegex, SkillNameRegex } from '../types';
+
+import { FormFieldManifestUrl, FormFieldEditName, MarginLeftSmall, FormModalBody } from './styles';
+import { validateManifestUrl } from './validateManifestUrl';
+
+export interface ICreateSkillModalProps {
+  isOpen: boolean;
+  editIndex?: number;
+  skills: Skill[];
+  projectId: string;
+  onSubmit: (skillFormData: ISkillFormData, editIndex: number) => void;
+  onDismiss: () => void;
+}
+
+const defaultFormData = {
+  manifestUrl: '',
+};
+
+const CreateSkillModal: React.FC<ICreateSkillModalProps> = props => {
+  const { editIndex = -1, skills, onSubmit, onDismiss, isOpen, projectId } = props;
+  const originFormData = skills[editIndex];
+  const initialFormData = originFormData
+    ? assignDefined(defaultFormData, { manifestUrl: originFormData.manifestUrl, name: originFormData.name })
+    : { ...defaultFormData };
+  const [formData, setFormData] = useState<ISkillFormData>(initialFormData);
+  const [formDataErrors, setFormDataErrors] = useState<ISkillFormDataErrors>({});
+  const [isValidating, setIsValidating] = useState(false);
+
+  const isModify = editIndex >= 0 && editIndex < skills.length;
+  useEffect(() => {
+    setFormData(initialFormData);
+  }, [editIndex]);
+
+  const asyncManifestUrlValidation = async (projectId: string, manifestUrl: string) => {
+    const err = await validateManifestUrl(projectId, manifestUrl);
+    if (err) {
+      setFormDataErrors(current => ({ ...current, manifestUrl: err }));
+    }
+    setIsValidating(false);
+  };
+
+  const debouncedManifestValidation = useRef(debounce(asyncManifestUrlValidation, 300)).current;
+
+  const validateForm = (newData: ISkillFormData): ISkillFormDataErrors => {
+    const errors: ISkillFormDataErrors = { ...formDataErrors };
+
+    if (has(newData, 'manifestUrl')) {
+      const { manifestUrl } = newData;
+
+      let currentError = '';
+      if (manifestUrl) {
+        if (!SkillUrlRegex.test(manifestUrl)) {
+          currentError = formatMessage('Url should start with http[s]://');
+        }
+
+        const duplicatedItemIndex = skills.findIndex(item => item.manifestUrl === manifestUrl);
+        if (duplicatedItemIndex !== -1 && (!isModify || (isModify && duplicatedItemIndex !== editIndex))) {
+          currentError = formatMessage('Duplicate skill manifest Url');
+        }
+      } else {
+        currentError = formatMessage('Please input a manifest url');
+      }
+
+      if (currentError) {
+        errors.manifestUrl = currentError;
+      } else {
+        delete errors.manifestUrl;
+      }
+    }
+
+    if (has(newData, 'name')) {
+      const { name } = newData;
+      let currentError = '';
+      if (name) {
+        if (!SkillNameRegex.test(name)) {
+          currentError = formatMessage('Name contains invalid charactors');
+        }
+
+        const duplicatedItemIndex = skills.findIndex(item => item.name === name);
+        if (duplicatedItemIndex !== -1 && (!isModify || (isModify && duplicatedItemIndex !== editIndex))) {
+          currentError = formatMessage('Duplicate skill name');
+        }
+      }
+
+      if (currentError) {
+        errors.name = currentError;
+      } else {
+        delete errors.name;
+      }
+    }
+
+    return errors;
+  };
+
+  // fetch manifest url
+  useEffect(() => {
+    if (!formDataErrors.manifestUrl) {
+      setIsValidating(true);
+      debouncedManifestValidation(projectId, formData.manifestUrl);
+    }
+
+    () => {
+      debouncedManifestValidation.cancel();
+    };
+  }, [formData.manifestUrl, formDataErrors.manifestUrl]);
+
+  const updateForm = (field: string) => (e: FormEvent, newValue: string | undefined) => {
+    const newData: ISkillFormData = {
+      ...formData,
+      [field]: newValue,
+    };
+    setFormData(newData);
+    const errors = validateForm(newData);
+    setFormDataErrors(errors);
+  };
+
+  const handleSubmit = useCallback(
+    e => {
+      e.preventDefault();
+      if (isValidating) return;
+      setIsValidating(true);
+
+      const errors = validateForm(formData);
+      setFormDataErrors(errors);
+      if (Object.keys(errors).length > 0) {
+        setIsValidating(false);
+        return;
+      }
+
+      // do async validation
+      validateManifestUrl(projectId, formData.manifestUrl).then(error => {
+        if (error) {
+          setFormDataErrors(current => ({ ...current, manifestUrl: error }));
+          setIsValidating(false);
+          return;
+        }
+        const newFormData = { ...formData };
+        onSubmit(newFormData, editIndex);
+        setIsValidating(false);
+      });
+    },
+    [formData, formDataErrors]
+  );
+
+  const formTitles =
+    editIndex === -1 ? { ...addSkillDialog.SKILL_MANIFEST_FORM } : { ...addSkillDialog.SKILL_MANIFEST_FORM_EDIT };
+
+  const isDisabled = !Object.values(formData).some(Boolean) || Object.values(formDataErrors).some(Boolean);
+
+  return (
+    <DialogWrapper isOpen={isOpen} onDismiss={onDismiss} {...formTitles} dialogType={DialogTypes.CreateFlow}>
+      <form onSubmit={handleSubmit} css={FormModalBody}>
+        <input type="submit" style={{ display: 'none' }} />
+        <Stack tokens={{ childrenGap: '3rem' }}>
+          <StackItem grow={0}>
+            <TextField
+              css={FormFieldManifestUrl}
+              label={formatMessage('Manifest url')}
+              value={formData.manifestUrl}
+              onChange={updateForm('manifestUrl')}
+              errorMessage={formDataErrors.manifestUrl}
+              data-testid="NewSkillManifestUrl"
+              required
+              autoFocus
+            />
+            <TextField
+              css={FormFieldEditName}
+              label={formatMessage('Custom name (optional)')}
+              value={formData.name}
+              onChange={updateForm('name')}
+              errorMessage={formDataErrors.name}
+              data-testid="NewSkillName"
+            />
+          </StackItem>
+
+          <StackItem>
+            <PrimaryButton
+              onClick={handleSubmit}
+              text={formatMessage('Confirm')}
+              disabled={isDisabled || isValidating}
+            />
+            <DefaultButton
+              css={MarginLeftSmall}
+              onClick={onDismiss}
+              text={formatMessage('Cancel')}
+              data-testid="SkillFormCancel"
+            />
+          </StackItem>
+        </Stack>
+      </form>
+    </DialogWrapper>
+  );
+};
+
+export default CreateSkillModal;

Composer/packages/client/src/components/SkillForm/CreateSkillModal/styles.ts

@@ -0,0 +1,21 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import { FontSizes } from 'office-ui-fabric-react/lib/Styling';
+import { css } from '@emotion/core';
+
+export const FormModalBody = css`
+  padding: 24px;
+`;
+
+export const FormFieldManifestUrl = css`
+  width: 40rem;
+`;
+
+export const FormFieldEditName = css`
+  width: 20rem;
+`;
+
+export const MarginLeftSmall = css`
+  margin-left: ${FontSizes.small};
+`;

Composer/packages/client/src/components/SkillForm/CreateSkillModal/validateManifestUrl.ts

@@ -0,0 +1,17 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+import httpClient from '../../../utils/httpUtil';
+
+export async function validateManifestUrl(projectId: string, manifestUrl: any): Promise<string | undefined> {
+  // skip validation if has local other errors.
+  if (!manifestUrl) {
+    return;
+  }
+
+  try {
+    await httpClient.post(`/projects/${projectId}/skill/check`, { url: manifestUrl });
+  } catch (err) {
+    return err.response && err.response.data.message ? err.response.data.message : err;
+  }
+}

Composer/packages/client/src/pages/skills/types.ts → Composer/packages/client/src/components/SkillForm/types.ts

@@ -8,7 +8,9 @@ export interface ISkillFormData {
 
 export interface ISkillFormDataErrors {
   manifestUrl?: string;
+  manifestUrlFetch?: string;
   name?: string;
 }
 
 export const SkillUrlRegex = /^http[s]?:\/\/\w+/;
+export const SkillNameRegex = /^\w[-\w]*$/;

Composer/packages/client/src/constants/index.ts

@@ -213,6 +213,10 @@ export const addSkillDialog = {
     title: formatMessage('Add a skill'),
     subText: formatMessage('Enter a manifest url to add a new skill to your bot.'),
   },
+  SKILL_MANIFEST_FORM_EDIT: {
+    title: formatMessage('Edit a skill'),
+    subText: formatMessage('Enter a manifest url to add a new skill to your bot.'),
+  },
 };
 
 export const SupportedFileTypes = [