From 292c916cb143a4de102d3eb3d44281c6e9cbb7ef Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Wed, 23 Jan 2013 10:36:02 -0500
Subject: [PATCH 1/3] Converting some actions to entity methods

The guidelines here are:

Entity methods:
  return a modified entity
  don't necessarily maintain whole-graph consistency

Actions:
  return a modified graph
  always maintain whole-graph consistency
  call entity methods liberally
  generally don't call other actions
---
 index.html                                  |  4 --
 js/id/actions/add_relation_member.js        |  9 ----
 js/id/actions/add_way_node.js               |  6 +--
 js/id/actions/delete_node.js                |  4 +-
 js/id/actions/delete_way.js                 |  4 +-
 js/id/actions/move_node.js                  |  3 +-
 js/id/actions/move_way.js                   |  2 +-
 js/id/actions/remove_relation_member.js     |  9 ----
 js/id/actions/remove_way_node.js            | 17 --------
 js/id/actions/reverse_way.js                |  3 +-
 js/id/actions/split_way.js                  | 14 ++----
 js/id/actions/unjoin_node.js                |  7 +--
 js/id/actions/update_relation_member.js     |  9 ----
 js/id/graph/node.js                         |  4 ++
 js/id/graph/relation.js                     | 17 ++++++++
 js/id/graph/way.js                          | 23 ++++++++++
 test/index.html                             |  9 ----
 test/index_packaged.html                    |  4 --
 test/spec/actions/add_relation_member.js    | 37 ----------------
 test/spec/actions/add_way_node.js           | 29 -------------
 test/spec/actions/remove_relation_member.js |  8 ----
 test/spec/actions/remove_way_node.js        |  8 ----
 test/spec/actions/update_relation_member.js |  8 ----
 test/spec/graph/relation.js                 | 36 ++++++++++++++++
 test/spec/graph/way.js                      | 48 +++++++++++++++++++++
 25 files changed, 143 insertions(+), 179 deletions(-)
 delete mode 100644 js/id/actions/add_relation_member.js
 delete mode 100644 js/id/actions/remove_relation_member.js
 delete mode 100644 js/id/actions/remove_way_node.js
 delete mode 100644 js/id/actions/update_relation_member.js
 delete mode 100644 test/spec/actions/add_relation_member.js
 delete mode 100644 test/spec/actions/add_way_node.js
 delete mode 100644 test/spec/actions/remove_relation_member.js
 delete mode 100644 test/spec/actions/remove_way_node.js
 delete mode 100644 test/spec/actions/update_relation_member.js

diff --git a/index.html b/index.html
index 93c5171a6..69f83f76b 100644
--- a/index.html
+++ b/index.html
@@ -71,7 +71,6 @@
 
         <script src='js/id/actions.js'></script>
         <script src='js/id/actions/add_node.js'></script>
-        <script src='js/id/actions/add_relation_member.js'></script>
         <script src='js/id/actions/add_way.js'></script>
         <script src='js/id/actions/add_way_node.js'></script>
         <script src='js/id/actions/change_entity_tags.js'></script>
@@ -80,12 +79,9 @@
         <script src='js/id/actions/move_node.js'></script>
         <script src='js/id/actions/move_way.js'></script>
         <script src='js/id/actions/noop.js'></script>
-        <script src='js/id/actions/remove_relation_member.js'></script>
-        <script src='js/id/actions/remove_way_node.js'></script>
         <script src='js/id/actions/reverse_way.js'></script>
         <script src='js/id/actions/split_way.js'></script>
         <script src='js/id/actions/unjoin_node.js'></script>
-        <script src='js/id/actions/update_relation_member.js'></script>
 
         <script src='js/id/behavior.js'></script>
         <script src='js/id/behavior/drag.js'></script>
diff --git a/js/id/actions/add_relation_member.js b/js/id/actions/add_relation_member.js
deleted file mode 100644
index 45e24a0b0..000000000
--- a/js/id/actions/add_relation_member.js
+++ /dev/null
@@ -1,9 +0,0 @@
-iD.actions.AddRelationMember = function(relationId, member, index) {
-    return function(graph) {
-        var relation = graph.entity(relationId),
-            members = relation.members.slice();
-
-        members.splice((index === undefined) ? members.length : index, 0, member);
-        return graph.replace(relation.update({members: members}));
-    };
-};
diff --git a/js/id/actions/add_way_node.js b/js/id/actions/add_way_node.js
index d685c3295..3e4fffcfc 100644
--- a/js/id/actions/add_way_node.js
+++ b/js/id/actions/add_way_node.js
@@ -1,10 +1,6 @@
 // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/AddNodeToWayAction.as
 iD.actions.AddWayNode = function(wayId, nodeId, index) {
     return function(graph) {
-        var way = graph.entity(wayId),
-            node = graph.entity(nodeId),
-            nodes = way.nodes.slice();
-        nodes.splice((index === undefined) ? nodes.length : index, 0, nodeId);
-        return graph.replace(way.update({nodes: nodes}));
+        return graph.replace(graph.entity(wayId).addNode(nodeId, index));
     };
 };
diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js
index a2d54fecb..d600bc5fc 100644
--- a/js/id/actions/delete_node.js
+++ b/js/id/actions/delete_node.js
@@ -5,12 +5,12 @@ iD.actions.DeleteNode = function(nodeId) {
 
         graph.parentWays(node)
             .forEach(function(parent) {
-                graph = iD.actions.RemoveWayNode(parent.id, nodeId)(graph);
+                graph = graph.replace(parent.removeNode(nodeId));
             });
 
         graph.parentRelations(node)
             .forEach(function(parent) {
-                graph = iD.actions.RemoveRelationMember(parent.id, nodeId)(graph);
+                graph = graph.replace(parent.removeMember(nodeId));
             });
 
         return graph.remove(node);
diff --git a/js/id/actions/delete_way.js b/js/id/actions/delete_way.js
index c2065a756..19e9e7c2a 100644
--- a/js/id/actions/delete_way.js
+++ b/js/id/actions/delete_way.js
@@ -5,7 +5,7 @@ iD.actions.DeleteWay = function(wayId) {
 
         graph.parentRelations(way)
             .forEach(function(parent) {
-                graph = iD.actions.RemoveRelationMember(parent.id, wayId)(graph);
+                graph = graph.replace(parent.removeMember(wayId));
             });
 
         way.nodes.forEach(function (nodeId) {
@@ -15,7 +15,7 @@ iD.actions.DeleteWay = function(wayId) {
             // can be deleted on earlier iterations of this loop.
             if (!node) return;
 
-            graph = iD.actions.RemoveWayNode(wayId, nodeId)(graph);
+            graph = graph.replace(way.removeNode(nodeId));
 
             if (!graph.parentWays(node).length &&
                 !graph.parentRelations(node).length) {
diff --git a/js/id/actions/move_node.js b/js/id/actions/move_node.js
index 9204fcb67..433242ff6 100644
--- a/js/id/actions/move_node.js
+++ b/js/id/actions/move_node.js
@@ -2,7 +2,6 @@
 // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MoveNodeAction.as
 iD.actions.MoveNode = function(nodeId, loc) {
     return function(graph) {
-        var node = graph.entity(nodeId);
-        return graph.replace(node.update({loc: loc}));
+        return graph.replace(graph.entity(nodeId).move(loc));
     };
 };
diff --git a/js/id/actions/move_way.js b/js/id/actions/move_way.js
index ea547cc44..09ab8fd50 100644
--- a/js/id/actions/move_way.js
+++ b/js/id/actions/move_way.js
@@ -6,7 +6,7 @@ iD.actions.MoveWay = function(wayId, delta, projection) {
             var node  = graph.entity(id),
                 start = projection(node.loc),
                 end   = projection.invert([start[0] + delta[0], start[1] + delta[1]]);
-            graph = iD.actions.MoveNode(id, end)(graph);
+            graph = graph.replace(node.move(end));
         });
 
         return graph;
diff --git a/js/id/actions/remove_relation_member.js b/js/id/actions/remove_relation_member.js
deleted file mode 100644
index e72dd7ca0..000000000
--- a/js/id/actions/remove_relation_member.js
+++ /dev/null
@@ -1,9 +0,0 @@
-iD.actions.RemoveRelationMember = function(relationId, memberId) {
-    return function(graph) {
-        var relation = graph.entity(relationId),
-            members = _.reject(relation.members, function(r) {
-                return r.id === memberId;
-            });
-        return graph.replace(relation.update({members: members}));
-    };
-};
diff --git a/js/id/actions/remove_way_node.js b/js/id/actions/remove_way_node.js
deleted file mode 100644
index 8c589a20c..000000000
--- a/js/id/actions/remove_way_node.js
+++ /dev/null
@@ -1,17 +0,0 @@
-iD.actions.RemoveWayNode = function(wayId, nodeId) {
-    return function(graph) {
-        var way = graph.entity(wayId), nodes;
-        // If this is the connecting node in a closed area
-        if (way.nodes.length > 1 &&
-            _.indexOf(way.nodes, nodeId) === 0 &&
-            _.lastIndexOf(way.nodes, nodeId) === way.nodes.length - 1) {
-            // Remove the node
-            nodes = _.without(way.nodes, nodeId);
-            // And reclose the way on the new first node.
-            nodes.push(nodes[0]);
-        } else {
-            nodes = _.without(way.nodes, nodeId);
-        }
-        return graph.replace(way.update({nodes: nodes}));
-    };
-};
diff --git a/js/id/actions/reverse_way.js b/js/id/actions/reverse_way.js
index a845fe844..160017637 100644
--- a/js/id/actions/reverse_way.js
+++ b/js/id/actions/reverse_way.js
@@ -65,7 +65,8 @@ iD.actions.ReverseWay = function(wayId) {
         graph.parentRelations(way).forEach(function (relation) {
             relation.members.forEach(function (member, index) {
                 if (member.id === way.id && (role = {forward: 'backward', backward: 'forward'}[member.role])) {
-                    graph = iD.actions.UpdateRelationMember(relation.id, {role: role}, index)(graph);
+                    relation = relation.updateMember({role: role}, index);
+                    graph = graph.replace(relation);
                 }
             });
         });
diff --git a/js/id/actions/split_way.js b/js/id/actions/split_way.js
index d716880cf..f6d502b97 100644
--- a/js/id/actions/split_way.js
+++ b/js/id/actions/split_way.js
@@ -37,11 +37,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) {
             if (relation.isRestriction()) {
                 var via = relation.memberByRole('via');
                 if (via && newWay.contains(via.id)) {
-                    graph = iD.actions.UpdateRelationMember(
-                        relation.id,
-                        {id: newWay.id},
-                        relation.memberById(way.id).index
-                    )(graph);
+                    relation = relation.updateMember({id: newWay.id}, relation.memberById(way.id).index);
+                    graph = graph.replace(relation);
                 }
             } else {
                 var role = relation.memberById(way.id).role,
@@ -55,11 +52,8 @@ iD.actions.SplitWay = function(nodeId, newWayId) {
                     }
                 }
 
-                graph = iD.actions.AddRelationMember(
-                    relation.id,
-                    {id: newWay.id, type: 'way', role: role},
-                    i <= j ? i + 1 : i
-                )(graph);
+                relation = relation.addMember({id: newWay.id, type: 'way', role: role}, i <= j ? i + 1 : i);
+                graph = graph.replace(relation);
             }
         });
 
diff --git a/js/id/actions/unjoin_node.js b/js/id/actions/unjoin_node.js
index 4bc7a5ca7..037c56137 100644
--- a/js/id/actions/unjoin_node.js
+++ b/js/id/actions/unjoin_node.js
@@ -20,13 +20,10 @@ iD.actions.UnjoinNode = function(nodeId, newNodeId) {
                 return;
 
             var index = parent.nodes.indexOf(nodeId),
-                newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags}),
-                nodes = parent.nodes.slice();
-
-            nodes.splice(index, 1, newNode.id);
+                newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags});
 
             graph = graph.replace(newNode);
-            graph = graph.replace(parent.update({nodes: nodes}));
+            graph = graph.replace(parent.updateNode(newNode.id, index));
         });
 
         return graph;
diff --git a/js/id/actions/update_relation_member.js b/js/id/actions/update_relation_member.js
deleted file mode 100644
index 29eb79afe..000000000
--- a/js/id/actions/update_relation_member.js
+++ /dev/null
@@ -1,9 +0,0 @@
-iD.actions.UpdateRelationMember = function(relationId, properties, index) {
-    return function(graph) {
-        var relation = graph.entity(relationId),
-            members = relation.members.slice();
-
-        members.splice(index, 1, _.extend({}, members[index], properties));
-        return graph.replace(relation.update({members: members}));
-    };
-};
diff --git a/js/id/graph/node.js b/js/id/graph/node.js
index d72bf6442..df9c253a4 100644
--- a/js/id/graph/node.js
+++ b/js/id/graph/node.js
@@ -17,5 +17,9 @@ _.extend(iD.Node.prototype, {
 
     geometry: function() {
         return this._poi ? 'point' : 'vertex';
+    },
+
+    move: function(loc) {
+        return this.update({loc: loc});
     }
 });
diff --git a/js/id/graph/relation.js b/js/id/graph/relation.js
index 2dd325438..48a86c920 100644
--- a/js/id/graph/relation.js
+++ b/js/id/graph/relation.js
@@ -48,6 +48,23 @@ _.extend(iD.Relation.prototype, {
         }
     },
 
+    addMember: function(member, index) {
+        var members = this.members.slice();
+        members.splice(index === undefined ? members.length : index, 0, member);
+        return this.update({members: members});
+    },
+
+    updateMember: function(member, index) {
+        var members = this.members.slice();
+        members.splice(index, 1, _.extend({}, members[index], member));
+        return this.update({members: members});
+    },
+
+    removeMember: function(id) {
+        var members = _.reject(this.members, function(m) { return m.id === id; });
+        return this.update({members: members});
+    },
+
     isRestriction: function() {
         return !!(this.tags.type && this.tags.type.match(/^restriction:?/));
     },
diff --git a/js/id/graph/way.js b/js/id/graph/way.js
index 35834e761..eb3cf4087 100644
--- a/js/id/graph/way.js
+++ b/js/id/graph/way.js
@@ -60,5 +60,28 @@ _.extend(iD.Way.prototype, {
 
     geometry: function() {
         return this.isArea() ? 'area' : 'line';
+    },
+
+    addNode: function(id, index) {
+        var nodes = this.nodes.slice();
+        nodes.splice(index === undefined ? nodes.length : index, 0, id);
+        return this.update({nodes: nodes});
+    },
+
+    updateNode: function(id, index) {
+        var nodes = this.nodes.slice();
+        nodes.splice(index, 1, id);
+        return this.update({nodes: nodes});
+    },
+
+    removeNode: function(id) {
+        var nodes = _.without(this.nodes, id);
+
+        // Preserve circularity
+        if (this.nodes.length > 1 && this.first() === id && this.last() === id) {
+            nodes.push(nodes[0]);
+        }
+
+        return this.update({nodes: nodes});
     }
 });
diff --git a/test/index.html b/test/index.html
index 6df3944d9..e7e6701a6 100644
--- a/test/index.html
+++ b/test/index.html
@@ -66,7 +66,6 @@
 
     <script src='../js/id/actions.js'></script>
     <script src='../js/id/actions/add_node.js'></script>
-    <script src="../js/id/actions/add_relation_member.js"></script>
     <script src='../js/id/actions/add_way.js'></script>
     <script src='../js/id/actions/add_way_node.js'></script>
     <script src='../js/id/actions/change_entity_tags.js'></script>
@@ -75,12 +74,9 @@
     <script src='../js/id/actions/move_node.js'></script>
     <script src='../js/id/actions/move_way.js'></script>
     <script src='../js/id/actions/noop.js'></script>
-    <script src='../js/id/actions/remove_relation_member.js'></script>
-    <script src='../js/id/actions/remove_way_node.js'></script>
     <script src='../js/id/actions/reverse_way.js'></script>
     <script src='../js/id/actions/split_way.js'></script>
     <script src='../js/id/actions/unjoin_node.js'></script>
-    <script src='../js/id/actions/update_relation_member.js'></script>
 
     <script src='../js/id/behavior.js'></script>
     <script src='../js/id/behavior/drag.js'></script>
@@ -128,21 +124,16 @@
     <script src="spec/lib/d3.keybinding.js"></script>
 
     <script src="spec/actions/add_node.js"></script>
-    <script src="spec/actions/add_relation_member.js"></script>
     <script src="spec/actions/add_way.js"></script>
-    <script src="spec/actions/add_way_node.js"></script>
     <script src="spec/actions/change_entity_tags.js"></script>
     <script src="spec/actions/delete_node.js"></script>
     <script src="spec/actions/delete_way.js"></script>
     <script src="spec/actions/move_node.js"></script>
     <script src="spec/actions/move_way.js"></script>
     <script src="spec/actions/noop.js"></script>
-    <script src="spec/actions/remove_way_node.js"></script>
-    <script src="spec/actions/remove_relation_member.js"></script>
     <script src="spec/actions/reverse_way.js"></script>
     <script src="spec/actions/split_way.js"></script>
     <script src='spec/actions/unjoin_node.js'></script>
-    <script src="spec/actions/update_relation_member.js"></script>
 
     <script src="spec/behavior/hover.js"></script>
 
diff --git a/test/index_packaged.html b/test/index_packaged.html
index 6dc1391a9..5a013b45c 100644
--- a/test/index_packaged.html
+++ b/test/index_packaged.html
@@ -33,7 +33,6 @@
     <script src="spec/lib/d3.keybinding.js"></script>
 
     <script src="spec/actions/add_node.js"></script>
-    <script src="spec/actions/add_relation_member.js"></script>
     <script src="spec/actions/add_way.js"></script>
     <script src="spec/actions/add_way_node.js"></script>
     <script src="spec/actions/change_entity_tags.js"></script>
@@ -42,12 +41,9 @@
     <script src="spec/actions/move_node.js"></script>
     <script src="spec/actions/move_way.js"></script>
     <script src="spec/actions/noop.js"></script>
-    <script src="spec/actions/remove_way_node.js"></script>
-    <script src="spec/actions/remove_relation_member.js"></script>
     <script src="spec/actions/reverse_way.js"></script>
     <script src="spec/actions/split_way.js"></script>
     <script src='spec/actions/unjoin_node.js'></script>
-    <script src="spec/actions/update_relation_member.js"></script>
 
     <script src="spec/behavior/hover.js"></script>
 
diff --git a/test/spec/actions/add_relation_member.js b/test/spec/actions/add_relation_member.js
deleted file mode 100644
index aeae8e353..000000000
--- a/test/spec/actions/add_relation_member.js
+++ /dev/null
@@ -1,37 +0,0 @@
-describe("iD.actions.AddRelationMember", function () {
-    it("adds a member at the end of the relation", function () {
-        var relation = iD.Relation(),
-            graph    = iD.Graph([relation]);
-
-        graph = iD.actions.AddRelationMember(relation.id, {id: '1'})(graph);
-
-        expect(graph.entity(relation.id).members).to.eql([{id: '1'}]);
-    });
-
-    it("adds a member at index 0", function () {
-        var relation = iD.Relation({members: [{id: '1'}]}),
-            graph    = iD.Graph([relation]);
-
-        graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, 0)(graph);
-
-        expect(graph.entity(relation.id).members).to.eql([{id: '2'}, {id: '1'}]);
-    });
-
-    it("adds a member at a positive index", function () {
-        var relation = iD.Relation({members: [{id: '1'}, {id: '3'}]}),
-            graph    = iD.Graph([relation]);
-
-        graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, 1)(graph);
-
-        expect(graph.entity(relation.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
-    });
-
-    it("adds a member at a negative index", function () {
-        var relation = iD.Relation({members: [{id: '1'}, {id: '3'}]}),
-            graph    = iD.Graph([relation]);
-
-        graph = iD.actions.AddRelationMember(relation.id, {id: '2'}, -1)(graph);
-
-        expect(graph.entity(relation.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
-    });
-});
diff --git a/test/spec/actions/add_way_node.js b/test/spec/actions/add_way_node.js
deleted file mode 100644
index 5c774e5e4..000000000
--- a/test/spec/actions/add_way_node.js
+++ /dev/null
@@ -1,29 +0,0 @@
-describe("iD.actions.AddWayNode", function () {
-    it("adds a node to the end of a way", function () {
-        var way = iD.Way(),
-            node = iD.Node({id: "n1"}),
-            graph = iD.actions.AddWayNode(way.id, node.id)(iD.Graph([way, node]));
-        expect(graph.entity(way.id).nodes).to.eql(["n1"]);
-    });
-
-    it("adds a node to a way at index 0", function () {
-        var way = iD.Way({nodes: ["n1", "n3"]}),
-            node = iD.Node({id: "n2"}),
-            graph = iD.actions.AddWayNode(way.id, node.id, 0)(iD.Graph([way, node]));
-        expect(graph.entity(way.id).nodes).to.eql(["n2", "n1", "n3"]);
-    });
-
-    it("adds a node to a way at a positive index", function () {
-        var way = iD.Way({nodes: ["n1", "n3"]}),
-            node = iD.Node({id: "n2"}),
-            graph = iD.actions.AddWayNode(way.id, node.id, 1)(iD.Graph([way, node]));
-        expect(graph.entity(way.id).nodes).to.eql(["n1", "n2", "n3"]);
-    });
-
-    it("adds a node to a way at a negative index", function () {
-        var way = iD.Way({nodes: ["n1", "n3"]}),
-            node = iD.Node({id: "n2"}),
-            graph = iD.actions.AddWayNode(way.id, node.id, -1)(iD.Graph([way, node]));
-        expect(graph.entity(way.id).nodes).to.eql(["n1", "n2", "n3"]);
-    });
-});
diff --git a/test/spec/actions/remove_relation_member.js b/test/spec/actions/remove_relation_member.js
deleted file mode 100644
index d989b749b..000000000
--- a/test/spec/actions/remove_relation_member.js
+++ /dev/null
@@ -1,8 +0,0 @@
-describe("iD.actions.RemoveRelationMember", function () {
-    it("removes a member from a relation", function () {
-        var node     = iD.Node(),
-            relation = iD.Way({members: [{ id: node.id }]}),
-            graph    = iD.actions.RemoveRelationMember(relation.id, node.id)(iD.Graph([node, relation]));
-        expect(graph.entity(relation.id).members).to.eql([]);
-    });
-});
diff --git a/test/spec/actions/remove_way_node.js b/test/spec/actions/remove_way_node.js
deleted file mode 100644
index 6e5751b76..000000000
--- a/test/spec/actions/remove_way_node.js
+++ /dev/null
@@ -1,8 +0,0 @@
-describe("iD.actions.RemoveWayNode", function () {
-    it("removes a node from a way", function () {
-        var node = iD.Node({id: "n1"}),
-            way = iD.Way({id: "w1", nodes: ["n1"]}),
-            graph = iD.actions.RemoveWayNode(way.id, node.id)(iD.Graph({n1: node, w1: way}));
-        expect(graph.entity(way.id).nodes).to.eql([]);
-    });
-});
diff --git a/test/spec/actions/update_relation_member.js b/test/spec/actions/update_relation_member.js
deleted file mode 100644
index c85d72b34..000000000
--- a/test/spec/actions/update_relation_member.js
+++ /dev/null
@@ -1,8 +0,0 @@
-describe("iD.actions.UpdateRelationMember", function () {
-    it("updates the properties of the relation member at the specified index", function () {
-        var node     = iD.Node(),
-            relation = iD.Relation({members: [{id: node.id, role: 'forward'}]}),
-            graph    = iD.actions.UpdateRelationMember(relation.id, {role: 'backward'}, 0)(iD.Graph([node, relation]));
-        expect(graph.entity(relation.id).members).to.eql([{id: node.id, role: 'backward'}]);
-    });
-});
diff --git a/test/spec/graph/relation.js b/test/spec/graph/relation.js
index 58b1767f4..d8619726a 100644
--- a/test/spec/graph/relation.js
+++ b/test/spec/graph/relation.js
@@ -104,6 +104,42 @@ describe('iD.Relation', function () {
         });
     });
 
+    describe("#addMember", function () {
+        it("adds a member at the end of the relation", function () {
+            var r = iD.Relation();
+            expect(r.addMember({id: '1'}).members).to.eql([{id: '1'}]);
+        });
+
+        it("adds a member at index 0", function () {
+            var r = iD.Relation({members: [{id: '1'}]});
+            expect(r.addMember({id: '2'}, 0).members).to.eql([{id: '2'}, {id: '1'}]);
+        });
+
+        it("adds a member at a positive index", function () {
+            var r = iD.Relation({members: [{id: '1'}, {id: '3'}]});
+            expect(r.addMember({id: '2'}, 1).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
+        });
+
+        it("adds a member at a negative index", function () {
+            var r = iD.Relation({members: [{id: '1'}, {id: '3'}]});
+            expect(r.addMember({id: '2'}, -1).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]);
+        });
+    });
+
+    describe("#updateMember", function () {
+        it("updates the properties of the relation member at the specified index", function () {
+            var r = iD.Relation({members: [{role: 'forward'}]});
+            expect(r.updateMember({role: 'backward'}, 0).members).to.eql([{role: 'backward'}]);
+        });
+    });
+
+    describe("#removeMember", function () {
+        it("removes a member", function () {
+            var r = iD.Relation({members: [{id: 'a'}]});
+            expect(r.removeMember('a').members).to.eql([]);
+        });
+    });
+
     describe("#multipolygon", function () {
         specify("single polygon consisting of a single way", function () {
             var a = iD.Node(),
diff --git a/test/spec/graph/way.js b/test/spec/graph/way.js
index 37df6b648..0c70ea692 100644
--- a/test/spec/graph/way.js
+++ b/test/spec/graph/way.js
@@ -126,4 +126,52 @@ describe('iD.Way', function() {
             expect(iD.Way({tags: { area: 'yes' }}).geometry()).to.equal('area');
         });
     });
+
+    describe("#addNode", function () {
+        it("adds a node to the end of a way", function () {
+            var w = iD.Way();
+            expect(w.addNode('a').nodes).to.eql(['a']);
+        });
+
+        it("adds a node to a way at index 0", function () {
+            var w = iD.Way({nodes: ['a', 'b']});
+            expect(w.addNode('c', 0).nodes).to.eql(['c', 'a', 'b']);
+        });
+
+        it("adds a node to a way at a positive index", function () {
+            var w = iD.Way({nodes: ['a', 'b']});
+            expect(w.addNode('c', 1).nodes).to.eql(['a', 'c', 'b']);
+        });
+
+        it("adds a node to a way at a negative index", function () {
+            var w = iD.Way({nodes: ['a', 'b']});
+            expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']);
+        });
+    });
+
+    describe("#updateNode", function () {
+        it("updates the node id at the specified index", function () {
+            var w = iD.Way({nodes: ['a', 'b', 'c']});
+            expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']);
+        });
+    });
+
+    describe("#removeNode", function () {
+        it("removes the node", function () {
+            var a = iD.Node({id: 'a'}),
+                w = iD.Way({nodes: ['a']});
+
+            expect(w.removeNode('a').nodes).to.eql([]);
+        });
+
+        it("preserves circularity", function () {
+            var a = iD.Node({id: 'a'}),
+                b = iD.Node({id: 'b'}),
+                c = iD.Node({id: 'c'}),
+                d = iD.Node({id: 'd'}),
+                w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'a']});
+
+            expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']);
+        });
+    });
 });

From 278bb4a51c4026718e7d68403e345c833f8217d2 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Wed, 23 Jan 2013 12:08:25 -0500
Subject: [PATCH 2/3] Fix some cases where a degenerate way was created

---
 js/id/actions/delete_node.js     |  7 +++++-
 js/id/graph/way.js               |  4 ++++
 js/id/modes/draw_area.js         | 37 +++++++++++++++++++++-----------
 js/id/modes/draw_line.js         | 33 +++++++++++++++++++---------
 js/id/modes/select.js            | 10 +++------
 test/spec/actions/delete_node.js | 18 ++++++++++++++++
 test/spec/graph/way.js           | 25 +++++++++++++++++++++
 7 files changed, 103 insertions(+), 31 deletions(-)

diff --git a/js/id/actions/delete_node.js b/js/id/actions/delete_node.js
index d600bc5fc..a93129ce3 100644
--- a/js/id/actions/delete_node.js
+++ b/js/id/actions/delete_node.js
@@ -5,7 +5,12 @@ iD.actions.DeleteNode = function(nodeId) {
 
         graph.parentWays(node)
             .forEach(function(parent) {
-                graph = graph.replace(parent.removeNode(nodeId));
+                parent = parent.removeNode(nodeId);
+                graph = graph.replace(parent);
+
+                if (parent.isDegenerate()) {
+                    graph = iD.actions.DeleteWay(parent.id)(graph);
+                }
             });
 
         graph.parentRelations(node)
diff --git a/js/id/graph/way.js b/js/id/graph/way.js
index eb3cf4087..0de0e55a1 100644
--- a/js/id/graph/way.js
+++ b/js/id/graph/way.js
@@ -58,6 +58,10 @@ _.extend(iD.Way.prototype, {
                 !this.tags.barrier);
     },
 
+    isDegenerate: function() {
+        return _.uniq(this.nodes).length < (this.isArea() ? 3 : 2);
+    },
+
     geometry: function() {
         return this.isArea() ? 'area' : 'line';
     },
diff --git a/js/id/modes/draw_area.js b/js/id/modes/draw_area.js
index a90d13c9c..494d1affb 100644
--- a/js/id/modes/draw_area.js
+++ b/js/id/modes/draw_area.js
@@ -12,9 +12,8 @@ iD.modes.DrawArea = function(wayId) {
             history = mode.history,
             controller = mode.controller,
             way = history.graph().entity(wayId),
-            headId = (way.nodes.length == 1) ?
-                way.nodes[0] :
-                way.nodes[way.nodes.length - 2],
+            index = way.nodes.length - 1,
+            headId = way.nodes[index - 1],
             tailId = way.first(),
             node = iD.Node({loc: map.mouseCoordinates()});
 
@@ -24,12 +23,20 @@ iD.modes.DrawArea = function(wayId) {
 
         history.perform(
             iD.actions.AddNode(node),
-            iD.actions.AddWayNode(way.id, node.id, -1));
+            iD.actions.AddWayNode(way.id, node.id, index));
 
         surface.selectAll('.way, .node')
             .filter(function (d) { return d.id === wayId || d.id === node.id; })
             .classed('active', true);
 
+        function ReplaceTemporaryNode(replacementId) {
+            return function(graph) {
+                graph = graph.replace(graph.entity(wayId).updateNode(replacementId, index));
+                graph = graph.remove(node);
+                return graph;
+            }
+        }
+
         function mousemove() {
             history.replace(iD.actions.MoveNode(node.id, map.mouseCoordinates()));
         }
@@ -55,8 +62,7 @@ iD.modes.DrawArea = function(wayId) {
             } else if (datum.type === 'node' && datum.id !== node.id) {
                 // connect the way to an existing node
                 history.replace(
-                    iD.actions.DeleteNode(node.id),
-                    iD.actions.AddWayNode(way.id, datum.id, -1),
+                    ReplaceTemporaryNode(datum.id),
                     way.nodes.length > 2 ? 'added to an area' : '');
 
                 controller.enter(iD.modes.DrawArea(wayId));
@@ -77,13 +83,11 @@ iD.modes.DrawArea = function(wayId) {
                 iD.actions.DeleteNode(node.id),
                 iD.actions.DeleteNode(headId));
 
-            if (history.graph().fetch(wayId).nodes.length === 2) {
-                history.replace(
-                    iD.actions.DeleteNode(way.nodes[0]),
-                    iD.actions.DeleteWay(wayId));
-                controller.enter(iD.modes.Browse());
-            } else {
+            if (history.graph().entity(wayId)) {
                 controller.enter(iD.modes.DrawArea(wayId));
+            } else {
+                // The way was deleted because it had too few nodes.
+                controller.enter(iD.modes.Browse());
             }
         }
 
@@ -95,8 +99,15 @@ iD.modes.DrawArea = function(wayId) {
 
         function ret() {
             d3.event.preventDefault();
+
             history.replace(iD.actions.DeleteNode(node.id));
-            controller.enter(iD.modes.Select(way, true));
+
+            if (history.graph().entity(wayId)) {
+                controller.enter(iD.modes.Select(way, true));
+            } else {
+                // The way was deleted because it had too few nodes.
+                controller.enter(iD.modes.Browse());
+            }
         }
 
         surface
diff --git a/js/id/modes/draw_line.js b/js/id/modes/draw_line.js
index 3b3892aa6..b8fabeca5 100644
--- a/js/id/modes/draw_line.js
+++ b/js/id/modes/draw_line.js
@@ -13,7 +13,7 @@ iD.modes.DrawLine = function(wayId, direction) {
             controller = mode.controller,
             way = history.graph().entity(wayId),
             node = iD.Node({loc: map.mouseCoordinates()}),
-            index = (direction === 'forward') ? undefined : 0,
+            index = (direction === 'forward') ? way.nodes.length : 0,
             headId = (direction === 'forward') ? way.last() : way.first(),
             tailId = (direction === 'forward') ? way.first() : way.last();
 
@@ -35,6 +35,14 @@ iD.modes.DrawLine = function(wayId, direction) {
             .filter(function (d) { return d.id === wayId || d.id === node.id; })
             .classed('active', true);
 
+        function ReplaceTemporaryNode(replacementId) {
+            return function(graph) {
+                graph = graph.replace(graph.entity(wayId).updateNode(replacementId, index));
+                graph = graph.remove(node);
+                return graph;
+            }
+        }
+
         function mousemove() {
             history.replace(iD.actions.MoveNode(node.id, map.mouseCoordinates()));
         }
@@ -46,8 +54,7 @@ iD.modes.DrawLine = function(wayId, direction) {
                 // connect the way in a loop
                 if (way.nodes.length > 2) {
                     history.replace(
-                        iD.actions.DeleteNode(node.id),
-                        iD.actions.AddWayNode(wayId, tailId, index),
+                        ReplaceTemporaryNode(tailId),
                         'added to a line');
 
                     controller.enter(iD.modes.Select(way, true));
@@ -66,8 +73,7 @@ iD.modes.DrawLine = function(wayId, direction) {
             } else if (datum.type === 'node' && datum.id !== node.id) {
                 // connect the way to an existing node
                 history.replace(
-                    iD.actions.DeleteNode(node.id),
-                    iD.actions.AddWayNode(wayId, datum.id, index),
+                    ReplaceTemporaryNode(datum.id),
                     'added to a line');
 
                 controller.enter(iD.modes.DrawLine(wayId, direction));
@@ -106,11 +112,11 @@ iD.modes.DrawLine = function(wayId, direction) {
                 iD.actions.DeleteNode(node.id),
                 iD.actions.DeleteNode(headId));
 
-            if (history.graph().fetch(wayId).nodes.length === 0) {
-                history.replace(iD.actions.DeleteWay(wayId));
-                controller.enter(iD.modes.Browse());
-            } else {
+            if (history.graph().entity(wayId)) {
                 controller.enter(iD.modes.DrawLine(wayId, direction));
+            } else {
+                // The way was deleted because it had too few nodes.
+                controller.enter(iD.modes.Browse());
             }
         }
 
@@ -122,8 +128,15 @@ iD.modes.DrawLine = function(wayId, direction) {
 
         function ret() {
             d3.event.preventDefault();
+
             history.replace(iD.actions.DeleteNode(node.id));
-            controller.enter(iD.modes.Select(way, true));
+
+            if (history.graph().entity(wayId)) {
+                controller.enter(iD.modes.Select(way, true));
+            } else {
+                // The way was deleted because it had too few nodes.
+                controller.enter(iD.modes.Browse());
+            }
         }
 
         surface
diff --git a/js/id/modes/select.js b/js/id/modes/select.js
index 2b92cfedd..d80d6fffd 100644
--- a/js/id/modes/select.js
+++ b/js/id/modes/select.js
@@ -15,13 +15,9 @@ iD.modes.Select = function(entity, initial) {
                 iD.actions.DeleteWay(entity.id),
                 'deleted a way');
         } else if (entity.type === 'node') {
-            var parents = mode.history.graph().parentWays(entity),
-                operations = [iD.actions.DeleteNode(entity.id)];
-            parents.forEach(function(parent) {
-                if (_.uniq(parent.nodes).length === 1) operations.push(iD.actions.DeleteWay(parent.id));
-            });
-            mode.history.perform.apply(mode.history,
-                operations.concat(['deleted a node']));
+            mode.history.perform(
+                iD.actions.DeleteNode(entity.id),
+                'deleted a node');
         }
 
         mode.controller.exit();
diff --git a/test/spec/actions/delete_node.js b/test/spec/actions/delete_node.js
index a73f7e33f..93860117f 100644
--- a/test/spec/actions/delete_node.js
+++ b/test/spec/actions/delete_node.js
@@ -24,4 +24,22 @@ describe("iD.actions.DeleteNode", function () {
             graph    = action(iD.Graph([node1, node2, relation]));
         expect(graph.entity(relation.id).members).to.eql([{ id: node2.id }]);
     });
+
+    it("deletes parent ways that would otherwise have less than two nodes", function () {
+        var node1  = iD.Node(),
+            node2  = iD.Node(),
+            way    = iD.Way({nodes: [node1.id, node2.id]}),
+            action = iD.actions.DeleteNode(node1.id),
+            graph  = action(iD.Graph([node1, node2, way]));
+        expect(graph.entity(way.id)).to.be.undefined;
+    });
+
+    it("deletes degenerate circular ways", function () {
+        var node1  = iD.Node(),
+            node2  = iD.Node(),
+            way    = iD.Way({nodes: [node1.id, node2.id, node1.id]}),
+            action = iD.actions.DeleteNode(node2.id),
+            graph  = action(iD.Graph([node1, node2, way]));
+        expect(graph.entity(way.id)).to.be.undefined;
+    });
 });
diff --git a/test/spec/graph/way.js b/test/spec/graph/way.js
index 0c70ea692..87d7d0029 100644
--- a/test/spec/graph/way.js
+++ b/test/spec/graph/way.js
@@ -117,6 +117,31 @@ describe('iD.Way', function() {
         });
     });
 
+    describe("#isDegenerate", function() {
+       it("returns true for a linear way with zero or one nodes", function () {
+           expect(iD.Way({nodes: []}).isDegenerate()).to.equal(true);
+           expect(iD.Way({nodes: ['a']}).isDegenerate()).to.equal(true);
+       });
+
+        it("returns true for a circular way with only one unique node", function () {
+            expect(iD.Way({nodes: ['a', 'a']}).isDegenerate()).to.equal(true);
+        });
+
+        it("returns false for a linear way with two or more nodes", function () {
+            expect(iD.Way({nodes: ['a', 'b']}).isDegenerate()).to.equal(false);
+        });
+
+        it("returns true for an area with zero, one, or two unique nodes", function () {
+            expect(iD.Way({tags: {area: 'yes'}, nodes: []}).isDegenerate()).to.equal(true);
+            expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'a']}).isDegenerate()).to.equal(true);
+            expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'b', 'a']}).isDegenerate()).to.equal(true);
+        });
+
+        it("returns false for an area with three or more unique nodes", function () {
+            expect(iD.Way({tags: {area: 'yes'}, nodes: ['a', 'b', 'c', 'a']}).isDegenerate()).to.equal(false);
+        });
+    });
+
     describe("#geometry", function() {
         it("returns 'line' when the way is not an area", function () {
             expect(iD.Way().geometry()).to.equal('line');

From 18b33310032e5df383c50a46016c51841b1d1d96 Mon Sep 17 00:00:00 2001
From: John Firebaugh <john.firebaugh@gmail.com>
Date: Wed, 23 Jan 2013 12:39:45 -0500
Subject: [PATCH 3/3] Fix event reference

---
 js/id/id.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/js/id/id.js b/js/id/id.js
index 9d7fb38bf..3646ba074 100644
--- a/js/id/id.js
+++ b/js/id/id.js
@@ -192,7 +192,7 @@ window.iD = function(container) {
             .on('⌃+Z', function() { history.undo(); })
             .on('⌘+⇧+Z', function() { history.redo(); })
             .on('⌃+⇧+Z', function() { history.redo(); })
-            .on('⌫', function(e) { e.preventDefault(); });
+            .on('⌫', function() { d3.event.preventDefault(); });
 
         d3.select(document)
             .call(keybinding);