I am facing an issue in my gojs diagram where inside a group when I add a node and then undo/redo it, its groupkey becomes undefined because of which the nodes doesnt recognize that it is a part of a group and whole diagram disrupts.
So basically in ideal case when node is removed and then I undo it, it keeps the group key there, so it works fine because i removed the node manually from removeNode button but when i directly undo the node, in the modelListener i can see node.oldValue/node.newValue has group:undefined and due to that when i redo it has same group:undefined causing this issue. Is this goJs issue or something I am missing.
Here’s what I just tested:
console.log("UNDO")
myNewData = { key: 4, group: 3, text: "member of group 3" };
myDiagram.model.commit(m => {
m.addNodeData(myNewData);
}, "add myNewData");
console.log("data.group:", myNewData.group);
myDiagram.commandHandler.undo();
console.log("data.group:", myNewData.group);
And also the delete node case that you mention:
console.log("DELETE")
myNewData = { key: 4, group: 3, text: "member of group 3" };
myDiagram.model.commit(m => {
m.addNodeData(myNewData);
}, "add myNewData");
console.log("data.group:", myNewData.group);
myDiagram.select(myDiagram.findNodeForData(myNewData));
myDiagram.commandHandler.deleteSelection();
console.log("data.group:", myNewData.group);
It is true that after the undo that the data.group property value is undefined. However a redo would correctly reset data.group to the group’s key and actually add the Node back into the Group. So the Diagram’s state is correctly maintained through undo and redo.
One obvious difference between an undo of adding a node via Model.addNodeData and a call to CommandHandler.deleteSelection causing in the former case only one Transaction in the UndoManager history, whereas in the latter case there are two quite different Transactions.
However you are wondering about why in the undo case the data.group is modified to be undefined. The reason is that when adding a node data object to the model’s nodeDataArray, a new Node is added to the Diagram in the default Layer, and the Node is added as a member of the existing Group. So an undo will first remove the Node from the Group and then remove the Node from the Layer/Diagram. That first step will reset the data.group to undefined.
But in the delete case, when the Node is removed from the Group, the Node is already not in the Diagram so it doesn’t bother recording its removal.
The order of operations is different. For undo the actions are undone in reverse; for delete the actions are just performed. But that’s OK because the intended state of the diagram is still achieved either way. I can see how you would want the model data to have the same state after the undo that it had before the add, but I’m sorry that that is not what happens.
But When I am redoing the status inside the group, it is not getting connected back properly and i thought it is happening because i was getting group:undefined in the node.newValue inside the modelChange when i perform redoing , due to this my whole diagram breaks. Please see the screenshot.
Added status
Redo Status after undo
What do you mean by this?
What code did you execute in a transaction that you are undoing?
This is my modelChange method for your reference.
modelChange = (e) => {
if ( !e.isTransactionFinished) {return; }
const txn = e.object;
if (txn === null) { return; }
const that = this;
const propertyName = e.propertyName;
function removeEdge(edgeVal){
that.updateSaveTimerIf5SecRemain();
let fromNodeId = edgeVal.from;
if(edgeVal.fromPort.includes('group')){
let groupNode = that.diagram.findNodeForKey(edgeVal.from);
if(!groupNode){
const toNodeData = that.diagram.model.findNodeDataForKey(edgeVal.to);
const groupKey = toNodeData?.group;
const parallelFlowGroup = that.diagram.findTopLevelGroups().filter(grp => grp.key == groupKey).first();
const forkNode = parallelFlowGroup?.memberParts.filter(memPart => memPart['data'] && ['fork'].includes(memPart['data']['nodeType'])).first().data;
fromNodeId = forkNode?.key;
} else {
fromNodeId = groupNode?.findLinksInto().first().data.from;
}
}
const fromNodeData = that.diagram.model.findNodeDataForKey(fromNodeId);
const fromNodePostData = that.data.wfNodes.find(wfNode => _.get(wfNode, 'metadata.apiNodeId') == fromNodeId);
if((_.get(fromNodeData, 'nodeType') == 'fork' || _.get(fromNodeData,'category') == 'groupType') && !edgeVal.toPort.includes('group') ) {
const secKey = _.get(fromNodeData, 'group');
if(edgeVal.fromPort && (!edgeVal.fromPort.includes('group'))) {
that.removeNodeFromForkForSec(secKey, edgeVal.to);
} else that.updateGroupOutNodeCnt( edgeVal, 'remove');
}
that.data.wfNodes.forEach(node => {
if (node.metadata.apiNodeId == fromNodeId) {
node.nodeTransitions = node.nodeTransitions.filter(obj => (!(obj.apiNodeId == fromNodeId && obj.apiNextNodeId == edgeVal.to)));
}
if(node.metadata.apiNodeId == edgeVal.to && edgeVal.fromPort){
if(_.get(node, 'nodeType.id') == 3) {
if(_.get(fromNodePostData, 'metadata.groupId')) that.removeGroupFromJoinForSec(_.get(node, 'metadata.data.group'), _.get(fromNodePostData, 'metadata.groupId'), node, fromNodePostData);
else if(_.get(fromNodePostData, 'metadata.data.rootNodeId')) that.removeNodeFromJoinForSec(_.get(node, 'metadata.data.group'), _.get(fromNodePostData, 'metadata.data.rootNodeId'), node, fromNodePostData);
}
if(_.get(node, 'metadata.groupId')) {
that.deleteGroupId(node, fromNodePostData);
} else if(_.get(node, 'metadata.data.rootNodeId')) {
that.deleteRootNodeId(node, fromNodePostData);
}
}
});
that.diagram.clearSelection();
const node = that.diagram.findNodeForKey(fromNodeId);
node && that.scrollToObjIfNotInVPBnds(node);
that.diagram.select(node);
that.displayFlags.hideEdgeForm = true;
that.clearAllLinkSelection();
that.selectedEdgeForm = null;
}
function removeNode(nodeVal){
that.updateSaveTimerIf5SecRemain();
that.data.wfNodes = that.data.wfNodes.filter(node => node.metadata.apiNodeId != nodeVal.key);
that.displayFlags.hideForm = _.get(that.selectedNodeForm, 'nodeType.id') != 3;
that.displayFlags.hideLock = true;
that.displayFlags.hideValueUpdate = true;
that.updateLockSelection(false, 'ruleLockCircle');
that.selectedNodeForm = _.get(that.selectedNodeForm, 'nodeType.id') != 3 || _.get(that.selectedNodeForm,'metadata.apiNodeId') == nodeVal.key ? null : that.selectedNodeForm;
}
function updateNodeName(nodeVal, value) {
const nodeOfOperation = that.data.wfNodes.find(node => node.metadata.apiNodeId== nodeVal.key);
if(nodeOfOperation) {
nodeOfOperation['name'] = value;
}
}
function linkToKey(nodeVal){
if(nodeVal.oldValue && nodeVal.newValue && nodeVal.object) {
let fromVal = nodeVal.object.from;
if(nodeVal.object.fromPort.includes("group")){
fromVal = that.data.wfNodes.find(node => _.has(node,'metadata.data.addedGroups') && node.metadata.data.addedGroups.find(group => group.id == fromVal))?.metadata?.apiNodeId;
}
if(that.data.wfNodes && that.data.wfNodes.find(node => _.has(node,'metadata.apiNodeId') && node.metadata.apiNodeId == fromVal)) {
const transitionArr = that.data.wfNodes.find(node => _.has(node,'metadata.apiNodeId') && node.metadata.apiNodeId == fromVal).nodeTransitions;
const transitionObj = transitionArr? transitionArr.find(obj => obj['apiNextNodeId'] && obj['apiNextNodeId'] == nodeVal.oldValue) : null;
if(transitionObj) transitionObj['apiNextNodeId'] = nodeVal.newValue;
const toNodePostDataOld = that.data.wfNodes.find(node => _.get(node,'metadata.apiNodeId') == nodeVal.oldValue);
const toNodePostDataNew = that.data.wfNodes.find(node => _.get(node,'metadata.apiNodeId') == nodeVal.newValue);
const fromNodePostData = that.data.wfNodes.find(node => _.get(node,'metadata.apiNodeId') == fromVal);
that.selectedStatus = toNodePostDataNew.status ? {name:_.get(toNodePostDataNew,'status.name'),id:_.get(toNodePostDataNew,'status.id'),apiNodeId:_.get(toNodePostDataNew,'metadata.apiNodeId')} : {name:_.get(toNodePostDataNew,'subWorkflow.name'),id:_.get(toNodePostDataNew,'subWorkflow.id'),apiNodeId:_.get(toNodePostDataNew,'metadata.apiNodeId')};
that.handleToReLinking(fromNodePostData, toNodePostDataOld, toNodePostDataNew, nodeVal, that);
}
const toDiagNodeNew = that.diagram.findNodeForKey(nodeVal.newValue);
const fromDiagNode = that.diagram.findNodeForKey(fromVal);
that.updateBgForNodePairIfValid(fromDiagNode, toDiagNodeNew);
}
}
function linkFromKey(val) {
if(val.oldValue && val.newValue && val.object) {
const toVal = val.object.to;
if(that.data.wfNodes && that.data.wfNodes.find(node => _.has(node,'metadata.apiNodeId') && node.metadata.apiNodeId == val.newValue) && that.data.wfNodes.find(node => _.has(node,'metadata.apiNodeId') && node.metadata.apiNodeId == val.oldValue)) {
const transitionArr = that.data.wfNodes.find(node => _.has(node,'metadata.apiNodeId') && node.metadata.apiNodeId == val.oldValue).nodeTransitions;
const newNodeObj = that.data.wfNodes.find(node => _.has(node,'metadata.apiNodeId') && node.metadata.apiNodeId == val.newValue);
const transitionObjInd = transitionArr? transitionArr.findIndex(obj => obj['apiNextNodeId'] && obj['apiNextNodeId'] == toVal) : -1;
if(transitionObjInd>-1) {
if(!_.has(newNodeObj,'nodeTransitions') || !(newNodeObj['nodeTransitions'] instanceof Array)) newNodeObj['nodeTransitions'] = [];
transitionArr[transitionObjInd]['apiNodeId'] = val.newValue;
newNodeObj['nodeTransitions'].push(_.cloneDeep(transitionArr[transitionObjInd]));
transitionArr.splice(transitionObjInd,1);
}
const fromNodePostDataOld = that.data.wfNodes.find(node => _.get(node,'metadata.apiNodeId') == val.oldValue);
const fromNodePostDataNew = that.data.wfNodes.find(node => _.get(node,'metadata.apiNodeId') == val.newValue);
const toNodePostData = that.data.wfNodes.find(node => _.get(node,'metadata.apiNodeId') == toVal);
that.fromNodeStatus = fromNodePostDataNew?.name;
that.handleFromReLinking(fromNodePostDataOld, fromNodePostDataNew, toNodePostData, val, that);
}
const fromDiagNodeNew = that.diagram.findNodeForKey(val.newValue);
const toDiagNode = that.diagram.findNodeForKey(toVal);
that.updateBgForNodePairIfValid(fromDiagNodeNew, toDiagNode);
}
}
if(['CommittedTransaction',"FinishedRedo"].includes(propertyName)){
txn && txn.changes && txn.changes.each(function(val) {
if (val.modelChange === 'nodeDataArray') {
// record node insertions and removals
if (val.change === go.ChangedEvent.Insert && val.newValue) {
that.createForm(val.newValue);
} else if (val.change === go.ChangedEvent.Remove && val.oldValue) {
removeNode(val.oldValue);
}
}
if (val.modelChange === 'linkDataArray') {
if (val.change === go.ChangedEvent.Insert && !val.newValue.toPort.includes('group')) {
that.addEdge(val.newValue);
}
else if (val.change === go.ChangedEvent.Remove) {
removeEdge(val.oldValue)
}
}
if(val.modelChange === 'linkFromKey') {
linkFromKey(val);
}
if(val.modelChange === 'linkFromPortId') {
val.oldValue && val.newValue && (val.oldValue.includes('group') && val.newValue.includes('fork')
|| val.oldValue.includes('fork') && val.newValue.includes('group')) && val.object &&
val.object.from && val.object.to && linkFromKey({oldValue: val.object.from, newValue: val.object.from, object: val.object});
}
if(val.modelChange === 'linkToKey') {
linkToKey(val);
}
else if (propertyName == 'FinishedRedo' && val.change && val.change == go.ChangedEvent.Property && val.propertyName == 'name') {
val.oldValue && val.newValue && val.object && updateNodeName(val.object,val.newValue);
}
});
if(txn && txn.name == 'Initial Layout') {
const scrollToStartNode = () => {
const node = this.diagram.findNodesByExample({'start': true}).first();
node && this.diagram && this.diagram.scroll("pixel", "down", (node.position.y - this.diagram.viewportBounds.y - 12));
};
scrollToStartNode();
}
}else if(propertyName == 'FinishedUndo') {
txn && txn.changes && txn.changes._dataArray.reverse().forEach(function (val) {
// during undo the case is reversed as the deleted node is inserted back.
if (val.modelChange === 'nodeDataArray') {
if (val.change === go.ChangedEvent.Insert && val.newValue) {removeNode(val.newValue)}
else if (val.change === go.ChangedEvent.Remove && val.oldValue) {that.createForm(val.oldValue)}
}
if (val.modelChange === 'linkDataArray') {
if (val.change === go.ChangedEvent.Insert) {removeEdge(val.newValue)}
else if (val.change === go.ChangedEvent.Remove) {that.addEdge(val.oldValue)}
}
if (val.modelChange === 'linkToKey') {
const valObj = { 'newValue': val['oldValue'], 'oldValue': val['newValue'], 'object': val['object'] };
linkToKey(valObj);
}
if(val.modelChange === 'linkFromKey') {
const valObj = { 'newValue': val['oldValue'], 'oldValue': val['newValue'], 'object': val['object'] };
linkFromKey(valObj);
}
else if (val.change && val.change == go.ChangedEvent.Property && val.propertyName == 'name') {
val.oldValue && val.newValue && val.object && updateNodeName(val.object,val.oldValue);
}
return val;
});
}
};
I can’t tell what your code is doing, partly because you call many other methods that I don’t know about.
Are you sure you aren’t intentionally or accidentally modifying the model or the diagram at any time in this code?
Note the warning in the documentation for Transaction ChangedEvents:
As a general rule, you should not make any changes to the model or any of its data in a listener for any Transaction ChangedEvent.
I understand that. I further debugged my whole modelChange and I noticed that my finishedUndo code block is the reason for this .
This is the code I am talking about.
else if(propertyName == 'FinishedUndo') {
txn && txn.changes && txn.changes._dataArray.reverse().forEach(function (val) {
// during undo the case is reversed as the deleted node is inserted back.
if (val.modelChange === 'nodeDataArray') {
if (val.change === go.ChangedEvent.Insert && val.newValue) {removeNode(val.newValue)}
else if (val.change === go.ChangedEvent.Remove && val.oldValue) {that.createForm(val.oldValue)}
}
if (val.modelChange === 'linkDataArray') {
if (val.change === go.ChangedEvent.Insert) {removeEdge(val.newValue)}
else if (val.change === go.ChangedEvent.Remove) {that.addEdge(val.oldValue)}
}
if (val.modelChange === 'linkToKey') {
const valObj = { 'newValue': val['oldValue'], 'oldValue': val['newValue'], 'object': val['object'] };
linkToKey(valObj);
}
if(val.modelChange === 'linkFromKey') {
const valObj = { 'newValue': val['oldValue'], 'oldValue': val['newValue'], 'object': val['object'] };
linkFromKey(valObj);
}
else if (val.change && val.change == go.ChangedEvent.Property && val.propertyName == 'name') {
val.oldValue && val.newValue && val.object && updateNodeName(val.object,val.oldValue);
}
return val;
});
}
};
here since I am traversing through txn.changes._dataArray.reverse().forEach() it is causing issue. When i am using each in place of this like txn.changes.each() group is coming properly in the node object. But since this is very stable code and working from last 2-3 years , I am not sure if i can replace _dataArray.reverse().forEach() with .each() to just solve this issue as it may break for other cases in case of undo/redo. Please suggest if this can be replaced or we can try something else here ?
You should not be trying to reimplement undo or redo – all of the changes that have happened in the model (assuming you have used Model methods to perform the modifications) and in the diagram will automatically be recorded in each Transaction and will perform their own undo or redo operations.
But then , how will this issue be resolved? Why we can’t use .each there in place of
_dataArray.reverse().forEach() .My entire diagram is getting corrupted due to this
My question is why you are trying to do anything to the model or its data or to the diagram when an undo or a redo is performed?
And if it had been working before, why change it?
I don’t know what the “_dataArray” property is.
And of course array.reverse().forEach(...) will do things in the opposite order of list.each(...).
_dataArray is something which is coming from gojs event object that is, e.object.changes._dataArray. We save our whole data in an array to send to backend and that array modification needs to be happened for this case as well. My question is if I use .each() in place of .reverse for this data modification, will it make any impact? As i earlier described you what issue I am facing because of this .reverse thing as it is directly trying to modifies the model object i guess
“_dataArray” is not a supported property, so you cannot depend on it.
Yes, Array.reverse modifies the Array. Again, that’s not something that you can do.