Merge pull request #2611 from openstreetmap/revert

Add ability to revert entity back to base version, use revert when resolving conflicts in favor of remote editor, don't save empty changesets.

index.html

@@ -169,6 +169,7 @@
         <script src='js/id/actions/rotate_way.js'></script>
         <script src='js/id/actions/restrict_turn.js'></script>
         <script src='js/id/actions/reverse.js'></script>
+        <script src='js/id/actions/revert.js'></script>
         <script src='js/id/actions/straighten.js'></script>
         <script src='js/id/actions/split.js'></script>
         <script src='js/id/actions/unrestrict_turn.js'></script>

js/id/actions/revert.js

@@ -0,0 +1,5 @@
+iD.actions.Revert = function(entity) {
+    return function(graph) {
+        return graph.revert(entity);
+    };
+};

js/id/core/graph.js

@@ -251,6 +251,16 @@ iD.Graph.prototype = {
         });
     },
 
+    revert: function(entity) {
+        if (this.entities[entity.id] === this.base().entities[entity.id])
+            return this;
+
+        return this.update(function() {
+            this._updateCalculated(entity, this.base().entities[entity.id]);
+            delete this.entities[entity.id];
+        });
+    },
+
     update: function() {
         var graph = this.frozen ? iD.Graph(this, true) : this;
 

js/id/modes/save.js

@@ -134,23 +134,30 @@ iD.modes.Save = function(context) {
             } else if (errors.length) {
                 showErrors();
             } else {
-                context.connection().putChangeset(
-                    history.changes(iD.actions.DiscardTags(history.difference())),
-                    e.comment,
-                    history.imageryUsed(),
-                    function(err, changeset_id) {
-                        if (err) {
-                            errors.push({
-                                msg: err.responseText,
-                                details: [ t('save.status_code', { code: err.status }) ]
-                            });
-                            showErrors();
-                        } else {
-                            loading.close();
-                            context.flush();
-                            success(e, changeset_id);
-                        }
-                    });
+                var changes = history.changes(iD.actions.DiscardTags(history.difference()));
+                if (changes.modified.length || changes.created.length || changes.deleted.length) {
+                    context.connection().putChangeset(
+                        changes,
+                        e.comment,
+                        history.imageryUsed(),
+                        function(err, changeset_id) {
+                            if (err) {
+                                errors.push({
+                                    msg: err.responseText,
+                                    details: [ t('save.status_code', { code: err.status }) ]
+                                });
+                                showErrors();
+                            } else {
+                                loading.close();
+                                context.flush();
+                                success(e, changeset_id);
+                            }
+                        });
+                } else {        // changes were insignificant or reverted by user
+                    loading.close();
+                    context.flush();
+                    cancel();
+                }
             }
         }
 
@@ -175,6 +182,20 @@ iD.modes.Save = function(context) {
                     selection.remove();
                 })
                 .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 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(entity));
+                        }
+                    }
+
                     selection.remove();
                     save(e, true);
                 })

js/id/renderer/map.js

@@ -409,10 +409,15 @@ iD.Map = function(context) {
         }
     };
 
-    map.trimmedExtent = function() {
-        var headerY = 60, footerY = 30, pad = 10;
-        return new iD.geo.Extent(projection.invert([pad, dimensions[1] - footerY - pad]),
-                projection.invert([dimensions[0] - pad, headerY + pad]));
+    map.trimmedExtent = function(_) {
+        if (!arguments.length) {
+            var headerY = 60, footerY = 30, pad = 10;
+            return new iD.geo.Extent(projection.invert([pad, dimensions[1] - footerY - pad]),
+                    projection.invert([dimensions[0] - pad, headerY + pad]));
+        } else {
+            var extent = iD.geo.Extent(_);
+            map.centerZoom(extent.center(), map.trimmedExtentZoom(extent));
+        }
     };
 
     function calcZoom(extent, dim) {

js/id/ui/conflicts.js

@@ -198,17 +198,31 @@ iD.ui.Conflicts = function(context) {
             .selectAll('input')
             .property('checked', function(d) { return d === datum; });
 
+        var extent = iD.geo.Extent(),
+            entity;
+
+        entity = context.graph().hasEntity(datum.id);
+        if (entity) extent._extend(entity.extent(context.graph()));
+
         datum.action();
-        zoomToEntity(datum.id);
+
+        entity = context.graph().hasEntity(datum.id);
+        if (entity) extent._extend(entity.extent(context.graph()));
+
+        zoomToEntity(datum.id, extent);
     }
 
-    function zoomToEntity(id) {
+    function zoomToEntity(id, extent) {
         context.surface().selectAll('.hover')
             .classed('hover', false);
 
         var entity = context.graph().hasEntity(id);
         if (entity) {
-            context.map().zoomTo(entity);
+            if (extent) {
+                context.map().trimmedExtent(extent);
+            } else {
+                context.map().zoomTo(entity);
+            }
             context.surface().selectAll(
                 iD.util.entityOrMemberSelector([entity.id], context.graph()))
                 .classed('hover', true);

test/index.html

@@ -147,6 +147,7 @@
     <script src='../js/id/actions/orthogonalize.js'></script>
     <script src='../js/id/actions/restrict_turn.js'></script>
     <script src='../js/id/actions/reverse.js'></script>
+    <script src='../js/id/actions/revert.js'></script>
     <script src='../js/id/actions/rotate_way.js'></script>
     <script src='../js/id/actions/split.js'></script>
     <script src='../js/id/actions/straighten.js'></script>
@@ -250,6 +251,7 @@
     <script src="spec/actions/noop.js"></script>
     <script src="spec/actions/restrict_turn.js"></script>
     <script src="spec/actions/reverse.js"></script>
+    <script src="spec/actions/revert.js"></script>
     <script src="spec/actions/split.js"></script>
     <script src="spec/actions/unrestrict_turn.js"></script>
 

test/index_packaged.html

@@ -54,6 +54,7 @@
     <script src="spec/actions/orthogonalize.js"></script>
     <script src="spec/actions/restrict_turn.js"></script>
     <script src="spec/actions/reverse.js"></script>
+    <script src="spec/actions/revert.js"></script>
     <script src="spec/actions/split.js"></script>
     <script src="spec/actions/straighten.js"></script>
     <script src="spec/actions/unrestrict_turn.js"></script>

test/spec/actions/revert.js

@@ -0,0 +1,11 @@
+describe('iD.actions.Revert', function() {
+    it('reverts an 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);
+    });
+});

test/spec/core/graph.js

@@ -334,28 +334,112 @@ describe('iD.Graph', function() {
             expect(graph.replace(w1).parentWays(node)).to.eql([w1]);
         });
 
-        it("adds parentRels",  function () {
+        it("adds parentRelations",  function () {
             var node = iD.Node({id: 'n' }),
-            r1 = iD.Relation({id: 'w', members: [{id: 'n'}]}),
-            graph = iD.Graph([node]);
+                r1 = iD.Relation({id: 'r', members: [{id: 'n'}]}),
+                graph = iD.Graph([node]);
             expect(graph.replace(r1).parentRelations(node)).to.eql([r1]);
         });
 
         it("removes parentRelations",  function () {
             var node = iD.Node({id: 'n' }),
-            r1 = iD.Relation({id: 'w', members: [{id: 'n'}]}),
-            graph = iD.Graph([node, r1]);
+                r1 = iD.Relation({id: 'r', members: [{id: 'n'}]}),
+                graph = iD.Graph([node, r1]);
             expect(graph.remove(r1).parentRelations(node)).to.eql([]);
         });
 
         it("doesn't add duplicate parentRelations",  function () {
             var node = iD.Node({id: 'n' }),
-            r1 = iD.Relation({id: 'w', members: [{id: 'n'}]}),
-            graph = iD.Graph([node, r1]);
+                r1 = iD.Relation({id: 'r', members: [{id: 'n'}]}),
+                graph = iD.Graph([node, r1]);
             expect(graph.replace(r1).parentRelations(node)).to.eql([r1]);
         });
     });
 
+    describe("#revert", function () {
+        it("is a no-op if the 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);
+        });
+
+        it("returns a new graph", function () {
+            var n1 = iD.Node({id: 'n' }),
+                n2 = n1.update({}),
+                graph = iD.Graph([n1]).replace(n2);
+            expect(graph.revert(n2)).not.to.equal(graph);
+        });
+
+        it("doesn't modify the receiver", function () {
+            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);
+        });
+
+        it("removes a new entity", function () {
+            var n1 = iD.Node({id: 'n' }),
+                graph = iD.Graph().replace(n1);
+
+            expect(graph.entity('n')).to.equal(n1);
+            graph = graph.revert(n1);
+            expect(graph.hasEntity('n')).to.be.undefined;
+        });
+
+        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]);
+        });
+
+        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]);
+        });
+
+        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);
+
+            expect(graph.parentWays(graph.entity('n'))).to.eql([w1]);
+            graph = graph.revert(w1);
+            expect(graph.parentWays(graph.entity('n'))).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);
+
+            expect(graph.parentRelations(graph.entity('n'))).to.eql([r1]);
+            graph = graph.revert(r1);
+            expect(graph.parentRelations(graph.entity('n'))).to.eql([]);
+        });
+    });
+
     describe("#update", function () {
         it("returns a new graph if self is frozen", function () {
             var graph = iD.Graph();