From 1f59cec66ebfc2b9203d83f43d79f62200fca412 Mon Sep 17 00:00:00 2001 From: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Date: Mon, 16 Dec 2024 22:08:54 +0530 Subject: [PATCH] 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 (cherry picked from commit 254fce41385e0f7debfd8e6210496f7567b3d1a1) --- .../GlossaryTermTab.component.tsx | 5 +- .../Glossary/GlossaryV1.component.tsx | 59 +++++--- .../components/Glossary/useGlossary.store.ts | 11 ++ .../main/resources/ui/src/rest/glossaryAPI.ts | 2 +- .../ui/src/utils/GlossaryUtils.test.ts | 136 ++++++++++++++++++ .../resources/ui/src/utils/GlossaryUtils.tsx | 32 +++++ 6 files changed, 221 insertions(+), 24 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermTab/GlossaryTermTab.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermTab/GlossaryTermTab.component.tsx index 1e69fa0736f..147f3dff4c1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermTab/GlossaryTermTab.component.tsx +++ b/openmetadata-ui/src/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(); 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 index c92528e87b9..a3c165428aa 100644 --- 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 @@ -98,8 +98,12 @@ const GlossaryV1 = ({ const [editMode, setEditMode] = useState(false); - const { activeGlossary, glossaryChildTerms, setGlossaryChildTerms } = - useGlossaryStore(); + const { + activeGlossary, + glossaryChildTerms, + setGlossaryChildTerms, + insertNewGlossaryTermToChildTerms, + } = useGlossaryStore(); const { id, fullyQualifiedName } = activeGlossary ?? {}; @@ -128,11 +132,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 { @@ -231,7 +233,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) { @@ -256,29 +262,38 @@ const GlossaryV1 = ({ } }; - const onTermModalSuccess = useCallback(() => { - loadGlossaryTerms(true); - if (!isGlossaryActive && tab !== 'terms') { - history.push( - getGlossaryTermDetailsPath( - selectedData.fullyQualifiedName || '', - '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 || '', + '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 index 81ec1d9bef8..f8799cb02a1 100644 --- 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) => 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 index 72d817f816e..2b6a4db5cd0 100644 --- 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 = ( data: CreateGlossaryTerm -): Promise => { +): Promise => { const url = '/glossaryTerms'; return 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 index bea6c81ae09..11263c62500 100644 --- 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 index b95492e8bf3..96c55340509 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/GlossaryUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/GlossaryUtils.tsx @@ -334,3 +334,35 @@ export const filterTreeNodeOptions = ( return filterNodes(options as ModifiedGlossaryTerm[]); }; + +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; + }); +};