diff --git a/js/id/actions/revert.js b/js/id/actions/revert.js index 21da5bf7e..83c1cb310 100644 --- a/js/id/actions/revert.js +++ b/js/id/actions/revert.js @@ -1,5 +1,15 @@ -iD.actions.Revert = function(entity) { - return function(graph) { - return graph.revert(entity); +iD.actions.Revert = function(id) { + + var action = function(graph) { + var entity = graph.hasEntity(id), + base = graph.base().entities[id]; + + if (entity && !base) { + return iD.actions.DeleteMultiple([id])(graph); + } else { + return graph.revert(id); + } }; + + return action; }; diff --git a/js/id/core/graph.js b/js/id/core/graph.js index 20d114bce..3252fc1c1 100644 --- a/js/id/core/graph.js +++ b/js/id/core/graph.js @@ -251,13 +251,16 @@ iD.Graph.prototype = { }); }, - revert: function(entity) { - if (this.entities[entity.id] === this.base().entities[entity.id]) + revert: function(id) { + var baseEntity = this.base().entities[id], + headEntity = this.entities[id]; + + if (headEntity === baseEntity) return this; return this.update(function() { - this._updateCalculated(entity, this.base().entities[entity.id]); - delete this.entities[entity.id]; + this._updateCalculated(headEntity, baseEntity); + delete this.entities[id]; }); }, diff --git a/js/id/modes/save.js b/js/id/modes/save.js index 58ef0dfea..ce843cd42 100644 --- a/js/id/modes/save.js +++ b/js/id/modes/save.js @@ -184,15 +184,14 @@ iD.modes.Save = function(context) { .on('save', function() { for (var i = 0; i < conflicts.length; i++) { if (conflicts[i].chosen === 1) { // user chose "keep theirs" - var entity = context.entity(conflicts[i].id); - if (entity.type === 'way') { + var entity = context.hasEntity(conflicts[i].id); + if (entity && entity.type === 'way') { var children = _.uniq(entity.nodes); for (var j = 0; j < children.length; j++) { - var child = context.entity(children[j]); - history.replace(iD.actions.Revert(child)); + history.replace(iD.actions.Revert(children[j])); } } - history.replace(iD.actions.Revert(entity)); + history.replace(iD.actions.Revert(conflicts[i].id)); } } diff --git a/test/spec/actions/revert.js b/test/spec/actions/revert.js index cdfa19da1..e8e64f0db 100644 --- a/test/spec/actions/revert.js +++ b/test/spec/actions/revert.js @@ -1,11 +1,27 @@ describe('iD.actions.Revert', function() { - it('reverts an entity', function() { - var n1 = iD.Node({id: 'n' }), + it('deletes a new entity', function() { + var n1 = iD.Node({id: 'n'}), + graph = iD.Graph().replace(n1); + + graph = iD.actions.Revert('n')(graph); + expect(graph.hasEntity('n')).to.be.undefined; + }); + + it('reverts an updated entity', function() { + var n1 = iD.Node({id: 'n'}), n2 = n1.update({}), graph = iD.Graph([n1]).replace(n2); - expect(graph.entity('n')).to.equal(n2); - graph = iD.actions.Revert(n2)(graph) - expect(graph.entity('n')).to.equal(n1); + graph = iD.actions.Revert('n')(graph); + expect(graph.hasEntity('n')).to.equal(n1); }); + + it('restores a deleted entity', function() { + var n1 = iD.Node({id: 'n'}), + graph = iD.Graph([n1]).remove(n1); + + graph = iD.actions.Revert('n')(graph); + expect(graph.hasEntity('n')).to.equal(n1); + }); + }); diff --git a/test/spec/core/graph.js b/test/spec/core/graph.js index 8ed01880e..46cb46a69 100644 --- a/test/spec/core/graph.js +++ b/test/spec/core/graph.js @@ -357,86 +357,112 @@ describe('iD.Graph', function() { }); describe("#revert", function () { - it("is a no-op if the entity is identical to the base entity", function () { - var n1 = iD.Node({id: 'n' }), + it("is a no-op if the head entity is identical to the base entity", function () { + var n1 = iD.Node({id: 'n'}), graph = iD.Graph([n1]); - expect(graph.revert(n1)).to.equal(graph); + expect(graph.revert('n')).to.equal(graph); }); it("returns a new graph", function () { - var n1 = iD.Node({id: 'n' }), + var n1 = iD.Node({id: 'n'}), n2 = n1.update({}), graph = iD.Graph([n1]).replace(n2); - expect(graph.revert(n2)).not.to.equal(graph); + expect(graph.revert('n')).not.to.equal(graph); }); it("doesn't modify the receiver", function () { - var n1 = iD.Node({id: 'n' }), + var n1 = iD.Node({id: 'n'}), n2 = n1.update({}), graph = iD.Graph([n1]).replace(n2); - graph.revert(n2); - expect(graph.entity(n2.id)).to.equal(n2); - }); - - it("reverts an updated entity to the base version", function () { - var n1 = iD.Node({id: 'n' }), - n2 = n1.update({}), - graph = iD.Graph([n1]).replace(n2); - - expect(graph.entity('n')).to.equal(n2); - graph = graph.revert(n2); - expect(graph.entity('n')).to.equal(n1); + graph.revert('n'); + expect(graph.hasEntity('n')).to.equal(n2); }); it("removes a new entity", function () { - var n1 = iD.Node({id: 'n' }), + var n1 = iD.Node({id: 'n'}), graph = iD.Graph().replace(n1); - expect(graph.entity('n')).to.equal(n1); - graph = graph.revert(n1); + graph = graph.revert('n'); expect(graph.hasEntity('n')).to.be.undefined; }); - it("reverts updated parentWays", function () { - var n1 = iD.Node({id: 'n' }), + it("reverts an updated entity to the base version", function () { + var n1 = iD.Node({id: 'n'}), + n2 = n1.update({}), + graph = iD.Graph([n1]).replace(n2); + + graph = graph.revert('n'); + expect(graph.hasEntity('n')).to.equal(n1); + }); + + it("restores a deleted entity", function () { + var n1 = iD.Node({id: 'n'}), + graph = iD.Graph([n1]).remove(n1); + + graph = graph.revert('n'); + expect(graph.hasEntity('n')).to.equal(n1); + }); + + it("removes new parentWays", function () { + var n1 = iD.Node({id: 'n'}), + w1 = iD.Way({id: 'w', nodes: ['n']}), + graph = iD.Graph().replace(n1).replace(w1); + + graph = graph.revert('w'); + expect(graph.hasEntity('n')).to.equal(n1); + expect(graph.parentWays(n1)).to.eql([]); + }); + + it("removes new parentRelations", function () { + var n1 = iD.Node({id: 'n'}), + r1 = iD.Relation({id: 'r', members: [{id: 'n'}]}), + graph = iD.Graph().replace(n1).replace(r1); + + graph = graph.revert('r'); + expect(graph.hasEntity('n')).to.equal(n1); + expect(graph.parentRelations(n1)).to.eql([]); + }); + + it("reverts updated parentWays", function () { + var n1 = iD.Node({id: 'n'}), w1 = iD.Way({id: 'w', nodes: ['n']}), w2 = w1.removeNode('n'), graph = iD.Graph([n1, w1]).replace(w2); - expect(graph.parentWays(graph.entity('n'))).to.eql([]); - graph = graph.revert(w2); - expect(graph.parentWays(graph.entity('n'))).to.eql([w1]); + graph = graph.revert('w'); + expect(graph.hasEntity('n')).to.equal(n1); + expect(graph.parentWays(n1)).to.eql([w1]); }); - it("reverts updated parentRelations", function () { - var n1 = iD.Node({id: 'n' }), + it("reverts updated parentRelations", function () { + var n1 = iD.Node({id: 'n'}), r1 = iD.Relation({id: 'r', members: [{id: 'n'}]}), r2 = r1.removeMembersWithID('n'), graph = iD.Graph([n1, r1]).replace(r2); - expect(graph.parentRelations(graph.entity('n'))).to.eql([]); - graph = graph.revert(r2); - expect(graph.parentRelations(graph.entity('n'))).to.eql([r1]); + graph = graph.revert('r'); + expect(graph.hasEntity('n')).to.equal(n1); + expect(graph.parentRelations(n1)).to.eql([r1]); }); - it("removes new parentWays", function () { - var n1 = iD.Node({id: 'n' }), + it("restores deleted parentWays", function () { + var n1 = iD.Node({id: 'n'}), w1 = iD.Way({id: 'w', nodes: ['n']}), - graph = iD.Graph().replace(n1).replace(w1); + graph = iD.Graph([n1, w1]).remove(w1); - expect(graph.parentWays(graph.entity('n'))).to.eql([w1]); - graph = graph.revert(w1); - expect(graph.parentWays(graph.entity('n'))).to.eql([]); + graph = graph.revert('w'); + expect(graph.hasEntity('n')).to.equal(n1); + expect(graph.parentWays(n1)).to.eql([w1]); }); - it("removes new parentRelations", function () { - var n1 = iD.Node({id: 'n' }), + it("restores deleted parentRelations", function () { + var n1 = iD.Node({id: 'n'}), r1 = iD.Relation({id: 'r', members: [{id: 'n'}]}), - graph = iD.Graph().replace(n1).replace(r1); + graph = iD.Graph([n1, r1]).remove(r1); - expect(graph.parentRelations(graph.entity('n'))).to.eql([r1]); - graph = graph.revert(r1); - expect(graph.parentRelations(graph.entity('n'))).to.eql([]); + graph = graph.revert('r'); + expect(graph.hasEntity('n')).to.equal(n1); + expect(graph.parentRelations(n1)).to.eql([r1]); }); });