CalvinBackup/iD
1
0
Fork 0
mirror of https://github.com/FoggedLens/iD.git synced 2026-06-05 14:38:05 +02:00
Code Issues Packages Projects Releases Wiki Activity

Update to prevent detachment of node when either a via or

Browse Source 
location_hint role in a turn restriction.
Update to move any other relation to new node.
This commit is contained in:
Jon D
2018-07-22 19:35:29 +01:00
parent dd3de593ca
commit 90bc0b8537
7 changed files with 385 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
data/core.yaml
+2 -1
View File
@@ -233,7 +233,8 @@ en:
title: Detach
key: T
description: Detach this node from these lines/areas.
annotation: Detached a node from owning lines/areas.
annotation: Detached a node from owning lines/areas.
via_restriction: "This can't be detached because it would damage a turn restriction."
restriction:
controls:
distance: Distance
dist/locales/en.json
Vendored
+2 -1
View File
@@ -302,7 +302,8 @@
"title": "Detach",
"key": "T",
"description": "Detach this node from these lines/areas.",
"annotation": "Detached a node from owning lines/areas."
"annotation": "Detached a node from owning lines/areas.",
"via_restriction": "This can't be detached because it would damage a turn restriction."
}
},
"restriction": {
modules/actions/detach_node.js
+16 -4
View File
@@ -9,19 +9,31 @@ export function actionDetachNode(nodeId) {
// Create a new node to replace the one we will detach
var replacementNode = osmNode({ loc: node.loc });
// We need to process each way in turn, updating the graph as we go
return parentWays
var updatedWaysGraph = parentWays
.reduce(function (accGraph, parentWay) {
// Make a note of where in the way our target node is inside this way
var originalIndex = parentWay.nodes.indexOf(nodeId);
// Swap out the target node for the replacement
var updatedWay = parentWay
.removeNode(nodeId) // Remove our target node from the parent way
var updatedWay = parentWay.removeNode(nodeId) // Remove our target node from the parent way
.addNode(replacementNode.id, originalIndex); // Add in the replacement node in its place
// Update the graph with the updated way
// Update the graph with the updated way and pass into the next cycle of the reduce operation
return accGraph.replace(updatedWay);
},
// Seed the reduction with the input graph, updated to include the replacementNode so
// that is accessible to the ways when we add it in to them
graph.replace(replacementNode));
// Process any relations too
var parentRels = updatedWaysGraph.parentRelations(node);
return parentRels
.reduce(function (accGraph, parentRel) {
// Move the relationship to the new node
var originalMember = parentRel.memberById(nodeId);
var newMember = { id: replacementNode.id, type: 'node', role: originalMember.role };
// Remove & replace with the new member
var updatedRel = parentRel.removeMembersWithID(nodeId)
.addMember(newMember, originalMember.index);
// Update graph and pass into the next cycle of the reduce operation
return accGraph.replace(updatedRel);
}, updatedWaysGraph);
};
}
modules/operations/detach_node.js
+55 -1
View File
@@ -2,6 +2,8 @@ import { actionDetachNode } from '../actions/index';
import { behaviorOperation } from '../behavior/index';
import { modeMove } from '../modes/index';
import { t } from '../util/locale';
import _flatMap from 'lodash-es/flatMap';
import _uniq from 'lodash-es/uniq';
export function operationDetachNode(selectedIDs, context) {
var selectedNode = selectedIDs[0];
@@ -37,7 +39,10 @@ export function operationDetachNode(selectedIDs, context) {
return false;
};
operation.tooltip = function () {
return t('operations.detachNode.description');
var disableReason = operation.disabled();
return disableReason
? t('operations.detachNode.' + disableReason)
: t('operations.detachNode.description');
};
operation.annotation = function () {
return t('operations.detachNode.annotation');
@@ -46,5 +51,54 @@ export function operationDetachNode(selectedIDs, context) {
operation.keys = [t('operations.detachNode.key')];
operation.title = t('operations.detachNode.title');
operation.behavior = behaviorOperation(context).which(operation);
operation.disabled = function () {
// We should prevent the node being detached if it represents a via/location_hint node of a turn restriction
var graph = context.graph();
// Get nodes for the Ids (although there should only be one, we can handle multiple here)
var nodes = selectedIDs.map(function (i) { return graph.hasEntity(i); })
.filter(isNotNullOrUndefined);
// Get all via nodes of restrictions involving the target nodes
var restrictionNodeIds = _flatMap(nodes, function (node) {
// Get the relations that this node belongs to
var relationsFromNode = graph.parentRelations(node);
// Check each relation in turn
return _flatMap(relationsFromNode, function (relation) {
// Check to see if this is a restriction relation, if not return null
if (!relation.isValidRestriction()) {
return null;
}
// We have identified that it is a restriction.
// https://wiki.openstreetmap.org/wiki/Relation:restriction indicates that
// from & to roles are only appropriate for Ways
// The via members can be either nodes or ways. Via-Ways do not prevent us removing a node
// from within them, as it is the way itself which is in the relation with the via role,
// and not the consitutent nodes (so if we switch out a constituent node, the way id
// does not change and therefore the relation will not be affected). Therefore we
// only need to examine the standalone nodes
return relation.members.filter(function (m) {
return (m.role === 'via' || m.role === 'location_hint') && m.type === 'node';
}).map(function (m) { return m.id; });
});
}).filter(isNotNullOrUndefined);
// Get unique list of ids in restrictionNodeIds to simplify checking
var nodeIds = _uniq(restrictionNodeIds);
// Now we have a list of via/location_hint nodes, we should prevent detachment if the target node is in this list
var anyInhibits = nodes.filter(function (n) {
return nodeIds.indexOf(n.id) !== -1;
});
if (anyInhibits.length > 0) {
// The node is a via/location_hint, do not permit
return 'via_restriction';
}
// We are ok to proceed
return false;
};
return operation;
}
function isNotNullOrUndefined(i) {
return i !== undefined && i !== null;
}
modules/util/index.js
+2 -2
View File
@@ -12,7 +12,7 @@ export { utilFunctor } from './util';
export { utilGetAllNodes } from './util';
export { utilGetPrototypeOf } from './util';
export { utilGetSetValue } from './get_set_value';
export { utilIdleWorker} from './idle_worker';
export { utilIdleWorker } from './idle_worker';
export { utilNoAuto } from './util';
export { utilPrefixCSSProperty } from './util';
export { utilPrefixDOMProperty } from './util';
@@ -24,4 +24,4 @@ export { utilStringQs } from './util';
export { utilSuggestNames } from './suggest_names';
export { utilTagText } from './util';
export { utilTriggerEvent } from './trigger_event';
export { utilWrap } from './util';
export { utilWrap } from './util';
test/spec/actions/detach_node.js
+76 -6
View File
@@ -79,7 +79,7 @@ describe('iD.actionDetachNode', function () {
// confirm that a still exists
var targetNode = assertionGraph.entity('a');
expect(targetNode).not.to.eql(undefined);
// .., and that the location is correct
// ... and that the location is correct
expect(targetNode.loc).to.eql([0, 0]);
// ... and that the tags are intact
expect(targetNode.tags).to.eql(tags);
@@ -149,7 +149,7 @@ describe('iD.actionDetachNode', function () {
// confirm that a still exists
var targetNode = assertionGraph.entity('b');
expect(targetNode).not.to.eql(undefined);
// .., and that the location is correct
// ... and that the location is correct
expect(targetNode.loc).to.eql([1, 0]);
// ... and that the tags are intact
expect(targetNode.tags).to.eql(tags);
@@ -240,7 +240,7 @@ describe('iD.actionDetachNode', function () {
// confirm that a still exists
var targetNode = assertionGraph.entity('a');
expect(targetNode).not.to.eql(undefined);
// .., and that the location is correct
// ... and that the location is correct
expect(targetNode.loc).to.eql([0, 0]);
// ... and that the tags are intact
expect(targetNode.tags).to.eql(tags);
@@ -312,7 +312,7 @@ describe('iD.actionDetachNode', function () {
// confirm that a still exists
var targetNode = assertionGraph.entity('b');
expect(targetNode).not.to.eql(undefined);
// .., and that the location is correct
// ... and that the location is correct
expect(targetNode.loc).to.eql([1, 0]);
// ... and that the tags are intact
expect(targetNode.tags).to.eql(tags);
@@ -416,7 +416,7 @@ describe('iD.actionDetachNode', function () {
// confirm that a still exists
var targetNode = assertionGraph.entity('c');
expect(targetNode).not.to.eql(undefined);
// .., and that the location is correct
// ... and that the location is correct
expect(targetNode.loc).to.eql([2, 0]);
// ... and that the tags are intact
expect(targetNode.tags).to.eql(tags);
@@ -529,7 +529,7 @@ describe('iD.actionDetachNode', function () {
// confirm that a still exists
var targetNode = assertionGraph.entity('c');
expect(targetNode).not.to.eql(undefined);
// .., and that the location is correct
// ... and that the location is correct
expect(targetNode.loc).to.eql([1, 1]);
// ... and that the tags are intact
expect(targetNode.tags).to.eql(tags);
@@ -537,4 +537,74 @@ describe('iD.actionDetachNode', function () {
expect(assertionGraph.parentWays(targetNode)).to.eql([]);
});
});
describe('with relation', function () {
var graph;
beforeEach(function () {
// Set up a simple way
// a-b-c (0,0)-(1,0)-(2,0)
// Node b represents the target
// With a relationship for the way including b
graph = iD.Graph([
iD.Node({ id: 'a', loc: [0, 0] }),
iD.Node({ id: 'b', loc: [1, 0], tags: tags }),
iD.Node({ id: 'c', loc: [2, 0] }),
iD.Way({ id: 'w', nodes: ['a', 'b', 'c'] }),
iD.Relation({
id: 'r',
tags: {
type: 'route',
route: 'foot'
},
members: [
{ id: 'a', type: 'node', role: 'point' },
{ id: 'b', type: 'node', role: 'point' },
{ id: 'c', type: 'node', role: 'point' }
]
})
]);
});
it('detached node not a member of relation', function () {
var assertionGraph = iD.actionDetachNode('b')(graph);
var targetNode = assertionGraph.entity('b');
// Confirm is not a member of the relation
expect(assertionGraph.parentRelations(targetNode).length).to.eql(0);
});
it('new node is a member of relation', function () {
var assertionGraph = iD.actionDetachNode('b')(graph);
// Find the new node
var targetWay = assertionGraph.entity('w');
var newNodeId = targetWay.nodes.filter(function (m) {
return m !== 'a' && m !== 'b' && m !== 'c';
})[0];
var newNode = assertionGraph.entity(newNodeId);
// Confirm is a member of the relation
expect(assertionGraph.parentRelations(newNode).length).to.eql(1);
expect(assertionGraph.parentRelations(newNode)[0].id).to.eql('r');
});
it('Relation membership has the same properties', function () {
var assertionGraph = iD.actionDetachNode('b')(graph);
// Find the new node
var targetWay = assertionGraph.entity('w');
var newNodeId = targetWay.nodes.filter(function (m) {
return m !== 'a' && m !== 'b' && m !== 'c';
})[0];
// Get the relation
var targetRelation = assertionGraph.entity('r');
// Find the member
var targetMember = targetRelation.memberById(newNodeId);
// Confirm membership is the same as original (except for the new id)
expect(targetMember).to.eql({ id: newNodeId, index: 1, type: 'node', role: 'point' });
});
});
});
test/spec/operations/detach_node.js
+232 -71
View File
@@ -1,85 +1,246 @@
describe('iD.operationDetachNode', function () {
var fakeContext;
var graph;
// Some common setup functions
// Set up the fake context
fakeContext = {};
fakeContext.graph = function () {
return graph;
};
var fakeTags = { 'name': 'fake' };
beforeEach(function () {
// Set up graph
var createFakeNode = function (id, hasTags) {
return hasTags
? { id: id, type: 'node', tags: fakeTags }
: { id: id, type: 'node' };
};
// a - node with tags & parent way
// b - node with tags & 2 parent ways
// c - node with no tags, parent way
// d - node with no tags, 2 parent ways
// e - node with tags, no parent way
// f - node with no tags, no parent way
graph = iD.Graph([
iD.Node(createFakeNode('a', true)),
iD.Node(createFakeNode('b', true)),
iD.Node(createFakeNode('c', false)),
iD.Node(createFakeNode('d', false)),
iD.Node(createFakeNode('e', true)),
iD.Node(createFakeNode('f', false)),
iD.Way({ id: 'x', nodes: ['a', 'b', 'c', 'd'] }),
iD.Way({ id: 'y', nodes: ['b', 'd'] })
]);
// Set up graph
var createFakeNode = function (id, hasTags) {
return hasTags
? { id: id, type: 'node', tags: fakeTags }
: { id: id, type: 'node' };
};
// Set up the fake context
fakeContext = {};
fakeContext.graph = function () {
return graph;
};
describe('available', function () {
beforeEach(function () {
// a - node with tags & parent way
// b - node with tags & 2 parent ways
// c - node with no tags, parent way
// d - node with no tags, 2 parent ways
// e - node with tags, no parent way
// f - node with no tags, no parent way
graph = iD.Graph([
iD.Node(createFakeNode('a', true)),
iD.Node(createFakeNode('b', true)),
iD.Node(createFakeNode('c', false)),
iD.Node(createFakeNode('d', false)),
iD.Node(createFakeNode('e', true)),
iD.Node(createFakeNode('f', false)),
iD.Way({ id: 'x', nodes: ['a', 'b', 'c', 'd'] }),
iD.Way({ id: 'y', nodes: ['b', 'd'] })
]);
});
it('is not available for no selected ids', function () {
var result = iD.operationDetachNode([], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for two selected ids', function () {
var result = iD.operationDetachNode(['a', 'b'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for unkown selected id', function () {
var result = iD.operationDetachNode(['z'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected way', function () {
var result = iD.operationDetachNode(['x'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected node with tags, no parent way', function () {
var result = iD.operationDetachNode(['e'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected node with no tags, no parent way', function () {
var result = iD.operationDetachNode(['f'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected node with no tags, parent way', function () {
var result = iD.operationDetachNode(['c'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected node with no tags, two parent ways', function () {
var result = iD.operationDetachNode(['d'], fakeContext).available();
expect(result).to.eql(false);
});
it('is available for selected node with tags, parent way', function () {
var result = iD.operationDetachNode(['a'], fakeContext).available();
expect(result).to.eql(true);
});
it('is available for selected node with tags, two parent ways', function () {
var result = iD.operationDetachNode(['b'], fakeContext).available();
expect(result).to.eql(true);
});
});
it('is not available for no selected ids', function () {
var result = iD.operationDetachNode([], fakeContext).available();
expect(result).to.eql(false);
});
describe('disabled', function () {
it('returns enabled for non-related node', function () {
graph = iD.Graph([
iD.Node(createFakeNode('a', false)),
iD.Node(createFakeNode('b', true)),
iD.Node(createFakeNode('c', false)),
iD.Way({ id: 'x', nodes: ['a', 'b', 'c'] })
]);
var result = iD.operationDetachNode(['b'], fakeContext).disabled();
expect(result).to.eql(false);
});
it('is not available for two selected ids', function () {
var result = iD.operationDetachNode(['a', 'b'], fakeContext).available();
expect(result).to.eql(false);
});
it('returns enabled for non-restriction related node', function () {
graph = iD.Graph([
iD.Node(createFakeNode('a', false)),
iD.Node(createFakeNode('b', true)),
iD.Node(createFakeNode('c', false)),
iD.Way({ id: 'x', nodes: ['a', 'b', 'c'] }),
iD.Relation({ id: 'r', members: [{ id: 'b', role: 'label' }] })
]);
var result = iD.operationDetachNode(['b'], fakeContext).disabled();
expect(result).to.eql(false);
});
it('is not available for unkown selected id', function () {
var result = iD.operationDetachNode(['z'], fakeContext).available();
expect(result).to.eql(false);
});
it('returns not-enabled for via node in restriction', function () {
// https://wiki.openstreetmap.org/wiki/Relation:restriction indicates that
// from & to roles are only appropriate for Ways
graph = iD.Graph([
iD.Node(createFakeNode('a', false)),
iD.Node(createFakeNode('b', false)),
iD.Node(createFakeNode('c', false)),
iD.Node(createFakeNode('d', true)),
iD.Node(createFakeNode('e', false)),
iD.Node(createFakeNode('f', false)),
iD.Node(createFakeNode('g', false)),
iD.Way({ id: 'x', nodes: ['a', 'b', 'c'] }),
iD.Way({ id: 'y', nodes: ['e', 'f', 'g'] }),
iD.Relation({
id: 'r',
tags: {
type: 'restriction',
restriction: 'no_right_turn'
},
members: [
{ id: 'x', type: 'way', role: 'from' },
{ id: 'd', type: 'node', role: 'via' },
{ id: 'z', type: 'way', role: 'to' }
]
})
]);
var result = iD.operationDetachNode(['d'], fakeContext).disabled();
expect(result).not.to.eql(false);
});
it('is not available for selected way', function () {
var result = iD.operationDetachNode(['x'], fakeContext).available();
expect(result).to.eql(false);
});
it('returns not-enabled for via node in restriction and other non-restriction relation', function () {
graph = iD.Graph([
iD.Node(createFakeNode('a', false)),
iD.Node(createFakeNode('b', false)),
iD.Node(createFakeNode('c', false)),
iD.Node(createFakeNode('d', true)),
iD.Node(createFakeNode('e', false)),
iD.Node(createFakeNode('f', false)),
iD.Node(createFakeNode('g', false)),
iD.Way({ id: 'x', nodes: ['a', 'b', 'c'] }),
iD.Way({ id: 'y', nodes: ['e', 'f', 'g'] }),
iD.Relation({
id: 'r',
tags: {
type: 'restriction',
restriction: 'no_right_turn'
},
members: [
{ id: 'x', type: 'way', role: 'from' },
{ id: 'd', type: 'node', role: 'via' },
{ id: 'z', type: 'way', role: 'to' }
]
}),
iD.Relation({
id: 's',
members: [
{ id: 'x', type: 'way' },
{ id: 'd', type: 'node' },
]
})
]);
var result = iD.operationDetachNode(['d'], fakeContext).disabled();
expect(result).not.to.eql(false);
});
it('is not available for selected node with tags, no parent way', function () {
var result = iD.operationDetachNode(['e'], fakeContext).available();
expect(result).to.eql(false);
});
it('returns not-enabled for location_hint node in restriction', function () {
// https://wiki.openstreetmap.org/wiki/Relation:restriction indicates that
// from & to roles are only appropriate for Ways
graph = iD.Graph([
iD.Node(createFakeNode('a', false)),
iD.Node(createFakeNode('b', false)),
iD.Node(createFakeNode('c', false)),
iD.Node(createFakeNode('d', true)),
iD.Node(createFakeNode('e', false)),
iD.Node(createFakeNode('f', false)),
iD.Node(createFakeNode('g', false)),
iD.Way({ id: 'x', nodes: ['a', 'b'] }),
iD.Way({ id: 'y', nodes: ['e', 'f', 'g'] }),
iD.Relation({
id: 'r',
tags: {
type: 'restriction',
restriction: 'no_right_turn'
},
members: [
{ id: 'x', type: 'way', role: 'from' },
{ id: 'c', type: 'node', role: 'via' },
{ id: 'd', type: 'node', role: 'location_hint' },
{ id: 'z', type: 'way', role: 'to' }
]
})
]);
var result = iD.operationDetachNode(['d'], fakeContext).disabled();
expect(result).not.to.eql(false);
});
it('is not available for selected node with no tags, no parent way', function () {
var result = iD.operationDetachNode(['f'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected node with no tags, parent way', function () {
var result = iD.operationDetachNode(['c'], fakeContext).available();
expect(result).to.eql(false);
});
it('is not available for selected node with no tags, two parent ways', function () {
var result = iD.operationDetachNode(['d'], fakeContext).available();
expect(result).to.eql(false);
});
it('is available for selected node with tags, parent way', function () {
var result = iD.operationDetachNode(['a'], fakeContext).available();
expect(result).to.eql(true);
});
it('is available for selected node with tags, two parent ways', function () {
var result = iD.operationDetachNode(['b'], fakeContext).available();
expect(result).to.eql(true);
it('returns not-enabled for location_hint node in restriction and other non-restriction relation', function () {
graph = iD.Graph([
iD.Node(createFakeNode('a', false)),
iD.Node(createFakeNode('b', false)),
iD.Node(createFakeNode('c', false)),
iD.Node(createFakeNode('d', true)),
iD.Node(createFakeNode('e', false)),
iD.Node(createFakeNode('f', false)),
iD.Node(createFakeNode('g', false)),
iD.Way({ id: 'x', nodes: ['a', 'b'] }),
iD.Way({ id: 'y', nodes: ['e', 'f', 'g'] }),
iD.Relation({
id: 'r',
tags: {
type: 'restriction',
restriction: 'no_right_turn'
},
members: [
{ id: 'x', type: 'way', role: 'from' },
{ id: 'c', type: 'node', role: 'via' },
{ id: 'd', type: 'node', role: 'location_hint' },
{ id: 'z', type: 'way', role: 'to' }
]
}),
iD.Relation({
id: 's',
members: [
{ id: 'x', type: 'way' },
{ id: 'd', type: 'node' },
]
})
]);
var result = iD.operationDetachNode(['d'], fakeContext).disabled();
expect(result).not.to.eql(false);
});
});
});