From a88f133a4613a93845f72303927fc535e45bf541 Mon Sep 17 00:00:00 2001 From: Pablo Cuadrado Date: Wed, 3 Aug 2022 12:02:58 -0300 Subject: [PATCH 1/4] Fix: connect cases to loop end. Ignore cases in execution. --- ui/src/components/diagram/WorkflowDAG.js | 31 ++++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/ui/src/components/diagram/WorkflowDAG.js b/ui/src/components/diagram/WorkflowDAG.js index 1dab1430f2..ce76eb1fdf 100644 --- a/ui/src/components/diagram/WorkflowDAG.js +++ b/ui/src/components/diagram/WorkflowDAG.js @@ -146,10 +146,14 @@ export default class WorkflowDAG { // E.g. the default edge not taken. // SWITCH is the newer version of DECISION and DECISION is deprecated if (antecedent.type === "SWITCH" || antecedent.type === "DECISION") { - edgeParams.caseValue = getCaseValue( - taskConfig.taskReferenceName, - antecedent - ); + // A switch inside a loop in execution won't + // have a decisionCases key + if (antecedent.decisionCases) { + edgeParams.caseValue = getCaseValue( + taskConfig.taskReferenceName, + antecedent + ); + } // Highlight edge as executed only after thorough test const branchTaken = this.switchBranchTaken( @@ -295,7 +299,24 @@ export default class WorkflowDAG { } else { // Definition view (or not executed) this.processTaskList(doWhileTask.loopOver, [doWhileTask]); - this.addVertex(endDoWhileTask, [_.last(doWhileTask.loopOver)]); + + const lastLoopTask = _.last(doWhileTask.loopOver); + + // Connect the end of each case to the loop end + if ( + lastLoopTask?.type === "SWITCH" || + lastLoopTask?.type === "DECISION" + ) { + Object.entries(lastLoopTask.decisionCases).forEach( + ([caseValue, tasks]) => { + const lastTaskInCase = _.last(tasks); + this.addVertex(endDoWhileTask, [lastTaskInCase]); + } + ); + } + + // Default case + this.addVertex(endDoWhileTask, [lastLoopTask]); } // Create cosmetic LOOP edges between top and bottom bars From 8cdb439b282f34cf9e9ed49b77918ee1e5d79410 Mon Sep 17 00:00:00 2001 From: Pablo Cuadrado Date: Mon, 8 Aug 2022 01:21:57 -0300 Subject: [PATCH 2/4] Add cosmetic edge to left of loop. Adjust bar size. --- ui/src/components/diagram/WorkflowDAG.js | 23 +++++++++++++++++++-- ui/src/components/diagram/WorkflowGraph.jsx | 5 +++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/ui/src/components/diagram/WorkflowDAG.js b/ui/src/components/diagram/WorkflowDAG.js index ce76eb1fdf..e1432f5d02 100644 --- a/ui/src/components/diagram/WorkflowDAG.js +++ b/ui/src/components/diagram/WorkflowDAG.js @@ -274,6 +274,16 @@ export default class WorkflowDAG { ); if (hasDoWhileExecuted) { + // Create cosmetic LOOP edges between top and bottom bars + this.graph.setEdge( + doWhileTask.taskReferenceName, + doWhileTask.taskReferenceName + "-END", + { + type: "loop", + executed: hasDoWhileExecuted, + } + ); + const loopOverRefs = Array.from(this.taskResults.keys()).filter((key) => { for (let prefix of loopOverRefPrefixes) { if (key.startsWith(prefix)) return true; @@ -297,6 +307,16 @@ export default class WorkflowDAG { } this.addVertex(endDoWhileTask, [...loopTasks]); } else { + // Create cosmetic LOOP edges between top and bottom bars + this.graph.setEdge( + doWhileTask.taskReferenceName, + doWhileTask.taskReferenceName + "-END", + { + type: "loop", + executed: hasDoWhileExecuted, + } + ); + // Definition view (or not executed) this.processTaskList(doWhileTask.loopOver, [doWhileTask]); @@ -319,10 +339,9 @@ export default class WorkflowDAG { this.addVertex(endDoWhileTask, [lastLoopTask]); } - // Create cosmetic LOOP edges between top and bottom bars this.graph.setEdge( - doWhileTask.taskReferenceName, doWhileTask.taskReferenceName + "-END", + doWhileTask.taskReferenceName, { type: "loop", executed: hasDoWhileExecuted, diff --git a/ui/src/components/diagram/WorkflowGraph.jsx b/ui/src/components/diagram/WorkflowGraph.jsx index ca3bc9cb70..35bcb499df 100644 --- a/ui/src/components/diagram/WorkflowGraph.jsx +++ b/ui/src/components/diagram/WorkflowGraph.jsx @@ -582,6 +582,11 @@ class WorkflowGraph extends React.Component { if (right > maxX) maxX = right; if (left < minX) minX = left; } + + if (minX < 0) { + maxX = maxX - minX; + } + translateX = minX; barNode.elem.setAttribute( "transform", From 55d083d77895b0e3140530bfa13be0f1c9271db3 Mon Sep 17 00:00:00 2001 From: Pablo Cuadrado Date: Mon, 8 Aug 2022 01:36:48 -0300 Subject: [PATCH 3/4] Add a bit of margin. --- ui/src/components/diagram/WorkflowGraph.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ui/src/components/diagram/WorkflowGraph.jsx b/ui/src/components/diagram/WorkflowGraph.jsx index 35bcb499df..197116907b 100644 --- a/ui/src/components/diagram/WorkflowGraph.jsx +++ b/ui/src/components/diagram/WorkflowGraph.jsx @@ -584,7 +584,8 @@ class WorkflowGraph extends React.Component { } if (minX < 0) { - maxX = maxX - minX; + maxX = maxX - minX + BAR_MARGIN; + minX = -BAR_MARGIN; } translateX = minX; From e0c37deb11bef3eb910717e3509926ab4dc8839d Mon Sep 17 00:00:00 2001 From: peterlau Date: Thu, 11 Aug 2022 12:43:03 -0700 Subject: [PATCH 4/4] [UI] - Some changes to PR # 3158 - Add Cypress test cases - Keep only one 'cosmetic' arc between DO_WHILE and DO_WHILE_END bars. The arc is flows top-to-bottom to maintain the acyclic nature of DAG, arrow being rendered in reverse by CSS only. --- .../fixtures/doWhile/doWhileSwitch.json | 401 ++++++++++++++++++ ui/src/components/diagram/WorkflowDAG.js | 16 +- .../diagram/WorkflowGraph.test.cy.js | 38 +- 3 files changed, 443 insertions(+), 12 deletions(-) create mode 100644 ui/cypress/fixtures/doWhile/doWhileSwitch.json diff --git a/ui/cypress/fixtures/doWhile/doWhileSwitch.json b/ui/cypress/fixtures/doWhile/doWhileSwitch.json new file mode 100644 index 0000000000..65de6e27fd --- /dev/null +++ b/ui/cypress/fixtures/doWhile/doWhileSwitch.json @@ -0,0 +1,401 @@ +{ + "createTime": 1660244521990, + "status": "COMPLETED", + "endTime": 1660244523050, + "workflowId": "c56d02d5-cc01-4a82-a611-00a3f7f56305", + "tasks": [ + { + "taskType": "DO_WHILE", + "status": "COMPLETED", + "inputData": { + "value": null + }, + "referenceTaskName": "LoopTask", + "retryCount": 0, + "seq": 1, + "pollCount": 0, + "taskDefName": "Loop Task", + "scheduledTime": 1660244522071, + "startTime": 1660244522070, + "endTime": 1660244522935, + "updateTime": 1660244522201, + "startDelayInSeconds": 0, + "retried": false, + "executed": false, + "callbackFromWorker": true, + "responseTimeoutSeconds": 0, + "workflowInstanceId": "c56d02d5-cc01-4a82-a611-00a3f7f56305", + "workflowType": "LoopTestWithSwitch", + "taskId": "426d651c-0c8c-498f-b328-10b0dc2d37d3", + "callbackAfterSeconds": 0, + "outputData": { + "1": { + "inline_task_1": { + "result": { + "result": "NODE_2" + } + }, + "Set_NODE_2": {}, + "switch_task": { + "evaluationResult": [ + "NODE_2" + ] + } + }, + "iteration": 1 + }, + "workflowTask": { + "name": "Loop Task", + "taskReferenceName": "LoopTask", + "inputParameters": { + "value": "${workflow.input.value}" + }, + "type": "DO_WHILE", + "startDelay": 0, + "optional": false, + "asyncComplete": false, + "loopCondition": "false", + "loopOver": [ + { + "name": "inline_task_1", + "taskReferenceName": "inline_task_1", + "inputParameters": { + "value": "${workflow.input.value}", + "evaluatorType": "javascript", + "expression": "function e() { if ($.value == 1){return {\"result\": 'NODE_1'}} else { return {\"result\": 'NODE_2'}}} e();" + }, + "type": "INLINE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + }, + { + "name": "switch_task", + "taskReferenceName": "switch_task", + "inputParameters": { + "switchCaseValue": "${inline_task_1.output.result.result}" + }, + "type": "SWITCH", + "decisionCases": { + "NODE_1": [ + { + "name": "Set_NODE_1", + "taskReferenceName": "Set_NODE_1", + "inputParameters": { + "node": "NODE_1" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + } + ], + "NODE_2": [ + { + "name": "Set_NODE_2", + "taskReferenceName": "Set_NODE_2", + "inputParameters": { + "node": "NODE_2" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + } + ] + }, + "startDelay": 0, + "optional": false, + "asyncComplete": false, + "evaluatorType": "value-param", + "expression": "switchCaseValue" + } + ] + }, + "rateLimitPerFrequency": 0, + "rateLimitFrequencyInSeconds": 1, + "workflowPriority": 0, + "iteration": 1, + "subworkflowChanged": false, + "taskDefinition": null, + "queueWaitTime": -1, + "loopOverTask": true + }, + { + "taskType": "INLINE", + "status": "COMPLETED", + "inputData": { + "evaluatorType": "javascript", + "expression": "function e() { if ($.value == 1){return {\"result\": 'NODE_1'}} else { return {\"result\": 'NODE_2'}}} e();", + "value": null + }, + "referenceTaskName": "inline_task_1__1", + "retryCount": 0, + "seq": 2, + "pollCount": 0, + "taskDefName": "inline_task_1", + "scheduledTime": 1660244522137, + "startTime": 1660244522134, + "endTime": 1660244522361, + "updateTime": 1660244522146, + "startDelayInSeconds": 0, + "retried": false, + "executed": true, + "callbackFromWorker": true, + "responseTimeoutSeconds": 0, + "workflowInstanceId": "c56d02d5-cc01-4a82-a611-00a3f7f56305", + "workflowType": "LoopTestWithSwitch", + "taskId": "19b0ecc8-37cb-4038-857a-8e9b885acdf0", + "callbackAfterSeconds": 0, + "outputData": { + "result": { + "result": "NODE_2" + } + }, + "workflowTask": { + "name": "inline_task_1", + "taskReferenceName": "inline_task_1", + "inputParameters": { + "value": "${workflow.input.value}", + "evaluatorType": "javascript", + "expression": "function e() { if ($.value == 1){return {\"result\": 'NODE_1'}} else { return {\"result\": 'NODE_2'}}} e();" + }, + "type": "INLINE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + }, + "rateLimitPerFrequency": 0, + "rateLimitFrequencyInSeconds": 0, + "workflowPriority": 0, + "iteration": 1, + "subworkflowChanged": false, + "taskDefinition": null, + "queueWaitTime": -3, + "loopOverTask": true + }, + { + "taskType": "SWITCH", + "status": "COMPLETED", + "inputData": { + "hasChildren": "true", + "case": "NODE_2" + }, + "referenceTaskName": "switch_task__1", + "retryCount": 0, + "seq": 3, + "pollCount": 0, + "taskDefName": "SWITCH", + "scheduledTime": 1660244522479, + "startTime": 1660244522477, + "endTime": 1660244522654, + "updateTime": 1660244522489, + "startDelayInSeconds": 0, + "retried": false, + "executed": true, + "callbackFromWorker": true, + "responseTimeoutSeconds": 0, + "workflowInstanceId": "c56d02d5-cc01-4a82-a611-00a3f7f56305", + "workflowType": "LoopTestWithSwitch", + "taskId": "aabe9d65-0f11-4d95-b8f6-e4c21cb544cb", + "callbackAfterSeconds": 0, + "outputData": { + "evaluationResult": [ + "NODE_2" + ] + }, + "workflowTask": { + "name": "switch_task", + "taskReferenceName": "switch_task", + "inputParameters": { + "switchCaseValue": "${inline_task_1.output.result.result}" + }, + "type": "SWITCH", + "decisionCases": { + "NODE_1": [ + { + "name": "Set_NODE_1", + "taskReferenceName": "Set_NODE_1", + "inputParameters": { + "node": "NODE_1" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + } + ], + "NODE_2": [ + { + "name": "Set_NODE_2", + "taskReferenceName": "Set_NODE_2", + "inputParameters": { + "node": "NODE_2" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + } + ] + }, + "startDelay": 0, + "optional": false, + "asyncComplete": false, + "evaluatorType": "value-param", + "expression": "switchCaseValue" + }, + "rateLimitPerFrequency": 0, + "rateLimitFrequencyInSeconds": 0, + "workflowPriority": 0, + "iteration": 1, + "subworkflowChanged": false, + "taskDefinition": null, + "queueWaitTime": -2, + "loopOverTask": true + }, + { + "taskType": "SET_VARIABLE", + "status": "COMPLETED", + "inputData": { + "node": "NODE_2" + }, + "referenceTaskName": "Set_NODE_2__1", + "retryCount": 0, + "seq": 4, + "pollCount": 0, + "taskDefName": "Set_NODE_2", + "scheduledTime": 1660244522482, + "startTime": 1660244522477, + "endTime": 1660244522709, + "updateTime": 1660244522542, + "startDelayInSeconds": 0, + "retried": false, + "executed": true, + "callbackFromWorker": true, + "responseTimeoutSeconds": 0, + "workflowInstanceId": "c56d02d5-cc01-4a82-a611-00a3f7f56305", + "workflowType": "LoopTestWithSwitch", + "taskId": "eeda447b-4040-47e1-935a-af9f62d30ce9", + "callbackAfterSeconds": 0, + "outputData": {}, + "workflowTask": { + "name": "Set_NODE_2", + "taskReferenceName": "Set_NODE_2", + "inputParameters": { + "node": "NODE_2" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + }, + "rateLimitPerFrequency": 0, + "rateLimitFrequencyInSeconds": 0, + "workflowPriority": 0, + "iteration": 1, + "subworkflowChanged": false, + "taskDefinition": null, + "queueWaitTime": -5, + "loopOverTask": true + } + ], + "input": {}, + "output": {}, + "taskToDomain": {}, + "failedReferenceTaskNames": [], + "workflowDefinition": { + "createTime": 1660244498873, + "name": "LoopTestWithSwitch", + "description": "Loop Test With Switch WF", + "version": 1, + "tasks": [ + { + "name": "Loop Task", + "taskReferenceName": "LoopTask", + "inputParameters": { + "value": "${workflow.input.value}" + }, + "type": "DO_WHILE", + "startDelay": 0, + "optional": false, + "asyncComplete": false, + "loopCondition": "false", + "loopOver": [ + { + "name": "inline_task_1", + "taskReferenceName": "inline_task_1", + "inputParameters": { + "value": "${workflow.input.value}", + "evaluatorType": "javascript", + "expression": "function e() { if ($.value == 1){return {\"result\": 'NODE_1'}} else { return {\"result\": 'NODE_2'}}} e();" + }, + "type": "INLINE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + }, + { + "name": "switch_task", + "taskReferenceName": "switch_task", + "inputParameters": { + "switchCaseValue": "${inline_task_1.output.result.result}" + }, + "type": "SWITCH", + "decisionCases": { + "NODE_1": [ + { + "name": "Set_NODE_1", + "taskReferenceName": "Set_NODE_1", + "inputParameters": { + "node": "NODE_1" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + } + ], + "NODE_2": [ + { + "name": "Set_NODE_2", + "taskReferenceName": "Set_NODE_2", + "inputParameters": { + "node": "NODE_2" + }, + "type": "SET_VARIABLE", + "startDelay": 0, + "optional": false, + "asyncComplete": false + } + ] + }, + "startDelay": 0, + "optional": false, + "asyncComplete": false, + "evaluatorType": "value-param", + "expression": "switchCaseValue" + } + ] + } + ], + "inputParameters": [], + "outputParameters": {}, + "schemaVersion": 2, + "restartable": true, + "workflowStatusListenerEnabled": true, + "ownerEmail": "abc@example.com", + "timeoutPolicy": "ALERT_ONLY", + "timeoutSeconds": 0, + "variables": {}, + "inputTemplate": {} + }, + "priority": 0, + "variables": { + "node": "NODE_2" + }, + "lastRetriedTime": 0, + "startTime": 1660244521990, + "workflowName": "LoopTestWithSwitch", + "workflowVersion": 1 + } \ No newline at end of file diff --git a/ui/src/components/diagram/WorkflowDAG.js b/ui/src/components/diagram/WorkflowDAG.js index 7535a66398..908503ac23 100644 --- a/ui/src/components/diagram/WorkflowDAG.js +++ b/ui/src/components/diagram/WorkflowDAG.js @@ -253,6 +253,7 @@ export default class WorkflowDAG { processDoWhileTask(doWhileTask, antecedents) { console.assert(Array.isArray(antecedents)); + console.log(this.taskResults); const hasDoWhileExecuted = !!this.getExecutionStatus( doWhileTask.taskReferenceName @@ -282,10 +283,11 @@ export default class WorkflowDAG { } ); - const loopOverRefs = Array.from(this.taskResults.keys()).filter((key) => { + const loopOverRefs = Array.from(this.taskResultsByRef.keys()).filter((key) => { for (let prefix of loopOverRefPrefixes) { if (key.startsWith(prefix)) return true; } + return false }); const loopTaskResults = []; @@ -306,15 +308,6 @@ export default class WorkflowDAG { this.addVertex(endDoWhileTask, [...loopTasks]); } else { - // Create cosmetic LOOP edges between top and bottom bars - this.graph.setEdge( - doWhileTask.taskReferenceName, - doWhileTask.taskReferenceName + "-END", - { - type: "loop", - executed: hasDoWhileExecuted, - } - ); // Definition view (or not executed) @@ -339,9 +332,10 @@ export default class WorkflowDAG { this.addVertex(endDoWhileTask, [lastLoopTask]); } + // Create reverse loop edge this.graph.setEdge( - doWhileTask.taskReferenceName + "-END", doWhileTask.taskReferenceName, + doWhileTask.taskReferenceName + "-END", { type: "loop", executed: hasDoWhileExecuted, diff --git a/ui/src/components/diagram/WorkflowGraph.test.cy.js b/ui/src/components/diagram/WorkflowGraph.test.cy.js index dcf076b130..c7dee76192 100644 --- a/ui/src/components/diagram/WorkflowGraph.test.cy.js +++ b/ui/src/components/diagram/WorkflowGraph.test.cy.js @@ -66,7 +66,7 @@ describe("", () => { cy.fixture("dynamicFork/noneSpawned").then((data) => { const dag = new WorkflowDAG(data); - mount(); + mount(); cy.get("#dynamic_tasks_DF_EMPTY_PLACEHOLDER") .should("contain", "No tasks spawned") .click(); @@ -74,4 +74,40 @@ describe("", () => { cy.get("@onClickSpy").should("not.be.called"); }); }); + + it("Do_while containing switch (definition)", () => { + const onClickSpy = cy.spy().as("onClickSpy"); + + cy.fixture("doWhile/doWhileSwitch").then((data) => { + const dag = new WorkflowDAG(null, data.workflowDefinition); + mount( + + ); + + cy.get('.edgePaths .edgePath.reverse').should('exist'); + cy.get('.edgePaths').find('.edgePath').should('have.length', 10); + cy.get('.edgeLabels').should('contain', 'LOOP'); + }); + }) + + + it("Do_while containing switch (execution)", () => { + const onClickSpy = cy.spy().as("onClickSpy"); + + cy.fixture("doWhile/doWhileSwitch").then((data) => { + const dag = new WorkflowDAG(data); + mount( + + ); + + cy.get("#LoopTask_DF_TASK_PLACEHOLDER") + .should("contain", "2 of 2 tasks succeeded") + .click(); + + cy.get("@onClickSpy").should("be.calledWith", { ref: "inline_task_1__1" }); + cy.get('.edgePaths').find('.edgePath').should('have.length', 5); + cy.get('.edgePaths .edgePath.reverse').should('exist'); + cy.get('.edgeLabels').should('contain', 'LOOP'); + }); + }) });