From 0706ae59819d965e529fe09ec1aea1b37fc54a68 Mon Sep 17 00:00:00 2001 From: Michael Abashian Date: Mon, 5 Dec 2016 15:02:18 -0500 Subject: [PATCH] Recognize edge conflicts when they arise and force the user to fix them before saving. --- awx/ui/client/src/forms/WorkflowMaker.js | 10 +-- .../workflow-chart/workflow-chart.block.less | 7 ++ .../workflow-chart.directive.js | 19 ++++- .../workflow-maker.controller.js | 78 ++++++++++++++++--- .../workflow-maker.partial.html | 2 +- .../templates/workflows/workflow.service.js | 42 +++++++++- 6 files changed, 138 insertions(+), 20 deletions(-) diff --git a/awx/ui/client/src/forms/WorkflowMaker.js b/awx/ui/client/src/forms/WorkflowMaker.js index 9b1c369e32..5bd7788db1 100644 --- a/awx/ui/client/src/forms/WorkflowMaker.js +++ b/awx/ui/client/src/forms/WorkflowMaker.js @@ -33,27 +33,27 @@ export default edgeType: { label: i18n._('Type'), type: 'radio_group', - ngShow: 'selectedTemplate && showTypeOptions', + ngShow: 'selectedTemplate && edgeFlags.showTypeOptions', ngDisabled: '!canAddWorkflowJobTemplate', options: [ { label: i18n._('On Success'), value: 'success', - ngShow: '!edgeTypeRestriction || edgeTypeRestriction === "successFailure"' + ngShow: '!edgeFlags.typeRestriction || edgeFlags.typeRestriction === "successFailure"' }, { label: i18n._('On Failure'), value: 'failure', - ngShow: '!edgeTypeRestriction || edgeTypeRestriction === "successFailure"' + ngShow: '!edgeFlags.typeRestriction || edgeFlags.typeRestriction === "successFailure"' }, { label: i18n._('Always'), value: 'always', - ngShow: '!edgeTypeRestriction || edgeTypeRestriction === "always"' + ngShow: '!edgeFlags.typeRestriction || edgeFlags.typeRestriction === "always"' } ], awRequiredWhen: { - reqExpression: 'showTypeOptions' + reqExpression: 'edgeFlags.showTypeOptions' } }, credential: { diff --git a/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.block.less b/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.block.less index 4c795d2a40..177ab6b35d 100644 --- a/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.block.less +++ b/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.block.less @@ -90,3 +90,10 @@ width: 90px; color: @default-interface-txt; } +.WorkflowChart-conflictIcon { + color: @default-err; +} +.WorkflowChart-conflictText { + width: 90px; + color: @default-interface-txt; +} diff --git a/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.directive.js b/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.directive.js index ac8e9c44ac..9e5ad95475 100644 --- a/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.directive.js +++ b/awx/ui/client/src/templates/workflows/workflow-chart/workflow-chart.directive.js @@ -119,8 +119,7 @@ export default [ '$state', .attr("class", "node") .attr("id", function(d){return "node-" + d.id;}) .attr("parent", function(d){return d.parent ? d.parent.id : null;}) - .attr("transform", function(d) { return "translate(" + d.y + "," + d.x + ")"; }) - .attr("fill", "red"); + .attr("transform", function(d) { return "translate(" + d.y + "," + d.x + ")"; }); nodeEnter.each(function(d) { let thisNode = d3.select(this); @@ -171,6 +170,16 @@ export default [ '$state', return (d.unifiedJobTemplate && d.unifiedJobTemplate.name) ? d.unifiedJobTemplate.name : ""; }).each(wrap); + thisNode.append("foreignObject") + .attr("x", 43) + .attr("y", 45) + .style("font-size","0.7em") + .attr("class", "WorkflowChart-conflictText") + .html(function () { + return "\uf06a EDGE CONFLICT"; + }) + .style("display", function(d) { return (d.edgeConflict && !d.placeholder) ? null : "none"; }); + thisNode.append("foreignObject") .attr("x", 17) .attr("y", 22) @@ -347,7 +356,7 @@ export default [ '$state', let link = svgGroup.selectAll("g.link") .data(links, function(d) { - return d.target.id; + return d.source.id + "-" + d.target.id; }); let linkEnter = link.enter().append("g") @@ -485,6 +494,7 @@ export default [ '$state', }); t.selectAll(".node") + .attr("parent", function(d){return d.parent ? d.parent.id : null;}) .attr("transform", function(d) {d.px = d.x; d.py = d.y; return "translate(" + d.y + "," + d.x + ")"; }); t.selectAll(".WorkflowChart-nodeTypeCircle") @@ -558,6 +568,9 @@ export default [ '$state', t.selectAll(".WorkflowChart-incompleteText") .style("display", function(d){ return d.unifiedJobTemplate || d.placeholder ? "none" : null; }); + t.selectAll(".WorkflowChart-conflictText") + .style("display", function(d) { return (d.edgeConflict && !d.placeholder) ? null : "none"; }); + } function add_node() { diff --git a/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.controller.js b/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.controller.js index 7414826f53..e6bd83d9b3 100644 --- a/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.controller.js +++ b/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.controller.js @@ -29,6 +29,12 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr value: "check" }]; + $scope.edgeFlags = { + conflict: false, + typeRestriction: null, + showTypeOptions: false + }; + function init() { $scope.treeDataMaster = angular.copy($scope.treeData.data); $scope.$broadcast("refreshWorkflowChart"); @@ -36,7 +42,7 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr function resetNodeForm() { $scope.workflowMakerFormConfig.nodeMode = "idle"; - $scope.showTypeOptions = false; + $scope.edgeFlags.showTypeOptions = false; delete $scope.selectedTemplate; delete $scope.workflow_job_templates; delete $scope.workflow_projects; @@ -44,7 +50,7 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr delete $scope.placeholderNode; delete $scope.betweenTwoNodes; $scope.nodeBeingEdited = null; - $scope.edgeTypeRestriction = null; + $scope.edgeFlags.typeRestriction = null; $scope.workflowMakerFormConfig.activeTab = "jobs"; } @@ -89,7 +95,8 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr let siblingConnectionTypes = WorkflowService.getSiblingConnectionTypes({ tree: $scope.treeData.data, - parentId: betweenTwoNodes ? parent.source.id : parent.id + parentId: betweenTwoNodes ? parent.source.id : parent.id, + childId: $scope.placeholderNode.id }); // Set the default to success @@ -99,21 +106,27 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr // We don't want to give the user the option to select // a type as this node will always be executed edgeType = "always"; - $scope.showTypeOptions = false; + $scope.edgeFlags.showTypeOptions = false; } else { if ((_.includes(siblingConnectionTypes, "success") || _.includes(siblingConnectionTypes, "failure")) && _.includes(siblingConnectionTypes, "always")) { - // This is a problem... + // This is a conflicted scenario but we'll just let the user keep building - they will have to remediate before saving + $scope.edgeFlags.typeRestriction = null; } else if (_.includes(siblingConnectionTypes, "success") || _.includes(siblingConnectionTypes, "failure")) { - $scope.edgeTypeRestriction = "successFailure"; + $scope.edgeFlags.typeRestriction = "successFailure"; edgeType = "success"; } else if (_.includes(siblingConnectionTypes, "always")) { - $scope.edgeTypeRestriction = "always"; + $scope.edgeFlags.typeRestriction = "always"; edgeType = "always"; + } else { + $scope.edgeFlags.typeRestriction = null; } - $scope.showTypeOptions = true; + $scope.edgeFlags.showTypeOptions = true; } + // Reset the edgeConflict flag + resetEdgeConflict(); + $scope.$broadcast("setEdgeType", edgeType); $scope.$broadcast("refreshWorkflowChart"); @@ -181,6 +194,9 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr } } + // Reset the edgeConflict flag + resetEdgeConflict(); + $scope.$broadcast("refreshWorkflowChart"); }; @@ -195,6 +211,9 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr $scope.nodeBeingEdited.isActiveEdit = false; } + // Reset the edgeConflict flag + resetEdgeConflict(); + // Reset the form resetNodeForm(); @@ -208,6 +227,12 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr if (!$scope.nodeBeingEdited || ($scope.nodeBeingEdited && $scope.nodeBeingEdited.id !== nodeToEdit.id)) { if ($scope.placeholderNode || $scope.nodeBeingEdited) { $scope.cancelNodeForm(); + + // Refresh this object as the parent has changed + nodeToEdit = WorkflowService.searchTree({ + element: $scope.treeData.data, + matchingId: nodeToEdit.id + }); } $scope.workflowMakerFormConfig.nodeMode = "edit"; @@ -330,7 +355,30 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr } } - $scope.showTypeOptions = (parent && parent.isStartNode) ? false : true; + let siblingConnectionTypes = WorkflowService.getSiblingConnectionTypes({ + tree: $scope.treeData.data, + parentId: parent.id, + childId: nodeToEdit.id + }); + + if (parent && parent.isStartNode) { + // We don't want to give the user the option to select + // a type as this node will always be executed + $scope.edgeFlags.showTypeOptions = false; + } else { + if ((_.includes(siblingConnectionTypes, "success") || _.includes(siblingConnectionTypes, "failure")) && _.includes(siblingConnectionTypes, "always")) { + // This is a conflicted scenario but we'll just let the user keep building - they will have to remediate before saving + $scope.edgeFlags.typeRestriction = null; + } else if (_.includes(siblingConnectionTypes, "success") || _.includes(siblingConnectionTypes, "failure") && (nodeToEdit.edgeType === "success" || nodeToEdit.edgeType === "failure")) { + $scope.edgeFlags.typeRestriction = "successFailure"; + } else if (_.includes(siblingConnectionTypes, "always") && nodeToEdit.edgeType === "always") { + $scope.edgeFlags.typeRestriction = "always"; + } else { + $scope.edgeFlags.typeRestriction = null; + } + + $scope.edgeFlags.showTypeOptions = true; + } $scope.$broadcast('setEdgeType', $scope.nodeBeingEdited.edgeType); @@ -441,6 +489,9 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr resetNodeForm(); } + // Reset the edgeConflict flag + resetEdgeConflict(); + resetDeleteNode(); $scope.$broadcast("refreshWorkflowChart"); @@ -515,6 +566,15 @@ export default ['$scope', 'WorkflowService', 'generateList', 'TemplateList', 'Pr }); }; + function resetEdgeConflict(){ + $scope.edgeFlags.conflict = false; + + WorkflowService.checkForEdgeConflicts({ + treeData: $scope.treeData.data, + edgeFlags: $scope.edgeFlags + }); + } + init(); } diff --git a/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.partial.html b/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.partial.html index b52b8ed381..f3ea67b990 100644 --- a/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.partial.html +++ b/awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.partial.html @@ -82,6 +82,6 @@
- +
diff --git a/awx/ui/client/src/templates/workflows/workflow.service.js b/awx/ui/client/src/templates/workflows/workflow.service.js index 36282fac29..4e551c7582 100644 --- a/awx/ui/client/src/templates/workflows/workflow.service.js +++ b/awx/ui/client/src/templates/workflows/workflow.service.js @@ -46,6 +46,8 @@ export default [function(){ child.edgeType = "always"; } + child.parent = parentNode; + parentNode.children.push(child); }); } @@ -81,9 +83,10 @@ export default [function(){ if(params.betweenTwoNodes) { _.forEach(parentNode.children, function(child, index) { if(child.id === params.parent.target.id) { - placeholder.children.push(angular.copy(child)); + placeholder.children.push(child); parentNode.children[index] = placeholder; placeholderRef = parentNode.children[index]; + child.parent = parentNode.children[index]; return false; } }); @@ -102,6 +105,7 @@ export default [function(){ }, getSiblingConnectionTypes: function(params) { // params.parentId + // params.childId // params.tree let siblingConnectionTypes = {}; @@ -114,7 +118,7 @@ export default [function(){ if(parentNode.children && parentNode.children.length > 0) { // Loop across them and add the types as keys to siblingConnectionTypes _.forEach(parentNode.children, function(child) { - if(!child.placeholder && child.edgeType) { + if(child.id !== params.childId && !child.placeholder && child.edgeType) { siblingConnectionTypes[child.edgeType] = true; } }); @@ -283,6 +287,40 @@ export default [function(){ }; } + }, + checkForEdgeConflicts: function(params) { + //params.treeData + //params.edgeFlags + + let hasAlways = false; + let hasSuccessFailure = false; + let _this = this; + + _.forEach(params.treeData.children, function(child) { + // Flip the flag to false for now - we'll set it to true later on + // if we detect a conflict + child.edgeConflict = false; + if(child.edgeType === 'always') { + hasAlways = true; + } + else if(child.edgeType === 'success' || child.edgeType === 'failure') { + hasSuccessFailure = true; + } + + _this.checkForEdgeConflicts({ + treeData: child, + edgeFlags: params.edgeFlags + }); + }); + + if(hasAlways && hasSuccessFailure) { + // We have a conflict + _.forEach(params.treeData.children, function(child) { + child.edgeConflict = true; + }); + + params.edgeFlags.conflict = true; + } } }; }];