From 1e1ef473d716fc2186274656d20ae23b4bddc998 Mon Sep 17 00:00:00 2001 From: Sweta Agarwalla <105535990+sweta1308@users.noreply.github.com> Date: Wed, 8 Jan 2025 01:03:43 +0530 Subject: [PATCH] FIX: Update glossary listing (#18880) * update fetch glossary function * fix minor issues * fix add glossaries issue * fix delete issue * update glossary listing * update glossary * refactor * updated glossary listing issues * add scrollIntoView * update glossary * fix fetching logic * fix init of glossaries * add playwright tests * add timeout --------- Co-authored-by: karanh37 Co-authored-by: Karan Hotchandani <33024356+karanh37@users.noreply.github.com> --- .../ui/playwright/e2e/Pages/Glossary.spec.ts | 46 +++++++ .../GlossaryLeftPanel.component.tsx | 41 +++++- .../GlossaryPage/GlossaryPage.component.tsx | 119 +++++++++++++++--- .../GlossaryPage/GlossaryPage.test.tsx | 8 +- 4 files changed, 195 insertions(+), 19 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 a8c9e8a8a47..e6ef19e99cc 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 @@ -1186,6 +1186,52 @@ test.describe('Glossary tests', () => { } }); + test('should check for glossary term pagination', async ({ browser }) => { + test.slow(true); + + const { page, afterAction, apiContext } = await performAdminLogin(browser); + const glossaries = []; + for (let i = 0; i < 60; i++) { + const glossary = new Glossary(`PW_GLOSSARY_TEST_${i + 1}`); + await glossary.create(apiContext); + glossaries.push(glossary); + } + + try { + await redirectToHomePage(page); + const glossaryRes = page.waitForResponse('/api/v1/glossaries?*'); + await sidebarClick(page, SidebarItem.GLOSSARY); + await glossaryRes; + + const glossaryAfterRes = page.waitForResponse( + '/api/v1/glossaries?*after=*' + ); + await page + .getByTestId('glossary-left-panel-scroller') + .scrollIntoViewIfNeeded(); + + const res = await glossaryAfterRes; + const json = await res.json(); + + const firstGlossaryName = json.data[0].displayName; + + await expect( + page.getByRole('menuitem', { name: firstGlossaryName }) + ).toBeVisible(); + + const lastGlossaryName = json.data[json.data.length - 1].displayName; + + await expect( + page.getByRole('menuitem', { name: lastGlossaryName }) + ).toBeVisible(); + } finally { + for (const glossary of glossaries) { + await glossary.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/pages/Glossary/GlossaryLeftPanel/GlossaryLeftPanel.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryLeftPanel/GlossaryLeftPanel.component.tsx index c0d87c641c2..b8ef1471c5c 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryLeftPanel/GlossaryLeftPanel.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryLeftPanel/GlossaryLeftPanel.component.tsx @@ -13,7 +13,7 @@ import { Button, Col, Menu, MenuProps, Row, Typography } from 'antd'; import { ItemType } from 'antd/lib/menu/hooks/useItems'; -import React, { useMemo } from 'react'; +import React, { useEffect, useMemo, useRef } from 'react'; import { useTranslation } from 'react-i18next'; import { useHistory } from 'react-router-dom'; import { ReactComponent as GlossaryIcon } from '../../../assets/svg/glossary.svg'; @@ -36,6 +36,7 @@ const GlossaryLeftPanel = ({ glossaries }: GlossaryLeftPanelProps) => { const { permissions } = usePermissionProvider(); const { fqn: glossaryFqn } = useFqn(); const history = useHistory(); + const menuRef = useRef(null); const createGlossaryPermission = useMemo( () => @@ -70,6 +71,43 @@ const GlossaryLeftPanel = ({ glossaries }: GlossaryLeftPanelProps) => { history.push(getGlossaryPath(event.key)); }; + useEffect(() => { + if (menuRef.current && glossaryFqn) { + const items = document?.querySelectorAll( + `[data-testid="glossary-left-panel"] > li > span` + ); + const menuItem = glossaries.find( + (item) => item.fullyQualifiedName === glossaryFqn + ); + + const itemToScroll = Array.from(items).find( + (item) => + item.textContent === menuItem?.name || + item.textContent === menuItem?.displayName + ); + + if (itemToScroll) { + const rect = itemToScroll.getBoundingClientRect(); + const isVisible = + rect.top >= 0 && + rect.bottom <= + (window.innerHeight || document.documentElement.clientHeight); + + if (!isVisible) { + const itemIndex = Array.from(items).findIndex( + (item) => item === itemToScroll + ); + const blockPosition = + itemIndex > Array.from(items).length - 10 ? 'nearest' : 'center'; + itemToScroll.scrollIntoView({ + behavior: 'smooth', + block: blockPosition, + }); + } + } + } + }, [glossaryFqn]); + return ( @@ -102,6 +140,7 @@ const GlossaryLeftPanel = ({ glossaries }: GlossaryLeftPanelProps) => { data-testid="glossary-left-panel" items={menuItems} mode="inline" + ref={menuRef} selectedKeys={[selectedKey]} onClick={handleMenuClick} /> diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.component.tsx index 2839a2b4727..43ab0c3de72 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.component.tsx @@ -14,7 +14,13 @@ import { AxiosError } from 'axios'; import { compare } from 'fast-json-patch'; import { isEmpty } from 'lodash'; -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { + RefObject, + useCallback, + useEffect, + useMemo, + useState, +} from 'react'; import { useTranslation } from 'react-i18next'; import { useHistory, useParams } from 'react-router-dom'; import ErrorPlaceHolder from '../../../components/common/ErrorWithPlaceholder/ErrorPlaceHolder'; @@ -32,6 +38,7 @@ import { import { FQN_SEPARATOR_CHAR } from '../../../constants/char.constants'; import { PAGE_SIZE_LARGE, ROUTES } from '../../../constants/constants'; import { GLOSSARIES_DOCS } from '../../../constants/docs.constants'; +import { observerOptions } from '../../../constants/Mydata.constants'; import { usePermissionProvider } from '../../../context/PermissionProvider/PermissionProvider'; import { ResourceEntity } from '../../../context/PermissionProvider/PermissionProvider.interface'; import { ERROR_PLACEHOLDER_TYPE } from '../../../enums/common.enum'; @@ -39,6 +46,8 @@ import { EntityAction, TabSpecificField } from '../../../enums/entity.enum'; import { Glossary } from '../../../generated/entity/data/glossary'; import { GlossaryTerm } from '../../../generated/entity/data/glossaryTerm'; import { Operation } from '../../../generated/entity/policies/policy'; +import { usePaging } from '../../../hooks/paging/usePaging'; +import { useElementInView } from '../../../hooks/useElementInView'; import { useFqn } from '../../../hooks/useFqn'; import { deleteGlossary, @@ -62,8 +71,16 @@ const GlossaryPage = () => { const { fqn: glossaryFqn } = useFqn(); const history = useHistory(); const { action } = useParams<{ action: EntityAction }>(); + const [initialised, setInitialised] = useState(false); const [isLoading, setIsLoading] = useState(true); + const [isMoreGlossaryLoading, setIsMoreGlossaryLoading] = + useState(false); + const [elementRef, isInView] = useElementInView({ + ...observerOptions, + rootMargin: '10px', + }); + const { paging, pageSize, handlePagingChange } = usePaging(); const [isRightPanelLoading, setIsRightPanelLoading] = useState(true); const [previewAsset, setPreviewAsset] = @@ -133,11 +150,58 @@ const GlossaryPage = () => { history.push(ROUTES.ADD_GLOSSARY); }; - const fetchGlossaryList = async () => { - setIsRightPanelLoading(true); - setIsLoading(true); + const fetchGlossaryList = useCallback(async () => { try { - const { data } = await getGlossariesList({ + let allGlossaries: Glossary[] = []; + let nextPage = paging.after; + let isGlossaryFound = false; + setInitialised(false); + setIsLoading(true); + + do { + const { data, paging: glossaryPaging } = await getGlossariesList({ + fields: [ + TabSpecificField.OWNERS, + TabSpecificField.TAGS, + TabSpecificField.REVIEWERS, + TabSpecificField.VOTES, + TabSpecificField.DOMAIN, + ], + limit: PAGE_SIZE_LARGE, + ...(nextPage && { after: nextPage }), + }); + + allGlossaries = [...allGlossaries, ...data]; + + if (glossaryFqn) { + isGlossaryFound = allGlossaries.some( + (item) => item.fullyQualifiedName === glossaryFqn + ); + } else { + isGlossaryFound = true; // limit to first 50 records only if no glossaryFqn + } + + nextPage = glossaryPaging?.after; + + handlePagingChange(glossaryPaging); + } while (nextPage && !isGlossaryFound); + + setGlossaries(allGlossaries); + } catch (error) { + showErrorToast(error as AxiosError); + } finally { + setIsLoading(false); + setInitialised(true); + } + }, [paging.after, glossaryFqn]); + + const fetchNextGlossaryItems = async (after?: string) => { + try { + let allGlossaries: Glossary[] = glossaries; + + setIsMoreGlossaryLoading(true); + + const { data, paging: glossaryPaging } = await getGlossariesList({ fields: [ TabSpecificField.OWNERS, TabSpecificField.TAGS, @@ -145,20 +209,32 @@ const GlossaryPage = () => { TabSpecificField.VOTES, TabSpecificField.DOMAIN, ], - limit: PAGE_SIZE_LARGE, + after: after, }); - setGlossaries(data); + + allGlossaries = [...allGlossaries, ...data]; + handlePagingChange(glossaryPaging); + + setGlossaries(allGlossaries); } catch (error) { showErrorToast(error as AxiosError); } finally { - setIsLoading(false); - setIsRightPanelLoading(false); + setIsMoreGlossaryLoading(false); } }; + useEffect(() => { - fetchGlossaryList(); - }, []); + if (!initialised) { + fetchGlossaryList(); + } + }, [initialised]); + + useEffect(() => { + if (paging?.after && isInView && !isMoreGlossaryLoading) { + fetchNextGlossaryItems(paging.after); + } + }, [paging, isInView, isMoreGlossaryLoading, pageSize]); const fetchGlossaryTermDetails = async () => { setIsRightPanelLoading(true); @@ -205,10 +281,7 @@ const GlossaryPage = () => { const jsonPatch = compare(activeGlossary as Glossary, updatedData); try { - const response = await patchGlossaries( - activeGlossary?.id as string, - jsonPatch - ); + const response = await patchGlossaries(activeGlossary?.id, jsonPatch); updateActiveGlossary({ ...updatedData, ...response }); @@ -256,13 +329,13 @@ const GlossaryPage = () => { setIsLoading(true); // check if the glossary available const updatedGlossaries = glossaries.filter((item) => item.id !== id); + setGlossaries(updatedGlossaries); const glossaryPath = updatedGlossaries.length > 0 ? getGlossaryPath(updatedGlossaries[0].fullyQualifiedName) : getGlossaryPath(); history.push(glossaryPath); - fetchGlossaryList(); } catch (error) { showErrorToast( error as AxiosError, @@ -270,6 +343,8 @@ const GlossaryPage = () => { entity: t('label.glossary'), }) ); + } finally { + setIsLoading(false); } }; @@ -395,7 +470,17 @@ const GlossaryPage = () => { className: 'content-resizable-panel-container', minWidth: 280, flex: 0.13, - children: , + children: ( + <> + +
} + /> + {isMoreGlossaryLoading && } + + ), }} hideFirstPanel={isImportAction} pageTitle={t('label.glossary')} diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.test.tsx index 8c1b0f7e52f..727c965f5fd 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/Glossary/GlossaryPage/GlossaryPage.test.tsx @@ -21,6 +21,10 @@ import { } from '../../../rest/glossaryAPI'; import GlossaryPage from './GlossaryPage.component'; +jest.mock('../../../hooks/useFqn', () => ({ + useFqn: jest.fn().mockReturnValue({ fqn: 'Business Glossary' }), +})); + jest.mock('react-router-dom', () => ({ useHistory: () => ({ push: jest.fn(), @@ -89,7 +93,9 @@ jest.mock('../../../rest/glossaryAPI', () => ({ .mockImplementation(() => Promise.resolve({ data: MOCK_GLOSSARY })), getGlossariesList: jest .fn() - .mockImplementation(() => Promise.resolve({ data: [MOCK_GLOSSARY] })), + .mockImplementation(() => + Promise.resolve({ data: [MOCK_GLOSSARY], paging: { total: 1 } }) + ), patchGlossaryTerm: jest .fn() .mockImplementation(() => Promise.resolve({ data: MOCK_GLOSSARY })),