From cbb0b837c0253debaf2ea12c9bcf4e961c11b6b7 Mon Sep 17 00:00:00 2001
From: darth-coder00 <86726556+darth-coder00@users.noreply.github.com>
Date: Tue, 11 Jan 2022 15:06:46 +0530
Subject: [PATCH] Fix: Misc issues for adding Tags, Teams and Service (#2149)

* Fix: Misc issues for adding Tags, Teams and Service

* Addressing comments
---
 .../AddServiceModal/AddServiceModal.tsx       |  11 +-
 .../src/components/Modals/FormModal/index.tsx |  18 ++-
 .../resources/ui/src/interface/types.d.ts     |   4 +
 .../main/resources/ui/src/pages/tags/Form.tsx |  33 +++---
 .../resources/ui/src/pages/tags/index.tsx     | 106 ++++++++++++++----
 .../resources/ui/src/pages/teams/Form.tsx     |  31 +++--
 .../resources/ui/src/pages/teams/index.tsx    |  63 +++++++++--
 .../resources/ui/src/utils/CommonUtils.tsx    |   8 ++
 8 files changed, 205 insertions(+), 69 deletions(-)

diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Modals/AddServiceModal/AddServiceModal.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Modals/AddServiceModal/AddServiceModal.tsx
index 0233297092a..9e749e58101 100644
--- a/openmetadata-ui/src/main/resources/ui/src/components/Modals/AddServiceModal/AddServiceModal.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/components/Modals/AddServiceModal/AddServiceModal.tsx
@@ -24,6 +24,7 @@ import {
 import { DatabaseService } from '../../../generated/entity/services/databaseService';
 import { MessagingService } from '../../../generated/entity/services/messagingService';
 import { PipelineService } from '../../../generated/entity/services/pipelineService';
+import { errorMsg } from '../../../utils/CommonUtils';
 // import { fromISOString } from '../../../utils/ServiceUtils';
 import { Button } from '../../buttons/Button/Button';
 import MarkdownWithPreview from '../../common/editor/MarkdownWithPreview';
@@ -161,14 +162,6 @@ const seprateUrl = (url?: string) => {
   return {};
 };
 
-const errorMsg = (value: string) => {
-  return (
-    <div className="tw-mt-1">
-      <strong className="tw-text-red-500 tw-text-xs tw-italic">{value}</strong>
-    </div>
-  );
-};
-
 export const AddServiceModal: FunctionComponent<Props> = ({
   header,
   serviceName,
@@ -338,7 +331,7 @@ export const AddServiceModal: FunctionComponent<Props> = ({
   const handleSave = () => {
     let setMsg: ErrorMsg = {
       selectService: !selectService,
-      name: !name,
+      name: !name.trim(),
     };
     switch (serviceName) {
       case ServiceCategory.DATABASE_SERVICES:
diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Modals/FormModal/index.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Modals/FormModal/index.tsx
index 05892f570b2..39583bc436f 100644
--- a/openmetadata-ui/src/main/resources/ui/src/components/Modals/FormModal/index.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/components/Modals/FormModal/index.tsx
@@ -11,27 +11,31 @@
  *  limitations under the License.
  */
 
-import { Team } from 'Models';
+import { FormErrorData, Team } from 'Models';
 import React, { useRef, useState } from 'react';
 import { TagsCategory } from '../../../pages/tags/tagsTypes';
 import { Button } from '../../buttons/Button/Button';
 type FormData = TagsCategory | Team;
 type FormModalProp = {
   onCancel: () => void;
-  onSave: (data: TagsCategory) => void;
+  onChange?: (data: TagsCategory | Team) => void;
+  onSave: (data: TagsCategory | Team) => void;
   form: React.ElementType;
   header: string;
   initialData: FormData;
+  errorData?: FormErrorData;
 };
 type FormRef = {
   fetchMarkDownData: () => string;
 };
 const FormModal = ({
   onCancel,
+  onChange,
   onSave,
   form: Form,
   header,
   initialData,
+  errorData,
 }: FormModalProp) => {
   const formRef = useRef<FormRef>();
   const [data, setData] = useState<FormData>(initialData);
@@ -57,7 +61,15 @@ const FormModal = ({
             </p>
           </div>
           <div className="tw-modal-body">
-            <Form initialData={initialData} ref={formRef} saveData={setData} />
+            <Form
+              errorData={errorData}
+              initialData={initialData}
+              ref={formRef}
+              saveData={(data: TagsCategory | Team) => {
+                setData(data);
+                onChange && onChange(data);
+              }}
+            />
           </div>
           <div className="tw-modal-footer" data-testid="cta-container">
             <Button
diff --git a/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts b/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts
index 8782f9b0800..19b256294e4 100644
--- a/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts
+++ b/openmetadata-ui/src/main/resources/ui/src/interface/types.d.ts
@@ -422,4 +422,8 @@ declare module 'Models' {
     openInNewTab?: boolean;
     showLabel?: boolean;
   };
+
+  export interface FormErrorData {
+    [key: string]: string | undefined;
+  }
 }
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/tags/Form.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/tags/Form.tsx
index af10a2dd0fd..e140754f622 100644
--- a/openmetadata-ui/src/main/resources/ui/src/pages/tags/Form.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/pages/tags/Form.tsx
@@ -11,7 +11,7 @@
  *  limitations under the License.
  */
 
-import PropTypes from 'prop-types';
+import { FormErrorData } from 'Models';
 import React, {
   forwardRef,
   useEffect,
@@ -21,6 +21,7 @@ import React, {
 } from 'react';
 import MarkdownWithPreview from '../../components/common/editor/MarkdownWithPreview';
 import { CreateTagCategory } from '../../generated/api/tags/createTagCategory';
+import { errorMsg } from '../../utils/CommonUtils';
 
 type CustomTagCategory = {
   categoryType: string;
@@ -31,18 +32,22 @@ type CustomTagCategory = {
 type FormProp = {
   saveData: (value: CreateTagCategory) => void;
   initialData: CustomTagCategory;
+  errorData?: FormErrorData;
 };
 type EditorContentRef = {
   getEditorContent: () => string;
 };
 const Form: React.FC<FormProp> = forwardRef(
-  ({ saveData, initialData }, ref): JSX.Element => {
+  ({ saveData, initialData, errorData }: FormProp, ref): JSX.Element => {
     const [data, setData] = useState<CustomTagCategory>({
       name: initialData.name,
       description: initialData.description,
       categoryType: initialData.categoryType,
     });
+
+    const isMounting = useRef<boolean>(true);
     const markdownRef = useRef<EditorContentRef>();
+
     const onChangeHadler = (
       e: React.ChangeEvent<HTMLInputElement | HTMLSelectElement>
     ) => {
@@ -62,11 +67,18 @@ const Form: React.FC<FormProp> = forwardRef(
     }));
 
     useEffect(() => {
-      saveData({
-        ...(data as CreateTagCategory),
-      });
+      if (!isMounting.current) {
+        saveData({
+          ...(data as CreateTagCategory),
+        });
+      }
     }, [data]);
 
+    // alwyas Keep this useEffect at the end...
+    useEffect(() => {
+      isMounting.current = false;
+    }, []);
+
     return (
       <div className="tw-w-full tw-flex ">
         <div className="tw-flex tw-w-full">
@@ -92,7 +104,6 @@ const Form: React.FC<FormProp> = forwardRef(
             <div className="tw-mb-4">
               <label className="tw-form-label required-field">Name</label>
               <input
-                required
                 autoComplete="off"
                 className="tw-text-sm tw-appearance-none tw-border tw-border-main
                 tw-rounded tw-w-full tw-py-2 tw-px-3 tw-text-grey-body  tw-leading-tight
@@ -103,6 +114,7 @@ const Form: React.FC<FormProp> = forwardRef(
                 value={data.name}
                 onChange={onChangeHadler}
               />
+              {errorData?.name && errorMsg(errorData.name)}
             </div>
             <div>
               <label className="tw-form-label required-field">
@@ -117,13 +129,4 @@ const Form: React.FC<FormProp> = forwardRef(
   }
 );
 
-Form.propTypes = {
-  saveData: PropTypes.func.isRequired,
-  initialData: PropTypes.shape({
-    name: PropTypes.string.isRequired,
-    description: PropTypes.string.isRequired,
-    categoryType: PropTypes.string.isRequired,
-  }).isRequired,
-};
-
 export default Form;
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/tags/index.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/tags/index.tsx
index a7ad9e794e9..40696d4aa5b 100644
--- a/openmetadata-ui/src/main/resources/ui/src/pages/tags/index.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/pages/tags/index.tsx
@@ -13,7 +13,8 @@
 
 import { AxiosError, AxiosResponse } from 'axios';
 import classNames from 'classnames';
-import { EntityTags } from 'Models';
+import { isUndefined } from 'lodash';
+import { EntityTags, FormErrorData } from 'Models';
 import React, { useEffect, useState } from 'react';
 import { Link } from 'react-router-dom';
 import {
@@ -64,6 +65,8 @@ const TagsPage = () => {
   const [editTag, setEditTag] = useState<TagClass>();
   const [error, setError] = useState<string>('');
   const [isLoading, setIsLoading] = useState<boolean>(false);
+  const [errorDataCategory, setErrorDataCategory] = useState<FormErrorData>();
+  const [errorDataTag, setErrorDataTag] = useState<FormErrorData>();
 
   const fetchCategories = () => {
     setIsLoading(true);
@@ -97,13 +100,36 @@ const TagsPage = () => {
     }
   };
 
-  const createCategory = (data: CreateTagCategory) => {
-    createTagCategory(data).then((res: AxiosResponse) => {
-      if (res.data) {
-        fetchCategories();
+  const onNewCategoryChange = (data: CreateTagCategory, forceSet = false) => {
+    if (errorDataCategory || forceSet) {
+      const errData: { [key: string]: string } = {};
+      if (!data.name.trim()) {
+        errData['name'] = 'Name is required';
+      } else if (/\s/g.test(data.name)) {
+        errData['name'] = 'Name with space is not allowed';
+      } else if (
+        !isUndefined(categories.find((item) => item.name === data.name))
+      ) {
+        errData['name'] = 'Name already exists';
       }
-    });
-    setIsAddingCategory(false);
+      setErrorDataCategory(errData);
+
+      return errData;
+    }
+
+    return {};
+  };
+
+  const createCategory = (data: CreateTagCategory) => {
+    const errData = onNewCategoryChange(data, true);
+    if (!Object.values(errData).length) {
+      createTagCategory(data).then((res: AxiosResponse) => {
+        if (res.data) {
+          fetchCategories();
+        }
+      });
+      setIsAddingCategory(false);
+    }
   };
 
   const UpdateCategory = (updatedHTML: string) => {
@@ -119,16 +145,43 @@ const TagsPage = () => {
     setIsEditCategory(false);
   };
 
-  const createPrimaryTag = (data: TagCategory) => {
-    createTag(currentCategory?.name, {
-      name: data.name,
-      description: data.description,
-    }).then((res: AxiosResponse) => {
-      if (res.data) {
-        fetchCurrentCategory(currentCategory?.name as string, true);
+  const onNewTagChange = (data: TagCategory, forceSet = false) => {
+    if (errorDataTag || forceSet) {
+      const errData: { [key: string]: string } = {};
+      if (!data.name.trim()) {
+        errData['name'] = 'Name is required';
+      } else if (/\s/g.test(data.name)) {
+        errData['name'] = 'Name with space is not allowed';
+      } else if (
+        !isUndefined(
+          currentCategory?.children?.find(
+            (item) => (item as TagClass)?.name === data.name
+          )
+        )
+      ) {
+        errData['name'] = 'Name already exists';
       }
-    });
-    setIsAddingTag(false);
+      setErrorDataTag(errData);
+
+      return errData;
+    }
+
+    return {};
+  };
+
+  const createPrimaryTag = (data: TagCategory) => {
+    const errData = onNewTagChange(data, true);
+    if (!Object.values(errData).length) {
+      createTag(currentCategory?.name, {
+        name: data.name,
+        description: data.description,
+      }).then((res: AxiosResponse) => {
+        if (res.data) {
+          fetchCurrentCategory(currentCategory?.name as string, true);
+        }
+      });
+      setIsAddingTag(false);
+    }
   };
   const updatePrimaryTag = (updatedHTML: string) => {
     updateTag(currentCategory?.name, editTag?.name, {
@@ -187,7 +240,10 @@ const TagsPage = () => {
               size="small"
               theme="primary"
               variant="contained"
-              onClick={() => setIsAddingCategory((prevState) => !prevState)}>
+              onClick={() => {
+                setIsAddingCategory((prevState) => !prevState);
+                setErrorDataCategory(undefined);
+              }}>
               <i aria-hidden="true" className="fa fa-plus" />
             </Button>
           </NonAdminAction>
@@ -250,9 +306,10 @@ const TagsPage = () => {
                         size="small"
                         theme="primary"
                         variant="contained"
-                        onClick={() =>
-                          setIsAddingTag((prevState) => !prevState)
-                        }>
+                        onClick={() => {
+                          setIsAddingTag((prevState) => !prevState);
+                          setErrorDataTag(undefined);
+                        }}>
                         Add new tag
                       </Button>
                     </NonAdminAction>
@@ -394,9 +451,8 @@ const TagsPage = () => {
                                         />
                                       </button>
                                     ) : (
-                                      <span className="tw-opacity-0 group-hover:tw-opacity-100">
+                                      <span className="tw-opacity-60 group-hover:tw-opacity-100 tw-text-grey-muted group-hover:tw-text-primary">
                                         <Tags
-                                          className="tw-border-main"
                                           startWith="+ "
                                           tag="Add tag"
                                           type="outlined"
@@ -427,6 +483,7 @@ const TagsPage = () => {
                 )}
                 {isAddingCategory && (
                   <FormModal
+                    errorData={errorDataCategory}
                     form={Form}
                     header="Adding new category"
                     initialData={{
@@ -435,11 +492,15 @@ const TagsPage = () => {
                       categoryType: TagCategoryType.Descriptive,
                     }}
                     onCancel={() => setIsAddingCategory(false)}
+                    onChange={(data) =>
+                      onNewCategoryChange(data as TagCategory)
+                    }
                     onSave={(data) => createCategory(data as TagCategory)}
                   />
                 )}
                 {isAddingTag && (
                   <FormModal
+                    errorData={errorDataTag}
                     form={Form}
                     header={`Adding new tag on ${currentCategory?.name}`}
                     initialData={{
@@ -448,6 +509,7 @@ const TagsPage = () => {
                       categoryType: '',
                     }}
                     onCancel={() => setIsAddingTag(false)}
+                    onChange={(data) => onNewTagChange(data as TagCategory)}
                     onSave={(data) => createPrimaryTag(data as TagCategory)}
                   />
                 )}
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/teams/Form.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/teams/Form.tsx
index e511ba9ea0b..f1c154f8e77 100644
--- a/openmetadata-ui/src/main/resources/ui/src/pages/teams/Form.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/pages/teams/Form.tsx
@@ -11,7 +11,7 @@
  *  limitations under the License.
  */
 
-import { Team } from 'Models';
+import { FormErrorData, Team } from 'Models';
 import React, {
   forwardRef,
   useEffect,
@@ -20,16 +20,18 @@ import React, {
   useState,
 } from 'react';
 import MarkdownWithPreview from '../../components/common/editor/MarkdownWithPreview';
+import { errorMsg } from '../../utils/CommonUtils';
 
 type FormProp = {
   saveData: (value: {}) => void;
   initialData: Team;
+  errorData?: FormErrorData;
 };
 type EditorContentRef = {
   getEditorContent: () => string;
 };
 const Form: React.FC<FormProp> = forwardRef(
-  ({ saveData, initialData }: FormProp, ref): JSX.Element => {
+  ({ saveData, initialData, errorData }: FormProp, ref): JSX.Element => {
     const [data, setData] = useState<Team>({
       name: initialData.name,
       description: initialData.description,
@@ -40,6 +42,7 @@ const Form: React.FC<FormProp> = forwardRef(
       users: initialData.users || [],
     });
 
+    const isMounting = useRef<boolean>(true);
     const markdownRef = useRef<EditorContentRef>();
 
     const onChangeHadler = (
@@ -60,11 +63,21 @@ const Form: React.FC<FormProp> = forwardRef(
     }));
 
     useEffect(() => {
-      saveData({
-        ...data,
-      });
+      if (!isMounting.current) {
+        saveData({
+          ...data,
+          name: data.name.trim(),
+          displayName: data.displayName.trim(),
+          description: data.description.trim(),
+        });
+      }
     }, [data]);
 
+    // alwyas Keep this useEffect at the end...
+    useEffect(() => {
+      isMounting.current = false;
+    }, []);
+
     return (
       <div className="tw-w-full tw-flex ">
         <div className="tw-flex tw-w-full">
@@ -72,7 +85,6 @@ const Form: React.FC<FormProp> = forwardRef(
             <div className="tw-mb-4">
               <label className="tw-form-label required-field">Name</label>
               <input
-                required
                 autoComplete="off"
                 className="tw-form-inputs tw-px-3 tw-py-1"
                 name="name"
@@ -81,13 +93,13 @@ const Form: React.FC<FormProp> = forwardRef(
                 value={data.name}
                 onChange={onChangeHadler}
               />
+              {errorData?.name && errorMsg(errorData.name)}
             </div>
             <div className="tw-mb-4">
               <label className="tw-form-label required-field">
                 Display name
               </label>
               <input
-                required
                 autoComplete="off"
                 className="tw-form-inputs tw-px-3 tw-py-1"
                 name="displayName"
@@ -96,11 +108,10 @@ const Form: React.FC<FormProp> = forwardRef(
                 value={data.displayName}
                 onChange={onChangeHadler}
               />
+              {errorData?.displayName && errorMsg(errorData.displayName)}
             </div>
             <div>
-              <label className="tw-form-label required-field">
-                Description
-              </label>
+              <label className="tw-form-label">Description</label>
               <MarkdownWithPreview ref={markdownRef} value={data.description} />
             </div>
           </div>
diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/teams/index.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/teams/index.tsx
index 81c08ef75da..68ec3acb577 100644
--- a/openmetadata-ui/src/main/resources/ui/src/pages/teams/index.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/pages/teams/index.tsx
@@ -14,7 +14,9 @@
 import { AxiosError, AxiosResponse } from 'axios';
 import classNames from 'classnames';
 import { compare } from 'fast-json-patch';
+import { isUndefined, toLower } from 'lodash';
 import { observer } from 'mobx-react';
+import { FormErrorData } from 'Models';
 import React, { useEffect, useState } from 'react';
 import { Link, useHistory, useParams } from 'react-router-dom';
 import AppState from '../../AppState';
@@ -40,6 +42,7 @@ import {
 import { Team } from '../../generated/entity/teams/team';
 import { User } from '../../generated/entity/teams/user';
 import { useAuth } from '../../hooks/authHooks';
+import useToastContext from '../../hooks/useToastContext';
 import { UserTeam } from '../../interface/team.interface';
 import { getActiveCatClass, getCountBadge } from '../../utils/CommonUtils';
 import AddUsersModal from './AddUsersModal';
@@ -59,6 +62,9 @@ const TeamsPage = () => {
   const [isAddingTeam, setIsAddingTeam] = useState<boolean>(false);
   const [isAddingUsers, setIsAddingUsers] = useState<boolean>(false);
   const [userList, setUserList] = useState<Array<User>>([]);
+  const [errorData, setErrorData] = useState<FormErrorData>();
+
+  const showToast = useToastContext();
 
   const fetchTeams = () => {
     setIsLoading(true);
@@ -100,16 +106,48 @@ const TeamsPage = () => {
     }
   };
 
+  const onNewDataChange = (data: Team, forceSet = false) => {
+    if (errorData || forceSet) {
+      const errData: { [key: string]: string } = {};
+      if (!data.name.trim()) {
+        errData['name'] = 'Name is required';
+      } else if (
+        !isUndefined(
+          teams.find((item) => toLower(item.name) === toLower(data.name))
+        )
+      ) {
+        errData['name'] = 'Name already exists';
+      }
+      if (!data.displayName?.trim()) {
+        errData['displayName'] = 'Display name is required';
+      }
+      setErrorData(errData);
+
+      return errData;
+    }
+
+    return {};
+  };
+
   const createNewTeam = (data: Team) => {
-    createTeam(data)
-      .then((res: AxiosResponse) => {
-        if (res.data) {
-          fetchTeams();
-        }
-      })
-      .finally(() => {
-        setIsAddingTeam(false);
-      });
+    const errData = onNewDataChange(data, true);
+    if (!Object.values(errData).length) {
+      createTeam(data)
+        .then((res: AxiosResponse) => {
+          if (res.data) {
+            fetchTeams();
+          }
+        })
+        .catch((error: AxiosError) => {
+          showToast({
+            variant: 'error',
+            body: error.message ?? 'Something went wrong!',
+          });
+        })
+        .finally(() => {
+          setIsAddingTeam(false);
+        });
+    }
   };
 
   const createUsers = (data: Array<UserTeam>) => {
@@ -280,7 +318,10 @@ const TeamsPage = () => {
               size="small"
               theme="primary"
               variant="contained"
-              onClick={() => setIsAddingTeam(true)}>
+              onClick={() => {
+                setErrorData(undefined);
+                setIsAddingTeam(true);
+              }}>
               <i aria-hidden="true" className="fa fa-plus" />
             </Button>
           </NonAdminAction>
@@ -440,6 +481,7 @@ const TeamsPage = () => {
 
                 {isAddingTeam && (
                   <FormModal
+                    errorData={errorData}
                     form={Form}
                     header="Adding new team"
                     initialData={{
@@ -448,6 +490,7 @@ const TeamsPage = () => {
                       displayName: '',
                     }}
                     onCancel={() => setIsAddingTeam(false)}
+                    onChange={(data) => onNewDataChange(data as Team)}
                     onSave={(data) => createNewTeam(data as Team)}
                   />
                 )}
diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/CommonUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/CommonUtils.tsx
index 3e1581b82c7..1dce286091c 100644
--- a/openmetadata-ui/src/main/resources/ui/src/utils/CommonUtils.tsx
+++ b/openmetadata-ui/src/main/resources/ui/src/utils/CommonUtils.tsx
@@ -278,3 +278,11 @@ export const getOwnerIds = (
 export const getActiveCatClass = (name: string, activeName = '') => {
   return activeName === name ? 'activeCategory' : '';
 };
+
+export const errorMsg = (value: string) => {
+  return (
+    <div className="tw-mt-1">
+      <strong className="tw-text-red-500 tw-text-xs tw-italic">{value}</strong>
+    </div>
+  );
+};