diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ContextCenterArticles.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ContextCenterArticles.spec.ts index 05ac57901e81..c48519b5a680 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ContextCenterArticles.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ContextCenterArticles.spec.ts @@ -735,6 +735,7 @@ test.describe('Context Center Articles', () => { ).not.toBeVisible(); await page.getByLabel('Expand All').click(); + await expect(page.getByLabel('Collapse All')).toBeVisible(); await expect( page.getByTestId(`page-node-${child.displayName}`) ).toBeVisible(); @@ -752,6 +753,98 @@ test.describe('Context Center Articles', () => { await cleanupAfterAction(); }); + test('Expanding a multi-level hierarchy does not throw and renders no duplicate nodes', async ({ + page, + }) => { + test.slow(); + const { apiContext, afterAction } = await getApiContext(page); + + const grandparent = await createArticleViaApi(apiContext, { + displayName: `CC Deep Grandparent ${uuid()}`, + name: `cc_deep_grandparent_${uuid()}`, + }); + const parent = await createArticleViaApi(apiContext, { + displayName: `CC Deep Parent ${uuid()}`, + name: `cc_deep_parent_${uuid()}`, + }); + const child = await createArticleViaApi(apiContext, { + displayName: `CC Deep Child ${uuid()}`, + name: `cc_deep_child_${uuid()}`, + }); + + const patchParent = async ( + pageId: string, + parentEntity: KnowledgeCenterResponseDataType + ) => { + await apiContext.patch(`/api/v1/contextCenter/pages/${pageId}`, { + data: [ + { + op: 'add', + path: '/parent', + value: { + id: parentEntity.id, + type: 'page', + fullyQualifiedName: parentEntity.fullyQualifiedName, + displayName: parentEntity.displayName, + name: parentEntity.name, + }, + }, + ], + headers: { 'Content-Type': 'application/json-patch+json' }, + }); + }; + + await patchParent(parent.id, grandparent); + await patchParent(child.id, parent); + await afterAction(); + + const pageErrors: string[] = []; + page.on('pageerror', (error) => + pageErrors.push(error.stack ?? error.message) + ); + + await navigateToArticles(page); + await scrollHierarchyToNode(page, grandparent.displayName); + + await page + .getByRole('button', { + name: `Expand ${grandparent.displayName}`, + }) + .click(); + await expect( + page.getByTestId(`page-node-${parent.displayName}`) + ).toBeVisible(); + + await page + .getByRole('button', { + name: `Expand ${parent.displayName}`, + }) + .click(); + await expect( + page.getByTestId(`page-node-${child.displayName}`) + ).toBeVisible(); + + await page.getByLabel('Expand All').click(); + await expect(page.getByLabel('Collapse All')).toBeVisible(); + await page.getByLabel('Collapse All').click(); + await expect(page.getByLabel('Expand All')).toBeVisible(); + await page.getByLabel('Expand All').click(); + await expect(page.getByLabel('Collapse All')).toBeVisible(); + await expect( + page.getByTestId(`page-node-${child.displayName}`) + ).toBeVisible(); + + for (const displayName of [ + grandparent.displayName, + parent.displayName, + child.displayName, + ]) { + await expect(page.getByTestId(`page-node-${displayName}`)).toHaveCount(1); + } + + expect(pageErrors).toEqual([]); + }); + test('Article detail layout, drawer, activity tab, and version page work', async ({ page, browser, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgeCenterLayout/KnowledgeCenterLayout.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgeCenterLayout/KnowledgeCenterLayout.test.tsx deleted file mode 100644 index acc8fbd8b19d..000000000000 --- a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgeCenterLayout/KnowledgeCenterLayout.test.tsx +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Copyright 2026 Collate. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { render, screen } from '@testing-library/react'; -import KnowledgeCenterLayout from './KnowledgeCenterLayout'; - -jest.mock('components/common/DocumentTitle/DocumentTitle', () => - jest.fn().mockImplementation(({ title }) => { - document.title = title; - - return null; - }) -); - -jest.mock('react-router-dom', () => ({ - useLocation: jest.fn().mockReturnValue({ - pathname: '/', - }), -})); - -describe('KnowledgeCenterLayout', () => { - const mockProps = { - children:
Test Children
, - leftSidebar:
Test Left Sidebar
, - rightSidebar:
Test Right Sidebar
, - pageTitle: 'Test Page Title', - }; - - it('should render correctly', () => { - render(); - - expect(screen.getByText('Test Children')).toBeInTheDocument(); - expect(screen.getByText('Test Left Sidebar')).toBeInTheDocument(); - expect(screen.getByText('Test Right Sidebar')).toBeInTheDocument(); - expect(document.title).toEqual('Test Page Title'); - }); -}); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgeCenterLayout/KnowledgeCenterLayout.tsx b/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgeCenterLayout/KnowledgeCenterLayout.tsx deleted file mode 100644 index 489f04ef0435..000000000000 --- a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgeCenterLayout/KnowledgeCenterLayout.tsx +++ /dev/null @@ -1,145 +0,0 @@ -/* - * Copyright 2026 Collate. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import { Card, Typography } from 'antd'; -import classNames from 'classnames'; -import React, { FC } from 'react'; -import { useTranslation } from 'react-i18next'; -import { ReflexContainer, ReflexElement, ReflexSplitter } from 'react-reflex'; -import DocumentTitle from '../../../components/common/DocumentTitle/DocumentTitle'; -import '../../../components/common/ResizablePanels/resizable-panels.less'; -import './knowledge-center-layout.less'; - -interface KnowledgeCenterLayoutProps { - children: React.ReactNode; - leftSidebar: React.ReactNode; - rightSidebar: React.ReactNode; - pageTitle: string; - className?: string; - leftSidebarTitle?: React.ReactNode; - rightSidebarTitle?: string; - leftSidebarExtra?: React.ReactNode; - rightSidebarExtra?: React.ReactNode; - centerNoPadding?: boolean; -} - -const KnowledgeCenterLayout: FC = ({ - children, - leftSidebar, - rightSidebar, - pageTitle, - className, - leftSidebarTitle, - rightSidebarTitle, - leftSidebarExtra, - rightSidebarExtra, - centerNoPadding = false, -}) => { - const { i18n } = useTranslation(); - const isLeftPanelCollapsed = !leftSidebar; - const isRightPanelCollapsed = !rightSidebar; - - return ( -
- - - {/* left */} - - - {leftSidebarTitle} - - ) - }> - {leftSidebar} - - - - - {!isLeftPanelCollapsed && ( -
-
-
- )} - - - {/* middle */} - - - {children} - - - - - {!isRightPanelCollapsed && ( -
-
-
- )} - - - - - {rightSidebarTitle} - - ) - }> - {rightSidebar} - - - -
- ); -}; - -export default KnowledgeCenterLayout; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.test.tsx index a541fc408adf..8b4c8c8babc4 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.test.tsx @@ -17,7 +17,9 @@ import { screen, waitFor, } from '@testing-library/react'; +import { createRef } from 'react'; import { MemoryRouter } from 'react-router-dom'; +import { KnowledgePagesHierarchyRef } from '../../../interface/knowledge-center.interface'; import { DEFAULT_ENTITY_PERMISSION } from '../../../utils/PermissionsUtils'; import KnowledgePagesHierarchy from './KnowledgePagesHierarchy'; @@ -325,6 +327,63 @@ describe('KnowledgePagesHierarchy', () => { ).toBeInTheDocument(); }); + it('should keep a manually collapsed ancestor of the active node collapsed', async () => { + const { rerender } = render( + , + { wrapper: MemoryRouter } + ); + + await act(async () => { + await Promise.resolve(); + }); + + // Manually expand the ancestor so the active descendant is visible, + // mirroring what the activeKey-driven auto-expand effect would do. + const row = screen + .getByText('How to Discover Assets of Interest') + .closest('[role="row"]'); + const chevron = row?.querySelector('button[slot="chevron"]'); + + expect(chevron).not.toBeNull(); + + await act(async () => { + fireEvent.click(chevron!); + }); + + expect( + screen.getByText('How to Discover Assets of Interest Child 1') + ).toBeInTheDocument(); + + await act(async () => { + fireEvent.click(chevron!); + }); + + expect( + screen.queryByText('How to Discover Assets of Interest Child 1') + ).not.toBeInTheDocument(); + + // Re-rendering with the same activeKey (e.g. triggered by an unrelated + // hierarchy state update) must not re-expand the ancestor that was just + // collapsed by the user. + rerender( + + ); + + await act(async () => { + await Promise.resolve(); + }); + + expect( + screen.queryByText('How to Discover Assets of Interest Child 1') + ).not.toBeInTheDocument(); + }); + it('delete flow should work', async () => { await act(async () => { render( @@ -346,6 +405,354 @@ describe('KnowledgePagesHierarchy', () => { expect(screen.getByTestId('delete-widget')).toBeInTheDocument(); }); + describe('loadNodeChildren', () => { + const mockGetPageHierarchyFromES = jest.requireMock( + 'rest/knowledgeCenterAPI' + ).getPageHierarchyFromES; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should stop fetching once a childrenCount/actual-children mismatch is detected, instead of calling the API forever', async () => { + const mismatchedHierarchy = [ + { + id: 'mismatch-parent-id', + pageType: 'Article', + name: 'Article_Mismatch', + description: '', + fullyQualifiedName: 'Article_Mismatch', + displayName: 'Mismatched Count Parent', + // childrenCount claims more children exist than any fetch will ever return. + childrenCount: 5, + children: [], + }, + ]; + + mockGetPageHierarchyFromES.mockImplementation((parent?: string) => + Promise.resolve({ + data: + parent === 'Article_Mismatch' + ? [ + { + id: 'mismatch-child-1', + pageType: 'Article', + name: 'Article_MismatchChild', + description: '', + fullyQualifiedName: + 'Article_Mismatch.Article_MismatchChild', + displayName: 'Mismatch Child', + childrenCount: 0, + children: [], + }, + ] + : mismatchedHierarchy, + paging: { limit: 100, offset: 0, total: 1 }, + }) + ); + + await act(async () => { + render( + , + { wrapper: MemoryRouter } + ); + }); + + const row = screen + .getByText('Mismatched Count Parent') + .closest('[role="row"]'); + const expandBtn = row?.querySelector('button[slot="chevron"]'); + + await act(async () => { + fireEvent.click(expandBtn!); + }); + + await waitFor(() => { + expect(screen.getByText('Mismatch Child')).toBeInTheDocument(); + }); + + const callCountAfterFirstLoad = + mockGetPageHierarchyFromES.mock.calls.length; + + // Allow further effect/render cycles to run; the call count must not + // keep growing once the single available page of children has loaded. + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(mockGetPageHierarchyFromES.mock.calls.length).toBe( + callCountAfterFirstLoad + ); + }); + + it('should request the next page using the actual returned count as offset, not the locally merged/deduped count', async () => { + const parentFqn = 'Article_Paged'; + const parentHierarchy = [ + { + id: 'paged-parent-id', + pageType: 'Article', + name: 'Article_Paged', + description: '', + fullyQualifiedName: parentFqn, + displayName: 'Paged Parent', + childrenCount: 101, + children: [], + }, + ]; + const buildPage = (index: number) => ({ + id: `paged-child-${index}`, + pageType: 'Article', + name: `Article_PagedChild${index}`, + description: '', + fullyQualifiedName: `${parentFqn}.Article_PagedChild${index}`, + displayName: `Paged Child ${index}`, + childrenCount: 0, + children: [], + }); + // First page returns a full 100-item page (children 0-99); the last of + // those (index 99) is also returned again at the start of what a + // naive "offset = children.length" fetch would treat as page two, + // simulating a sibling shift. `unionBy` dedupes it, so the locally + // merged/deduped count (100, since the duplicate collapses) would + // equal `children.length` and mask the bug — the fix must instead + // track that the server already returned 100 raw items and request + // offset=100 for the next page, landing on the genuinely new item at + // index 100 instead of skipping it. + const firstPage = Array.from({ length: 100 }, (_, i) => buildPage(i)); + const newFinalChild = buildPage(100); + + mockGetPageHierarchyFromES.mockImplementation( + (parent?: string, _pageType?: string, offset = 0) => { + if (parent !== parentFqn) { + return Promise.resolve({ + data: parentHierarchy, + paging: { limit: 100, offset: 0, total: 1 }, + }); + } + + return Promise.resolve({ + data: offset === 0 ? firstPage : [newFinalChild], + paging: { limit: 100, offset, total: 101 }, + }); + } + ); + + await act(async () => { + render( + , + { wrapper: MemoryRouter } + ); + }); + + const row = screen.getByText('Paged Parent').closest('[role="row"]'); + const expandBtn = row?.querySelector('button[slot="chevron"]'); + + await act(async () => { + fireEvent.click(expandBtn!); + }); + + await waitFor(() => { + expect(screen.getByText('Paged Child 99')).toBeInTheDocument(); + }); + + // Second page fetch must use offset=100 (the count actually returned + // by the server on page one), not the locally-mutated/deduped count. + await waitFor(() => { + const callsForParent = mockGetPageHierarchyFromES.mock.calls.filter( + (call: unknown[]) => call[0] === parentFqn + ); + + expect(callsForParent).toHaveLength(2); + expect(callsForParent[1]).toEqual([parentFqn, undefined, 100, 100]); + }); + + await waitFor(() => { + expect(screen.getByText('Paged Child 100')).toBeInTheDocument(); + }); + }); + }); + + describe('force refresh', () => { + const mockGetPageHierarchyFromES = jest.requireMock( + 'rest/knowledgeCenterAPI' + ).getPageHierarchyFromES; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should allow re-fetching a node that was previously marked exhausted, after a force refresh', async () => { + const parentFqn = 'Article_Exhausted'; + const exhaustedParentHierarchy = [ + { + id: 'exhausted-parent-id', + pageType: 'Article', + name: 'Article_Exhausted', + description: '', + fullyQualifiedName: parentFqn, + displayName: 'Exhausted Parent', + childrenCount: 1, + children: [], + }, + ]; + const onlyChild = { + id: 'exhausted-child-1', + pageType: 'Article', + name: 'Article_ExhaustedChild', + description: '', + fullyQualifiedName: `${parentFqn}.Article_ExhaustedChild`, + displayName: 'Exhausted Child', + childrenCount: 0, + children: [], + }; + + mockGetPageHierarchyFromES.mockImplementation((parent?: string) => + Promise.resolve({ + data: parent === parentFqn ? [onlyChild] : exhaustedParentHierarchy, + paging: { limit: 100, offset: 0, total: 1 }, + }) + ); + + const ref = createRef(); + + await act(async () => { + render( + , + { wrapper: MemoryRouter } + ); + }); + + const row = screen.getByText('Exhausted Parent').closest('[role="row"]'); + const expandBtn = row?.querySelector('button[slot="chevron"]'); + + await act(async () => { + fireEvent.click(expandBtn!); + }); + + await waitFor(() => { + expect(screen.getByText('Exhausted Child')).toBeInTheDocument(); + }); + + const callCountBeforeRefresh = + mockGetPageHierarchyFromES.mock.calls.length; + + await act(async () => { + await ref.current?.fetchKnowledgePageHierarchy(true); + }); + + // A force refresh must clear stale exhaustion/offset tracking so the + // node can be expanded and its children re-fetched again. + const rowAfterRefresh = screen + .getByText('Exhausted Parent') + .closest('[role="row"]'); + const expandBtnAfterRefresh = rowAfterRefresh?.querySelector( + 'button[slot="chevron"]' + ); + + await act(async () => { + fireEvent.click(expandBtnAfterRefresh!); + }); + + await waitFor(() => { + expect(mockGetPageHierarchyFromES.mock.calls.length).toBeGreaterThan( + callCountBeforeRefresh + ); + }); + + expect(screen.getByText('Exhausted Child')).toBeInTheDocument(); + }); + }); + + describe('handleExpandAll', () => { + const mockGetPageHierarchyFromES = jest.requireMock( + 'rest/knowledgeCenterAPI' + ).getPageHierarchyFromES; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should page through a node with more direct children than the page limit instead of looping forever', async () => { + const parentFqn = 'Article_ManyChildren'; + const rootHierarchy = [ + { + id: 'many-children-parent-id', + pageType: 'Article', + name: 'Article_ManyChildren', + description: '', + fullyQualifiedName: parentFqn, + displayName: 'Many Children Parent', + childrenCount: 150, + children: [], + }, + ]; + const buildPage = (index: number) => ({ + id: `many-children-${index}`, + pageType: 'Article', + name: `Article_ManyChildren_${index}`, + description: '', + fullyQualifiedName: `${parentFqn}.Article_ManyChildren_${index}`, + displayName: `Many Children ${index}`, + childrenCount: 0, + children: [], + }); + const allChildren = Array.from({ length: 150 }, (_, i) => buildPage(i)); + + mockGetPageHierarchyFromES.mockImplementation( + (parent?: string, _pageType?: string, offset = 0, limit = 100) => { + if (parent !== parentFqn) { + return Promise.resolve({ + data: rootHierarchy, + paging: { limit: 100, offset: 0, total: 1 }, + }); + } + + return Promise.resolve({ + data: allChildren.slice(offset, offset + limit), + paging: { limit, offset, total: 150 }, + }); + } + ); + + await act(async () => { + render( + , + { wrapper: MemoryRouter } + ); + }); + + const expandAllButton = screen.getByRole('button', { + name: 'label.expand-all', + }); + + await act(async () => { + fireEvent.click(expandAllButton); + }); + + // The while-loop must terminate: exactly 2 pages (100 + 50) fetched + // for the 150-child node, not an unbounded number of calls. + const callsForParent = mockGetPageHierarchyFromES.mock.calls.filter( + (call: unknown[]) => call[0] === parentFqn + ); + + expect(callsForParent).toHaveLength(2); + expect(callsForParent[0][2]).toBe(0); + expect(callsForParent[1][2]).toBe(100); + + await waitFor(() => { + expect(screen.getByText('Many Children 149')).toBeInTheDocument(); + }); + + expect( + screen.getByRole('button', { name: 'label.collapse-all' }) + ).toBeInTheDocument(); + }); + }); + describe('Scroll Pagination', () => { const mockGetPageHierarchyFromES = jest.requireMock( 'rest/knowledgeCenterAPI' diff --git a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx b/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx index bc1e31e67774..7b65c7264bc0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/KnowledgeCenter/KnowledgePagesHierarchy/KnowledgePagesHierarchy.tsx @@ -88,6 +88,7 @@ import { hierarchyPaginationInitialState, hierarchyPaginationReducer, integrateNodesIntoHierarchy, + remapSubtreeFqn, updateTreeData, } from '../../../utils/KnowledgePagePureUtils'; import { updateKnowledgeCenterRecentViewed } from '../../../utils/KnowledgePageUtils'; @@ -154,6 +155,10 @@ const KnowledgePagesHierarchy = forwardRef< hierarchyPaginationInitialState ); + const nodesLoadingChildrenRef = useRef>(new Set()); + const nodesWithNoMoreChildrenRef = useRef>(new Set()); + const nodeChildrenOffsetRef = useRef>(new Map()); + const handleExpandAll = useCallback(async () => { setIsExpandingAll(true); try { @@ -166,7 +171,10 @@ const KnowledgePagesHierarchy = forwardRef< ): PageHierarchy[] => { const unloaded: PageHierarchy[] = []; nodes.forEach((n) => { - if (n.childrenCount > (n.children?.length ?? 0)) { + const isExhausted = nodesWithNoMoreChildrenRef.current.has( + n.fullyQualifiedName + ); + if (n.childrenCount > (n.children?.length ?? 0) && !isExhausted) { unloaded.push(n); } else if (n.children) { unloaded.push(...collectUnloadedExpandableNodes(n.children)); @@ -181,19 +189,45 @@ const KnowledgePagesHierarchy = forwardRef< while (nodesPendingChildren.length > 0) { const childrenResults = await Promise.all( - nodesPendingChildren.map((node) => - getPageHierarchyFromES(node.fullyQualifiedName) - ) + nodesPendingChildren.map((node) => { + const offset = + nodeChildrenOffsetRef.current.get(node.fullyQualifiedName) ?? + node.children?.length ?? + 0; + + return getPageHierarchyFromES( + node.fullyQualifiedName, + undefined, + offset, + KNOWLEDGE_CENTER_PAGINATION_LIMIT + ); + }) ); nodesPendingChildren.forEach((node, index) => { - fetchedChildrenByParentFqn.set( + const fetchedChildren = childrenResults[index].data; + const offset = + nodeChildrenOffsetRef.current.get(node.fullyQualifiedName) ?? + node.children?.length ?? + 0; + + nodeChildrenOffsetRef.current.set( node.fullyQualifiedName, - childrenResults[index].data + offset + fetchedChildren.length ); + + if (fetchedChildren.length < KNOWLEDGE_CENTER_PAGINATION_LIMIT) { + nodesWithNoMoreChildrenRef.current.add(node.fullyQualifiedName); + } + + fetchedChildrenByParentFqn.set(node.fullyQualifiedName, [ + ...(fetchedChildrenByParentFqn.get(node.fullyQualifiedName) ?? + []), + ...fetchedChildren, + ]); traversalHierarchy = updateTreeData( traversalHierarchy, - childrenResults[index].data, + fetchedChildren, node.fullyQualifiedName ); }); @@ -299,6 +333,9 @@ const KnowledgePagesHierarchy = forwardRef< if (forceRefresh) { setExpandedKeys([]); setIsUserExpandedAll(false); + nodesWithNoMoreChildrenRef.current.clear(); + nodesLoadingChildrenRef.current.clear(); + nodeChildrenOffsetRef.current.clear(); } if (isCreateHash) { consumedCreateHashFqnRef.current = fqn; @@ -339,16 +376,43 @@ const KnowledgePagesHierarchy = forwardRef< const loadNodeChildren = useCallback( async (nodeKey: string) => { const node = findPageInTreeData(knowledgePageHierarchy, nodeKey); - if (!node || node.childrenCount <= (node.children?.length ?? 0)) { + const loadedCount = node?.children?.length ?? 0; + if (!node || node.childrenCount <= loadedCount) { return; } + if ( + nodesLoadingChildrenRef.current.has(nodeKey) || + nodesWithNoMoreChildrenRef.current.has(nodeKey) + ) { + return; + } + nodesLoadingChildrenRef.current.add(nodeKey); try { - const { data: children } = await getPageHierarchyFromES(nodeKey); + const fetchOffset = + nodeChildrenOffsetRef.current.get(nodeKey) ?? loadedCount; + const { data: children } = await getPageHierarchyFromES( + nodeKey, + undefined, + fetchOffset, + KNOWLEDGE_CENTER_PAGINATION_LIMIT + ); + nodeChildrenOffsetRef.current.set( + nodeKey, + fetchOffset + children.length + ); + if (children.length < KNOWLEDGE_CENTER_PAGINATION_LIMIT) { + nodesWithNoMoreChildrenRef.current.add(nodeKey); + } + if (children.length === 0) { + return; + } setKnowledgePageHierarchy( updateTreeData(knowledgePageHierarchy, children, nodeKey) ); } catch { // do nothing + } finally { + nodesLoadingChildrenRef.current.delete(nodeKey); } }, [knowledgePageHierarchy] @@ -467,10 +531,32 @@ const KnowledgePagesHierarchy = forwardRef< targetNode.fullyQualifiedName ); + nodesWithNoMoreChildrenRef.current.delete( + targetNode.fullyQualifiedName + ); + nodesLoadingChildrenRef.current.delete(targetNode.fullyQualifiedName); + nodeChildrenOffsetRef.current.delete(targetNode.fullyQualifiedName); + + const targetChildrenWithMovedSubtree = targetNodeChildren.data.map( + (child) => + child.fullyQualifiedName === newSourceFQN && + isEmpty(child.children) && + !isEmpty(sourceNode.children) + ? { + ...child, + children: remapSubtreeFqn( + sourceNode.children ?? [], + oldSourceFQN, + newSourceFQN + ), + } + : child + ); + setKnowledgePageHierarchy((prev) => getUpdatePageHierarchy( prev, - { ...targetNode, children: targetNodeChildren.data }, + { ...targetNode, children: targetChildrenWithMovedSubtree }, true ) ); @@ -482,6 +568,16 @@ const KnowledgePagesHierarchy = forwardRef< sourceNodeParent.fullyQualifiedName ); + nodesWithNoMoreChildrenRef.current.delete( + sourceNodeParent.fullyQualifiedName + ); + nodesLoadingChildrenRef.current.delete( + sourceNodeParent.fullyQualifiedName + ); + nodeChildrenOffsetRef.current.delete( + sourceNodeParent.fullyQualifiedName + ); + setKnowledgePageHierarchy((prev) => getUpdatePageHierarchy( prev, @@ -696,8 +792,11 @@ const KnowledgePagesHierarchy = forwardRef< fetchKnowledgePagesTotalCount(); }, [fetchKnowledgePagesTotalCount]); + const autoExpandedForKeyRef = useRef(undefined); + useEffect(() => { - if (activeKey) { + if (activeKey && autoExpandedForKeyRef.current !== activeKey) { + autoExpandedForKeyRef.current = activeKey; setExpandedKeys((prev) => uniq([ ...prev, diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterArticlesPage/ContextCenterArticlesPage.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterArticlesPage/ContextCenterArticlesPage.tsx index 0c4f910ad2f7..ec5b70e019d1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterArticlesPage/ContextCenterArticlesPage.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/ContextCenterPage/ContextCenterArticlesPage/ContextCenterArticlesPage.tsx @@ -34,6 +34,7 @@ import { useNavigate } from 'react-router-dom'; import { ReactComponent as FileIcon } from '../../../assets/svg/common/file.svg'; import { withActivityFeed } from '../../../components/AppRouter/withActivityFeed'; import DocumentTitle from '../../../components/common/DocumentTitle/DocumentTitle'; +import '../../../components/common/ResizablePanels/resizable-panels.less'; import ArticleDetailHeader from '../../../components/ContextCenter/ArticleDetailHeader/ArticleDetailHeader.component'; import ArticleVersionHeader from '../../../components/ContextCenter/ArticleVersionHeader/ArticleVersionHeader.component'; import ContextCenterHeader from '../../../components/ContextCenter/ContextCenterHeader/ContextCenterHeader.component'; @@ -444,7 +445,6 @@ const ContextCenterArticlesPage = () => { style={showArticlesEmptyState ? { display: 'none' } : undefined}> {/* left */} = {} +): PageHierarchy => ({ + id: fullyQualifiedName, + name: fullyQualifiedName, + fullyQualifiedName, + pageType: PageType.ARTICLE, + childrenCount: 0, + ...overrides, +}); + +describe('updateTreeData', () => { + it('appends new children under the matching parent', () => { + const existing = [buildPage('parent', { childrenCount: 1 })]; + const newChildren = [buildPage('parent.child')]; + + const result = updateTreeData(existing, newChildren, 'parent'); + + expect(result[0].children).toEqual(newChildren); + }); + + it('does not mutate duplicate children when the same parent is updated twice with overlapping results', () => { + const existing = [buildPage('parent', { childrenCount: 1 })]; + const child = buildPage('parent.child'); + + const afterFirstFetch = updateTreeData(existing, [child], 'parent'); + const afterSecondFetch = updateTreeData(afterFirstFetch, [child], 'parent'); + + expect(afterSecondFetch[0].children).toHaveLength(1); + expect(afterSecondFetch[0].children).toEqual([child]); + }); + + it('merges non-overlapping children from a second fetch without dropping the first batch', () => { + const existing = [buildPage('parent', { childrenCount: 2 })]; + const firstChild = buildPage('parent.child1'); + const secondChild = buildPage('parent.child2'); + + const afterFirstFetch = updateTreeData(existing, [firstChild], 'parent'); + const afterSecondFetch = updateTreeData( + afterFirstFetch, + [secondChild], + 'parent' + ); + + expect(afterSecondFetch[0].children).toHaveLength(2); + expect(afterSecondFetch[0].children).toEqual([firstChild, secondChild]); + }); + + it('pushes to the root when no parentKey is provided', () => { + const existing = [buildPage('root1')]; + const newPages = [buildPage('root2')]; + + const result = updateTreeData(existing, newPages); + + expect(result).toHaveLength(2); + }); +}); + +describe('getUpdatePageHierarchy', () => { + it('preserves an already-loaded grandchild subtree when a shallow refresh of the parent does not include it', () => { + // A -> C -> D, where C's subtree was already loaded locally. + const grandchild = buildPage('A.C.D', { childrenCount: 0 }); + const nodeC = buildPage('A.C', { + childrenCount: 1, + children: [grandchild], + }); + const existing = [buildPage('A', { childrenCount: 1, children: [nodeC] })]; + + // A one-level-deep refresh of A's children (e.g. after an unrelated + // sibling move) returns C without its nested children. + const shallowRefreshedC = buildPage('A.C', { childrenCount: 1 }); + + const result = getUpdatePageHierarchy( + existing, + { ...buildPage('A'), children: [shallowRefreshedC] }, + true + ); + + expect(result[0].children).toHaveLength(1); + expect(result[0].children?.[0].children).toEqual([grandchild]); + }); + + it('drops a child that the authoritative fresh fetch no longer reports, instead of leaving a stale duplicate', () => { + // A used to have children [B, C]; B was moved out from under A, so a + // fresh fetch of A's children now only reports C. + const nodeB = buildPage('A.B'); + const nodeC = buildPage('A.C'); + const existing = [ + buildPage('A', { childrenCount: 2, children: [nodeB, nodeC] }), + ]; + + const result = getUpdatePageHierarchy( + existing, + { ...buildPage('A'), children: [nodeC] }, + true + ); + + expect(result[0].children).toEqual([nodeC]); + }); + + it('replaces a child with its fresh version when the fresh fetch does include updated children', () => { + const existingChild = buildPage('A.C.D'); + const nodeC = buildPage('A.C', { + childrenCount: 1, + children: [existingChild], + }); + const existing = [buildPage('A', { childrenCount: 1, children: [nodeC] })]; + + const refreshedChild = buildPage('A.C.D', { displayName: 'Renamed' }); + const refreshedC = buildPage('A.C', { + childrenCount: 1, + children: [refreshedChild], + }); + + const result = getUpdatePageHierarchy( + existing, + { ...buildPage('A'), children: [refreshedC] }, + true + ); + + expect(result[0].children?.[0].children).toEqual([refreshedChild]); + }); +}); + +describe('remapSubtreeFqn', () => { + it('rewrites the fullyQualifiedName prefix at every depth of the subtree', () => { + const grandchild = buildPage('oldParent.source.child.grandchild'); + const child = buildPage('oldParent.source.child', { + childrenCount: 1, + children: [grandchild], + }); + + const result = remapSubtreeFqn( + [child], + 'oldParent.source', + 'newParent.source' + ); + + expect(result[0].fullyQualifiedName).toBe('newParent.source.child'); + expect(result[0].children?.[0].fullyQualifiedName).toBe( + 'newParent.source.child.grandchild' + ); + }); + + it('leaves nodes without children untouched beyond the FQN rewrite', () => { + const leaf = buildPage('oldParent.source.leaf'); + + const result = remapSubtreeFqn([leaf], 'oldParent.source', 'newParent'); + + expect(result[0].fullyQualifiedName).toBe('newParent.leaf'); + expect(result[0].children).toBeUndefined(); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts index fa52125027d8..e959def8c3dd 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts +++ b/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePagePureUtils.ts @@ -11,7 +11,7 @@ * limitations under the License. */ import type { DataNode } from 'antd/lib/tree'; -import { cloneDeep, isEmpty } from 'lodash'; +import { cloneDeep, isEmpty, unionBy } from 'lodash'; import { FQN_SEPARATOR_CHAR } from '../constants/char.constants'; import { PLACEHOLDER_ROUTE_FQN, @@ -220,10 +220,11 @@ export const updateTreeData = ( ): boolean => { for (const page of pages) { if (page.fullyQualifiedName === key) { - if (!page.children) { - page.children = []; - } - page.children.push(...children); + page.children = unionBy( + page.children ?? [], + children, + 'fullyQualifiedName' + ); return true; } @@ -262,8 +263,22 @@ export const getUpdatePageHierarchy = ( nodes.forEach((node) => { if (node.fullyQualifiedName === activePage?.fullyQualifiedName) { if (updateChildren) { - const children = (activePage?.children ?? []) as PageHierarchy[]; - node.children = children; + const freshChildren = (activePage?.children ?? []) as PageHierarchy[]; + const previousChildrenByFqn = new Map( + (node.children ?? []).map((child) => [ + child.fullyQualifiedName, + child, + ]) + ); + node.children = freshChildren.map((freshChild) => { + const previousChild = previousChildrenByFqn.get( + freshChild.fullyQualifiedName + ); + + return isEmpty(freshChild.children) && previousChild?.children + ? { ...freshChild, children: previousChild.children } + : freshChild; + }); } else { node.displayName = activePage?.displayName; } @@ -279,6 +294,20 @@ export const getUpdatePageHierarchy = ( return newPages; }; +export const remapSubtreeFqn = ( + nodes: PageHierarchy[], + fromPrefix: string, + toPrefix: string +): PageHierarchy[] => + nodes.map((node) => ({ + ...node, + fullyQualifiedName: + toPrefix + node.fullyQualifiedName.slice(fromPrefix.length), + children: node.children + ? remapSubtreeFqn(node.children, fromPrefix, toPrefix) + : node.children, + })); + export const getUpdatePageHierarchyForDelete = ( fullyQualifiedName: string, pages: PageHierarchy[] = [] diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.test.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.test.tsx index 69a99f3484ea..a62318dc2a1a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.test.tsx @@ -13,12 +13,15 @@ import { render, screen } from '@testing-library/react'; import React from 'react'; import { BrowserRouter } from 'react-router-dom'; +import { usePersistentStorage } from '../hooks/currentUserStore/useCurrentUserStore'; +import { useApplicationStore } from '../hooks/useApplicationStore'; import { Article, KnowledgePage, PageHierarchy, PageType, QuickLink, + RecentViewedKnowledgePage, } from '../interface/knowledge-center.interface'; import { extractKnowledgePageParentFQN, @@ -28,7 +31,10 @@ import { getUpdatePageHierarchy, integrateNodesIntoHierarchy, } from './KnowledgePagePureUtils'; -import { getLink } from './KnowledgePageUtils'; +import { + addToKnowledgeCenterRecentViewed, + getLink, +} from './KnowledgePageUtils'; describe('getKnowledgePageName', () => { it('returns displayName when present', () => { @@ -595,4 +601,98 @@ describe('KnowledgePageUtils', () => { expect(result[1].children).toBeUndefined(); }); }); + + describe('addToKnowledgeCenterRecentViewed', () => { + const userName = 'test-user'; + + const buildPage = ( + overrides: Partial + ): RecentViewedKnowledgePage => + ({ + id: '1', + fullyQualifiedName: 'Article_A', + displayName: 'Article A', + pageType: PageType.ARTICLE, + name: 'Article_A', + timestamp: 0, + ...overrides, + } as RecentViewedKnowledgePage); + + beforeEach(() => { + jest.spyOn(useApplicationStore, 'getState').mockReturnValue({ + currentUser: { name: userName }, + } as unknown as ReturnType); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('replaces the existing entry for the same id even when its fullyQualifiedName has changed', () => { + const existingEntry = buildPage({ + id: 'page-1', + fullyQualifiedName: 'Article_Old_FQN', + }); + const setUserPreference = jest.fn(); + + jest.spyOn(usePersistentStorage, 'getState').mockReturnValue({ + preferences: { + [userName]: { recentlyViewedQuickLinks: [existingEntry] }, + }, + setUserPreference, + } as unknown as ReturnType); + + const renamedEntry = buildPage({ + id: 'page-1', + fullyQualifiedName: 'Article_New_FQN', + }); + + addToKnowledgeCenterRecentViewed(renamedEntry); + + expect(setUserPreference).toHaveBeenCalledTimes(1); + + const [, updatedPreferences] = setUserPreference.mock.calls[0]; + const recentlyViewedQuickLinks = + updatedPreferences.recentlyViewedQuickLinks; + + expect(recentlyViewedQuickLinks).toHaveLength(1); + expect(recentlyViewedQuickLinks[0].id).toBe('page-1'); + expect(recentlyViewedQuickLinks[0].fullyQualifiedName).toBe( + 'Article_New_FQN' + ); + }); + + it('keeps entries for different ids even when they share a fullyQualifiedName', () => { + const existingEntry = buildPage({ + id: 'page-1', + fullyQualifiedName: 'Article_A', + }); + const setUserPreference = jest.fn(); + + jest.spyOn(usePersistentStorage, 'getState').mockReturnValue({ + preferences: { + [userName]: { recentlyViewedQuickLinks: [existingEntry] }, + }, + setUserPreference, + } as unknown as ReturnType); + + const otherEntry = buildPage({ + id: 'page-2', + fullyQualifiedName: 'Article_A', + }); + + addToKnowledgeCenterRecentViewed(otherEntry); + + const [, updatedPreferences] = setUserPreference.mock.calls[0]; + const recentlyViewedQuickLinks = + updatedPreferences.recentlyViewedQuickLinks; + + expect(recentlyViewedQuickLinks).toHaveLength(2); + expect( + recentlyViewedQuickLinks.map( + (item: RecentViewedKnowledgePage) => item.id + ) + ).toEqual(expect.arrayContaining(['page-1', 'page-2'])); + }); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.tsx index 958ad29392e7..e8b998165e67 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/KnowledgePageUtils.tsx @@ -64,9 +64,7 @@ export const addToKnowledgeCenterRecentViewed = ( } if (recentlyViewed) { const arrData = recentlyViewed - .filter( - (item) => item.fullyQualifiedName !== entityData.fullyQualifiedName - ) + .filter((item) => item.id !== entityData.id) .sort(arraySorterByKey('timestamp', true)); arrData.unshift(entityData);