Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kie-issues#886: On the DMN Editor, adding waypoints to edges that don't have a corresponding DMNEdge associated with it should create the DMNEdge and add the waypoint normally #2546

Merged
merged 51 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
efa4577
kie-issues#886 On the DMN Editor, adding waypoints to edges that don'…
jomarko Aug 23, 2024
8eac192
Merge branch 'main' into kie-issues#886
jomarko Aug 26, 2024
7a79145
information requirement tests
jomarko Aug 26, 2024
de4b79f
add TODO tasks
jomarko Aug 26, 2024
5b1f21f
incorporate review feedback - @_href
jomarko Aug 26, 2024
05c48a2
drgNodes.toggle
jomarko Aug 26, 2024
dc80ddc
add tests
jomarko Aug 26, 2024
94db899
add missing test screenshots
jomarko Aug 28, 2024
8cfc2f5
cleanup spec.ts javadoc
jomarko Aug 28, 2024
eadc55e
Cleanup Diagram.tsx
jomarko Aug 28, 2024
b25ca85
addEdge in usePotentialWaypointControls
jomarko Aug 28, 2024
babe9a2
add logs
jomarko Aug 28, 2024
c2cbace
display waypoints on the edges targetting external nodes
jomarko Aug 30, 2024
632adeb
create DMNEdge targeting external node
jomarko Aug 30, 2024
f650d0a
Merge branch 'main' into kie-issues#886
jomarko Sep 9, 2024
d95740b
dmnElementRef in format [namespace]:[id]
jomarko Sep 9, 2024
ff2c7fc
documentation
jomarko Sep 9, 2024
77c96e5
Merge branch 'main' into kie-issues#886
jomarko Sep 10, 2024
fb13967
Merge branch 'main' into kie-issues#886
jomarko Sep 13, 2024
dda9611
allowExternalTarget refactoring
jomarko Sep 13, 2024
ba8b40d
remove target.qname.prefix usage
jomarko Sep 13, 2024
e3d0b79
remove non used code
jomarko Sep 13, 2024
4ea5a26
remove addEdgeTargetingExternal
jomarko Sep 13, 2024
8a2b8eb
Merge branch 'main' into kie-issues#886
jomarko Sep 16, 2024
411814d
fix onConnect
jomarko Sep 16, 2024
5bad3f8
fix deleteEdge
jomarko Sep 16, 2024
8b98787
Object.entries
jomarko Sep 17, 2024
3de7056
fix typo
jomarko Sep 18, 2024
b28961d
logs
jomarko Sep 18, 2024
1b7d807
simplified null checks
jomarko Sep 18, 2024
dd67a91
Merge branch 'main' into kie-issues#886
jomarko Sep 18, 2024
302ec19
review feedback
jomarko Sep 19, 2024
cb56865
review feedback
jomarko Sep 19, 2024
63c9187
remove extraArg
jomarko Sep 19, 2024
269d5a5
Merge branch 'main' into kie-issues#886
jomarko Sep 25, 2024
20ebd51
MUTATION -> DIAGRAM
jomarko Sep 25, 2024
e5eb1a0
Update packages/dmn-editor/src/diagram/edges/usePotentialWaypointCont…
jomarko Sep 25, 2024
f4207a8
Update packages/dmn-editor/src/diagram/edges/usePotentialWaypointCont…
jomarko Sep 25, 2024
bc8495e
targetsExternalNode
jomarko Sep 25, 2024
1da182f
throw error
jomarko Sep 25, 2024
4ac0a99
is valid connection
jomarko Sep 25, 2024
8eaa8b9
dmnEdgeIndex
jomarko Sep 25, 2024
6d6cc5b
incorporate review feedback
jomarko Sep 26, 2024
9be924f
Merge branch 'main' into kie-issues#886
jomarko Sep 26, 2024
a5044c8
fix != vs !==
jomarko Sep 26, 2024
d3fc589
Revert "incorporate review feedback"
jomarko Sep 26, 2024
79f64d2
incorporate review feedback - v2
jomarko Sep 26, 2024
38ce320
Merge branch 'main' into kie-issues#886
jomarko Sep 27, 2024
006000e
fix deleteEdge
jomarko Sep 27, 2024
3a77fd3
resolve conflicts with main
jomarko Oct 1, 2024
5ea16c2
Remove unnecessary Hook dependency
tiagobento Oct 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/dmn-editor/src/diagram/Diagram.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@
const sourceBounds = sourceNode.data.shape["dc:Bounds"];
const targetBounds = targetNode.data.shape["dc:Bounds"];
if (!sourceBounds || !targetBounds) {
throw new Error("Cannot create connection without target bounds!");
throw new Error("Cannot create connection without source and target bounds!");
}

// --------- This is where we draw the line between the diagram and the model.
Expand Down Expand Up @@ -897,7 +897,7 @@
}
});
},
[reactFlowInstance, dmnEditorStoreApi, externalModelsByNamespace]

Check warning on line 900 in packages/dmn-editor/src/diagram/Diagram.tsx

View workflow job for this annotation

GitHub Actions / run (ubuntu-latest, 2)

React Hook useCallback has an unnecessary dependency: 'reactFlowInstance'. Either exclude it or remove the dependency array

Check warning on line 900 in packages/dmn-editor/src/diagram/Diagram.tsx

View workflow job for this annotation

GitHub Actions / run (macos-13, 2)

React Hook useCallback has an unnecessary dependency: 'reactFlowInstance'. Either exclude it or remove the dependency array

Check warning on line 900 in packages/dmn-editor/src/diagram/Diagram.tsx

View workflow job for this annotation

GitHub Actions / run (windows-latest, 2)

React Hook useCallback has an unnecessary dependency: 'reactFlowInstance'. Either exclude it or remove the dependency array
);

const resetToBeforeEditingBegan = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,18 @@ export function checkIsValidConnection(
export function _checkIsValidConnection(
sourceNode: { type?: string; data: DmnDiagramNodeData } | undefined,
targetNode: { type?: string; data: DmnDiagramNodeData } | undefined,
edgeType: string | null | undefined
edgeType: string | null | undefined,
extraArg?: { allowExternalTarget: boolean }
) {
if (!sourceNode?.type || !targetNode?.type || !edgeType) {
return false;
}

// External nodes cannot be targeted
if (targetNode.data.dmnObjectQName.prefix) {
// External nodes cannot be targeted by default
// However there are exceptions, for example adding a waypoint on the edge
const targetsExternalNode = targetNode.data.dmnObjectQName.prefix !== undefined;
const allowExternalTarget = extraArg?.allowExternalTarget ?? false;
if (targetsExternalNode && !allowExternalTarget) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ import { snapPoint } from "../SnapGrid";
import { DC__Point } from "@kie-tools/dmn-marshaller/dist/schemas/dmn-1_5/ts-gen/types";
import { DmnDiagramNodeData } from "../nodes/Nodes";
import { DmnDiagramEdgeData } from "./Edges";
import { useExternalModels } from "../../includedModels/DmnEditorDependenciesContext";
import { addEdge } from "../../mutations/addEdge";
import { EdgeType, NodeType } from "../connections/graphStructure";
import { PositionalNodeHandleId } from "../connections/PositionalNodeHandles";
import { getHandlePosition } from "../maths/DmnMaths";
import { xmlHrefToQName } from "../../xml/xmlHrefToQName";

export function usePotentialWaypointControls(
waypoints: DC__Point[],
Expand All @@ -38,6 +44,7 @@ export function usePotentialWaypointControls(
const isDraggingWaypoint = useDmnEditorStore((s) => !!s.diagram.draggingWaypoints.find((e) => e === edgeId));
const dmnEditorStoreApi = useDmnEditorStoreApi();
const reactFlowInstance = RF.useReactFlow<DmnDiagramNodeData, DmnDiagramEdgeData>();
const { externalModelsByNamespace } = useExternalModels();

const [potentialWaypoint, setPotentialWaypoint] = useState<ReturnType<typeof approximateClosestPoint> | undefined>(
undefined
Expand Down Expand Up @@ -74,12 +81,82 @@ export function usePotentialWaypointControls(
}, [snapGrid, potentialWaypoint]);

const onDoubleClick = useCallback(() => {
if (!potentialWaypoint || !snappedPotentialWaypoint || edgeIndex === undefined) {
if (!potentialWaypoint || !snappedPotentialWaypoint) {
return;
}

if (edgeIndex === undefined) {
/**
* This means we are adding a first waypoint to one of following edges:
* - an edge in a non default DRD
* - an edge targeting an external node
*/
dmnEditorStoreApi.setState((state) => {
const nodesById = state.computed(state).getDiagramData(externalModelsByNamespace).nodesById;
const edge = state.computed(state).getDiagramData(externalModelsByNamespace).edgesById.get(edgeId);
if (edge === undefined || edge.data?.dmnShapeSource === undefined || edge.data?.dmnShapeTarget == undefined) {
console.debug(
`DMN DIAGRAM: We can not add DMNEdge for '${edgeId}' edge into diagram. There are missing data edge: ${edge}, edge.data: ${edge?.data}`
);
return;
}

const edgeSourceBounds = edge.data?.dmnShapeSource["dc:Bounds"];
const edgeTargetBounds = edge.data?.dmnShapeTarget["dc:Bounds"];
if (edgeSourceBounds === undefined || edgeTargetBounds === undefined) {
tiagobento marked this conversation as resolved.
Show resolved Hide resolved
console.debug(
`DMN DIAGRAM: We can not add DMNEdge for '${edgeId}' edge into diagram. There are missing data edgeSourceBounds: ${edgeSourceBounds}, edgeTargetBounds: ${edgeTargetBounds}`
);
return;
}

const sourceNode = nodesById.get(edge.source);
const targetNode = nodesById.get(edge.target);
if (sourceNode === undefined || targetNode === undefined) {
console.debug(
`DMN DIAGRAM: We can not add DMNEdge for '${edgeId}' edge into diagram. There are missing data sourceNode: ${sourceNode}, targetNode: ${targetNode}`
);
return;
}

const targetsExternalNode = targetNode.data.dmnObjectQName.prefix !== undefined;
const requirementEdgeQNameRelativeToThisDmn = xmlHrefToQName(edgeId, state.dmn.model.definitions);
addEdge({
definitions: state.dmn.model.definitions,
drdIndex: state.computed(state).getDrdIndex(),
edge: {
type: edge.type as EdgeType,
targetHandle: getHandlePosition({ shapeBounds: edgeTargetBounds, waypoint: snappedPotentialWaypoint })
.handlePosition as PositionalNodeHandleId,
sourceHandle: getHandlePosition({ shapeBounds: edgeSourceBounds, waypoint: snappedPotentialWaypoint })
.handlePosition as PositionalNodeHandleId,
autoPositionedEdgeMarker: undefined,
},
sourceNode: {
type: sourceNode.type as NodeType,
data: sourceNode.data,
href: edge.source,
bounds: edgeSourceBounds,
shapeId: edge.data?.dmnShapeSource["@_id"],
},
targetNode: {
type: targetNode.type as NodeType,
href: edge.target,
data: targetNode.data,
bounds: edgeTargetBounds,
index: nodesById.get(edge.target)?.data.index ?? 0,
shapeId: edge.data?.dmnShapeTarget["@_id"],
},
keepWaypoints: false,
dmnElementRefOfDmnEdge: targetsExternalNode ? requirementEdgeQNameRelativeToThisDmn : undefined,
});

console.debug(`DMN DIAGRAM: DMNEdge for '${edgeId}' edge was added into diagram.`);
});
}

if (isExistingWaypoint(snappedPotentialWaypoint)) {
console.debug("Preventing overlapping waypoint creation.");
console.debug("DMN DIAGRAM: Preventing overlapping waypoint creation.");
return;
}

Expand All @@ -96,18 +173,27 @@ export function usePotentialWaypointControls(
}

dmnEditorStoreApi.setState((state) => {
const edgeQName = xmlHrefToQName(edgeId, state.dmn.model.definitions);
const dmnEdgeIndex = state.computed(state).indexedDrd().dmnEdgesByDmnElementRef.get(edgeQName)?.index;
if (dmnEdgeIndex === undefined) {
throw new Error(`DMN DIAGRAM: Diagram computed state does not contain DMNEdge for '${edgeId}' edge.`);
}
addEdgeWaypoint({
definitions: state.dmn.model.definitions,
drdIndex,
beforeIndex: i - 1,
edgeIndex,
dmnEdgeIndex,
waypoint: snappedPotentialWaypoint,
});

console.debug(`DMN DIAGRAM: Waypoint on the DMNEdge for '${edgeId}' edge was added.`);
});
}, [
drdIndex,
dmnEditorStoreApi,
edgeId,
edgeIndex,
externalModelsByNamespace,
isExistingWaypoint,
potentialWaypoint,
snappedPotentialWaypoint,
Expand Down
31 changes: 17 additions & 14 deletions packages/dmn-editor/src/mutations/addEdge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
DMN15__tInformationRequirement,
DMN15__tKnowledgeRequirement,
DMNDI15__DMNEdge,
DMNDI15__DMNShape,
} from "@kie-tools/dmn-marshaller/dist/schemas/dmn-1_5/ts-gen/types";
import { PositionalNodeHandleId } from "../diagram/connections/PositionalNodeHandles";
import { EdgeType, NodeType } from "../diagram/connections/graphStructure";
Expand All @@ -49,6 +48,7 @@ export function addEdge({
targetNode,
edge,
keepWaypoints,
dmnElementRefOfDmnEdge,
}: {
definitions: Normalized<DMN15__tDefinitions>;
drdIndex: number;
Expand All @@ -74,14 +74,17 @@ export function addEdge({
autoPositionedEdgeMarker: AutoPositionedEdgeMarker | undefined;
};
keepWaypoints: boolean;
// QName format, used as dmnElementRef for DMNEdge targeting an external node
dmnElementRefOfDmnEdge?: string;
}) {
if (!_checkIsValidConnection(sourceNode, targetNode, edge.type)) {
const externalTargetAllowed = dmnElementRefOfDmnEdge !== undefined;
if (!_checkIsValidConnection(sourceNode, targetNode, edge.type, { allowExternalTarget: externalTargetAllowed })) {
throw new Error(`DMN MUTATION: Invalid structure: (${sourceNode.type}) --${edge.type}--> (${targetNode.type}) `);
}

const newEdgeId = generateUuid();

let existingEdgeId: string | undefined = undefined;
let existingRequirementOrAssociationId: string | undefined = dmnElementRefOfDmnEdge;

// Associations
if (edge.type === EDGE_TYPES.association) {
Expand All @@ -99,29 +102,29 @@ export function addEdge({
definitions.artifact,
(a) => a.__$$element === "association" && areAssociationsEquivalent(a, newAssociation)
);
existingEdgeId = removed?.["@_id"];
existingRequirementOrAssociationId = removed?.["@_id"];

// Replace with the new one.
definitions.artifact?.push({
__$$element: "association",
...newAssociation,
"@_id": tryKeepingEdgeId(existingEdgeId, newEdgeId),
"@_id": tryKeepingEdgeId(existingRequirementOrAssociationId, newEdgeId),
});
}
// Requirements
else {
else if (!externalTargetAllowed) {
const requirements = getRequirementsFromEdge(sourceNode, newEdgeId, edge.type);
const drgElement = definitions.drgElement![targetNode.index] as Normalized<DMN15__tDecision>; // We cast to tDecision here because it has all three types of requirement.
if (requirements?.informationRequirement) {
drgElement.informationRequirement ??= [];
const removed = removeFirstMatchIfPresent(drgElement.informationRequirement, (ir) =>
doesInformationRequirementsPointTo(ir, sourceNode.href)
);
existingEdgeId = removed?.["@_id"];
existingRequirementOrAssociationId = removed?.["@_id"];
drgElement.informationRequirement?.push(
...requirements.informationRequirement.map((s) => ({
...s,
"@_id": tryKeepingEdgeId(existingEdgeId, newEdgeId),
"@_id": tryKeepingEdgeId(existingRequirementOrAssociationId, newEdgeId),
}))
);
}
Expand All @@ -131,11 +134,11 @@ export function addEdge({
const removed = removeFirstMatchIfPresent(drgElement.knowledgeRequirement, (kr) =>
doesKnowledgeRequirementsPointTo(kr, sourceNode.href)
);
existingEdgeId = removed?.["@_id"];
existingRequirementOrAssociationId = removed?.["@_id"];
drgElement.knowledgeRequirement?.push(
...requirements.knowledgeRequirement.map((s) => ({
...s,
"@_id": tryKeepingEdgeId(existingEdgeId, newEdgeId),
"@_id": tryKeepingEdgeId(existingRequirementOrAssociationId, newEdgeId),
}))
);
}
Expand All @@ -145,11 +148,11 @@ export function addEdge({
const removed = removeFirstMatchIfPresent(drgElement.authorityRequirement, (ar) =>
doesAuthorityRequirementsPointTo(ar, sourceNode.href)
);
existingEdgeId = removed?.["@_id"];
existingRequirementOrAssociationId = removed?.["@_id"];
drgElement.authorityRequirement?.push(
...requirements.authorityRequirement.map((s) => ({
...s,
"@_id": tryKeepingEdgeId(existingEdgeId, newEdgeId),
"@_id": tryKeepingEdgeId(existingRequirementOrAssociationId, newEdgeId),
}))
);
}
Expand All @@ -160,7 +163,7 @@ export function addEdge({
// Remove existing
const removedDmnEdge = removeFirstMatchIfPresent(
diagramElements,
(e) => e.__$$element === "dmndi:DMNEdge" && e["@_dmnElementRef"] === existingEdgeId
(e) => e.__$$element === "dmndi:DMNEdge" && e["@_dmnElementRef"] === existingRequirementOrAssociationId
) as Normalized<DMNDI15__DMNEdge> | undefined;

const newWaypoints = keepWaypoints
Expand All @@ -179,7 +182,7 @@ export function addEdge({
"@_id":
withoutDiscreteAutoPosinitioningMarker(removedDmnEdge?.["@_id"] ?? generateUuid()) +
(edge.autoPositionedEdgeMarker ?? ""),
"@_dmnElementRef": existingEdgeId ?? newEdgeId,
"@_dmnElementRef": existingRequirementOrAssociationId ?? newEdgeId,
"@_sourceElement": sourceNode.shapeId,
"@_targetElement": targetNode.shapeId,
"di:waypoint": newWaypoints,
Expand Down
8 changes: 4 additions & 4 deletions packages/dmn-editor/src/mutations/addEdgeWaypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,21 @@ import { Normalized } from "../normalization/normalize";
export function addEdgeWaypoint({
definitions,
drdIndex,
edgeIndex,
dmnEdgeIndex,
beforeIndex,
waypoint,
}: {
definitions: Normalized<DMN15__tDefinitions>;
drdIndex: number;
edgeIndex: number;
dmnEdgeIndex: number;
beforeIndex: number;
waypoint: DC__Point;
}) {
const { diagramElements } = addOrGetDrd({ definitions, drdIndex });

const diagramElement = diagramElements[edgeIndex];
const diagramElement = diagramElements[dmnEdgeIndex];
if (diagramElement.__$$element !== "dmndi:DMNEdge") {
throw new Error("DMN MUTATION: Can't remove a waypoint from an element that is not a DMNEdge.");
throw new Error("DMN MUTATION: Can't add a waypoint for an element that is not a DMNEdge.");
tiagobento marked this conversation as resolved.
Show resolved Hide resolved
}

if (beforeIndex > (diagramElement["di:waypoint"]?.length ?? 0) - 1) {
Expand Down
1 change: 0 additions & 1 deletion packages/dmn-editor/src/mutations/addShape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import {
DC__Point,
DMN15__tDefinitions,
DMNDI15__DMNDecisionServiceDividerLine,
DMNDI15__DMNShape,
} from "@kie-tools/dmn-marshaller/dist/schemas/dmn-1_5/ts-gen/types";
import { NodeType } from "../diagram/connections/graphStructure";
Expand Down
2 changes: 1 addition & 1 deletion packages/dmn-editor/src/mutations/addStandaloneNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { switchExpression } from "@kie-tools-core/switch-expression-ts";
import { DmnBuiltInDataType, generateUuid } from "@kie-tools/boxed-expression-component/dist/api";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been doing that kind of cleanup every time I see it. Nice!

import { generateUuid } from "@kie-tools/boxed-expression-component/dist/api";
import { DC__Bounds, DMN15__tDefinitions } from "@kie-tools/dmn-marshaller/dist/schemas/dmn-1_5/ts-gen/types";
import { NodeType } from "../diagram/connections/graphStructure";
import { NODE_TYPES } from "../diagram/nodes/NodeTypes";
Expand Down
Loading
Loading