From b62f106be8df9a1aa4d3bd54a5e939db87c476c8 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 5 Feb 2013 18:02:43 -0800 Subject: [PATCH] Extract Relation#replaceMember --- js/id/actions/connect.js | 6 +----- js/id/actions/join.js | 13 ++++--------- js/id/graph/relation.js | 32 ++++++++++++++++++++++++++++++++ test/spec/actions/connect.js | 2 +- test/spec/actions/join.js | 2 +- test/spec/graph/relation.js | 29 +++++++++++++++++++++++++++++ 6 files changed, 68 insertions(+), 16 deletions(-) diff --git a/js/id/actions/connect.js b/js/id/actions/connect.js index 88bf35932..68b2bf079 100644 --- a/js/id/actions/connect.js +++ b/js/id/actions/connect.js @@ -30,11 +30,7 @@ iD.actions.Connect = function(nodeIds) { }); graph.parentRelations(node).forEach(function (parent) { - var memberA = parent.memberById(survivor.id), - memberB = parent.memberById(node.id); - if (!memberA) { - graph = graph.replace(parent.addMember({id: survivor.id, role: memberB.role, type: 'node'})); - } + graph = graph.replace(parent.replaceMember(node, survivor)); }); survivor = survivor.mergeTags(node.tags); diff --git a/js/id/actions/join.js b/js/id/actions/join.js index 1e3a5c57b..a4fd913d8 100644 --- a/js/id/actions/join.js +++ b/js/id/actions/join.js @@ -10,7 +10,7 @@ iD.actions.Join = function(idA, idB) { var action = function(graph) { var a = graph.entity(idA), b = graph.entity(idB), - nodes, tags; + nodes; if (a.first() === b.first()) { // a <-- b ==> c @@ -37,14 +37,9 @@ iD.actions.Join = function(idA, idB) { nodes = a.nodes.concat(b.nodes.slice().reverse().slice(1)); } - graph.parentRelations(b) - .forEach(function (parent) { - var memberA = parent.memberById(idA), - memberB = parent.memberById(idB); - if (!memberA) { - graph = graph.replace(parent.addMember({id: idA, role: memberB.role, type: 'way'})); - } - }); + graph.parentRelations(b).forEach(function (parent) { + graph = graph.replace(parent.replaceMember(b, a)); + }); graph = graph.replace(a.mergeTags(b.tags).update({nodes: nodes})); graph = iD.actions.DeleteWay(idB)(graph); diff --git a/js/id/graph/relation.js b/js/id/graph/relation.js index f8540db1f..950f56230 100644 --- a/js/id/graph/relation.js +++ b/js/id/graph/relation.js @@ -49,6 +49,16 @@ _.extend(iD.Relation.prototype, { } }, + // Return the first member with the given id and role. A copy of the member object + // is returned, extended with an 'index' property whose value is the member index. + memberByIdAndRole: function(id, role) { + for (var i = 0; i < this.members.length; i++) { + if (this.members[i].id === id && this.members[i].role === role) { + return _.extend({}, this.members[i], {index: i}); + } + } + }, + addMember: function(member, index) { var members = this.members.slice(); members.splice(index === undefined ? members.length : index, 0, member); @@ -66,6 +76,28 @@ _.extend(iD.Relation.prototype, { return this.update({members: members}); }, + // Wherever a member appears with id `needle.id`, replace it with a member + // with id `replacement.id`, type `replacement.type`, and the original role, + // unless a member already exists with that id and role. Return an updated + // relation. + replaceMember: function(needle, replacement) { + if (!this.memberById(needle.id)) + return this; + + var members = []; + + for (var i = 0; i < this.members.length; i++) { + var member = this.members[i]; + if (member.id !== needle.id) { + members.push(member); + } else if (!this.memberByIdAndRole(replacement.id, member.role)) { + members.push({id: replacement.id, type: replacement.type, role: member.role}); + } + } + + return this.update({members: members}); + }, + asJXON: function(changeset_id) { var r = { relation: { diff --git a/test/spec/actions/connect.js b/test/spec/actions/connect.js index 3838d83c2..fc585b43c 100644 --- a/test/spec/actions/connect.js +++ b/test/spec/actions/connect.js @@ -99,7 +99,7 @@ describe("iD.actions.Connect", function() { '-': iD.Way({id: '-', nodes: ['a', 'b']}), '=': iD.Way({id: '=', nodes: ['c', 'd']}), 'r1': iD.Relation({id: 'r1', members: [{id: 'b', role: 'r1', type: 'node'}]}), - 'r2': iD.Relation({id: 'r2', members: [{id: 'b', role: 'r1', type: 'node'}, {id: 'c', role: 'r2', type: 'node'}]}) + 'r2': iD.Relation({id: 'r2', members: [{id: 'b', role: 'r2', type: 'node'}, {id: 'c', role: 'r2', type: 'node'}]}) }); graph = iD.actions.Connect(['b', 'c'])(graph); diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index 261fe607d..88b4d5acc 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -161,7 +161,7 @@ describe("iD.actions.Join", function () { '-': iD.Way({id: '-', nodes: ['a', 'b']}), '=': iD.Way({id: '=', nodes: ['b', 'c']}), 'r1': iD.Relation({id: 'r1', members: [{id: '=', role: 'r1', type: 'way'}]}), - 'r2': iD.Relation({id: 'r2', members: [{id: '=', role: 'r1', type: 'way'}, {id: '-', role: 'r2', type: 'way'}]}) + 'r2': iD.Relation({id: 'r2', members: [{id: '=', role: 'r2', type: 'way'}, {id: '-', role: 'r2', type: 'way'}]}) }); graph = iD.actions.Join('-', '=')(graph); diff --git a/test/spec/graph/relation.js b/test/spec/graph/relation.js index b07f9bee7..39b748e0a 100644 --- a/test/spec/graph/relation.js +++ b/test/spec/graph/relation.js @@ -131,6 +131,35 @@ describe('iD.Relation', function () { }); }); + describe("#replaceMember", function () { + it("returns self if self does not contain needle", function () { + var r = iD.Relation({members: []}); + expect(r.replaceMember({id: 'a'}, {id: 'b'})).to.equal(r); + }); + + it("replaces a member which doesn't already exist", function () { + var r = iD.Relation({members: [{id: 'a', role: 'a'}]}); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members).to.eql([{id: 'b', role: 'a', type: 'node'}]); + }); + + it("preserves the existing role", function () { + var r = iD.Relation({members: [{id: 'a', role: 'a', type: 'node'}]}); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members).to.eql([{id: 'b', role: 'a', type: 'node'}]); + }); + + it("uses the replacement type", function () { + var r = iD.Relation({members: [{id: 'a', role: 'a', type: 'node'}]}); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'way'}).members).to.eql([{id: 'b', role: 'a', type: 'way'}]); + }); + + it("removes members if replacing them would produce duplicates", function () { + var r = iD.Relation({members: [ + {id: 'a', role: 'b', type: 'node'}, + {id: 'b', role: 'b', type: 'node'}]}); + expect(r.replaceMember({id: 'a'}, {id: 'b', type: 'node'}).members).to.eql([{id: 'b', role: 'b', type: 'node'}]); + }); + }); + describe("#asJXON", function () { it('converts a relation to jxon', function() { var relation = iD.Relation({id: 'r-1', members: [{id: 'w1', role: 'forward', type: 'way'}], tags: {type: 'route'}});