From 163c85bacbd254ee62153f6c7f6fbb09bf711213 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 27 Aug 2013 12:23:25 -0700 Subject: [PATCH] Delete relations that become empty Fixes #465 Fixes #1454 --- js/id/actions/delete_node.js | 7 ++++++- js/id/actions/delete_relation.js | 7 ++++++- js/id/actions/delete_way.js | 7 ++++++- js/id/core/relation.js | 4 ++++ test/spec/actions/delete_node.js | 8 ++++++++ test/spec/actions/delete_relation.js | 8 ++++++++ test/spec/actions/delete_way.js | 10 +++++++++- test/spec/core/relation.js | 10 ++++++++++ 8 files changed, 57 insertions(+), 4 deletions(-) diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js index 3262f9b64..bd131de68 100644 --- a/js/id/actions/delete_node.js +++ b/js/id/actions/delete_node.js @@ -15,7 +15,12 @@ iD.actions.DeleteNode = function(nodeId) { graph.parentRelations(node) .forEach(function(parent) { - graph = graph.replace(parent.removeMembersWithID(nodeId)); + parent = parent.removeMembersWithID(nodeId); + graph = graph.replace(parent); + + if (parent.isDegenerate()) { + graph = iD.actions.DeleteRelation(parent.id)(graph); + } }); return graph.remove(node); diff --git a/js/id/actions/delete_relation.js b/js/id/actions/delete_relation.js index 08834c67c..69222f39f 100644 --- a/js/id/actions/delete_relation.js +++ b/js/id/actions/delete_relation.js @@ -11,7 +11,12 @@ iD.actions.DeleteRelation = function(relationId) { graph.parentRelations(relation) .forEach(function(parent) { - graph = graph.replace(parent.removeMembersWithID(relationId)); + parent = parent.removeMembersWithID(relationId); + graph = graph.replace(parent); + + if (parent.isDegenerate()) { + graph = iD.actions.DeleteRelation(parent.id)(graph); + } }); _.uniq(_.pluck(relation.members, 'id')).forEach(function(memberId) { diff --git a/js/id/actions/delete_way.js b/js/id/actions/delete_way.js index ee3254df7..c87b31011 100644 --- a/js/id/actions/delete_way.js +++ b/js/id/actions/delete_way.js @@ -11,7 +11,12 @@ iD.actions.DeleteWay = function(wayId) { graph.parentRelations(way) .forEach(function(parent) { - graph = graph.replace(parent.removeMembersWithID(wayId)); + parent = parent.removeMembersWithID(wayId); + graph = graph.replace(parent); + + if (parent.isDegenerate()) { + graph = iD.actions.DeleteRelation(parent.id)(graph); + } }); _.uniq(way.nodes).forEach(function(nodeId) { diff --git a/js/id/core/relation.js b/js/id/core/relation.js index 049c4ddd9..9fabc3725 100644 --- a/js/id/core/relation.js +++ b/js/id/core/relation.js @@ -31,6 +31,10 @@ _.extend(iD.Relation.prototype, { }); }, + isDegenerate: function() { + return this.members.length === 0; + }, + // Return an array of members, each extended with an 'index' property whose value // is the member index. indexedMembers: function() { diff --git a/test/spec/actions/delete_node.js b/test/spec/actions/delete_node.js index 67cd1f812..2ec541ddc 100644 --- a/test/spec/actions/delete_node.js +++ b/test/spec/actions/delete_node.js @@ -42,4 +42,12 @@ describe("iD.actions.DeleteNode", function () { graph = action(iD.Graph([node1, node2, way])); expect(graph.hasEntity(way.id)).to.be.undefined; }); + + it("deletes parent relations that become empty", function () { + var node1 = iD.Node(), + relation = iD.Relation({members: [{ id: node1.id }]}), + action = iD.actions.DeleteNode(node1.id), + graph = action(iD.Graph([node1, relation])); + expect(graph.hasEntity(relation.id)).to.be.undefined; + }); }); diff --git a/test/spec/actions/delete_relation.js b/test/spec/actions/delete_relation.js index b3637e4c9..9fa49974f 100644 --- a/test/spec/actions/delete_relation.js +++ b/test/spec/actions/delete_relation.js @@ -74,6 +74,14 @@ describe("iD.actions.DeleteRelation", function () { expect(graph.hasEntity(node.id)).to.be.undefined; }); + it("deletes parent relations that become empty", function () { + var child = iD.Relation(), + parent = iD.Relation({members: [{ id: child.id }]}), + action = iD.actions.DeleteRelation(child.id), + graph = action(iD.Graph([child, parent])); + expect(graph.hasEntity(parent.id)).to.be.undefined; + }); + describe("#disabled", function() { it("returns 'incomplete_relation' if the relation is incomplete", function() { var relation = iD.Relation({members: [{id: 'w'}]}), diff --git a/test/spec/actions/delete_way.js b/test/spec/actions/delete_way.js index 2f5007c7b..ce678ef4f 100644 --- a/test/spec/actions/delete_way.js +++ b/test/spec/actions/delete_way.js @@ -8,7 +8,7 @@ describe("iD.actions.DeleteWay", function() { it("removes a way from parent relations", function() { var way = iD.Way(), - relation = iD.Relation({members: [{ id: way.id }]}), + relation = iD.Relation({members: [{ id: way.id }, { id: 'w-2' }]}), action = iD.actions.DeleteWay(way.id), graph = iD.Graph([way, relation]).update(action); expect(_.pluck(graph.entity(relation.id).members, 'id')).not.to.contain(way.id); @@ -60,4 +60,12 @@ describe("iD.actions.DeleteWay", function() { graph = iD.Graph([node, way]).update(action); expect(graph.hasEntity(node.id)).not.to.be.undefined; }); + + it("deletes parent relations that become empty", function () { + var way = iD.Way(), + relation = iD.Relation({members: [{ id: way.id }]}), + action = iD.actions.DeleteWay(way.id), + graph = iD.Graph([way, relation]).update(action); + expect(graph.hasEntity(relation.id)).to.be.undefined; + }); }); diff --git a/test/spec/core/relation.js b/test/spec/core/relation.js index 7abe4f781..e003d2565 100644 --- a/test/spec/core/relation.js +++ b/test/spec/core/relation.js @@ -56,6 +56,16 @@ describe('iD.Relation', function () { }); }); + describe("#isDegenerate", function () { + it("returns true for a relation without members", function () { + expect(iD.Relation().isDegenerate()).to.equal(true); + }); + + it("returns false for a relation with members", function () { + expect(iD.Relation({members: [{id: 'a', role: 'inner'}]}).isDegenerate()).to.equal(false); + }); + }); + describe("#memberByRole", function () { it("returns the first member with the given role", function () { var r = iD.Relation({members: [