From 3f12dd510704025bdbe27553af40c71c5f953f24 Mon Sep 17 00:00:00 2001 From: Kyle Hensel Date: Tue, 21 Sep 2021 12:34:32 +1200 Subject: [PATCH] keep the oldest Way when merging --- modules/actions/join.js | 16 +++++++++------- test/spec/actions/join.js | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/modules/actions/join.js b/modules/actions/join.js index 75c7469c9..3515a243b 100644 --- a/modules/actions/join.js +++ b/modules/actions/join.js @@ -27,7 +27,6 @@ export function actionJoin(ids) { var action = function(graph) { var ways = ids.map(graph.entity, graph); - var survivorID = ways[0].id; // if any of the ways are sided (e.g. coastline, cliff, kerb) // sort them first so they establish the overall order - #6033 @@ -40,12 +39,15 @@ export function actionJoin(ids) { }); // Prefer to keep an existing way. - for (var i = 0; i < ways.length; i++) { - if (!ways[i].isNew()) { - survivorID = ways[i].id; - break; - } - } + // if there are mulitple existing ways, keep the oldest one + // the oldest way is determined by the ID of the way + const survivorID = ( + ways + .filter((way) => !way.isNew()) + .sort((a, b) => +a.osmId() - +b.osmId())[0] || ways[0] + ).id; + + var sequences = osmJoinWays(ways, graph); var joined = sequences[0]; diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index deb30e77f..fecc421de 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -424,6 +424,29 @@ describe('iD.actionJoin', function () { expect(graph.hasEntity('w-2')).to.be.undefined; }); + it('prefers to keep the oldest way', function () { + // n1 ==> n2 ++> n3 --> n4 + // ==> is existing, ++> is existing, --> is new + // Expected result: + // n1 ==> n2 ==> n3 ==> n4 + var graph = iD.coreGraph([ + iD.osmNode({ id: 'n1', loc: [0,0] }), + iD.osmNode({ id: 'n2', loc: [2,0] }), + iD.osmNode({ id: 'n3', loc: [4,0] }), + iD.osmNode({ id: 'n4', loc: [6,0] }), + iD.osmWay({ id: 'w1', nodes: ['n2', 'n3'] }), + iD.osmWay({ id: 'w2', nodes: ['n1', 'n2'] }), + iD.osmWay({ id: 'w-1', nodes: ['n3', 'n4'] }) + ]); + + graph = iD.actionJoin(['w1', 'w2', 'w-1'])(graph); + + // way 1 is the oldest (it has the lower id) so it kept that one + expect(graph.entity('w1').nodes).to.eql(['n1', 'n2', 'n3', 'n4']); + expect(graph.hasEntity('w2')).to.be.undefined; + expect(graph.hasEntity('w-1')).to.be.undefined; + }); + it('merges tags', function () { var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0,0]}),