diff --git a/CHANGELOG.md b/CHANGELOG.md index f1dd8c4..ca84d7c 100644 --- a/CHANGELOG.md +++ b/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, diff --git a/src/data-model.js b/src/data-model.js index 800a097..3bdea9a 100644 --- a/src/data-model.js +++ b/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; diff --git a/test/contract/data-model-edit.test.js b/test/contract/data-model-edit.test.js index 60e216d..cbf5c3a 100644 --- a/test/contract/data-model-edit.test.js +++ b/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); + } +});