Merge pull request #4668 from node-red/undo-history-fix

Fix undo history of moves and post-deploy handling

packages/node_modules/@node-red/editor-api/lib/auth/index.js

@@ -205,10 +205,9 @@ function genericStrategy(adminApp,strategy) {
     passport.use(new strategy.strategy(options, verify));
 
     adminApp.get('/auth/strategy',
-        passport.authenticate(strategy.name, {
-            session:false,
+        passport.authenticate(strategy.name, {session:false,
             failureMessage: true,
-            failureRedirect: settings.httpAdminRoot + '?session_message=Login Failed'
+            failureRedirect: settings.httpAdminRoot
         }),
         completeGenerateStrategyAuth,
         handleStrategyError
@@ -222,7 +221,7 @@ function genericStrategy(adminApp,strategy) {
         passport.authenticate(strategy.name, {
             session:false,
             failureMessage: true,
-            failureRedirect: settings.httpAdminRoot + '?session_message=Login Failed'
+            failureRedirect: settings.httpAdminRoot
         }),
         completeGenerateStrategyAuth,
         handleStrategyError

packages/node_modules/@node-red/editor-client/src/js/history.js

@@ -706,11 +706,36 @@ RED.history = (function() {
 
     }
 
+    function markEventDirty (evt) {
+        // This isn't 100% thorough - just covers the main move/edit/delete cases
+        evt.dirty = true
+        if (evt.multi) {
+            for (let i = 0; i < evt.events.length-1; i++) {
+                markEventDirty(evt.events[i])
+            }
+        } else if (evt.t === 'move') {
+            for (let i=0;i<evt.nodes.length;i++) {
+                evt.nodes[i].moved = true
+            }
+        } else if (evt.t === 'edit') {
+            evt.changed = true
+        } else if (evt.t === 'delete') {
+            if (evt.nodes) {
+                for (let i=0;i<evt.nodes.length;i++) {
+                    evt.nodes[i].changed = true
+                }
+            }
+        }
+    }
     return {
-        //TODO: this function is a placeholder until there is a 'save' event that can be listened to
         markAllDirty: function() {
-            for (var i=0;i<undoHistory.length;i++) {
+            // A deploy has happened meaning any undo into the history will represent
+            // an undeployed change - regardless of what it was when the event was recorded.
+            // This goes back through the history any marks them all as being dirty events
+            // and also ensures individual node states are marked dirty
+            for (let i=0;i<undoHistory.length;i++) {
                 undoHistory[i].dirty = true;
+                markEventDirty(undoHistory[i])
             }
         },
         list: function() {

packages/node_modules/@node-red/editor-client/src/js/ui/editor.js

@@ -741,16 +741,9 @@ RED.editor = (function() {
             }
 
             try {
-                const rc = editing_node._def.oneditsave.call(editing_node);
+                var rc = editing_node._def.oneditsave.call(editing_node);
                 if (rc === true) {
                     editState.changed = true;
-                } else if (typeof rc === 'object' && rc !== null ) {
-                    if (rc.changed === true) {
-                        editState.changed = true
-                    }
-                    if (Array.isArray(rc.history) && rc.history.length > 0) {
-                        editState.history = rc.history
-                    }
                 }
             } catch(err) {
                 console.warn("oneditsave",editing_node.id,editing_node.type,err.toString());
@@ -1022,7 +1015,7 @@ RED.editor = (function() {
                                     }
                                 });
                             }
-                            let historyEvent = {
+                            var historyEvent = {
                                 t:'edit',
                                 node:editing_node,
                                 changes:editState.changes,
@@ -1038,15 +1031,6 @@ RED.editor = (function() {
                                     instances:subflowInstances
                                 }
                             }
-
-                            if (editState.history) {
-                                historyEvent = {
-                                    t: 'multi',
-                                    events: [ historyEvent, ...editState.history ],
-                                    dirty: wasDirty
-                                }
-                            }
-
                             RED.history.push(historyEvent);
                         }
                         editing_node.dirty = true;

packages/node_modules/@node-red/editor-client/src/js/ui/editors/code-editors/monaco.js

@@ -514,7 +514,7 @@ RED.editor.codeEditor.monaco = (function() {
                 _monaco.languages.json.jsonDefaults.setDiagnosticsOptions(diagnosticOptions);
                 if(modeConfiguration) { _monaco.languages.json.jsonDefaults.setModeConfiguration(modeConfiguration); }
             } catch (error) {
-                console.warn("monaco - Error setting up json options", error)
+                console.warn("monaco - Error setting up json options", err)
             }
         }
 
@@ -526,7 +526,7 @@ RED.editor.codeEditor.monaco = (function() {
                 if(htmlDefaults) { _monaco.languages.html.htmlDefaults.setOptions(htmlDefaults); }
                 if(handlebarDefaults) { _monaco.languages.html.handlebarDefaults.setOptions(handlebarDefaults); }
             } catch (error) {
-                console.warn("monaco - Error setting up html options", error)
+                console.warn("monaco - Error setting up html options", err)
             }
         }
 
@@ -546,7 +546,7 @@ RED.editor.codeEditor.monaco = (function() {
                 if(lessDefaults_modeConfiguration) { _monaco.languages.css.cssDefaults.setDiagnosticsOptions(lessDefaults_modeConfiguration); }
                 if(scssDefaults_modeConfiguration) { _monaco.languages.css.cssDefaults.setDiagnosticsOptions(scssDefaults_modeConfiguration); }
             } catch (error) {
-                console.warn("monaco - Error setting up CSS/SCSS/LESS options", error)
+                console.warn("monaco - Error setting up CSS/SCSS/LESS options", err)
             }
         }
 

packages/node_modules/@node-red/editor-client/src/js/ui/view.js

@@ -2167,9 +2167,9 @@ RED.view = (function() {
                     if (n.ox !== n.n.x || n.oy !== n.n.y || addedToGroup) {
                         // This node has moved or added to a group
                         if (rehomedNodes.has(n)) {
-                            moveAndChangedGroupEvent.nodes.push({...n})
+                            moveAndChangedGroupEvent.nodes.push({...n, moved: n.n.moved})
                         } else {
-                            moveEvent.nodes.push({...n})
+                            moveEvent.nodes.push({...n, moved: n.n.moved})
                         }
                         n.n.dirty = true;
                         n.n.moved = true;

packages/node_modules/@node-red/nodes/core/common/60-link.html

@@ -194,46 +194,27 @@
                 nodeMap[node.links[i]].new = true;
             }
         }
-
-        let editHistories = []
-        let n;
-        for (let id in nodeMap) {
+        var n;
+        for (var id in nodeMap) {
             if (nodeMap.hasOwnProperty(id)) {
                 n = RED.nodes.node(id);
                 if (n) {
-                    editHistories.push({
-                        t:'edit',
-                        node: n,
-                        changes: {
-                            links: [...n.links]
-                        },
-                        changed: n.changed
-                    })
                     if (nodeMap[id].old && !nodeMap[id].new) {
                         // Removed id
                         i = n.links.indexOf(node.id);
                         if (i > -1) {
                             n.links.splice(i,1);
-                            n.changed = true
-                            n.dirty = true
                         }
                     } else if (!nodeMap[id].old && nodeMap[id].new) {
                         // Added id
                         i = n.links.indexOf(id);
                         if (i === -1) {
                             n.links.push(node.id);
-                            n.changed = true
-                            n.dirty = true
                         }
                     }
                 }
             }
         }
-        if (editHistories.length > 0) {
-            return {
-                history: editHistories
-            }
-        }
     }
 
     function onAdd() {
@@ -273,14 +254,13 @@
             onEditPrepare(this,"link out");
         },
         oneditsave: function() {
-            const result = onEditSave(this);
+            onEditSave(this);
             // In case the name has changed, ensure any link call nodes on this
             // tab are redrawn with the updated name
             var localCallNodes = RED.nodes.filterNodes({z:RED.workspaces.active(), type:"link call"});
             localCallNodes.forEach(function(node) {
                 node.dirty = true;
             });
-            return result
         },
         onadd: onAdd,
         oneditresize: resizeNodeList
@@ -349,7 +329,7 @@
             onEditPrepare(this,"link in");
         },
         oneditsave: function() {
-            return onEditSave(this);
+            onEditSave(this);
         },
         oneditresize: resizeNodeList
     });
@@ -393,7 +373,7 @@
 
         },
         oneditsave: function() {
-            return onEditSave(this);
+            onEditSave(this);
         },
         onadd: onAdd,
         oneditresize: resizeNodeList

packages/node_modules/@node-red/nodes/core/function/10-function.js

@@ -374,7 +374,7 @@ module.exports = function(RED) {
                         iniOpt.breakOnSigint = true;
                     }
                 }
-                node.script = new vm.Script(functionText, createVMOpt(node, ""));
+                node.script = vm.createScript(functionText, createVMOpt(node, ""));
                 if (node.fin && (node.fin !== "")) {
                     var finText = `(function () {
                         var node = {
@@ -438,9 +438,10 @@ module.exports = function(RED) {
 
                             //store the error in msg to be used in flows
                             msg.error = err;
+
+                            var line = 0;
+                            var errorMessage;
                             if (stack.length > 0) {
-                                let line = 0;
-                                let errorMessage;
                                 while (line < stack.length && stack[line].indexOf("ReferenceError") !== 0) {
                                     line++;
                                 }
@@ -454,13 +455,11 @@ module.exports = function(RED) {
                                         errorMessage += " (line "+lineno+", col "+cha+")";
                                     }
                                 }
-                                if (errorMessage) {
-                                    err.message = errorMessage
-                                }
                             }
-                            // Pass the whole error object so any additional properties
-                            // (such as cause) are preserved
-                            done(err);
+                            if (!errorMessage) {
+                                errorMessage = err.toString();
+                            }
+                            done(errorMessage);
                         }
                         else if (typeof err === "string") {
                             done(err);

packages/node_modules/@node-red/runtime/lib/flows/Flow.js

@@ -678,9 +678,6 @@ class Flow {
                 if (logMessage.hasOwnProperty('stack')) {
                     errorMessage.error.stack = logMessage.stack;
                 }
-                if (logMessage.hasOwnProperty('cause')) {
-                    errorMessage.error.cause = logMessage.cause;
-                }
                 targetCatchNode.receive(errorMessage);
                 handled = true;
             });

test/nodes/core/function/10-function_spec.js

@@ -390,8 +390,7 @@ describe('function node', function() {
                     msg.should.have.property('level', helper.log().ERROR);
                     msg.should.have.property('id', 'n1');
                     msg.should.have.property('type', 'function');
-                    msg.should.have.property('msg')
-                    msg.msg.message.should.equal('ReferenceError: retunr is not defined (line 2, col 1)');
+                    msg.should.have.property('msg', 'ReferenceError: retunr is not defined (line 2, col 1)');
                     done();
                 } catch(err) {
                     done(err);
@@ -660,8 +659,7 @@ describe('function node', function() {
                 msg.should.have.property('level', helper.log().ERROR);
                 msg.should.have.property('id', name);
                 msg.should.have.property('type', 'function');
-                msg.should.have.property('msg')
-                msg.msg.message.should.equal('Callback must be a function');
+                msg.should.have.property('msg', 'Error: Callback must be a function');
                 done();
             }
             catch (e) {