Test/refine iD.actions.remove

js/id/actions/actions.js

@@ -20,9 +20,19 @@ iD.actions.startWay = function(way) {
 };
 
 // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/DeleteWayAction.as
-iD.actions.remove = function(node) {
+iD.actions.remove = function(entity) {
     return function(graph) {
-        return graph.remove(node, 'removed a feature');
+        graph.parentWays(entity.id)
+            .forEach(function(parent) {
+                graph = iD.actions.removeWayNode(parent, entity)(graph);
+            });
+
+        graph.parentRelations(entity.id)
+            .forEach(function(parent) {
+                graph = iD.actions.removeRelationEntity(parent, entity)(graph);
+            });
+
+        return graph.remove(entity, 'removed a feature');
     };
 };
 
@@ -42,6 +52,13 @@ iD.actions.removeWayNode = function(way, node) {
     };
 };
 
+iD.actions.removeRelationEntity = function(relation, entity) {
+    return function(graph) {
+        var members = _.without(relation.members, entity.id);
+        return graph.replace(relation.update({members: members}), 'removed from a relation');
+    };
+};
+
 // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as
 iD.actions.changeWayDirection = function(way) {
     return function(graph) {

js/id/graph/graph.js

@@ -15,11 +15,17 @@ iD.Graph.prototype = {
         return this.entities[id];
     },
 
-    parents: function(id) {
+    parentWays: function(id) {
         // This is slow and a bad hack.
         return _.filter(this.entities, function(e) {
-            if (e.type !== 'way') return false;
-            return e.nodes.indexOf(id) !== -1;
+            return e.type === 'way' && e.nodes.indexOf(id) !== -1;
+        });
+    },
+
+    parentRelations: function(id) {
+        // This is slow and a bad hack.
+        return _.filter(this.entities, function(e) {
+            return e.type === 'relation' && e.members.indexOf(id) !== -1;
         });
     },
 

js/id/modes/add_road.js

@@ -21,7 +21,7 @@ iD.modes.AddRoad = function() {
                 node = datum;
 
                 var id = datum.id;
-                var parents = mode.history.graph().parents(id);
+                var parents = mode.history.graph().parentWays(id);
                 if (parents.length) {
                     if (parents[0].nodes[0] === id) {
                         way = parents[0];

js/id/modes/select.js

@@ -15,7 +15,7 @@ iD.modes.Select = function (entity) {
 
             if (!dragging) {
                 dragging = iD.util.trueObj([entity.id].concat(
-                    _.pluck(mode.history.graph().parents(entity.id), 'id')));
+                    _.pluck(mode.history.graph().parentWays(entity.id), 'id')));
                 mode.history.perform(iD.actions.noop());
             }
 
@@ -33,12 +33,6 @@ iD.modes.Select = function (entity) {
         });
 
     function remove() {
-        // Remove this node from any ways that is a member of
-        mode.history.graph().parents(entity.id)
-            .filter(function(d) { return d.type === 'way'; })
-            .forEach(function(parent) {
-                mode.history.perform(iD.actions.removeWayNode(parent, entity));
-            });
         mode.history.perform(iD.actions.remove(entity));
         mode.controller.exit();
     }

js/id/renderer/map.js

@@ -30,7 +30,7 @@ iD.Map = function() {
                     }
 
                     dragging = iD.util.trueObj([entity.id].concat(
-                        _.pluck(history.graph().parents(entity.id), 'id')));
+                        _.pluck(history.graph().parentWays(entity.id), 'id')));
                     history.perform(iD.actions.noop());
                 }
 

test/index.html

@@ -73,6 +73,7 @@
     <!-- include spec files here... -->
     <script src="spec/actions/add_way_node.js"></script>
     <script src="spec/actions/remove_way_node.js"></script>
+    <script src="spec/actions/remove.js"></script>
     <script src="spec/format/geojson.js"></script>
     <script src="spec/format/xml.js"></script>
     <script src="spec/graph/graph.js"></script>

test/index_packaged.html

@@ -27,6 +27,7 @@
     <!-- include spec files here... -->
     <script src="spec/actions/add_way_node.js"></script>
     <script src="spec/actions/remove_way_node.js"></script>
+    <script src="spec/actions/remove.js"></script>
     <script src="spec/format/geojson.js"></script>
     <script src="spec/format/xml.js"></script>
     <script src="spec/graph/graph.js"></script>

test/spec/actions/remove.js

@@ -0,0 +1,24 @@
+describe("iD.actions.remove", function () {
+    it("removes the entity from the graph", function () {
+        var entity = iD.Way(),
+            action = iD.actions.remove(entity),
+            graph  = action(iD.Graph().replace(entity));
+        expect(graph.entity(entity.id)).to.be.undefined;
+    });
+
+    it("removes a node from parent ways", function () {
+        var node   = iD.Node(),
+            way    = iD.Way().update({nodes: [node.id]}),
+            action = iD.actions.remove(node),
+            graph  = action(iD.Graph().replace(node).replace(way));
+        expect(graph.entity(way.id).nodes).not.to.contain(node.id);
+    });
+
+    it("removes a way from parent relations", function () {
+        var way      = iD.Way(),
+            relation = iD.Relation().update({members: [way.id]}),
+            action   = iD.actions.remove(way),
+            graph    = action(iD.Graph().replace(way).replace(relation));
+        expect(graph.entity(relation.id).members).not.to.contain(way.id);
+    });
+});

test/spec/graph/graph.js

@@ -50,6 +50,26 @@ describe('Graph', function() {
         });
     });
 
+    describe("#parentWays", function() {
+        it("returns an array of ways that contain the given node id", function () {
+            var node  = iD.Node({id: "n1"}),
+                way   = iD.Way({id: "w1", nodes: ["n1"]}),
+                graph = iD.Graph({n1: node, w1: way});
+            expect(graph.parentWays("n1")).to.eql([way]);
+            expect(graph.parentWays("n2")).to.eql([]);
+        });
+    });
+
+    describe("#parentRelations", function() {
+        it("returns an array of relations that contain the given entity id", function () {
+            var node     = iD.Node({id: "n1"}),
+                relation = iD.Relation({id: "r1", members: ["n1"]}),
+                graph    = iD.Graph({n1: node, r1: relation});
+            expect(graph.parentRelations("n1")).to.eql([relation]);
+            expect(graph.parentRelations("n2")).to.eql([]);
+        });
+    });
+
     describe("#fetch", function () {
         it("replaces node ids with references", function () {
             var node  = iD.Node({id: "n1"}),