From c2713c3a3f95eb20b0c7e37e4ae3f918c7032ee8 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 31 Dec 2016 02:01:13 -0500 Subject: [PATCH] For node-way merge, preserve original node if possible (closes #3683) --- modules/actions/merge.js | 25 ++++++++++++++++++++++--- modules/operations/merge.js | 5 ++++- test/spec/actions/merge.js | 19 +++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/modules/actions/merge.js b/modules/actions/merge.js index a2ff40ffc..2797253f7 100644 --- a/modules/actions/merge.js +++ b/modules/actions/merge.js @@ -17,15 +17,34 @@ export function actionMerge(ids) { points.forEach(function(point) { target = target.mergeTags(point.tags); + graph = graph.replace(target); graph.parentRelations(point).forEach(function(parent) { graph = graph.replace(parent.replaceMember(point, target)); }); - graph = graph.remove(point); - }); + var nodes = _.uniq(graph.childNodes(target)), + removeNode = point; - graph = graph.replace(target); + for (var i = 0; i < nodes.length; i++) { + var node = nodes[i]; + if (graph.parentWays(node).length > 1 || + graph.parentRelations(node).length || + node.hasInterestingTags()) { + continue; + } + + // Found an uninteresting child node on the target way. + // Move orig point into its place to preserve point's history. #3683 + graph = graph.replace(point.update({ tags: {}, loc: node.loc })); + target = target.replaceNode(node.id, point.id); + graph = graph.replace(target); + removeNode = node; + break; + } + + graph = graph.remove(removeNode); + }); return graph; }; diff --git a/modules/operations/merge.js b/modules/operations/merge.js index d58a9a8bf..c55e3c350 100644 --- a/modules/operations/merge.js +++ b/modules/operations/merge.js @@ -27,7 +27,10 @@ export function operationMerge(selectedIDs, context) { } context.perform(action, annotation); - var ids = selectedIDs.filter(function(id) { return context.hasEntity(id); }); + var ids = selectedIDs.filter(function(id) { + var entity = context.hasEntity(id); + return entity && entity.type !== 'node'; + }); context.enter(modeSelect(context, ids).suppressMenu(true)); }; diff --git a/test/spec/actions/merge.js b/test/spec/actions/merge.js index 6b23ed9a8..dc3d27059 100644 --- a/test/spec/actions/merge.js +++ b/test/spec/actions/merge.js @@ -36,4 +36,23 @@ describe('iD.actionMerge', function () { expect(graph.entity('w').tags).to.eql({a: 'a', b: 'b', area: 'yes'}); expect(graph.entity('r').members).to.eql([{id: 'w', role: 'r', type: 'way'}]); }); + + it('preserves original point if possible', function () { + var graph = iD.Graph([ + iD.Node({id: 'a', loc: [1, 0], tags: {a: 'a'}}), + iD.Node({id: 'p', loc: [0, 0], tags: {p: 'p'}}), + iD.Node({id: 'q', loc: [0, 1]}), + iD.Way({id: 'w', nodes: ['p', 'q'], tags: {w: 'w'}}) + ]), + action = iD.actionMerge(['a', 'w']); + + graph = action(graph); + expect(graph.hasEntity('a')).to.be.ok; + expect(graph.hasEntity('p')).to.be.ok; + expect(graph.hasEntity('q')).to.be.undefined; + expect(graph.entity('w').tags).to.eql({a: 'a', w: 'w'}); + expect(graph.entity('w').nodes).to.eql(['p', 'a']); + expect(graph.entity('a').loc[0]).to.eql(0); + expect(graph.entity('a').loc[1]).to.eql(1); + }); });