From 241df76cae080623fb2fe126a5800d4d08b48336 Mon Sep 17 00:00:00 2001 From: Sachin Chaurasiya Date: Fri, 20 May 2022 10:08:31 +0530 Subject: [PATCH] Fix : UI manual Lineage Editor Issues (#4532) * Fix : UI manual Lineage Editor Issues * Keep node if only edge is deleted. * Adding hidden handler * Add invisible handle on custom node * Fix funtion name typo * Fix node overlapping issue * Fix #3508 Manual Lineage Editor: Do not reorganize the graph as the user is connecting the nodes * Fix code smell * Minor Fix * Styling fix * Fix Flaky state issue * Refactor onConnect Method * Fix duplicate edge and node issue * Fix Failing Unit test * Fix confirmation modal source and target node name issue * Add check for isNode in Element Click Handler * Add makeEdge Helper * Add JSDoc for helper methods * Remove onElementsRemove prop * Refactor node remove button * Move util method to util file * Allow users to delete edge and node separately * Add unit test * Fix Node Styling * Minor Fix * Add invisble edges --- .../CustomEdge.component.test.tsx | 23 +- .../EntityLineage/CustomEdge.component.tsx | 43 +- .../CustomNode.component.test.tsx | 2 + .../EntityLineage/CustomNode.component.tsx | 152 ++- .../EntityLineage/EntityLineage.component.tsx | 865 ++++++++++-------- .../EntityLineage/EntityLineage.interface.ts | 4 +- .../Entitylineage.component.test.tsx | 1 + .../ui/src/constants/Lineage.constants.ts | 6 + .../main/resources/ui/src/styles/tailwind.css | 4 +- .../main/resources/ui/src/styles/x-master.css | 9 + .../ui/src/utils/EntityLineageUtils.tsx | 87 +- 11 files changed, 767 insertions(+), 429 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.test.tsx index 23e77ab4cf9..286ba89baf4 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.test.tsx @@ -11,7 +11,12 @@ * limitations under the License. */ -import { findByTestId, queryByTestId, render } from '@testing-library/react'; +import { + findAllByTestId, + findByTestId, + queryByTestId, + render, +} from '@testing-library/react'; import React from 'react'; import { ArrowHeadType, EdgeProps, Position } from 'react-flow-renderer'; import { MemoryRouter } from 'react-router-dom'; @@ -40,6 +45,7 @@ const mockCustomEdgeProp = { id: 'node1', }, }, + selected: true, } as EdgeProps; describe('Test CustomEdge Component', () => { @@ -49,27 +55,24 @@ describe('Test CustomEdge Component', () => { }); const deleteButton = await findByTestId(container, 'delete-button'); - const edgePathElement = await findByTestId( + const edgePathElement = await findAllByTestId( container, 'react-flow-edge-path' ); expect(deleteButton).toBeInTheDocument(); - expect(edgePathElement).toBeInTheDocument(); + expect(edgePathElement).toHaveLength(edgePathElement.length); }); - it('Check if CustomEdge has selectedNode as empty object', async () => { + it('Check if CustomEdge has selected as false', async () => { const { container } = render( - , + , { wrapper: MemoryRouter, } ); - const edgePathElement = await findByTestId( + const edgePathElement = await findAllByTestId( container, 'react-flow-edge-path' ); @@ -77,6 +80,6 @@ describe('Test CustomEdge Component', () => { const deleteButton = queryByTestId(container, 'delete-button'); expect(deleteButton).not.toBeInTheDocument(); - expect(edgePathElement).toBeInTheDocument(); + expect(edgePathElement).toHaveLength(edgePathElement.length); }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.tsx index 99401e4aea1..be7bfad468d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/CustomEdge.component.tsx @@ -34,8 +34,10 @@ export const CustomEdge = ({ arrowHeadType, markerEndId, data, + selected, }: EdgeProps) => { - const { onEdgeClick, selectedNode, ...rest } = data; + const { onEdgeClick, ...rest } = data; + const offset = 4; const edgePath = getBezierPath({ sourceX, @@ -45,6 +47,22 @@ export const CustomEdge = ({ targetY, targetPosition, }); + const invisibleEdgePath = getBezierPath({ + sourceX: sourceX + offset, + sourceY: sourceY + offset, + sourcePosition, + targetX: targetX + offset, + targetY: targetY + offset, + targetPosition, + }); + const invisibleEdgePath1 = getBezierPath({ + sourceX: sourceX - offset, + sourceY: sourceY - offset, + sourcePosition, + targetX: targetX - offset, + targetY: targetY - offset, + targetPosition, + }); const markerEnd = getMarkerEnd(arrowHeadType, markerEndId); const [edgeCenterX, edgeCenterY] = getEdgeCenter({ sourceX, @@ -53,6 +71,19 @@ export const CustomEdge = ({ targetY, }); + const getInvisiblePath = (path: string) => { + return ( + + ); + }; + return ( - {(rest as CustomEdgeData)?.source?.includes(selectedNode?.id) || - (rest as CustomEdgeData)?.target?.includes(selectedNode?.id) ? ( + {getInvisiblePath(invisibleEdgePath)} + {getInvisiblePath(invisibleEdgePath1)} + + {selected ? ( + x={edgeCenterX - foreignObjectSize / offset} + y={edgeCenterY - foreignObjectSize / offset}> + {getNodeRemoveButton(() => { + removeNodeHandler(newNode as Node); + })}
), + removeNodeHandler, + isEditMode, isNewNode: true, }, }; setNewAddedNode(newNode as FlowElement); - setElements((es) => es.concat(newNode as FlowElement)); + setElements((es) => + getUniqueFlowElements(es.concat(newNode as FlowElement)) + ); } }; /** - * handle onNode select logic + * After dropping node to graph user will search and select entity + * and this method will take care of changing node information based on selected entity. */ const onEntitySelect = () => { if (!isEmpty(selectedEntity)) { - const isExistingNode = elements.some((n) => - n.id.includes(selectedEntity.id) - ); + const isExistingNode = elements.some((n) => n.id === selectedEntity.id); if (isExistingNode) { setElements((es) => es .map((n) => n.id.includes(selectedEntity.id) - ? { ...n, className: `${n.className} selected-node` } + ? { + ...n, + selectable: true, + className: `${n.className} selected`, + } : n ) .filter((es) => es.id !== newAddedNode.id) @@ -659,25 +727,21 @@ const Entitylineage: FunctionComponent = ({ return { ...el, connectable: true, + selectable: true, id: selectedEntity.id, data: { + ...el.data, + removeNodeHandler, + isEditMode, label: ( - {getNodeLable(selectedEntity)} - + {getNodeLabel(selectedEntity)} + {getNodeRemoveButton(() => { + removeNodeHandler({ + ...el, + id: selectedEntity.id, + } as Node); + })} ), }, @@ -691,10 +755,13 @@ const Entitylineage: FunctionComponent = ({ } }; + /** + * This method will handle the delete edge modal confirmation + */ const onRemove = () => { - setDeletionState({ loading: true, status: 'initial' }); + setDeletionState({ ...ELEMENT_DELETE_STATE, loading: true }); setTimeout(() => { - setDeletionState({ loading: false, status: 'success' }); + setDeletionState({ ...ELEMENT_DELETE_STATE, status: 'success' }); setTimeout(() => { setShowDeleteModal(false); setConfirmDelete(true); @@ -703,13 +770,188 @@ const Entitylineage: FunctionComponent = ({ }, 500); }; - useEffect(() => { - setElements(getLayoutedElements(setElementsHandle())); + /** + * + * @returns Custom control elements + */ + const getCustomControlElements = () => { + return ( + + {!deleted && ( + +

You do not have permission to edit the lineage

+ + } + permission={Operation.UpdateLineage}> + { + setEditMode((pre) => !pre && !deleted); + setSelectedNode({} as SelectedNode); + setIsDrawerOpen(false); + setNewAddedNode({} as FlowElement); + }}> + {loading ? ( + + ) : status === 'success' ? ( + + ) : ( + + )} + + + )} + + ); + }; + + /** + * + * @returns Grid background if editmode is enabled otherwise null + */ + const getGraphBackGround = () => { + if (!isEditMode) { + return null; + } else { + return ; + } + }; + + /** + * + * @returns Side drawer if node is selected and view mode is enabled otherwise null + */ + const getEntityDrawer = () => { + if (isEmpty(selectedNode) || isEditMode) { + return null; + } else { + return ( + + ); + } + }; + + const getConfirmationModal = () => { + if (!showdeleteModal) { + return null; + } else { + return ( + + Cancel + + } + confirmText={ + deletionState.loading ? ( + + ) : deletionState.status === 'success' ? ( + + ) : ( + 'Confirm' + ) + } + header="Remove lineage edge" + onCancel={() => { + setShowDeleteModal(false); + }} + onConfirm={onRemove} + /> + ); + } + }; + + /** + * Reset State between view and edit mode toggle + */ + const resetViewEditState = () => { setExpandNode(undefined); setTableColumns([]); setConfirmDelete(false); + }; + + /** + * Handle updated linegae nodes + * Change newly added node label based on entity:EntityReference + */ + const handleUpdatedLineageNode = () => { + const nodes = updatedLineageData.nodes; + const newlyAddedNodeElement = elements.find((el) => el?.data?.isNewNode); + const newlyAddedNode = nodes?.find( + (node) => node.id === newlyAddedNodeElement?.id + ); + + setElements((els) => { + return els.map((el) => { + if (el.id === newlyAddedNode?.id) { + return { + ...el, + data: { ...el.data, label: getNodeLabel(newlyAddedNode) }, + }; + } else { + return el; + } + }); + }); + }; + + useEffect(() => { + setElements(getLayoutedElements(setElementsHandle())); + resetViewEditState(); }, [lineageData, isNodeLoading, isEditMode]); + useEffect(() => { + const newNodes = updatedLineageData.nodes?.filter( + (n) => + !isUndefined( + updatedLineageData.downstreamEdges?.find((d) => d.toEntity === n.id) + ) || + !isUndefined( + updatedLineageData.upstreamEdges?.find((u) => u.fromEntity === n.id) + ) + ); + entityLineageHandler({ ...updatedLineageData, nodes: newNodes }); + }, [isEditMode]); + + useEffect(() => { + handleUpdatedLineageNode(); + }, [updatedLineageData]); + useEffect(() => { onNodeExpand(); getTableColumns(expandNode); @@ -719,9 +961,6 @@ const Entitylineage: FunctionComponent = ({ if (!isEmpty(selectedNode)) { setExpandNode(undefined); } - setElements((pre) => { - return pre.map((el) => ({ ...el, data: { ...el.data, selectedNode } })); - }); }, [selectedNode]); useEffect(() => { @@ -741,158 +980,56 @@ const Entitylineage: FunctionComponent = ({ useEffect(() => { if (!isEmpty(entityLineage)) { setLineageData(entityLineage); + setUpdatedLineageData(entityLineage); } }, [entityLineage]); - return ( + return deleted ? ( + getDeletedLineagePlaceholder() + ) : ( - {!deleted ? ( -
-
- - onElementClick(el)} - onElementsRemove={onElementsRemove} - onLoad={(reactFlowInstance: OnLoadParams) => { - onLoad(reactFlowInstance); - setReactFlowInstance(reactFlowInstance); - }} - onNodeContextMenu={onNodeContextMenu} - onNodeDrag={dragHandle} - onNodeDragStart={dragHandle} - onNodeDragStop={dragHandle} - onNodeMouseEnter={onNodeMouseEnter} - onNodeMouseLeave={onNodeMouseLeave} - onNodeMouseMove={onNodeMouseMove}> - - {!deleted && ( - -

You do not have permission to edit the lineage

- - } - permission={Operation.UpdateLineage}> - { - setEditMode((pre) => !pre && !deleted); - setSelectedNode({} as SelectedNode); - setIsDrawerOpen(false); - setNewAddedNode({} as FlowElement); - }}> - {loading ? ( - - ) : status === 'success' ? ( - - ) : ( - - )} - - - )} - - {isEditMode ? ( - - ) : null} - - -
- {!isEmpty(selectedNode) && !isEditMode ? ( - - ) : null} - - {showdeleteModal ? ( - - Cancel - - } - confirmText={ - deletionState.loading ? ( - - ) : deletionState.status === 'success' ? ( - - ) : ( - 'Confirm' - ) - } - header="Remove lineage edge" - onCancel={() => { - setShowDeleteModal(false); +
+
+ + onElementClick(el)} + onLoad={(reactFlowInstance: OnLoadParams) => { + onLoad(reactFlowInstance); + setReactFlowInstance(reactFlowInstance); }} - onConfirm={onRemove} - /> - ) : null} + onNodeContextMenu={onNodeContextMenu} + onNodeDrag={dragHandle} + onNodeDragStart={dragHandle} + onNodeDragStop={dragHandle} + onNodeMouseEnter={onNodeMouseEnter} + onNodeMouseLeave={onNodeMouseLeave} + onNodeMouseMove={onNodeMouseMove}> + {getCustomControlElements()} + {getGraphBackGround()} + +
- ) : ( - getDeletedLineagePlaceholder() - )} + {getEntityDrawer()} + + {getConfirmationModal()} +
); }; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/EntityLineage.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/EntityLineage.interface.ts index 272c3236e37..520c17f550a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/EntityLineage.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/EntityLineage.interface.ts @@ -11,7 +11,7 @@ * limitations under the License. */ -import { LeafNodes, LineagePos, LoadingNodeState } from 'Models'; +import { LeafNodes, LineagePos, LoadingNodeState, LoadingState } from 'Models'; import { EntityLineage, EntityReference, @@ -71,3 +71,5 @@ export interface SelectedEdge { source: EntityReference; target: EntityReference; } + +export type ElementLoadingState = Exclude; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/Entitylineage.component.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/Entitylineage.component.test.tsx index 3e7b05dad28..1f461cd9b17 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/Entitylineage.component.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/EntityLineage/Entitylineage.component.test.tsx @@ -146,6 +146,7 @@ jest.mock('../../utils/EntityLineageUtils', () => ({ onNodeMouseEnter: jest.fn(), onNodeMouseLeave: jest.fn(), onNodeMouseMove: jest.fn(), + getUniqueFlowElements: jest.fn().mockReturnValue([]), })); jest.mock('../../utils/TableUtils', () => ({ diff --git a/openmetadata-ui/src/main/resources/ui/src/constants/Lineage.constants.ts b/openmetadata-ui/src/main/resources/ui/src/constants/Lineage.constants.ts index b1bee503a64..c83570f1b34 100644 --- a/openmetadata-ui/src/main/resources/ui/src/constants/Lineage.constants.ts +++ b/openmetadata-ui/src/main/resources/ui/src/constants/Lineage.constants.ts @@ -1,4 +1,5 @@ import { capitalize } from 'lodash'; +import { ElementLoadingState } from '../components/EntityLineage/EntityLineage.interface'; import { EntityType } from '../enums/entity.enum'; export const foreignObjectSize = 40; @@ -17,3 +18,8 @@ export const positionY = 60; export const nodeWidth = 300; export const nodeHeight = 40; + +export const ELEMENT_DELETE_STATE = { + loading: false, + status: 'initial' as ElementLoadingState, +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/styles/tailwind.css b/openmetadata-ui/src/main/resources/ui/src/styles/tailwind.css index 4ba0e19a6b3..d3578908525 100644 --- a/openmetadata-ui/src/main/resources/ui/src/styles/tailwind.css +++ b/openmetadata-ui/src/main/resources/ui/src/styles/tailwind.css @@ -330,8 +330,8 @@ @apply tw-border-main; box-shadow: 0 0 0 0.5px #e2dce4; } - .leaf-node.selected-node, - .leaf-node.selected-node:hover { + .leaf-node.selected, + .leaf-node.selected:hover { @apply tw-border-primary-active; box-shadow: 0 0 0 0.5px #7147e8; } diff --git a/openmetadata-ui/src/main/resources/ui/src/styles/x-master.css b/openmetadata-ui/src/main/resources/ui/src/styles/x-master.css index be37cae1060..01b48862418 100644 --- a/openmetadata-ui/src/main/resources/ui/src/styles/x-master.css +++ b/openmetadata-ui/src/main/resources/ui/src/styles/x-master.css @@ -741,6 +741,15 @@ body .profiler-graph .recharts-active-dot circle { .leaf-node.core .react-flow__handle { background-color: #7147e8; } +.react-flow__edge { + pointer-events: all; + cursor: pointer; +} + +.react-flow__edge .react-flow__edge-path { + stroke-width: 2px; +} + .react-flow__edge.selected .react-flow__edge-path { stroke: #7147e8; } diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx index 944976fea9c..65587fe7952 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/EntityLineageUtils.tsx @@ -59,6 +59,7 @@ import { prepareLabel, } from './CommonUtils'; import { isLeafNode } from './EntityUtils'; +import SVGIcons from './SvgUtils'; import { getEntityLink } from './TableUtils'; export const getHeaderLabel = ( @@ -119,13 +120,14 @@ export const getLineageData = ( loadNodeHandler: (node: EntityReference, pos: LineagePos) => void, lineageLeafNodes: LeafNodes, isNodeLoading: LoadingNodeState, - getNodeLable: (node: EntityReference) => React.ReactNode, + getNodeLabel: (node: EntityReference) => React.ReactNode, isEditMode: boolean, edgeType: string, onEdgeClick: ( evt: React.MouseEvent, data: CustomEdgeData - ) => void + ) => void, + removeNodeHandler: (node: Node) => void ) => { const [x, y] = [0, 0]; const nodes = [ @@ -140,6 +142,7 @@ export const getLineageData = ( isMapped: false, ...down, })) || []; + const mainNode = entityLineage['entity']; const UPStreamNodes: Elements = []; @@ -161,8 +164,10 @@ export const getLineageData = ( type: 'default', className: 'leaf-node', data: { - label: getNodeLable(node), + label: getNodeLabel(node), entityType: node.type, + removeNodeHandler, + isEditMode, }, position: { x: pos === 'from' ? -xVal : xVal, @@ -171,6 +176,10 @@ export const getLineageData = ( }; }; + const makeEdge = (edge: FlowElement) => { + lineageEdges.push(edge); + }; + const getNodes = ( id: string, pos: LineagePos, @@ -187,7 +196,7 @@ export const getLineageData = ( if (node) { UPNodes.push(node); UPStreamNodes.push(makeNode(node, 'from', depth, upDepth)); - lineageEdges.push({ + makeEdge({ id: `edge-${up.fromEntity}-${id}-${depth}`, source: `${node.id}`, target: edg ? edg.id : `${id}`, @@ -229,7 +238,7 @@ export const getLineageData = ( if (node) { DOWNNodes.push(node); DOWNStreamNodes.push(makeNode(node, 'to', depth, downDepth)); - lineageEdges.push({ + makeEdge({ id: `edge-${id}-${down.toEntity}`, source: edg ? edg.id : `${id}`, target: `${node.id}`, @@ -328,7 +337,9 @@ export const getLineageData = ( : 'input', className: `leaf-node ${!isEditMode ? 'core' : ''}`, data: { - label: getNodeLable(mainNode), + label: getNodeLabel(mainNode), + isEditMode, + removeNodeHandler, }, position: { x: x, y: y }, }, @@ -343,6 +354,7 @@ export const getLineageData = ( ...up, type: isEditMode ? 'default' : 'input', data: { + ...up.data, label: (
{down?.data?.label}
@@ -444,7 +457,7 @@ export const getDataLabel = ( } else { return ( {type === 'table' ? databaseName && schemaName @@ -496,8 +509,8 @@ export const getLayoutedElements = ( elements.forEach((el) => { if (isNode(el)) { dagreGraph.setNode(el.id, { - width: el?.__rf?.width ?? nodeWidth, - height: el?.__rf?.height ?? nodeHeight, + width: nodeWidth, + height: nodeHeight, }); } else { dagreGraph.setEdge(el.source, el.target); @@ -512,11 +525,8 @@ export const getLayoutedElements = ( el.targetPosition = isHorizontal ? Position.Left : Position.Top; el.sourcePosition = isHorizontal ? Position.Right : Position.Bottom; el.position = { - x: - nodeWithPosition.x - - (el?.__rf?.width ?? nodeWidth) / 2 + - Math.random() / 1000, - y: nodeWithPosition.y - (el?.__rf?.height ?? nodeHeight) / 2, + x: nodeWithPosition.x - nodeWidth / 2, + y: nodeWithPosition.y - nodeHeight / 2, }; } @@ -527,27 +537,19 @@ export const getLayoutedElements = ( export const getModalBodyText = (selectedEdge: SelectedEdge) => { let sourceEntity = ''; let targetEntity = ''; + const sourceFQN = selectedEdge.source.fullyQualifiedName || ''; + const targetFQN = selectedEdge.target.fullyQualifiedName || ''; if (selectedEdge.source.type === EntityType.TABLE) { - sourceEntity = getPartialNameFromTableFQN( - selectedEdge.source.name as string, - [FqnPart.Table] - ); + sourceEntity = getPartialNameFromTableFQN(sourceFQN, [FqnPart.Table]); } else { - sourceEntity = getPartialNameFromFQN(selectedEdge.source.name as string, [ - 'database', - ]); + sourceEntity = getPartialNameFromFQN(sourceFQN, ['database']); } if (selectedEdge.target.type === EntityType.TABLE) { - targetEntity = getPartialNameFromTableFQN( - selectedEdge.target.name as string, - [FqnPart.Table] - ); + targetEntity = getPartialNameFromTableFQN(targetFQN, [FqnPart.Table]); } else { - targetEntity = getPartialNameFromFQN(selectedEdge.target.name as string, [ - 'database', - ]); + targetEntity = getPartialNameFromFQN(targetFQN, ['database']); } return `Are you sure you want to remove the edge between "${ @@ -560,3 +562,32 @@ export const getModalBodyText = (selectedEdge: SelectedEdge) => { : targetEntity }"?`; }; + +export const getUniqueFlowElements = (elements: FlowElement[]) => { + const flag: { [x: string]: boolean } = {}; + const uniqueElements: Elements = []; + + elements.forEach((elem) => { + if (!flag[elem.id]) { + flag[elem.id] = true; + uniqueElements.push(elem); + } + }); + + return uniqueElements; +}; + +/** + * + * @param onClick - callback + * @returns - Button element with attach callback + */ +export const getNodeRemoveButton = (onClick: () => void) => { + return ( + + ); +};