Skip to content

Commit

Permalink
Prevent merging over restriction relations (#1512)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfirebaugh committed Jun 18, 2013
1 parent 6d506ec commit 21fa8c9
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 1 deletion.
1 change: 1 addition & 0 deletions data/core.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ en:
annotation: "Merged {n} lines."
not_eligible: These features can't be merged.
not_adjacent: These lines can't be merged because they aren't connected.
restriction: These lines can't be merged because at least one is a member of a "{relation}" relation.
move:
title: Move
description: Move this to a different location.
Expand Down
3 changes: 2 additions & 1 deletion dist/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@
"key": "C",
"annotation": "Merged {n} lines.",
"not_eligible": "These features can't be merged.",
"not_adjacent": "These lines can't be merged because they aren't connected."
"not_adjacent": "These lines can't be merged because they aren't connected.",
"restriction": "These lines can't be merged because at least one is a member of a \"{relation}\" relation."
},
"move": {
"title": "Move",
Expand Down
14 changes: 14 additions & 0 deletions js/id/actions/join.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ iD.actions.Join = function(ids) {
var joined = iD.geo.joinWays(ids.map(graph.entity, graph), graph);
if (joined.length > 1)
return 'not_adjacent';

var nodeIds = _.pluck(joined[0].nodes, 'id').slice(1, -1),
relation;

joined[0].forEach(function(way) {
var parents = graph.parentRelations(way);
parents.forEach(function(parent) {
if (parent.isRestriction() && parent.members.some(function(m) { return nodeIds.indexOf(m.id) >= 0; }))
relation = parent;
});
});

if (relation)
return 'restriction';
};

return action;
Expand Down
3 changes: 3 additions & 0 deletions js/id/operations/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ iD.operations.Merge = function(selectedIDs, context) {
m = merge.disabled(context.graph()),
p = mergePolygon.disabled(context.graph());

if (j === 'restriction' && m && p)
return t('operations.merge.restriction', {relation: context.presets().item('type/restriction').name()});

if (j && m && p)
return t('operations.merge.' + j);

Expand Down
99 changes: 99 additions & 0 deletions test/spec/actions/join.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,105 @@ describe("iD.actions.Join", function () {

expect(iD.actions.Join(['-', '=']).disabled(graph)).to.equal('not_adjacent');
});

it("returns 'restriction' in situations where a turn restriction would be damaged (a)", function () {
// a --> b ==> c
// from: -
// to: =
// via: b
var graph = iD.Graph({
'a': iD.Node({id: 'a'}),
'b': iD.Node({id: 'b'}),
'c': iD.Node({id: 'c'}),
'-': iD.Way({id: '-', nodes: ['a', 'b']}),
'=': iD.Way({id: '=', nodes: ['b', 'c']}),
'r': iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [
{type: 'way', id: '-', role: 'from'},
{type: 'way', id: '=', role: 'to'},
{type: 'node', id: 'b', role: 'via'}
]})
});

expect(iD.actions.Join(['-', '=']).disabled(graph)).to.equal('restriction');
});

it("returns 'restriction' in situations where a turn restriction would be damaged (b)", function () {
// a --> b ==> c
// |
// d
// from: -
// to: |
// via: b
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']}),
'=': iD.Way({id: '=', nodes: ['b', 'c']}),
'|': iD.Way({id: '|', nodes: ['b', 'd']}),
'r': iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [
{type: 'way', id: '-', role: 'from'},
{type: 'way', id: '|', role: 'to'},
{type: 'node', id: 'b', role: 'via'}
]})
});

expect(iD.actions.Join(['-', '=']).disabled(graph)).to.equal('restriction');
});

it("returns falsy in situations where a turn restriction wouldn't be damaged (a)", function () {
// a --> b ==> c
// |
// d
// from: -
// to: |
// via: a
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']}),
'=': iD.Way({id: '=', nodes: ['b', 'c']}),
'|': iD.Way({id: '|', nodes: ['a', 'd']}),
'r': iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [
{type: 'way', id: '-', role: 'from'},
{type: 'way', id: '|', role: 'to'},
{type: 'node', id: 'a', role: 'via'}
]})
});

expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok;
});

it("returns falsy in situations where a turn restriction wouldn't be damaged (b)", function () {
// d
// |
// a --> b ==> c
// \
// e
// from: |
// to: \
// via: b
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']}),
'=': iD.Way({id: '=', nodes: ['b', 'c']}),
'|': iD.Way({id: '|', nodes: ['d', 'b']}),
'\\': iD.Way({id: '\\', nodes: ['b', 'e']}),
'r': iD.Relation({id: 'r', tags: {type: 'restriction'}, members: [
{type: 'way', id: '|', role: 'from'},
{type: 'way', id: '\\', role: 'to'},
{type: 'node', id: 'b', role: 'via'}
]})
});

expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok;
});
});

it("joins a --> b ==> c", function () {
Expand Down

0 comments on commit 21fa8c9

Please sign in to comment.