Merge branch '4320' of https://github.com/Psigio/iD into Psigio-4320

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

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": {

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);
+    };
+}

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';

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;
+}

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';

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';

svg/iD-sprite/operations/operation-detachNode.svg

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg
+    xmlns="http://www.w3.org/2000/svg" version="1.1" x="0" y="0" width="20" height="20" viewBox="0 0 20 20" id="svg838">
+    <g id="g823">
+        <path id="path834" d="m 11.414214,2.9289322 v 1.4142136 l 1.414213,1.4142135 -1.54715,1.5471497 c 0.294864,0.1421284 0.596092,0.3302188 0.840043,0.5741707 0.25173,0.25173 0.431335,0.5359869 0.574171,0.8400428 l 1.54715,-1.5471496 1.414213,1.4142135 h 1.414214 V 2.9289322 Z" style="fill:currentColor" />
+        <path id="path836" fill="inherit" d="M10,7.987 C8.895,7.987 8,8.883 8,9.987 C8,11.092 8.895,11.987 10,11.987 C11.105,11.987 12,11.092 12,9.987 C12,8.883 11.105,7.987 10,7.987 z" />
+    </g>
+</svg>

test/index.html

@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <html>
+
 <head>
     <meta charset='utf-8'>
     <title>Mocha Tests</title>
@@ -7,6 +8,7 @@
     <link rel='stylesheet' href='../dist/iD.css'>
     <!-- <script src='../node_modules/d3/build/d3.js'></script> -->
 </head>
+
 <body style="overflow:scroll">
     <div id='mocha'></div>
 
@@ -17,9 +19,9 @@
     <script src='../node_modules/happen/happen.js'></script>
 
     <script>
-    if (typeof initMochaPhantomJS === 'function') {
-        initMochaPhantomJS()
-    }
+        if (typeof initMochaPhantomJS === 'function') {
+            initMochaPhantomJS()
+        }
     </script>
 
     <!-- include source files here... -->
@@ -60,6 +62,7 @@
     <script src='spec/actions/straighten.js'></script>
     <script src='spec/actions/unrestrict_turn.js'></script>
     <script src='spec/actions/reflect.js'></script>
+    <script src='spec/actions/detach_node.js'></script>
 
     <script src='spec/behavior/hash.js'></script>
     <script src='spec/behavior/hover.js'></script>
@@ -139,8 +142,10 @@
     <script src='spec/util/suggest_names.js'></script>
     <script src='spec/util/util.js'></script>
 
+    <script src='spec/operations/detach_node.js'></script>
     <script>
-    window.mocha.run();
+        window.mocha.run();
     </script>
 </body>
-</html>
+
+</html>

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' });
+        });
+
+    });
+});

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);
+        });
+    });
+});