From db41fc5ac50e175c006ac900b0f07b95fea22c64 Mon Sep 17 00:00:00 2001 From: Karan Hotchandani <33024356+karanh37@users.noreply.github.com> Date: Mon, 14 Apr 2025 16:21:32 +0530 Subject: [PATCH] fix glossary duplicates (#20801) --- .../ui/playwright/e2e/Pages/Glossary.spec.ts | 50 +++++++++++ .../GlossaryTermModal.component.tsx | 31 ++++++- .../Glossary/GlossaryV1.component.tsx | 85 +++++-------------- 3 files changed, 102 insertions(+), 64 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts index cb9e65a468f..393b2f40682 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Glossary.spec.ts @@ -55,6 +55,7 @@ import { deselectColumns, dragAndDropColumn, dragAndDropTerm, + fillGlossaryTermDetails, filterStatus, goToAssetsTab, openColumnDropdown, @@ -1384,6 +1385,55 @@ test.describe('Glossary tests', () => { } }); + test('Check for duplicate Glossary Term', async ({ browser }) => { + const { page, afterAction, apiContext } = await performAdminLogin(browser); + const glossary1 = new Glossary('PW_TEST_GLOSSARY'); + const glossaryTerm1 = new GlossaryTerm( + glossary1, + undefined, + 'PW_TEST_TERM' + ); + const glossaryTerm2 = new GlossaryTerm( + glossary1, + undefined, + 'Pw_test_term' + ); + await glossary1.create(apiContext); + + try { + await sidebarClick(page, SidebarItem.GLOSSARY); + await selectActiveGlossary(page, glossary1.data.displayName); + + await test.step('Create Glossary Term One', async () => { + await fillGlossaryTermDetails(page, glossaryTerm1.data, false, false); + + const glossaryTermResponse = page.waitForResponse( + '/api/v1/glossaryTerms' + ); + await page.click('[data-testid="save-glossary-term"]'); + await glossaryTermResponse; + }); + + await test.step('Create Glossary Term Two', async () => { + await fillGlossaryTermDetails(page, glossaryTerm2.data, false, false); + + const glossaryTermResponse = page.waitForResponse( + '/api/v1/glossaryTerms' + ); + await page.click('[data-testid="save-glossary-term"]'); + await glossaryTermResponse; + + await expect(page.locator('#name_help')).toHaveText( + `A term with the name '${glossaryTerm2.data.name}' already exists in '${glossary1.data.name}' glossary.` + ); + }); + } finally { + await glossaryTerm1.delete(apiContext); + await glossary1.delete(apiContext); + await afterAction(); + } + }); + test.afterAll(async ({ browser }) => { const { afterAction, apiContext } = await performAdminLogin(browser); await user1.delete(apiContext); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermModal/GlossaryTermModal.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermModal/GlossaryTermModal.component.tsx index 69a46dc394f..81ba38482da 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermModal/GlossaryTermModal.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Glossary/GlossaryTermModal/GlossaryTermModal.component.tsx @@ -73,6 +73,33 @@ const GlossaryTermModal: FC = ({ setSaving(true); try { await onSave(values); + } catch (error) { + if ((error as AxiosError)?.response?.status === 400) { + const errorMessage = + (error as AxiosError<{ message: string }>)?.response?.data?.message ?? + ''; + + // Handle name duplication error + if (errorMessage.includes('already exists')) { + form.setFields([ + { + name: 'name', + errors: [errorMessage], + }, + ]); + } + // Handle tag mutual exclusivity error + else if (errorMessage.includes('mutually exclusive')) { + form.setFields([ + { + name: 'tags', + errors: [errorMessage], + }, + ]); + } + } + + throw error; } finally { setSaving(false); } @@ -84,7 +111,9 @@ const GlossaryTermModal: FC = ({ } else { setIsLoading(false); } - !visible && form.resetFields(); + if (!visible) { + form.resetFields(); + } }, [visible]); return ( 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 022cc4d4f2b..f13340fe7b2 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 @@ -18,7 +18,6 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { useHistory, useParams } from 'react-router-dom'; import { withActivityFeed } from '../../components/AppRouter/withActivityFeed'; -import { HTTP_STATUS_CODE } from '../../constants/Auth.constants'; import { usePermissionProvider } from '../../context/PermissionProvider/PermissionProvider'; import { OperationPermission, @@ -196,40 +195,21 @@ const GlossaryV1 = ({ currentData: GlossaryTerm, updatedData: GlossaryTerm ) => { - try { - const jsonPatch = compare(currentData, updatedData); - const response = await patchGlossaryTerm(currentData?.id, jsonPatch); - if (!response) { - throw t('server.entity-updating-error', { + const jsonPatch = compare(currentData, updatedData); + const response = await patchGlossaryTerm(currentData?.id, jsonPatch); + if (!response) { + throw new Error( + t('server.entity-updating-error', { entity: t('label.glossary-term'), - }); - } else { - updateGlossaryTermInStore({ - ...response, - // Since patch didn't respond with childrenCount preserve it from currentData - childrenCount: currentData.childrenCount, - }); - setIsEditModalOpen(false); - } - } catch (error) { - if ( - (error as AxiosError).response?.status === HTTP_STATUS_CODE.CONFLICT - ) { - showErrorToast( - t('server.entity-already-exist', { - entity: t('label.glossary-term'), - entityPlural: t('label.glossary-term-lowercase-plural'), - name: updatedData.name, - }) - ); - } else { - showErrorToast( - error as AxiosError, - t('server.entity-updating-error', { - entity: t('label.glossary-term-lowercase'), - }) - ); - } + }) + ); + } else { + updateGlossaryTermInStore({ + ...response, + // Since patch didn't respond with childrenCount preserve it from currentData + childrenCount: currentData.childrenCount, + }); + setIsEditModalOpen(false); } }; @@ -255,36 +235,15 @@ const GlossaryV1 = ({ ); const handleGlossaryTermAdd = async (formData: GlossaryTermForm) => { - try { - const term = await addGlossaryTerm({ - ...formData, - glossary: - activeGlossaryTerm?.glossary?.name || - (selectedData.fullyQualifiedName ?? ''), - parent: activeGlossaryTerm?.fullyQualifiedName, - }); + const term = await addGlossaryTerm({ + ...formData, + glossary: + activeGlossaryTerm?.glossary?.name || + (selectedData.fullyQualifiedName ?? ''), + parent: activeGlossaryTerm?.fullyQualifiedName, + }); - onTermModalSuccess(term); - } catch (error) { - if ( - (error as AxiosError).response?.status === HTTP_STATUS_CODE.CONFLICT - ) { - showErrorToast( - t('server.entity-already-exist', { - entity: t('label.glossary-term'), - entityPlural: t('label.glossary-term-lowercase-plural'), - name: formData.name, - }) - ); - } else { - showErrorToast( - error as AxiosError, - t('server.create-entity-error', { - entity: t('label.glossary-term-lowercase'), - }) - ); - } - } + onTermModalSuccess(term); }; const handleGlossaryTermSave = async (formData: GlossaryTermForm) => {