From 422f76be7732f50d083f82fafef303a700f64b67 Mon Sep 17 00:00:00 2001
From: tyr <tyr.asd@gmail.com>
Date: Sun, 9 Feb 2014 17:42:58 +0100
Subject: [PATCH] merge-polygons: do not merge tags from all possible members

previously when merging polygons, all tags from all involved
members were merged into the resulting multipolygon. This includes
existing inner members with different tags or outer members that
are not directly involved in the merge operations.

Example: A forest (landuse=forest) containing some clearings
(landuse=meadow), whose outline is in part a river (waterway=river)
is merged with a new untagged area. The resulting multipolygon
would have falsely been tagged: landuse=forest;meadow + waterway=river
---
 js/id/actions/merge_polygon.js     | 13 +++++++++----
 test/spec/actions/merge_polygon.js | 13 +++++++++----
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/js/id/actions/merge_polygon.js b/js/id/actions/merge_polygon.js
index 4ffde0e4c..6dda35581 100644
--- a/js/id/actions/merge_polygon.js
+++ b/js/id/actions/merge_polygon.js
@@ -86,10 +86,15 @@ iD.actions.MergePolygon = function(ids, newRelationId) {
             graph = graph.remove(m);
         });
 
-        members.forEach(function(m) {
-            var entity = graph.entity(m.id);
-            relation = relation.mergeTags(entity.tags);
-            graph = graph.replace(entity.update({ tags: {} }));
+        entities.closedWay.forEach(function(cw) {
+            function isThisOuter(m) {
+                return m.id === cw.id && m.role !== "inner";
+            }
+            if (members.some(isThisOuter)) {
+                var entity = graph.entity(cw.id);
+                relation = relation.mergeTags(entity.tags);
+                graph = graph.replace(entity.update({ tags: {} }));
+            }
         });
 
         return graph.replace(relation.update({
diff --git a/test/spec/actions/merge_polygon.js b/test/spec/actions/merge_polygon.js
index 5270ce29f..85ac68f8e 100644
--- a/test/spec/actions/merge_polygon.js
+++ b/test/spec/actions/merge_polygon.js
@@ -85,15 +85,20 @@ describe("iD.actions.MergePolygon", function () {
         expect(find(r, 'w5').role).to.equal('outer');
     });
 
-    it("moves all tags to the relation", function() {
+    it("moves tags to the relation", function() {
         graph = graph.replace(graph.entity('w0').update({ tags: { 'building': 'yes' }}));
+        // this is a new inner member whose tags should not be moved to the relation
         graph = graph.replace(graph.entity('w1').update({ tags: { 'natural': 'water' }}));
-        graph = iD.actions.MergePolygon(['w0', 'w1'], 'r')(graph);
+        // this is a new outer member whose tags should be moved to the relation
+        graph = graph.replace(graph.entity('w5').update({ tags: { 'amenity': 'school' }}));
+        graph = iD.actions.MergePolygon(['w0', 'w1','w5'], 'r')(graph);
         var r = graph.entity('r');
         expect(graph.entity('w0').tags.building).to.equal(undefined);
-        expect(graph.entity('w1').tags.natural).to.equal(undefined);
-        expect(r.tags.natural).to.equal('water');
+        expect(graph.entity('w1').tags.natural).to.equal('water');
+        expect(graph.entity('w5').tags.amenity).to.equal(undefined);
         expect(r.tags.building).to.equal('yes');
+        expect(r.tags.natural).to.equal(undefined);
+        expect(r.tags.amenity).to.equal('school');
     });
 
     it("doesn't copy area tags from ways", function() {