diff --git a/data/core.yaml b/data/core.yaml index b317df32a..f06a69983 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -229,6 +229,12 @@ en: annotation: create: Added a turn restriction delete: Deleted a turn restriction + detachNode: + title: Detach + key: T + description: Detach this node from these 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 diff --git a/dist/locales/en.json b/dist/locales/en.json index 8781f6338..8363a101f 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -297,6 +297,13 @@ "create": "Added a turn restriction", "delete": "Deleted a turn restriction" } + }, + "detachNode": { + "title": "Detach", + "key": "T", + "description": "Detach this node from these 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": { diff --git a/modules/actions/detach_node.js b/modules/actions/detach_node.js new file mode 100644 index 000000000..7ff5cd8e3 --- /dev/null +++ b/modules/actions/detach_node.js @@ -0,0 +1,39 @@ +import { osmNode } from '../osm'; + +export function actionDetachNode(nodeId) { + return function (graph) { + // Get the point in question + var node = graph.entity(nodeId); + // Get all of the ways it's currently attached to + var parentWays = graph.parentWays(node); + // 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 + 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 + .addNode(replacementNode.id, originalIndex); // Add in the replacement node in its place + // 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); + }; +} diff --git a/modules/actions/index.js b/modules/actions/index.js index 330690db2..4f15456d8 100644 --- a/modules/actions/index.js +++ b/modules/actions/index.js @@ -33,3 +33,4 @@ export { actionSplit } from './split'; export { actionStraighten } from './straighten'; export { actionUnrestrictTurn } from './unrestrict_turn'; export { actionReflect } from './reflect.js'; +export { actionDetachNode } from './detach_node'; \ No newline at end of file diff --git a/modules/operations/detach_node.js b/modules/operations/detach_node.js new file mode 100644 index 000000000..bb1f985cf --- /dev/null +++ b/modules/operations/detach_node.js @@ -0,0 +1,104 @@ +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]; + var operation = function () { + context.perform(actionDetachNode(selectedNode)); + context.enter(modeMove(context, [selectedNode], context.graph)); + }; + var hasTags = function (entity) { + return Object.keys(entity.tags).length > 0; + }; + operation.available = function () { + // Check multiple items aren't selected + if (selectedIDs.length !== 1) { + return false; + } + // Get the entity itself + var graph = context.graph(); + var entity = graph.hasEntity(selectedNode); + if (!entity) { + // This probably isn't possible + return false; + } + // Confirm entity is a node with tags + if (entity.type === 'node' && hasTags(entity)) { + // Confirm that the node is owned by at least 1 parent way + var parentWays = graph.parentWays(entity); + return parentWays && parentWays.length > 0; + } + // Not appropriate + return false; + }; + operation.disabled = function () { + return false; + }; + operation.tooltip = function () { + var disableReason = operation.disabled(); + return disableReason + ? t('operations.detachNode.' + disableReason) + : t('operations.detachNode.description'); + }; + operation.annotation = function () { + return t('operations.detachNode.annotation'); + }; + operation.id = 'detachNode'; + 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; +} \ No newline at end of file diff --git a/modules/operations/index.js b/modules/operations/index.js index 1ad24cd97..8339d8205 100644 --- a/modules/operations/index.js +++ b/modules/operations/index.js @@ -10,3 +10,4 @@ export { operationReverse } from './reverse'; export { operationRotate } from './rotate'; export { operationSplit } from './split'; export { operationStraighten } from './straighten'; +export { operationDetachNode } from './detach_node'; diff --git a/modules/util/index.js b/modules/util/index.js index 4d4c53269..bc06a0a41 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -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'; @@ -25,4 +25,4 @@ export { utilSuggestNames } from './suggest_names'; export { utilTagText } from './util'; export { utilTiler } from './tiler'; export { utilTriggerEvent } from './trigger_event'; -export { utilWrap } from './util'; +export { utilWrap } from './util'; \ No newline at end of file diff --git a/svg/iD-sprite/operations/operation-detachNode.svg b/svg/iD-sprite/operations/operation-detachNode.svg new file mode 100644 index 000000000..504acc2cb --- /dev/null +++ b/svg/iD-sprite/operations/operation-detachNode.svg @@ -0,0 +1,9 @@ + + + + + + + + diff --git a/test/index.html b/test/index.html index b094f4b32..3f03cdda0 100644 --- a/test/index.html +++ b/test/index.html @@ -1,5 +1,6 @@ + Mocha Tests @@ -7,6 +8,7 @@ +
@@ -17,9 +19,9 @@ @@ -60,6 +62,7 @@ + @@ -139,8 +142,10 @@ + - + + \ No newline at end of file diff --git a/test/spec/actions/detach_node.js b/test/spec/actions/detach_node.js new file mode 100644 index 000000000..16fcc5e3a --- /dev/null +++ b/test/spec/actions/detach_node.js @@ -0,0 +1,610 @@ +describe('iD.actionDetachNode', function () { + var tags = { 'name': 'test' }; + function createTargetNode(id, lonlat) { + return iD.Node({ id: id, loc: lonlat, tags: tags }); + } + describe('simple way', function () { + var graph; + beforeEach(function () { + // Set up a simple way + // a-b-c-d + // (0,0)-(1,0)-(2,0)-(3,0) + graph = iD.Graph([ + iD.Node({ id: 'a', loc: [0, 0] }), + iD.Node({ id: 'b', loc: [1, 0] }), + iD.Node({ id: 'c', loc: [2, 0] }), + iD.Node({ id: 'd', loc: [3, 0] }), + iD.Way({ id: 'w', nodes: ['a', 'b', 'c', 'd'] }) + ]); + }); + + describe('target in first position', function () { + beforeEach(function () { + // Swap target into the location & position of A + var targetNode = createTargetNode('a', graph.entity('a').loc); + graph = graph.replace(targetNode); + }); + + it('does not change length of way', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // Confirm that the way still has 4 nodes + var target = assertionGraph.entity('w'); + expect(target.nodes.length).to.eql(4); + }); + + it('does not change order of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // Confirm that the way is ordered correctly + var target = assertionGraph.entity('w'); + // Note that we can't be sure of the id of the replacement node + // so we only assert the nodes we know the ids for + // As we have already confirmed the size of the array we can assume + // that the replacement node is in the correct posisiton by a process of elimination + expect(target.nodes[1]).to.eql('b'); + expect(target.nodes[2]).to.eql('c'); + expect(target.nodes[3]).to.eql('d'); + }); + + it('does not change location of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // Confirm that the nodes have not moved, including the replacement node + var nodes = assertionGraph.entity('w').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([0, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([1, 0]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([2, 0]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([3, 0]); + }); + + it('does replace target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + var nodes = assertionGraph.entity('w').nodes; + // Confirm that the target is no longer "a" + expect(nodes[0]).not.to.eql('a'); + // and that the tags are not present + expect(assertionGraph.entity(nodes[0]).tags).to.eql({}); + }); + + it('does detach target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // confirm that a still exists + var targetNode = assertionGraph.entity('a'); + expect(targetNode).not.to.eql(undefined); + // ... 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); + // ... and that the parentWay is empty + expect(assertionGraph.parentWays(targetNode)).to.eql([]); + }); + }); + + describe('target in second position', function () { + beforeEach(function () { + // Swap target into the location & position of B + var targetNode = createTargetNode('b', graph.entity('b').loc); + graph = graph.replace(targetNode); + }); + + it('does not change length of way', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // Confirm that the way still has 4 nodes + var target = assertionGraph.entity('w'); + expect(target.nodes.length).to.eql(4); + }); + + it('does not change order of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // Confirm that the way is ordered correctly + var target = assertionGraph.entity('w'); + // Note that we can't be sure of the id of the replacement node + // so we only assert the nodes we know the ids for + // As we have already confirmed the size of the array we can assume + // that the replacement node is in the correct posisiton by a process of elimination + expect(target.nodes[0]).to.eql('a'); + expect(target.nodes[2]).to.eql('c'); + expect(target.nodes[3]).to.eql('d'); + }); + + it('does not change location of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // Confirm that the nodes have not moved, including the replacement node + var nodes = assertionGraph.entity('w').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([0, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([1, 0]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([2, 0]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([3, 0]); + }); + + it('does replace target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + var nodes = assertionGraph.entity('w').nodes; + // Confirm that the target is no longer "a" + expect(nodes[1]).not.to.eql('b'); + // and that the tags are not present + expect(assertionGraph.entity(nodes[1]).tags).to.eql({}); + }); + + it('does detach target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // confirm that a still exists + var targetNode = assertionGraph.entity('b'); + expect(targetNode).not.to.eql(undefined); + // ... 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); + // ... and that the parentWay is empty + expect(assertionGraph.parentWays(targetNode)).to.eql([]); + }); + }); + }); + describe('closed way', function () { + var graph; + beforeEach(function () { + // Set up a closed way + // a-b (0,0)-(1,0) + // | | + // d-c (0,1)-(1,1) + graph = iD.Graph([ + iD.Node({ id: 'a', loc: [0, 0] }), + iD.Node({ id: 'b', loc: [1, 0] }), + iD.Node({ id: 'c', loc: [1, 1] }), + iD.Node({ id: 'd', loc: [0, 1] }), + iD.Way({ id: 'w', nodes: ['a', 'b', 'c', 'd', 'a'] }) + ]); + }); + + describe('target in first position', function () { + beforeEach(function () { + // Swap target into the location & position of A + var targetNode = createTargetNode('a', graph.entity('a').loc); + graph = graph.replace(targetNode); + }); + + it('does not change length of way', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // Confirm that the way still has 5 nodes + var target = assertionGraph.entity('w'); + expect(target.nodes.length).to.eql(5); + }); + + it('does not change order of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // Confirm that the way is ordered correctly + var target = assertionGraph.entity('w'); + // Note that we can't be sure of the id of the replacement node + // so we only assert the nodes we know the ids for + // As we have already confirmed the size of the array we can assume + // that the replacement node is in the correct posisiton by a process of elimination + expect(target.nodes[1]).to.eql('b'); + expect(target.nodes[2]).to.eql('c'); + expect(target.nodes[3]).to.eql('d'); + // Need to confirm that the id of the first & last node is the same so that the way remains closed + expect(target.nodes[0]).to.eql(target.nodes[4]); + }); + + it('does not change location of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // Confirm that the nodes have not moved, including the replacement node + var nodes = assertionGraph.entity('w').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([0, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([1, 0]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([1, 1]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([0, 1]); + // We don't need to assert node[4] location as we've already confirmed that it is the same as node 0 + }); + + it('does replace target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + var nodes = assertionGraph.entity('w').nodes; + // Confirm that the target is no longer "a" + expect(nodes[0]).not.to.eql('a'); + // .. also in the tail position + expect(nodes[4]).not.to.eql('a'); + // and that the tags are not present (already confirmed same node in position 0 & 4, so only need to check tags once) + expect(assertionGraph.entity(nodes[0]).tags).to.eql({}); + }); + + it('does detach target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('a')(graph); + + // confirm that a still exists + var targetNode = assertionGraph.entity('a'); + expect(targetNode).not.to.eql(undefined); + // ... 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); + // ... and that the parentWay is empty + expect(assertionGraph.parentWays(targetNode)).to.eql([]); + }); + }); + + describe('target in second position', function () { + beforeEach(function () { + // Swap target into the location & position of B + var targetNode = createTargetNode('b', graph.entity('b').loc); + graph = graph.replace(targetNode); + }); + + it('does not change length of way', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // Confirm that the way still has 5 nodes + var target = assertionGraph.entity('w'); + expect(target.nodes.length).to.eql(5); + }); + + it('does not change order of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // Confirm that the way is ordered correctly + var target = assertionGraph.entity('w'); + // Note that we can't be sure of the id of the replacement node + // so we only assert the nodes we know the ids for + // As we have already confirmed the size of the array we can assume + // that the replacement node is in the correct posisiton by a process of elimination + expect(target.nodes[0]).to.eql('a'); + expect(target.nodes[2]).to.eql('c'); + expect(target.nodes[3]).to.eql('d'); + expect(target.nodes[4]).to.eql('a'); + }); + + it('does not change location of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // Confirm that the nodes have not moved, including the replacement node + var nodes = assertionGraph.entity('w').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([0, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([1, 0]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([1, 1]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([0, 1]); + // Confirmed already that node[4] is node[0] so no further assertion needed + }); + + it('does replace target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + var nodes = assertionGraph.entity('w').nodes; + // Confirm that the target is no longer "a" + expect(nodes[1]).not.to.eql('b'); + // and that the tags are not present + expect(assertionGraph.entity(nodes[1]).tags).to.eql({}); + }); + + it('does detach target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('b')(graph); + + // confirm that a still exists + var targetNode = assertionGraph.entity('b'); + expect(targetNode).not.to.eql(undefined); + // ... 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); + // ... and that the parentWay is empty + expect(assertionGraph.parentWays(targetNode)).to.eql([]); + }); + }); + }); + describe('intersecting simple ways', function () { + var graph; + beforeEach(function () { + // Set up two simple ways + // a-b-c-d (0,0)-(1,0)-(2,0)-(3,0) + // e (2,1) + // f (2,2) + // Node c represents the target + graph = iD.Graph([ + iD.Node({ id: 'a', loc: [0, 0] }), + iD.Node({ id: 'b', loc: [1, 0] }), + iD.Node({ id: 'c', loc: [2, 0], tags: tags }), + iD.Node({ id: 'd', loc: [3, 0] }), + iD.Node({ id: 'e', loc: [2, 1] }), + iD.Node({ id: 'f', loc: [2, 2] }), + iD.Way({ id: 'w', nodes: ['a', 'b', 'c', 'd'] }), + iD.Way({ id: 'x', nodes: ['c', 'e', 'f'] }) + ]); + }); + + it('does not change length of ways', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // Confirm that the way still has 4 nodes + var target = assertionGraph.entity('w'); + expect(target.nodes.length).to.eql(4); + // .. and second way has 3 + target = assertionGraph.entity('x'); + expect(target.nodes.length).to.eql(3); + }); + + it('does not change order of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // Confirm that the way is ordered correctly + var target = assertionGraph.entity('w'); + // Note that we can't be sure of the id of the replacement node + // so we only assert the nodes we know the ids for + // As we have already confirmed the size of the array we can assume + // that the replacement node is in the correct posisiton by a process of elimination + expect(target.nodes[0]).to.eql('a'); + expect(target.nodes[1]).to.eql('b'); + expect(target.nodes[3]).to.eql('d'); + // and second way + target = assertionGraph.entity('x'); + expect(target.nodes[1]).to.eql('e'); + expect(target.nodes[2]).to.eql('f'); + }); + + it('does not change location of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // Confirm that the nodes have not moved, including the replacement node + var nodes = assertionGraph.entity('w').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([0, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([1, 0]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([2, 0]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([3, 0]); + // and second way + nodes = assertionGraph.entity('x').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([2, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([2, 1]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([2, 2]); + }); + + it('uses same replacement node at intersection', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + // Confirm both ways have the same replacement node + expect(assertionGraph.entity('w').nodes[2]).to.eql(assertionGraph.entity('x').nodes[0]); + }); + + it('does replace target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + var nodes = assertionGraph.entity('w').nodes; + // Confirm that the target is no longer "c" + expect(nodes[2]).not.to.eql('c'); + // and that the tags are not present + expect(assertionGraph.entity(nodes[2]).tags).to.eql({}); + // Confirm that the second way's first node is the same + expect(assertionGraph.entity('x').nodes[0]).to.eql(nodes[2]); + }); + + it('does detach target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // confirm that a still exists + var targetNode = assertionGraph.entity('c'); + expect(targetNode).not.to.eql(undefined); + // ... 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); + // ... and that the parentWay is empty + expect(assertionGraph.parentWays(targetNode)).to.eql([]); + }); + }); + describe('intersecting closed way', function () { + var graph; + beforeEach(function () { + // Set up two intersecting closed ways + // a-b (0,0)-(1,0) + // | | + // d-c-e (0,1)-(1,1)-(2,1) + // | | + // g f (0,2) - (1,2) + // C is the target node + graph = iD.Graph([ + iD.Node({ id: 'a', loc: [0, 0] }), + iD.Node({ id: 'b', loc: [1, 0] }), + iD.Node({ id: 'c', loc: [1, 1], tags: tags }), + iD.Node({ id: 'd', loc: [0, 1] }), + iD.Node({ id: 'e', loc: [2, 1] }), + iD.Node({ id: 'f', loc: [1, 2] }), + iD.Node({ id: 'g', loc: [0, 2] }), + iD.Way({ id: 'w', nodes: ['a', 'b', 'c', 'd', 'a'] }), + iD.Way({ id: 'x', nodes: ['c', 'e', 'f', 'g', 'c'] }) + ]); + }); + + it('does not change length of ways', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // Confirm that the way still has 5 nodes + var target = assertionGraph.entity('w'); + expect(target.nodes.length).to.eql(5); + // and the second + target = assertionGraph.entity('x'); + expect(target.nodes.length).to.eql(5); + }); + + it('does not change order of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // Confirm that the way is ordered correctly + var target = assertionGraph.entity('w'); + // Note that we can't be sure of the id of the replacement node + // so we only assert the nodes we know the ids for + // As we have already confirmed the size of the array we can assume + // that the replacement node is in the correct posisiton by a process of elimination + expect(target.nodes[0]).to.eql('a'); + expect(target.nodes[1]).to.eql('b'); + expect(target.nodes[3]).to.eql('d'); + // Need to confirm that the id of the first & last node is the same so that the way remains closed + expect(target.nodes[0]).to.eql(target.nodes[4]); + // and the same for the other way + target = assertionGraph.entity('x'); + expect(target.nodes[1]).to.eql('e'); + expect(target.nodes[2]).to.eql('f'); + expect(target.nodes[3]).to.eql('g'); + expect(target.nodes[0]).to.eql(target.nodes[4]); + }); + + it('does not change location of nodes', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // Confirm that the nodes have not moved, including the replacement node + var nodes = assertionGraph.entity('w').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([0, 0]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([1, 0]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([1, 1]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([0, 1]); + // We don't need to assert node[4] location as we've already confirmed that it is the same as node 0 + // and the other way + nodes = assertionGraph.entity('x').nodes; + expect(assertionGraph.entity(nodes[0]).loc).to.eql([1, 1]); + expect(assertionGraph.entity(nodes[1]).loc).to.eql([2, 1]); + expect(assertionGraph.entity(nodes[2]).loc).to.eql([1, 2]); + expect(assertionGraph.entity(nodes[3]).loc).to.eql([0, 2]); + }); + + it('uses same replacement node at intersection', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + // Confirm both ways have the same replacement node + expect(assertionGraph.entity('w').nodes[2]).to.eql(assertionGraph.entity('x').nodes[0]); + }); + + it('does replace target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + var nodes = assertionGraph.entity('w').nodes; + // Confirm that the target is no longer "c" + expect(nodes[0]).not.to.eql('c'); + // .. also in the tail position + expect(nodes[4]).not.to.eql('c'); + // and that the tags are not present (already confirmed same node in position 0 & 4, so only need to check tags once) + expect(assertionGraph.entity(nodes[0]).tags).to.eql({}); + // Don't need to check for way 2 since we've already confirmed it is the same node + }); + + it('does detach target node', function () { + // Act + var assertionGraph = iD.actionDetachNode('c')(graph); + + // confirm that a still exists + var targetNode = assertionGraph.entity('c'); + expect(targetNode).not.to.eql(undefined); + // ... 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); + // ... and that the parentWay is empty + 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' }); + }); + + }); +}); \ No newline at end of file diff --git a/test/spec/operations/detach_node.js b/test/spec/operations/detach_node.js new file mode 100644 index 000000000..357d390fa --- /dev/null +++ b/test/spec/operations/detach_node.js @@ -0,0 +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' }; + // Set up graph + var createFakeNode = function (id, hasTags) { + return hasTags + ? { id: id, type: 'node', tags: fakeTags } + : { id: id, type: 'node' }; + }; + + 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); + }); + }); + + 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('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('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('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('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('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); + }); + }); +});