You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "aswathitv (via GitHub)" <gi...@apache.org> on 2025/04/03 16:39:40 UTC
[PR] Kie issues#892 : Collapsing and expanding Decision Services [incubator-kie-tools]
aswathitv opened a new pull request, #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046
Closes [kie#892](https://github.com/apache/incubator-kie-issues/issues/892)
- Added collapse button to expanded decision services
- Expanding and collapsing of decision services on click of respective buttons
- Hiding and showing of contained decisions
- For decision services with conflicted decisions, expand button is greyed out so that there are no other expanded Decision Services with conflicted decisions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento merged PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "danielzhe (via GitHub)" <gi...@apache.org>.
danielzhe commented on code in PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#discussion_r2086842818
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -121,13 +179,55 @@ test.describe("Model Decision Services - DRD", () => {
// TODO
});
- test("892 expand - same depiction as other DRDs", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/892");
+ test("892 expand - same depiction as other DRDs", async ({
+ drds,
+ drgNodes,
+ palette,
+ nodes,
+ diagram,
+ editor,
+ decisionServicePropertiesPanel,
+ }) => {
test.info().annotations.push({
type: TestAnnotations.REGRESSION,
description: "https://github.com/apache/incubator-kie-issues/issues/892",
});
- // TODO
+ await drds.toggle();
+ await drds.create({ name: "First DRD" });
+ await drgNodes.toggle();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION_SERVICE,
+ targetPosition: { x: 400, y: 200 },
+ thenRenameTo: "DS1",
+ });
+ await expect(nodes.get({ name: "DS1" })).toBeAttached();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION,
+ targetPosition: { x: 200, y: 200 },
+ thenRenameTo: "D1",
+ });
+ await expect(nodes.get({ name: "D1" })).toBeAttached();
+
+ await nodes.move({ name: "D1", targetPosition: { x: 500, y: 300 } });
+ await expect(nodes.get({ name: "D1" })).toBeAttached();
+ await nodes.select({ name: "DS1", position: NodePosition.TOP });
+ await editor.collapseDecisionService();
+
+ await drds.toggle();
+ await drds.create({ name: "Second DRD" });
+ await drgNodes.dragNode({ name: "DS1", targetPosition: { x: 400, y: 200 } });
+ await drgNodes.toggle();
+
+ await drds.toggle();
+ await drds.navigateTo({ name: "First DRD" });
+ await drds.toggle();
+
+ await nodes.select({ name: "DS1", position: NodePosition.TOP });
+ await editor.expandDecisionService();
Review Comment:
I think it is better to keep it as it is because the expand button is only available after the node is selected. This way makes it clear that the node needs to be selected first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "danielzhe (via GitHub)" <gi...@apache.org>.
danielzhe commented on PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#issuecomment-2873803493
Great work, @aswathitv !
I just found an issue in an edge case when adding Decision Services from included models.
I have this fancy DMN model attached. If you include this model in another one and add the included Decision Services, when I try to collapse/expand, the node disappears.
See the attached video for a better explanation:
https://github.com/user-attachments/assets/d7675bd3-eddc-4d60-ad70-16b4f272f645
[Fancy Model.dmn.txt](https://github.com/user-attachments/files/20175159/Fancy.Model.dmn.txt)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#discussion_r2086264749
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -121,13 +179,55 @@ test.describe("Model Decision Services - DRD", () => {
// TODO
});
- test("892 expand - same depiction as other DRDs", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/892");
+ test("892 expand - same depiction as other DRDs", async ({
+ drds,
+ drgNodes,
+ palette,
+ nodes,
+ diagram,
+ editor,
+ decisionServicePropertiesPanel,
+ }) => {
test.info().annotations.push({
type: TestAnnotations.REGRESSION,
description: "https://github.com/apache/incubator-kie-issues/issues/892",
});
- // TODO
+ await drds.toggle();
+ await drds.create({ name: "First DRD" });
+ await drgNodes.toggle();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION_SERVICE,
+ targetPosition: { x: 400, y: 200 },
+ thenRenameTo: "DS1",
+ });
+ await expect(nodes.get({ name: "DS1" })).toBeAttached();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION,
+ targetPosition: { x: 200, y: 200 },
+ thenRenameTo: "D1",
+ });
+ await expect(nodes.get({ name: "D1" })).toBeAttached();
+
+ await nodes.move({ name: "D1", targetPosition: { x: 500, y: 300 } });
+ await expect(nodes.get({ name: "D1" })).toBeAttached();
+ await nodes.select({ name: "DS1", position: NodePosition.TOP });
+ await editor.collapseDecisionService();
+
+ await drds.toggle();
+ await drds.create({ name: "Second DRD" });
+ await drgNodes.dragNode({ name: "DS1", targetPosition: { x: 400, y: 200 } });
+ await drgNodes.toggle();
+
+ await drds.toggle();
+ await drds.navigateTo({ name: "First DRD" });
+ await drds.toggle();
+
+ await nodes.select({ name: "DS1", position: NodePosition.TOP });
+ await editor.expandDecisionService();
Review Comment:
Don't we want have:
```
await editor.expandDecisionService({ name: "DS1", position: NodePosition.TOP });
```
that would internally do `nodes.select`?
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -121,13 +179,55 @@ test.describe("Model Decision Services - DRD", () => {
// TODO
});
- test("892 expand - same depiction as other DRDs", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/892");
+ test("892 expand - same depiction as other DRDs", async ({
+ drds,
+ drgNodes,
+ palette,
+ nodes,
+ diagram,
+ editor,
+ decisionServicePropertiesPanel,
+ }) => {
test.info().annotations.push({
type: TestAnnotations.REGRESSION,
description: "https://github.com/apache/incubator-kie-issues/issues/892",
});
- // TODO
+ await drds.toggle();
+ await drds.create({ name: "First DRD" });
+ await drgNodes.toggle();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION_SERVICE,
+ targetPosition: { x: 400, y: 200 },
+ thenRenameTo: "DS1",
+ });
+ await expect(nodes.get({ name: "DS1" })).toBeAttached();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION,
+ targetPosition: { x: 200, y: 200 },
+ thenRenameTo: "D1",
+ });
+ await expect(nodes.get({ name: "D1" })).toBeAttached();
+
+ await nodes.move({ name: "D1", targetPosition: { x: 500, y: 300 } });
+ await expect(nodes.get({ name: "D1" })).toBeAttached();
+ await nodes.select({ name: "DS1", position: NodePosition.TOP });
+ await editor.collapseDecisionService();
Review Comment:
similar comment as for `editor.expand`
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -103,13 +145,29 @@ test.describe("Model Decision Services - DRD", () => {
// TODO, question, not understand
});
- test("892 collapse - remove its content", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/892");
+ test("892 collapse - remove its content", async ({ editor, palette, nodes, diagram }) => {
test.info().annotations.push({
type: TestAnnotations.REGRESSION,
description: "https://github.com/apache/incubator-kie-issues/issues/892",
});
- // TODO
+ await palette.dragNewNode({ type: NodeType.DECISION_SERVICE, targetPosition: { x: 400, y: 200 } });
+ await expect(nodes.get({ name: DefaultNodeName.DECISION_SERVICE })).toBeAttached();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION,
+ targetPosition: { x: 200, y: 200 },
+ thenRenameTo: "User Decision",
+ });
+ await expect(nodes.get({ name: "User Decision" })).toBeAttached();
+
+ await nodes.move({ name: "User Decision", targetPosition: { x: 500, y: 300 } });
+ await expect(nodes.get({ name: "User Decision" })).toBeAttached();
+
+ await nodes.select({ name: DefaultNodeName.DECISION_SERVICE, position: NodePosition.TOP });
+ await editor.collapseDecisionService();
Review Comment:
similar comment as for `editor.expand`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#discussion_r2103720360
##########
packages/dmn-editor/src/diagram/nodes/Nodes.tsx:
##########
@@ -843,13 +854,50 @@ export const DecisionServiceNode = React.memo(
(s) => (isHovered || isResizing) && s.diagram.draggingNodes.length === 0
);
const isDropTarget = useDmnEditorStore((s) => s.diagram.dropTargetNode?.id === id);
+ const dmnEditorStoreApi = useDmnEditorStoreApi();
+
+ const dropPoint = useMemo(
+ () => ({
+ x: shape["dc:Bounds"]?.["@_x"] ?? 0,
+ y: shape["dc:Bounds"]?.["@_y"] ?? 0,
+ }),
+ [shape]
+ );
const { isEditingLabel, setEditingLabel, triggerEditing, triggerEditingIfEnter } = useEditableNodeLabel(id);
useHoveredNodeAlwaysOnTop(ref, zIndex, shouldActLikeHovered, dragging, selected, isEditingLabel);
- const dmnEditorStoreApi = useDmnEditorStoreApi();
+ let expandButtonClassname = "kie-dmn-editor--decision-service-collapsed-button";
Review Comment:
Not a good practice to have a mutable variable like this directly inside a Function Component. Please wrap this logic inside a `useMemo`.
##########
packages/dmn-editor/src/diagram/nodes/Nodes.tsx:
##########
@@ -883,6 +931,170 @@ export const DecisionServiceNode = React.memo(
};
}, [decisionService.encapsulatedDecision, decisionService.outputDecision, dmnEditorStoreApi, id]);
+ const defaultSizeNode = DEFAULT_NODE_SIZES[NODE_TYPES.decisionService]({
+ snapGrid: snapGrid,
+ });
+
+ const { computed, ...state } = dmnEditorStoreApi.getState();
+ const dereferencedState: State = { computed, ...JSON.parse(JSON.stringify(state)) };
+ const drdIndex = dereferencedState.computed(dereferencedState).getDrdIndex();
+
+ const drds = dereferencedState.dmn.model.definitions["dmndi:DMNDI"]?.["dmndi:DMNDiagram"] ?? [];
+ let indexedDrdContainingDecisionServiceDepiction: ReturnType<Computed["indexedDrd"]> | undefined;
+ for (let i = 0; i < drds.length; i++) {
+ if (
+ dereferencedState.dmn.model.definitions["@_namespace"] ===
+ dereferencedState.dmn.model.definitions["@_namespace"] &&
+ i === drdIndex
+ ) {
+ continue;
+ }
+
+ const _indexedDrd = computeIndexedDrd(
+ dereferencedState.dmn.model.definitions["@_namespace"],
+ dereferencedState.dmn.model.definitions,
+ i
+ );
+ const dsShape = _indexedDrd.dmnShapesByHref.get(id);
+ const hasCompleteExpandedDepictionOfDecisionService =
+ dsShape &&
+ !(dsShape["@_isCollapsed"] ?? false) &&
+ decisionsInCurrentDecisionService.every((dHref) => _indexedDrd.dmnShapesByHref.has(dHref));
+ if (hasCompleteExpandedDepictionOfDecisionService) {
+ indexedDrdContainingDecisionServiceDepiction = _indexedDrd;
+ break;
+ }
+ }
Review Comment:
Un-memoized logic too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "aswathitv (via GitHub)" <gi...@apache.org>.
aswathitv commented on code in PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#discussion_r2086722316
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -103,13 +145,29 @@ test.describe("Model Decision Services - DRD", () => {
// TODO, question, not understand
});
- test("892 collapse - remove its content", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/892");
+ test("892 collapse - remove its content", async ({ editor, palette, nodes, diagram }) => {
test.info().annotations.push({
type: TestAnnotations.REGRESSION,
description: "https://github.com/apache/incubator-kie-issues/issues/892",
});
- // TODO
+ await palette.dragNewNode({ type: NodeType.DECISION_SERVICE, targetPosition: { x: 400, y: 200 } });
+ await expect(nodes.get({ name: DefaultNodeName.DECISION_SERVICE })).toBeAttached();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION,
+ targetPosition: { x: 200, y: 200 },
+ thenRenameTo: "User Decision",
+ });
+ await expect(nodes.get({ name: "User Decision" })).toBeAttached();
+
+ await nodes.move({ name: "User Decision", targetPosition: { x: 500, y: 300 } });
+ await expect(nodes.get({ name: "User Decision" })).toBeAttached();
+
+ await nodes.select({ name: DefaultNodeName.DECISION_SERVICE, position: NodePosition.TOP });
+ await editor.collapseDecisionService();
Review Comment:
Collapse button will be available only after selecting the node
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#issuecomment-2899213519
Build is failing, please make sure we get a green build so that people can re-review it. Thanks @aswathitv
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#892: Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "aswathitv (via GitHub)" <gi...@apache.org>.
aswathitv commented on code in PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#discussion_r2088303578
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -103,13 +145,29 @@ test.describe("Model Decision Services - DRD", () => {
// TODO, question, not understand
});
- test("892 collapse - remove its content", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/892");
+ test("892 collapse - remove its content", async ({ editor, palette, nodes, diagram }) => {
test.info().annotations.push({
type: TestAnnotations.REGRESSION,
description: "https://github.com/apache/incubator-kie-issues/issues/892",
});
- // TODO
+ await palette.dragNewNode({ type: NodeType.DECISION_SERVICE, targetPosition: { x: 400, y: 200 } });
+ await expect(nodes.get({ name: DefaultNodeName.DECISION_SERVICE })).toBeAttached();
+
+ await palette.dragNewNode({
+ type: NodeType.DECISION,
+ targetPosition: { x: 200, y: 200 },
+ thenRenameTo: "User Decision",
+ });
+ await expect(nodes.get({ name: "User Decision" })).toBeAttached();
+
+ await nodes.move({ name: "User Decision", targetPosition: { x: 500, y: 300 } });
+ await expect(nodes.get({ name: "User Decision" })).toBeAttached();
+
+ await nodes.select({ name: DefaultNodeName.DECISION_SERVICE, position: NodePosition.TOP });
+ await editor.collapseDecisionService();
Review Comment:
Added selecting node step to selectAndCollapseDecisionService
##########
packages/dmn-editor/tests-e2e/drds/modelDecisionServicesInDrds.spec.ts:
##########
@@ -48,15 +49,56 @@ test.describe("Model Decision Services - DRD", () => {
// its content should be automatically layout-ed
});
- test("888 add to DRD, mix of collapsed and expanded", async ({ drds }) => {
- test.skip(true, "https://github.com/apache/incubator-kie-issues/issues/888");
+ test("888 add to DRD, mix of collapsed and expanded", async ({ drds, palette, nodes, diagram, drgNodes }) => {
Review Comment:
Added all the changes as mentioned.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] Kie issues#892 : Collapsing and expanding Decision Services [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on PR #3046:
URL: https://github.com/apache/incubator-kie-tools/pull/3046#issuecomment-2779265221
@aswathitv Thanks for the PR! Some comments from my side:
- Adding `conflictedNodes` to the Store forces you to update it, which is error-prone and won't work that well depending on how you're adding things to a DRD. Instead, create a new computed value so that the list of all conflicting Decisions inside Decision Services is available for you at all times.
- Great change on the minimum node sizes for when adding a Decision Service to a DRD requires it to be collapsed from the start.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org