[fix] keep parent links after convertNode, do not share state with subtree copies

CHANGELOG.md

@@ -5,6 +5,21 @@
 Changes since `0.2.11`. The version number is not changed yet, and the
 committed `dist/` is still the `0.2.11` build.
 
+### Behaviour changes (bug fixes)
+
+- `DataModel.convertNode` (via `replaceNode`) sets `$parent` of the children
+  to the new node. They kept pointing to the replaced node, which is no
+  longer in the model: sd-computations then initialised the expression scope
+  of the children from that node (variable values of the time of the
+  conversion), and sd-tree-designer treated the children of a selected
+  converted node as separate subtrees (copy and paste duplicated them).
+  Saving and loading the diagram repaired the links.
+- `DataModel.cloneSubtree` (copy / paste) copies the field status
+  (`$fieldStatus`) of the nodes and edges and the computed values of the
+  nodes deeply. The copy shared these objects with the original: marking a
+  field of the pasted copy as invalid also marked the original field, and
+  sd-computations then skipped the evaluation of the original's expression.
+
 ### Build and tests
 
 - The bundles are built with Rollup and Babel 7 instead of gulp/browserify,

src/data-model.js

@@ -368,7 +368,10 @@ export class DataModel {
         }
 
         newNode.childEdges = oldNode.childEdges;
-        newNode.childEdges.forEach(e=>e.parentNode=newNode);
+        newNode.childEdges.forEach(e=>{
+            e.parentNode=newNode;
+            e.childNode.$parent=newNode;
+        });
 
         var index = this.nodes.indexOf(oldNode);
         if(~index){
@@ -394,6 +397,7 @@ export class DataModel {
             childClone.$parent = clone;
             var edge = Utils.clone(e);
             edge.id = Utils.guid();
+            edge.$fieldStatus = Utils.cloneDeep(e.$fieldStatus);
             edge.parentNode = clone;
             edge.childNode = childClone;
             edge.payoff = Utils.cloneDeep(e.payoff);
@@ -437,7 +441,8 @@ export class DataModel {
         var clone = Utils.clone(node)
         clone.id = Utils.guid();
         clone.location = Utils.clone(node.location);
-        clone.computed = Utils.clone(node.computed);
+        clone.computed = Utils.cloneDeep(node.computed);
+        clone.$fieldStatus = Utils.cloneDeep(node.$fieldStatus);
         clone.$parent = null;
         clone.childEdges = [];
         return clone;

test/contract/data-model-edit.test.js

@@ -222,3 +222,54 @@ test('clear, clearComputedValues, clearExpressionScope, reversePayoffs, getGloba
     assert.deepEqual([dm.code, dm.$codeError, dm.$codeDirty, dm.defaultCriterion1Weight, dm.weightLowerBound, dm.weightUpperBound],
         ['', null, false, 1, 0, Infinity]);
 });
+
+// Regression: replaceNode (used by convertNode) did not update $parent of the
+// children, which kept pointing to the removed node. sd-computations then took
+// the expression scope of the children from the removed node (stale variable
+// values) and sd-tree-designer treated the children of a selected converted
+// node as separate subtrees (copy / paste duplicated them).
+test('convertNode: the children of the new node have it as $parent', () => {
+    const {dm, root, chance, toChance, toT1, toT2} = sampleTree();
+    dm.convertNode(chance, 'decision');
+    const converted = toChance.childNode;
+    assert.equal(toT1.childNode.$parent, converted);
+    assert.equal(toT2.childNode.$parent, converted);
+    checkIntegrity(assert, dm);
+    const subtree = dm.getAllNodesInSubtree(converted);
+    assert.deepEqual(dm.findSubtreeRoots(subtree), [converted]);
+    // a converted leaf keeps the parent chain
+    dm.convertNode(toT1.childNode, 'chance');
+    assert.equal(toT1.childNode.$parent, converted);
+    checkIntegrity(assert, dm);
+    // and the relinked state survives undo / redo
+    dm.saveState();
+    dm.removeNodes([converted]);
+    dm.undo();
+    checkIntegrity(assert, dm);
+    assert.equal(dm.getRoots().length, 1);
+    assert.equal(dm.getRoots()[0].id, root.id);
+});
+
+// Regression: cloneSubtree shared the field status objects of the nodes and
+// edges (and the nested computed values of the nodes) with the original, so
+// marking a field of the pasted copy as invalid marked the original too, and
+// sd-computations skipped evaluating the original's expressions.
+test('cloneSubtree: the copy does not share field status or computed values with the original', () => {
+    const {dm, chance, toT1} = sampleTree();
+    chance.computedValue('rule', 'payoff', 1);
+    chance.setValueValidity('name', true);
+    toT1.setValueValidity('probability', true);
+    for (const cloneComputed of [false, true]) {
+        const copy = dm.cloneSubtree(chance, cloneComputed);
+        const copiedEdge = copy.childEdges[0];
+        copiedEdge.setSyntaxValidity('probability', false);
+        copy.setSyntaxValidity('name', false);
+        copy.computedValue('rule', 'payoff', 2);
+        assert.equal(toT1.isFieldValid('probability'), true);
+        assert.equal(chance.isFieldValid('name'), true);
+        assert.equal(chance.computedValue('rule', 'payoff'), 1);
+        // the copy keeps the state it was copied with
+        assert.equal(dm.cloneSubtree(copy).childEdges[0].isFieldValid('probability'), false);
+        assert.equal(dm.cloneSubtree(copy).computedValue('rule', 'payoff'), 2);
+    }
+});