Merge pull request #4295 from mabashian/3964-edge-conflict-v2

Implement edge conflicts when editing the workflow graph

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: {

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;
+}

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("transform", function(d) { return "translate(" + d.y + "," + d.x + ")"; });
-                    .attr("fill", "red");
 
                 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 "<span class=\"WorkflowChart-conflictIcon\">\uf06a</span><span> EDGE CONFLICT</span>";
+                             })
+                             .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() {

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();
 
     }

awx/ui/client/src/templates/workflows/workflow-maker/workflow-maker.partial.html

@@ -82,6 +82,6 @@
     </div>
     <div class="WorkflowMaker-buttonHolder">
         <button type="button" class="btn btn-sm WorkflowMaker-cancelButton" ng-click="closeWorkflowMaker()"> Close</button>
-        <button type="button" class="btn btn-sm WorkflowMaker-saveButton" ng-click="saveWorkflowMaker()" ng-show="workflow_job_template_obj.summary_fields.user_capabilities.edit || canAddWorkflowJobTemplate"> Save</button>
+        <button type="button" class="btn btn-sm WorkflowMaker-saveButton" ng-click="saveWorkflowMaker()" ng-show="workflow_job_template_obj.summary_fields.user_capabilities.edit || canAddWorkflowJobTemplate" ng-disabled="edgeFlags.conflict"> Save</button>
     </div>
 </div>

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;
+            }
         }
     };
 }];