diff --git a/js/id/actions/revert.js b/js/id/actions/revert.js index 83c1cb310..3dbdca96c 100644 --- a/js/id/actions/revert.js +++ b/js/id/actions/revert.js @@ -4,11 +4,31 @@ iD.actions.Revert = function(id) { var entity = graph.hasEntity(id), base = graph.base().entities[id]; - if (entity && !base) { - return iD.actions.DeleteMultiple([id])(graph); - } else { - return graph.revert(id); + if (entity && !base) { // entity will be removed.. + if (entity.type === 'node') { + graph.parentWays(entity) + .forEach(function(parent) { + parent = parent.removeNode(id); + graph = graph.replace(parent); + + if (parent.isDegenerate()) { + graph = iD.actions.DeleteWay(parent.id)(graph); + } + }); + } + + graph.parentRelations(entity) + .forEach(function(parent) { + parent = parent.removeMembersWithID(id); + graph = graph.replace(parent); + + if (parent.isDegenerate()) { + graph = iD.actions.DeleteRelation(parent.id)(graph); + } + }); } + + return graph.revert(id); }; return action; diff --git a/test/spec/actions/revert.js b/test/spec/actions/revert.js index e8e64f0db..82f71e1a1 100644 --- a/test/spec/actions/revert.js +++ b/test/spec/actions/revert.js @@ -1,27 +1,195 @@ describe('iD.actions.Revert', function() { - it('deletes a new entity', function() { - var n1 = iD.Node({id: 'n'}), - graph = iD.Graph().replace(n1); + describe("basic", function () { + it('removes a new entity', function() { + var n1 = iD.Node({id: 'n-1'}), + graph = iD.Graph().replace(n1); - graph = iD.actions.Revert('n')(graph); - expect(graph.hasEntity('n')).to.be.undefined; + graph = iD.actions.Revert('n-1')(graph); + expect(graph.hasEntity('n-1')).to.be.undefined; + }); + + it('reverts an updated entity', function() { + var n1 = iD.Node({id: 'n1'}), + n1up = n1.update({}), + graph = iD.Graph([n1]).replace(n1up); + + graph = iD.actions.Revert('n1')(graph); + expect(graph.hasEntity('n1')).to.equal(n1); + }); + + it('restores a deleted entity', function() { + var n1 = iD.Node({id: 'n1'}), + graph = iD.Graph([n1]).remove(n1); + + graph = iD.actions.Revert('n1')(graph); + expect(graph.hasEntity('n1')).to.equal(n1); + }); }); - it('reverts an updated entity', function() { - var n1 = iD.Node({id: 'n'}), - n2 = n1.update({}), - graph = iD.Graph([n1]).replace(n2); + describe("reverting way child nodes", function () { + it('removes new node, updates parent way nodelist', function() { + // note: test with a 3 node way so w1 doesnt go degenerate.. + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n2'}), + n3 = iD.Node({id: 'n-3'}), + w1 = iD.Way({id: 'w1', nodes: ['n1', 'n2']}), + w1up = w1.addNode('n-3', 2), + graph = iD.Graph([n1, n2, w1]).replace(n3).replace(w1up); - graph = iD.actions.Revert('n')(graph); - expect(graph.hasEntity('n')).to.equal(n1); + graph = iD.actions.Revert('n-3')(graph); + + var w1_1 = graph.hasEntity('w1'); + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n2'), 'n2 unchanged').to.equal(n2); + expect(graph.hasEntity('n-3'), 'n-3 removed').to.be.undefined; + expect(graph.parentWays(n1), 'n1 has w1 as parent way').to.deep.equal([w1_1]); + expect(graph.parentWays(n2), 'n2 has w1 as parent way').to.deep.equal([w1_1]); + expect(w1_1.nodes, 'w1 nodes updated').to.deep.equal(w1.nodes); + }); + + it('reverts existing node, preserves parent way nodelist', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n2'}), + w1 = iD.Way({id: 'w1', nodes: ['n1', 'n2']}), + n1up = n1.update({}), + graph = iD.Graph([n1, n2, w1]).replace(n1up); + + graph = iD.actions.Revert('n1')(graph); + + var w1_1 = graph.hasEntity('w1'); + expect(graph.hasEntity('n1'), 'n1 reverted').to.equal(n1); + expect(graph.hasEntity('n2'), 'n2 unchanged').to.equal(n2); + expect(graph.parentWays(n1), 'n1 has w1 as parent way').to.deep.equal([w1_1]); + expect(graph.parentWays(n2), 'n2 has w1 as parent way').to.deep.equal([w1_1]); + expect(w1_1.nodes, 'w1 nodes preserved').to.deep.equal(w1.nodes); + }); }); - it('restores a deleted entity', function() { - var n1 = iD.Node({id: 'n'}), - graph = iD.Graph([n1]).remove(n1); + describe("reverting relation members", function () { + it('removes new node, updates parent relation memberlist', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + r1 = iD.Relation({id: 'r1', members: [{id: 'n1'}]}), + r1up = r1.addMember({id: 'n-2'}, 1), + graph = iD.Graph([n1, r1]).replace(n2).replace(r1up); - graph = iD.actions.Revert('n')(graph); - expect(graph.hasEntity('n')).to.equal(n1); + graph = iD.actions.Revert('n-2')(graph); + + var r1_1 = graph.hasEntity('r1'); + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 removed').to.be.undefined; + expect(graph.parentRelations(n1), 'n1 has r1 as parent relation').to.deep.equal([r1_1]); + expect(r1_1.members, 'r1 members updated').to.deep.equal(r1.members); + }); + + it('reverts existing node, preserves parent relation memberlist', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n2'}), + r1 = iD.Relation({id: 'r1', members: [{id: 'n1'}, {id: 'n2'}]}), + n1up = n1.update({}), + graph = iD.Graph([n1, n2, r1]).replace(n1up); + + graph = iD.actions.Revert('n1')(graph); + + var r1_1 = graph.hasEntity('r1'); + expect(graph.hasEntity('n1'), 'n1 reverted').to.equal(n1); + expect(graph.hasEntity('n2'), 'n2 unchanged').to.equal(n2); + expect(graph.parentRelations(n1), 'n1 has r1 as parent relation').to.deep.equal([r1_1]); + expect(graph.parentRelations(n2), 'n2 has r1 as parent relation').to.deep.equal([r1_1]); + expect(r1_1.members, 'r1 members preserved').to.deep.equal(r1.members); + }); + }); + + describe("reverting parent ways", function () { + it('removes new way, preserves new and existing child nodes', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + w1 = iD.Way({id: 'w-1', nodes: ['n1', 'n-2']}), + graph = iD.Graph([n1]).replace(n2).replace(w1); + + graph = iD.actions.Revert('w-1')(graph); + expect(graph.hasEntity('w-1'), 'w-1 removed').to.be.undefined; + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 unchanged').to.equal(n2); + expect(graph.parentWays(n1), 'n1 has no parent ways').to.be.empty; + expect(graph.parentWays(n2), 'n-2 has no parent ways').to.be.empty; + }); + + it('reverts an updated way, preserves new and existing child nodes', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + w1 = iD.Way({id: 'w1', nodes: ['n1']}), + w1up = w1.addNode('n-2', 1), + graph = iD.Graph([n1, w1]).replace(n2).replace(w1up); + + graph = iD.actions.Revert('w1')(graph); + expect(graph.hasEntity('w1'), 'w1 reverted').to.equal(w1); + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 unchanged').to.equal(n2); + expect(graph.parentWays(n1), 'n1 has w1 as parent way').to.deep.equal([w1]); + expect(graph.parentWays(n2), 'n-2 has no parent ways').to.be.empty; + }); + + it('restores a deleted way, preserves new and existing child nodes', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + w1 = iD.Way({id: 'w1', nodes: ['n1']}), + w1up = w1.addNode('n-2', 1), + graph = iD.Graph([n1, w1]).replace(n2).replace(w1up).remove(w1up); + + graph = iD.actions.Revert('w1')(graph); + expect(graph.hasEntity('w1'), 'w1 reverted').to.equal(w1); + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 unchanged').to.equal(n2); + expect(graph.parentWays(n1), 'n1 has w1 as parent way').to.deep.equal([w1]); + expect(graph.parentWays(n2), 'n-2 has no parent ways').to.be.empty; + }); + }); + + describe("reverting parent relations", function () { + it('removes new relation, preserves new and existing members', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + r1 = iD.Relation({id: 'r-1', members: [{id: 'n1'}, {id: 'n-2'}]}), + graph = iD.Graph([n1]).replace(n2).replace(r1); + + graph = iD.actions.Revert('r-1')(graph); + expect(graph.hasEntity('r-1'), 'r-1 removed').to.be.undefined; + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 unchanged').to.equal(n2); + expect(graph.parentRelations(n1), 'n1 has no parent relations').to.be.empty; + expect(graph.parentRelations(n2), 'n-2 has no parent relations').to.be.empty; + }); + + it('reverts an updated relation, preserves new and existing members', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + r1 = iD.Relation({id: 'r1', members: [{id: 'n1'}]}), + r1up = r1.addMember({id: 'n-2'}, 1), + graph = iD.Graph([n1, r1]).replace(n2).replace(r1up); + + graph = iD.actions.Revert('r1')(graph); + expect(graph.hasEntity('r1'), 'r1 reverted').to.equal(r1); + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 unchanged').to.equal(n2); + expect(graph.parentRelations(n1), 'n1 has r1 as parent relation').to.deep.equal([r1]); + expect(graph.parentRelations(n2), 'n-2 has no parent relations').to.be.empty; + }); + + it('restores a deleted relation, preserves new and existing members', function() { + var n1 = iD.Node({id: 'n1'}), + n2 = iD.Node({id: 'n-2'}), + r1 = iD.Relation({id: 'r1', members: [{id: 'n1'}]}), + r1up = r1.addMember({id: 'n-2'}, 1), + graph = iD.Graph([n1, r1]).replace(n2).replace(r1up).remove(r1up); + + graph = iD.actions.Revert('r1')(graph); + expect(graph.hasEntity('r1'), 'r1 reverted').to.equal(r1); + expect(graph.hasEntity('n1'), 'n1 unchanged').to.equal(n1); + expect(graph.hasEntity('n-2'), 'n-2 unchanged').to.equal(n2); + expect(graph.parentRelations(n1), 'n1 has r1 as parent relation').to.deep.equal([r1]); + expect(graph.parentRelations(n2), 'n-2 has no parent relations').to.be.empty; + }); }); }); diff --git a/test/spec/core/difference.js b/test/spec/core/difference.js index ac51bd810..3a51c4926 100644 --- a/test/spec/core/difference.js +++ b/test/spec/core/difference.js @@ -66,6 +66,31 @@ describe("iD.Difference", function () { diff = iD.Difference(base, head); expect(diff.changes()).to.eql({}); }); + + it("doesn't include created entities that were subsequently reverted", function () { + var node = iD.Node({id: 'n-1'}), + base = iD.Graph(), + head = base.replace(node).revert('n-1'), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({}); + }); + + it("doesn't include modified entities that were subsequently reverted", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.update({ tags: { yes: 'no' } }), + base = iD.Graph([n1]), + head = base.replace(n2).revert('n'), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({}); + }); + + it("doesn't include deleted entities that were subsequently reverted", function () { + var node = iD.Node({id: 'n'}), + base = iD.Graph([node]), + head = base.remove(node).revert('n'), + diff = iD.Difference(base, head); + expect(diff.changes()).to.eql({}); + }); }); describe("#extantIDs", function () {