From 2a0ef21548abd18533aa532d86e1a3bb07776b4c Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 18 Jan 2013 13:33:43 -0800 Subject: [PATCH] Fix restriction handling --- js/id/actions/split_way.js | 24 ++----- test/spec/actions/split_way.js | 122 +++++++++++++++++++++++++-------- 2 files changed, 100 insertions(+), 46 deletions(-) diff --git a/js/id/actions/split_way.js b/js/id/actions/split_way.js index 88566c70e..dda0c23a0 100644 --- a/js/id/actions/split_way.js +++ b/js/id/actions/split_way.js @@ -26,25 +26,13 @@ iD.actions.SplitWay = function(nodeId, newWayId) { // Reduce the original way to only contain the first set of nodes graph = graph.replace(way.update({nodes: way.nodes.slice(0, idx + 1)})); - var parentRelations = graph.parentRelations(way); + graph.parentRelations(way).forEach(function(relation) { + if (relation.isRestriction()) { + var via = relation.memberByRole('via'), + member = relation.memberById(way.id); - function isVia(x) { return x.role = 'via'; } - function isSelf(x) { return x.id = way.id; } - - parentRelations.forEach(function(relation) { - if (relation.tags.type === 'restriction') { - var via = _.find(relation.members, isVia); - var ownrole = _.find(relation.members, isSelf).role; - if (via && !_.contains(newWay.nodes, via.id)) { - // the new way doesn't contain the node that's important - // to the turn restriction, so we don't need to worry - // about adding it to the turn restriction. - } else { - graph = graph.replace(iD.actions.AddRelationMember(relation.id, { - role: ownrole, - id: newWay.id, - type: 'way' - })); + if (via && newWay.contains(via.id)) { + graph = iD.actions.UpdateRelationMember(relation.id, member.index, {id: newWay.id})(graph); } } }); diff --git a/test/spec/actions/split_way.js b/test/spec/actions/split_way.js index f7d1e4938..b12708f9c 100644 --- a/test/spec/actions/split_way.js +++ b/test/spec/actions/split_way.js @@ -37,35 +37,101 @@ describe("iD.actions.SplitWay", function () { expect(graph.entity('=').tags).to.equal(tags); }); - it("moves restriction relations to the new way", function () { - // Situation: - // a ---- b ---- c ~~~~ d - // A restriction from ---- to ~~~~ via c. - // - // Split at b. - // - // Expected result: - // a ---- b ==== c ~~~~ d - // A restriction from ==== to ~~~~ via c. - // - var graph = iD.Graph({ - 'a': iD.Node({id: 'a'}), - 'b': iD.Node({id: 'b'}), - 'c': iD.Node({id: 'c'}), - 'd': iD.Node({id: 'd'}), - '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), - '~': iD.Way({id: '~', nodes: ['c', 'd']}), - 'r': iD.Relation({tags: {type: 'restriction'}, members: [ - {id: '=', role: 'from'}, - {id: '~', role: 'to'}, - {id: 'c', role: 'via'}]}) - }); + ['restriction', 'restriction:bus'].forEach(function (type) { + it("updates a restriction's 'from' role", function () { + // Situation: + // a ----> b ----> c ~~~~ d + // A restriction from ---- to ~~~~ via c. + // + // Split at b. + // + // Expected result: + // a ----> b ====> c ~~~~ d + // A restriction from ==== to ~~~~ via c. + // + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), + '~': iD.Way({id: '~', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', tags: {type: type}, members: [ + {id: '-', role: 'from'}, + {id: '~', role: 'to'}, + {id: 'c', role: 'via'}]}) + }); - graph = iD.actions.SplitWay('b', '=')(graph); + graph = iD.actions.SplitWay('b', '=')(graph); - expect(graph.entity('r').members).to.eql([ - {id: '=', role: 'from'}, - {id: '~', role: 'to'}, - {id: 'c', role: 'via'}]); + expect(graph.entity('r').members).to.eql([ + {id: '=', role: 'from'}, + {id: '~', role: 'to'}, + {id: 'c', role: 'via'}]); + }); + + it("updates a restriction's 'to' role", function () { + // Situation: + // a ----> b ----> c ~~~~ d + // A restriction from ~~~~ to ---- via c. + // + // Split at b. + // + // Expected result: + // a ----> b ====> c ~~~~ d + // A restriction from ~~~~ to ==== via c. + // + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c']}), + '~': iD.Way({id: '~', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', tags: {type: type}, members: [ + {id: '~', role: 'from'}, + {id: '-', role: 'to'}, + {id: 'c', role: 'via'}]}) + }); + + graph = iD.actions.SplitWay('b', '=')(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '~', role: 'from'}, + {id: '=', role: 'to'}, + {id: 'c', role: 'via'}]); + }); + + it("leaves unaffected restrictions unchanged", function () { + // Situation: + // a <---- b <---- c ~~~~ d + // A restriction from ---- to ~~~~ via c. + // + // Split at b. + // + // Expected result: + // a <==== b <---- c ~~~~ d + // A restriction from ---- to ~~~~ via c. + // + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + '-': iD.Way({id: '-', nodes: ['c', 'b', 'a']}), + '~': iD.Way({id: '~', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', tags: {type: type}, members: [ + {id: '-', role: 'from'}, + {id: '~', role: 'to'}, + {id: 'c', role: 'via'}]}) + }); + + graph = iD.actions.SplitWay('b', '=')(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '-', role: 'from'}, + {id: '~', role: 'to'}, + {id: 'c', role: 'via'}]); + }); }); });