Fix: Misc issues for adding Tags, Teams and Service (#2149)

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

* Addressing comments

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:

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

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;
+  }
 }

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({
+      if (!isMounting.current) {
-        ...(data as CreateTagCategory),
+        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;

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) => {
+  const onNewCategoryChange = (data: CreateTagCategory, forceSet = false) => {
-    createTagCategory(data).then((res: AxiosResponse) => {
+    if (errorDataCategory || forceSet) {
-      if (res.data) {
+      const errData: { [key: string]: string } = {};
-        fetchCategories();
+      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';
       }
-    });
+      setErrorDataCategory(errData);
-    setIsAddingCategory(false);
+
+      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) => {
+  const onNewTagChange = (data: TagCategory, forceSet = false) => {
-    createTag(currentCategory?.name, {
+    if (errorDataTag || forceSet) {
-      name: data.name,
+      const errData: { [key: string]: string } = {};
-      description: data.description,
+      if (!data.name.trim()) {
-    }).then((res: AxiosResponse) => {
+        errData['name'] = 'Name is required';
-      if (res.data) {
+      } else if (/\s/g.test(data.name)) {
-        fetchCurrentCategory(currentCategory?.name as string, true);
+        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';
       }
-    });
+      setErrorDataTag(errData);
-    setIsAddingTag(false);
+
+      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={() =>
+                        onClick={() => {
-                          setIsAddingTag((prevState) => !prevState)
+                          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)}
                   />
                 )}

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({
+      if (!isMounting.current) {
-        ...data,
+        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">
+              <label className="tw-form-label">Description</label>
-                Description
-              </label>
               <MarkdownWithPreview ref={markdownRef} value={data.description} />
             </div>
           </div>

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)
+    const errData = onNewDataChange(data, true);
-      .then((res: AxiosResponse) => {
+    if (!Object.values(errData).length) {
-        if (res.data) {
+      createTeam(data)
-          fetchTeams();
+        .then((res: AxiosResponse) => {
-        }
+          if (res.data) {
-      })
+            fetchTeams();
-      .finally(() => {
+          }
-        setIsAddingTeam(false);
+        })
-      });
+        .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)}
                   />
                 )}

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>
+  );
+};