Repository: node-red/node-red
Pull request #4185 (2023-05-22 21:39:39)

feature: new node selection group for catch and status nodes
This PR builds on the work done by @NetHans in #3843. This PR includes their commits, so will supersede that PR.

It adds a new 'in same group' scope for catch/status nodes. They will handle messages where the source node is in the same group (or a lower nested group).

On top of the work done by @NetHans, this PR modifies how it checks which catch/status nodes are considered 'in scope' of an error/status message.

In particular, it means nested groups operate as expected; a catch/status node can be added at each level of a nested group hierachy and each will get triggered in the right order.


Patch:
diff --git a/packages/node_modules/@node-red/nodes/core/common/25-catch.html b/packages/node_modules/@node-red/nodes/core/common/25-catch.html
index 1a13d1337..fe68b46db 100644
--- a/packages/node_modules/@node-red/nodes/core/common/25-catch.html
+++ b/packages/node_modules/@node-red/nodes/core/common/25-catch.html
@@ -4,6 +4,7 @@
         <label style="width: auto" for="node-input-scope" data-i18n="catch.label.source"></label>
         <select id="node-input-scope-select">
             <option value="all" data-i18n="catch.scope.all"></option>
+            <option value="group" data-i18n="catch.scope.group"></option>
             <option value="target" data-i18n="catch.scope.selected"></option>
         </select>
     </div>
@@ -170,6 +171,8 @@
             });
             if (this.scope === null) {
                 $("#node-input-scope-select").val("all");
+            } else if(this.scope === "group"){
+                $("#node-input-scope-select").val("group");
             } else {
                 $("#node-input-scope-select").val("target");
             }
@@ -179,6 +182,8 @@
             var scope = $("#node-input-scope-select").val();
             if (scope === 'all') {
                 this.scope = null;
+            } else if(scope === 'group') {
+                this.scope = "group";
             } else {
                 $("#node-input-uncaught").prop("checked",false);
                 this.scope = $("#node-input-catch-target-container-div").treeList('selected').map(function(i) { return i.node.id})
diff --git a/packages/node_modules/@node-red/nodes/core/common/25-status.html b/packages/node_modules/@node-red/nodes/core/common/25-status.html
index 16a318833..0efdede33 100644
--- a/packages/node_modules/@node-red/nodes/core/common/25-status.html
+++ b/packages/node_modules/@node-red/nodes/core/common/25-status.html
@@ -4,6 +4,7 @@
         <label style="width: auto" for="node-input-scope" data-i18n="status.label.source"></label>
         <select id="node-input-scope-select">
             <option value="all" data-i18n="status.scope.all"></option>
+            <option value="group" data-i18n="status.scope.group"></option>
             <option value="target" data-i18n="status.scope.selected"></option>
         </select>
     </div>
@@ -157,6 +158,8 @@
             });
             if (this.scope === null) {
                 $("#node-input-scope-select").val("all");
+            } else if(this.scope === "group"){
+                $("#node-input-scope-select").val("group");
             } else {
                 $("#node-input-scope-select").val("target");
             }
@@ -166,6 +169,8 @@
             var scope = $("#node-input-scope-select").val();
             if (scope === 'all') {
                 this.scope = null;
+            } else if(scope === 'group') {
+                this.scope = "group";
             } else {
                 this.scope = $("#node-input-status-target-container-div").treeList('selected').map(function(i) { return i.node.id})
             }
diff --git a/packages/node_modules/@node-red/nodes/locales/de/messages.json b/packages/node_modules/@node-red/nodes/locales/de/messages.json
index 4ac593504..828cd4252 100644
--- a/packages/node_modules/@node-red/nodes/locales/de/messages.json
+++ b/packages/node_modules/@node-red/nodes/locales/de/messages.json
@@ -98,6 +98,7 @@
         },
         "scope": {
             "all": "allen Nodes",
+            "group": "in gleicher Gruppe",
             "selected": "ausgewählten Nodes"
         }
     },
@@ -110,6 +111,7 @@
         },
         "scope": {
             "all": "allen Nodes",
+            "group": "in gleicher Gruppe",
             "selected": "ausgewählten Nodes"
         }
     },
diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json
index 801aeace8..52fd1455d 100644
--- a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json
+++ b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json
@@ -103,6 +103,7 @@
         },
         "scope": {
             "all": "all nodes",
+            "group": "in same group",
             "selected": "selected nodes"
         }
     },
@@ -115,6 +116,7 @@
         },
         "scope": {
             "all": "all nodes",
+            "group": "in same group",
             "selected": "selected nodes"
         }
     },
diff --git a/packages/node_modules/@node-red/runtime/lib/flows/Flow.js b/packages/node_modules/@node-red/runtime/lib/flows/Flow.js
index b5bedb2d6..4430e59ed 100644
--- a/packages/node_modules/@node-red/runtime/lib/flows/Flow.js
+++ b/packages/node_modules/@node-red/runtime/lib/flows/Flow.js
@@ -128,6 +128,65 @@ class Flow {
         }
         this.parent.log(msg);
     }
+	
+    /**
+     * Checks if node A and node B are in the same group. 
+     * Node B can also be placed in a subgroup.
+     * If node A is not in any group, false is returned
+     * @param {Node} nodeIdA   Node which defines the first search level
+     * @param {Node} nodeIdB   Node which is to be searched in the group or a subgroup
+     * @returns {boolean}      Returns true if all nodes are in the same group. If not, then false or if node A is not in a group then also false.
+     */
+    isNodeInSameGroup(nodeIdA, nodeIdB) {
+        const groups = this.global.groups;
+        let result = false;
+        for(let key in groups) {
+            let group = groups[key];
+            
+            if(!group.nodes.includes(nodeIdA.id)) {
+                continue;
+            }
+			
+            if(group.nodes.includes(nodeIdB.id)) {
+                result = true;
+                break;
+            }
+
+            /**
+             * Subfunction to recursively search the groups for matches
+             * @param {Node}      Node which is to be searched in the group or a subgroup
+             * @param {Group}    targetGroup group currently under analysis
+             * @returns {boolean} Returns true if a match was found. Otherwise false.
+             */
+            const isInSubGroup = (targetNode, targetGroup) => {
+                let _result = false;
+                if(targetGroup.nodes.includes(targetNode.id)) {
+                    _result = true;
+                } else {
+                    for(let nodeId of targetGroup.nodes) {
+                        let node = this.getGroupNode(nodeId);
+
+                        if(!node){
+                            continue;
+                        }
+
+                        if(node.type === "group"){
+                            let result = isInSubGroup(targetNode, node);
+                            if(result === true){
+                                _result = true;
+                                break;
+                            }
+                        }
+                    }
+                }
+
+                return _result;
+            };
+
+            result = isInSubGroup(nodeIdB, group);
+        }
+        return result;
+    }
 
     /**
      * Start this flow.
@@ -606,10 +665,36 @@ class Flow {
             }
             handled = true;
         } else {
-            this.statusNodes.forEach(function(targetStatusNode) {
-                if (targetStatusNode.scope && targetStatusNode.scope.indexOf(reportingNode.id) === -1) {
+            const candidateNodes = [];
+            this.statusNodes.forEach(targetStatusNode => {
+                if (targetStatusNode.g && targetStatusNode.scope === 'group' && !reportingNode.g) {
+                    // Status node inside a group, reporting node not in a group - skip it
+                    return
+                }
+                if (Array.isArray(targetStatusNode.scope) && targetStatusNode.scope.indexOf(reportingNode.id) === -1) {
                     return;
                 }
+                let distance = 0
+                if (reportingNode.g) {
+                    // Reporting node inside a group. Calculate the distance between it and the status node
+                    let containingGroup = this.global.groups[reportingNode.g]
+                    while (containingGroup && containingGroup.id !== targetStatusNode.g) {
+                        distance++
+                        containingGroup = this.global.groups[containingGroup.g]
+                    }
+                    if (!containingGroup && targetStatusNode.g && targetStatusNode.scope === 'group') {
+                        // This status node is in a group, but not in the same hierachy
+                        // the reporting node is in
+                        return
+                    }
+                }
+                candidateNodes.push({ d: distance, n: targetStatusNode })
+            })
+            candidateNodes.sort((A,B) => {
+                return A.d - B.d
+            })
+            candidateNodes.forEach(candidate => {
+                const targetStatusNode = candidate.n
                 var message = {
                     status: clone(statusMessage)
                 }
@@ -667,21 +752,46 @@ class Flow {
             }
             handled = true;
         } else {
-            var handledByUncaught = false;
-
-            this.catchNodes.forEach(function(targetCatchNode) {
-                if (targetCatchNode.scope && targetCatchNode.scope.indexOf(reportingNode.id) === -1) {
+            const candidateNodes = [];
+            this.catchNodes.forEach(targetCatchNode => {
+                if (targetCatchNode.g && targetCatchNode.scope === 'group' && !reportingNode.g) {
+                    // Catch node inside a group, reporting node not in a group - skip it
+                    return
+                }
+                if (Array.isArray(targetCatchNode.scope) && targetCatchNode.scope.indexOf(reportingNode.id) === -1) {
+                    // Catch node has a scope set and it doesn't include the reporting node
                     return;
                 }
-                if (!targetCatchNode.scope && targetCatchNode.uncaught && !handledByUncaught) {
+                let distance = 0
+                if (reportingNode.g) {
+                    // Reporting node inside a group. Calculate the distance between it and the catch node
+                    let containingGroup = this.global.groups[reportingNode.g]
+                    while (containingGroup && containingGroup.id !== targetCatchNode.g) {
+                        distance++
+                        containingGroup = this.global.groups[containingGroup.g]
+                    }
+                    if (!containingGroup && targetCatchNode.g && targetCatchNode.scope === 'group') {
+                        // This catch node is in a group, but not in the same hierachy
+                        // the reporting node is in
+                        return
+                    }
+                }
+                candidateNodes.push({ d: distance, n: targetCatchNode })
+            })
+            candidateNodes.sort((A,B) => {
+                return A.d - B.d
+            })
+            let handledByUncaught = false
+            candidateNodes.forEach(candidate => {
+                const targetCatchNode = candidate.n
+                if (targetCatchNode.uncaught && !handledByUncaught) {
+                    // This node only wants errors that haven't already been handled
                     if (handled) {
-                        // This has been handled by a !uncaught catch node
-                        return;
+                        return
                     }
-                    // This is an uncaught error
-                    handledByUncaught = true;
+                    handledByUncaught = true
                 }
-                var errorMessage;
+                let errorMessage;
                 if (msg) {
                     errorMessage = redUtil.cloneMessage(msg);
                 } else {


Test patch:
diff --git a/test/unit/@node-red/runtime/lib/flows/Flow_spec.js b/test/unit/@node-red/runtime/lib/flows/Flow_spec.js
index ca30868d7..6d53747ef 100644
--- a/test/unit/@node-red/runtime/lib/flows/Flow_spec.js
+++ b/test/unit/@node-red/runtime/lib/flows/Flow_spec.js
@@ -686,6 +686,44 @@ describe('Flow', function() {
             },50);
         });
 
+        it.only("passes a status event to the group scoped status node",function(done) {
+            var config = flowUtils.parseConfig([
+                {id:"t1",type:"tab"},
+                {id: "g1", type: "group", g: "g3" },
+                {id: "g2", type: "group" },
+                {id: "g3", type: "group" },
+                {id:"1",x:10,y:10,z:"t1",g:"g1", type:"test",name:"a",wires:["2"]},
+                // sn - in the same group as source node
+                {id:"sn",x:10,y:10,z:"t1",g:"g1", type:"status",scope:"group",wires:[]},
+                // sn2 - in a different group hierarchy to the source node
+                {id:"sn2",x:10,y:10,z:"t1", g:"g2", type:"status",scope:"group",wires:[]},
+                // sn3 - in a higher-level group to the source node
+                {id:"sn3",x:10,y:10,z:"t1", g:"g3", type:"status",scope:"group",wires:[]},
+                // sn2 - in a different group hierarchy, but not scope to the group
+                {id:"sn4",x:10,y:10,z:"t1", g:"g2", type:"status",wires:[]},
+                
+            ]);
+            var flow = Flow.create({},config,config.flows["t1"]);
+
+            flow.start();
+
+            var activeNodes = flow.getActiveNodes();
+            flow.handleStatus(config.flows["t1"].nodes["1"],{text:"my-status"});
+            setTimeout(function() {
+                try {
+                    currentNodes["sn"].should.have.a.property("handled",1);
+                    currentNodes["sn2"].should.have.a.property("handled",0);
+                    currentNodes["sn3"].should.have.a.property("handled",1);
+                    currentNodes["sn3"].should.have.a.property("handled",1);
+                    done()
+                } catch(err) {
+                    done(err)
+                }
+            },50);
+        });
+
+
+
     });
 
     describe("#handleError",function() {
@@ -796,6 +834,42 @@ describe('Flow', function() {
                 },50);
             },50);
         });
+        it("passes an error event to the group scoped catch node",function(done) {
+            var config = flowUtils.parseConfig([
+                {id:"t1",type:"tab"},
+                {id: "g1", type: "group", g: "g3" },
+                {id: "g2", type: "group" },
+                {id: "g3", type: "group" },
+                {id:"1",x:10,y:10,z:"t1",g:"g1", type:"test",name:"a",wires:["2"]},
+                // sn - in the same group as source node
+                {id:"sn",x:10,y:10,z:"t1",g:"g1", type:"catch",scope:"group",wires:[]},
+                // sn2 - in a different group hierarchy to the source node
+                {id:"sn2",x:10,y:10,z:"t1", g:"g2", type:"catch",scope:"group",wires:[]},
+                // sn3 - in a higher-level group to the source node
+                {id:"sn3",x:10,y:10,z:"t1", g:"g3", type:"catch",scope:"group",wires:[]},
+                // sn2 - in a different group hierarchy, but not scope to the group
+                {id:"sn4",x:10,y:10,z:"t1", g:"g2", type:"catch",wires:[]},
+                
+            ]);
+            var flow = Flow.create({},config,config.flows["t1"]);
+
+            flow.start();
+
+            var activeNodes = flow.getActiveNodes();
+
+            flow.handleError(config.flows["t1"].nodes["1"],"my-error",{a:"foo"});
+            setTimeout(function() {
+                try {
+                    currentNodes["sn"].should.have.a.property("handled",1);
+                    currentNodes["sn2"].should.have.a.property("handled",0);
+                    currentNodes["sn3"].should.have.a.property("handled",1);
+                    currentNodes["sn3"].should.have.a.property("handled",1);
+                    done()
+                } catch(err) {
+                    done(err)
+                }
+            },50);
+        });
         it("moves any existing error object sideways",function(done){
             var config = flowUtils.parseConfig([
                 {id:"t1",type:"tab"},