ljmotta commented on code in PR #3580:
URL:
https://github.com/apache/incubator-kie-tools/pull/3580#discussion_r3310836001
##########
packages/bpmn-editor/src/mutations/moveNodesOutOfSubProcess.ts:
##########
@@ -24,25 +24,128 @@ import { Normalized } from "../normalization/normalize";
import { State } from "../store/Store";
import { addOrGetProcessAndDiagramElements } from
"./addOrGetProcessAndDiagramElements";
+export type SubProcessElement = Normalized<
+ Extract<
+ Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
+ { __$$element: "subProcess" | "adHocSubProcess" | "transaction" }
+ >
+>;
+
+export function isSubProcessElement(element: { __$$element?: string }):
element is SubProcessElement {
+ return (
+ element.__$$element === "subProcess" ||
+ element.__$$element === "adHocSubProcess" ||
+ element.__$$element === "transaction"
+ );
+}
+
+export function findSubProcessRecursively(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ subProcessId: string
+): SubProcessElement | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === subProcessId && isSubProcessElement(element)) {
+ return element;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findSubProcessRecursively(element.flowElement,
subProcessId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentFlowElements(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ elementId: string
+): NonNullable<BPMN20__tProcess["flowElement"]> | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === elementId) {
+ return flowElements;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findParentFlowElements(element.flowElement, elementId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentSubProcess(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ elementId: string
+): SubProcessElement | undefined {
+ for (const element of flowElements) {
+ if (isSubProcessElement(element) && element.flowElement) {
+ // Check if this subprocess directly contains the target
+ for (const child of element.flowElement) {
+ if (child["@_id"] === elementId) {
+ return element;
+ }
+ }
+ // Recurse into nested subprocesses
+ const found = findParentSubProcess(element.flowElement, elementId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function shouldMoveSequenceFlow(
+ flowElement: Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
+ nodeIds: Set<string>,
+ existingNodesIds: Set<string>
+): boolean {
+ return (
+ flowElement.__$$element === "sequenceFlow" &&
+ !!flowElement["@_sourceRef"] &&
+ !!flowElement["@_targetRef"] &&
+ ((nodeIds.has(flowElement["@_sourceRef"]) &&
nodeIds.has(flowElement["@_targetRef"])) ||
+ (existingNodesIds.has(flowElement["@_sourceRef"]) &&
nodeIds.has(flowElement["@_targetRef"])) ||
+ (nodeIds.has(flowElement["@_sourceRef"]) &&
existingNodesIds.has(flowElement["@_targetRef"])))
+ );
+}
+
export function moveNodesOutOfSubProcess({
definitions,
__readonly_subProcessId,
__readonly_nodeIds,
+ __readonly_targetSubProcessId,
}: {
definitions: State["bpmn"]["model"]["definitions"];
__readonly_subProcessId: string | undefined;
__readonly_nodeIds: string[];
+ __readonly_targetSubProcessId?: string | null;
}) {
const { process } = addOrGetProcessAndDiagramElements({ definitions });
- const subProcess = process.flowElement?.find((s) => s["@_id"] ===
__readonly_subProcessId);
- if (
- !(
- subProcess?.__$$element === "subProcess" ||
- subProcess?.__$$element === "adHocSubProcess" ||
- subProcess?.__$$element === "transaction"
- )
- ) {
- throw new Error(`Can't find subProcess with ID
${__readonly_subProcessId}`);
+
+ const subProcess = findSubProcessRecursively(process.flowElement ?? [],
__readonly_subProcessId ?? "");
+ if (!subProcess) {
+ throw new Error(`Cannot find subprocess with ID:
${__readonly_subProcessId}`);
+ }
+
+ let parentFlowElements: NonNullable<BPMN20__tProcess["flowElement"]>;
+ let targetSubProcess: SubProcessElement | undefined;
Review Comment:
We should use the `Normalized<>` type to ensure the `id` is present.
##########
packages/bpmn-editor/src/mutations/addConnectedNode.ts:
##########
@@ -107,8 +117,8 @@ export function addConnectedNode({
)
);
} else if (newNodeNature === NodeNature.ARTIFACT) {
- process.artifact ??= [];
- process.artifact.push(
+ targetArtifacts ??= [];
+ targetArtifacts.push(
Review Comment:
Same as previous comment.
##########
packages/bpmn-editor/src/mutations/addConnectedNode.ts:
##########
@@ -51,10 +52,19 @@ export function addConnectedNode({
const { process, diagramElements } = addOrGetProcessAndDiagramElements({
definitions });
+ const parentFlowElements = findParentFlowElements(
+ process.flowElement ?? [],
+ __readonly_sourceNode.bpmnElement["@_id"]
+ );
+ const parentSubProcess = findParentSubProcess(process.flowElement ?? [],
__readonly_sourceNode.bpmnElement["@_id"]);
+
+ let targetFlowElements = parentFlowElements ?? process.flowElement;
+ let targetArtifacts = parentSubProcess?.artifact ?? process.artifact;
+
if (newNodeNature === NodeNature.PROCESS_FLOW_ELEMENT || newNodeNature ===
NodeNature.CONTAINER) {
- process.flowElement ??= [];
+ targetFlowElements ??= [];
- process.flowElement?.push(
+ targetFlowElements.push(
Review Comment:
Why not move the `parentSubProcess` to the `if` block if it is not reused?
```suggestion
const parentFlowElements = findParentFlowElements(
process.flowElement ?? [],
__readonly_sourceNode.bpmnElement["@_id"]
);
const targetFlowElements = parentFlowElements ?? process.flowElement; ??
[];
targetFlowElements.push(
```
##########
packages/bpmn-editor/src/mutations/moveNodesOutOfSubProcess.ts:
##########
@@ -24,25 +24,128 @@ import { Normalized } from "../normalization/normalize";
import { State } from "../store/Store";
import { addOrGetProcessAndDiagramElements } from
"./addOrGetProcessAndDiagramElements";
+export type SubProcessElement = Normalized<
+ Extract<
+ Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
+ { __$$element: "subProcess" | "adHocSubProcess" | "transaction" }
+ >
+>;
+
+export function isSubProcessElement(element: { __$$element?: string }):
element is SubProcessElement {
+ return (
+ element.__$$element === "subProcess" ||
+ element.__$$element === "adHocSubProcess" ||
+ element.__$$element === "transaction"
+ );
+}
+
+export function findSubProcessRecursively(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
Review Comment:
All BPMN types must use the `Normalized<>`
##########
packages/bpmn-editor/src/mutations/moveNodesOutOfSubProcess.ts:
##########
@@ -24,25 +24,128 @@ import { Normalized } from "../normalization/normalize";
import { State } from "../store/Store";
import { addOrGetProcessAndDiagramElements } from
"./addOrGetProcessAndDiagramElements";
+export type SubProcessElement = Normalized<
+ Extract<
+ Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
+ { __$$element: "subProcess" | "adHocSubProcess" | "transaction" }
+ >
+>;
+
+export function isSubProcessElement(element: { __$$element?: string }):
element is SubProcessElement {
+ return (
+ element.__$$element === "subProcess" ||
+ element.__$$element === "adHocSubProcess" ||
+ element.__$$element === "transaction"
+ );
+}
+
+export function findSubProcessRecursively(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ subProcessId: string
+): SubProcessElement | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === subProcessId && isSubProcessElement(element)) {
+ return element;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findSubProcessRecursively(element.flowElement,
subProcessId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentFlowElements(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ elementId: string
+): NonNullable<BPMN20__tProcess["flowElement"]> | undefined {
Review Comment:
Same here.
##########
packages/bpmn-editor/src/mutations/moveNodesOutOfSubProcess.ts:
##########
@@ -51,34 +154,36 @@ export function moveNodesOutOfSubProcess({
>[] = [];
const nodeIdsToMoveOut = new Set(__readonly_nodeIds);
- const subProcessNodes = new Set();
- subProcess.flowElement?.forEach((flowElement) => {
- if (flowElement.__$$element !== "sequenceFlow") {
- subProcessNodes.add(flowElement["@_id"]);
+
+ const subProcessNodes = new Set<string>();
+ const collectSubProcessNodeIds = (flowElements:
NonNullable<BPMN20__tProcess["flowElement"]>): void => {
Review Comment:
Same here.
##########
packages/bpmn-editor/src/mutations/moveNodesOutOfSubProcess.ts:
##########
@@ -24,25 +24,128 @@ import { Normalized } from "../normalization/normalize";
import { State } from "../store/Store";
import { addOrGetProcessAndDiagramElements } from
"./addOrGetProcessAndDiagramElements";
+export type SubProcessElement = Normalized<
+ Extract<
+ Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
+ { __$$element: "subProcess" | "adHocSubProcess" | "transaction" }
+ >
+>;
+
+export function isSubProcessElement(element: { __$$element?: string }):
element is SubProcessElement {
+ return (
+ element.__$$element === "subProcess" ||
+ element.__$$element === "adHocSubProcess" ||
+ element.__$$element === "transaction"
+ );
+}
+
+export function findSubProcessRecursively(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ subProcessId: string
+): SubProcessElement | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === subProcessId && isSubProcessElement(element)) {
+ return element;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findSubProcessRecursively(element.flowElement,
subProcessId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentFlowElements(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ elementId: string
+): NonNullable<BPMN20__tProcess["flowElement"]> | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === elementId) {
+ return flowElements;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findParentFlowElements(element.flowElement, elementId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentSubProcess(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
Review Comment:
Same here.
##########
packages/bpmn-editor/src/mutations/moveNodesOutOfSubProcess.ts:
##########
@@ -24,25 +24,128 @@ import { Normalized } from "../normalization/normalize";
import { State } from "../store/Store";
import { addOrGetProcessAndDiagramElements } from
"./addOrGetProcessAndDiagramElements";
+export type SubProcessElement = Normalized<
+ Extract<
+ Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
+ { __$$element: "subProcess" | "adHocSubProcess" | "transaction" }
+ >
+>;
+
+export function isSubProcessElement(element: { __$$element?: string }):
element is SubProcessElement {
+ return (
+ element.__$$element === "subProcess" ||
+ element.__$$element === "adHocSubProcess" ||
+ element.__$$element === "transaction"
+ );
+}
+
+export function findSubProcessRecursively(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ subProcessId: string
+): SubProcessElement | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === subProcessId && isSubProcessElement(element)) {
+ return element;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findSubProcessRecursively(element.flowElement,
subProcessId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentFlowElements(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ elementId: string
+): NonNullable<BPMN20__tProcess["flowElement"]> | undefined {
+ for (const element of flowElements) {
+ if (element["@_id"] === elementId) {
+ return flowElements;
+ }
+ if (isSubProcessElement(element) && element.flowElement) {
+ const found = findParentFlowElements(element.flowElement, elementId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function findParentSubProcess(
+ flowElements: NonNullable<BPMN20__tProcess["flowElement"]>,
+ elementId: string
+): SubProcessElement | undefined {
+ for (const element of flowElements) {
+ if (isSubProcessElement(element) && element.flowElement) {
+ // Check if this subprocess directly contains the target
+ for (const child of element.flowElement) {
+ if (child["@_id"] === elementId) {
+ return element;
+ }
+ }
+ // Recurse into nested subprocesses
+ const found = findParentSubProcess(element.flowElement, elementId);
+ if (found) {
+ return found;
+ }
+ }
+ }
+ return undefined;
+}
+
+export function shouldMoveSequenceFlow(
+ flowElement: Unpacked<NonNullable<BPMN20__tProcess["flowElement"]>>,
Review Comment:
Same here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
