fix(ui): expand invalid state upon glossary term add (#18968)

* fix(ui): expand invalid state upon glossary term add invalid expand state for glossary term update * fix tests * update glossary store upon changes in tree * fix tests
open-metadata · Dec 16, 2024 · 254fce4 · 254fce4
1 parent 6c6b760
commit 254fce4
Show file tree

Hide file tree

Showing 6 changed files with 221 additions and 24 deletions.
diff --git a/...c/main/resources/ui/src/components/Glossary/GlossaryTermTab/GlossaryTermTab.component.tsx b/...c/main/resources/ui/src/components/Glossary/GlossaryTermTab/GlossaryTermTab.component.tsx
@@ -106,7 +106,10 @@ const GlossaryTermTab = ({
     useGlossaryStore();
   const { t } = useTranslation();
 
-  const glossaryTerms = (glossaryChildTerms as ModifiedGlossaryTerm[]) ?? [];
+  const glossaryTerms = useMemo(
+    () => (glossaryChildTerms as ModifiedGlossaryTerm[]) ?? [],
+    [glossaryChildTerms]
+  );
 
   const [movedGlossaryTerm, setMovedGlossaryTerm] =
     useState<MoveGlossaryTermType>();

diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryV1.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryV1.component.tsx
@@ -99,8 +99,12 @@ const GlossaryV1 = ({
 
   const [editMode, setEditMode] = useState(false);
 
-  const { activeGlossary, glossaryChildTerms, setGlossaryChildTerms } =
-    useGlossaryStore();
+  const {
+    activeGlossary,
+    glossaryChildTerms,
+    setGlossaryChildTerms,
+    insertNewGlossaryTermToChildTerms,
+  } = useGlossaryStore();
 
   const { id, fullyQualifiedName } = activeGlossary ?? {};
 
@@ -129,11 +133,9 @@ const GlossaryV1 = ({
       const { data } = await getFirstLevelGlossaryTerms(
         params?.glossary ?? params?.parent ?? ''
       );
-      const children = data.map((data) =>
-        data.childrenCount ?? 0 > 0 ? { ...data, children: [] } : data
-      );
-
-      setGlossaryChildTerms(children as ModifiedGlossary[]);
+      // We are considering childrenCount fot expand collapse state
+      // Hence don't need any intervention to list response here
+      setGlossaryChildTerms(data as ModifiedGlossary[]);
     } catch (error) {
       showErrorToast(error as AxiosError);
     } finally {
@@ -232,7 +234,11 @@ const GlossaryV1 = ({
           entity: t('label.glossary-term'),
         });
       } else {
-        updateGlossaryTermInStore(response);
+        updateGlossaryTermInStore({
+          ...response,
+          // Since patch didn't respond with childrenCount preserve it from currentData
+          childrenCount: currentData.childrenCount,
+        });
         setIsEditModalOpen(false);
       }
     } catch (error) {
@@ -257,29 +263,38 @@ const GlossaryV1 = ({
     }
   };
 
-  const onTermModalSuccess = useCallback(() => {
-    loadGlossaryTerms(true);
-    if (!isGlossaryActive && tab !== 'terms') {
-      history.push(
-        getGlossaryTermDetailsPath(
-          selectedData.fullyQualifiedName || '',
-          EntityTabs.TERMS
-        )
-      );
-    }
-    setIsEditModalOpen(false);
-  }, [isGlossaryActive, tab, selectedData]);
+  const onTermModalSuccess = useCallback(
+    (term: GlossaryTerm) => {
+      // Setting loading so that nested terms are rendered again on table with change
+      setIsTermsLoading(true);
+      // Update store with newly created term
+      insertNewGlossaryTermToChildTerms(term);
+      if (!isGlossaryActive && tab !== 'terms') {
+        history.push(
+          getGlossaryTermDetailsPath(
+            selectedData.fullyQualifiedName || '',
+            EntityTabs.TERMS
+          )
+        );
+      }
+      // Close modal and set loading to false
+      setIsEditModalOpen(false);
+      setIsTermsLoading(false);
+    },
+    [isGlossaryActive, tab, selectedData]
+  );
 
   const handleGlossaryTermAdd = async (formData: GlossaryTermForm) => {
     try {
-      await addGlossaryTerm({
+      const term = await addGlossaryTerm({
         ...formData,
         glossary:
           activeGlossaryTerm?.glossary?.name ||
           (selectedData.fullyQualifiedName ?? ''),
         parent: activeGlossaryTerm?.fullyQualifiedName,
       });
-      onTermModalSuccess();
+
+      onTermModalSuccess(term);
     } catch (error) {
       if (
         (error as AxiosError).response?.status === HTTP_STATUS_CODE.CONFLICT

diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Glossary/useGlossary.store.ts b/openmetadata-ui/src/main/resources/ui/src/components/Glossary/useGlossary.store.ts
@@ -12,7 +12,9 @@
  */
 import { create } from 'zustand';
 import { Glossary } from '../../generated/entity/data/glossary';
+import { GlossaryTerm } from '../../generated/entity/data/glossaryTerm';
 import { GlossaryTermWithChildren } from '../../rest/glossaryAPI';
+import { findAndUpdateNested } from '../../utils/GlossaryUtils';
 
 export type ModifiedGlossary = Glossary & {
   children?: GlossaryTermWithChildren[];
@@ -27,6 +29,7 @@ export const useGlossaryStore = create<{
   updateGlossary: (glossary: Glossary) => void;
   updateActiveGlossary: (glossary: Partial<ModifiedGlossary>) => void;
   setGlossaryChildTerms: (glossaryChildTerms: ModifiedGlossary[]) => void;
+  insertNewGlossaryTermToChildTerms: (glossary: GlossaryTerm) => void;
 }>()((set, get) => ({
   glossaries: [],
   activeGlossary: {} as ModifiedGlossary,
@@ -67,6 +70,14 @@ export const useGlossaryStore = create<{
       glossaries[index] = updatedGlossary;
     }
   },
+  insertNewGlossaryTermToChildTerms: (glossary: GlossaryTerm) => {
+    const { glossaryChildTerms } = get();
+
+    // Typically used to updated the glossary term list in the glossary page
+    set({
+      glossaryChildTerms: findAndUpdateNested(glossaryChildTerms, glossary),
+    });
+  },
   setGlossaryChildTerms: (glossaryChildTerms: ModifiedGlossary[]) => {
     set({ glossaryChildTerms });
   },

diff --git a/openmetadata-ui/src/main/resources/ui/src/rest/glossaryAPI.ts b/openmetadata-ui/src/main/resources/ui/src/rest/glossaryAPI.ts
@@ -147,7 +147,7 @@ export const getGlossaryTermByFQN = async (fqn = '', params?: ListParams) => {
 
 export const addGlossaryTerm = async (
   data: CreateGlossaryTerm
-): Promise<AxiosResponse> => {
+): Promise<GlossaryTerm> => {
   const url = '/glossaryTerms';
 
   const response = await APIClient.post(url, data);

diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/GlossaryUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/GlossaryUtils.test.ts
@@ -11,8 +11,10 @@
  *  limitations under the License.
  */
 import { ModifiedGlossaryTerm } from '../components/Glossary/GlossaryTermTab/GlossaryTermTab.interface';
+import { ModifiedGlossary } from '../components/Glossary/useGlossary.store';
 import { EntityType } from '../enums/entity.enum';
 import { Glossary } from '../generated/entity/data/glossary';
+import { GlossaryTerm } from '../generated/entity/data/glossaryTerm';
 import {
   MOCKED_GLOSSARY_TERMS,
   MOCKED_GLOSSARY_TERMS_1,
@@ -22,6 +24,7 @@ import {
 import {
   buildTree,
   filterTreeNodeOptions,
+  findAndUpdateNested,
   findExpandableKeys,
   findExpandableKeysForArray,
   getQueryFilterToExcludeTerm,
@@ -219,3 +222,136 @@ describe('Glossary Utils', () => {
     expect(filteredOptions).toEqual(expected_glossary);
   });
 });
+
+describe('findAndUpdateNested', () => {
+  it('should add new term to the correct parent', () => {
+    const terms: ModifiedGlossary[] = [
+      {
+        fullyQualifiedName: 'parent1',
+        children: [],
+        id: 'parent1',
+        name: 'parent1',
+        description: 'parent1',
+      },
+      {
+        fullyQualifiedName: 'parent2',
+        children: [],
+        id: 'parent2',
+        name: 'parent2',
+        description: 'parent2',
+      },
+    ];
+
+    const newTerm: GlossaryTerm = {
+      fullyQualifiedName: 'child1',
+      parent: {
+        fullyQualifiedName: 'parent1',
+        id: 'parent1',
+        type: 'Glossary',
+      },
+      id: 'child1',
+      name: 'child1',
+      description: 'child1',
+      glossary: {
+        fullyQualifiedName: 'child1',
+        id: 'child1',
+        name: 'child1',
+        description: 'child1',
+        type: 'Glossary',
+      },
+    };
+
+    const updatedTerms = findAndUpdateNested(terms, newTerm);
+
+    expect(updatedTerms[0].children).toHaveLength(1);
+    expect(updatedTerms?.[0].children?.[0]).toEqual(newTerm);
+  });
+
+  it('should add new term to nested parent', () => {
+    const terms: ModifiedGlossary[] = [
+      {
+        fullyQualifiedName: 'parent1',
+        children: [
+          {
+            fullyQualifiedName: 'child1',
+            children: [],
+            glossary: {
+              fullyQualifiedName: 'child1',
+              id: 'child1',
+              name: 'child1',
+              description: 'child1',
+              type: 'Glossary',
+            },
+            id: 'child1',
+            name: 'child1',
+            description: 'child1',
+          },
+        ],
+        id: 'parent1',
+        name: 'parent1',
+        description: 'parent1',
+      },
+    ];
+
+    const newTerm: GlossaryTerm = {
+      fullyQualifiedName: 'child2',
+      parent: { fullyQualifiedName: 'child1', id: 'child1', type: 'Glossary' },
+      id: 'child2',
+      name: 'child2',
+      description: 'child2',
+      glossary: {
+        fullyQualifiedName: 'child2',
+        id: 'child2',
+        name: 'child2',
+        description: 'child2',
+        type: 'Glossary',
+      },
+    };
+
+    const updatedTerms = findAndUpdateNested(terms, newTerm);
+
+    expect(
+      updatedTerms?.[0].children && updatedTerms?.[0].children[0].children
+    ).toHaveLength(1);
+    expect(
+      updatedTerms?.[0].children &&
+        updatedTerms?.[0].children[0].children &&
+        updatedTerms?.[0].children[0].children[0]
+    ).toEqual(newTerm);
+  });
+
+  it('should not modify terms if parent is not found', () => {
+    const terms: ModifiedGlossary[] = [
+      {
+        fullyQualifiedName: 'parent1',
+        children: [],
+        id: 'parent1',
+        name: 'parent1',
+        description: 'parent1',
+      },
+    ];
+
+    const newTerm: GlossaryTerm = {
+      fullyQualifiedName: 'child1',
+      parent: {
+        fullyQualifiedName: 'nonexistent',
+        id: 'nonexistent',
+        type: 'Glossary',
+      },
+      id: 'child1',
+      name: 'child1',
+      description: 'child1',
+      glossary: {
+        fullyQualifiedName: 'child1',
+        id: 'child1',
+        name: 'child1',
+        description: 'child1',
+        type: 'Glossary',
+      },
+    };
+
+    const updatedTerms = findAndUpdateNested(terms, newTerm);
+
+    expect(updatedTerms).toEqual(terms);
+  });
+});
diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/GlossaryUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/GlossaryUtils.tsx
@@ -394,3 +394,35 @@ export const renderReferenceElement = (
     </Tag>
   );
 };
+
+export const findAndUpdateNested = (
+  terms: ModifiedGlossary[],
+  newTerm: GlossaryTerm
+): ModifiedGlossary[] => {
+  // If new term has no parent, it's a top level term
+  // So just update 0 level terms no need to iterate over it
+  if (!newTerm.parent) {
+    return [...terms, newTerm as ModifiedGlossary];
+  }
+
+  // If parent is there means term is  created within a term
+  // So we need to find the parent term and update it's children
+  return terms.map((term) => {
+    if (term.fullyQualifiedName === newTerm.parent?.fullyQualifiedName) {
+      return {
+        ...term,
+        children: [...(term.children || []), newTerm] as GlossaryTerm[],
+      } as ModifiedGlossary;
+    } else if ('children' in term && term.children?.length) {
+      return {
+        ...term,
+        children: findAndUpdateNested(
+          term.children as ModifiedGlossary[],
+          newTerm
+        ),
+      } as ModifiedGlossary;
+    }
+
+    return term;
+  });
+};