Don't consider untagged multipolygons as old multipolygons

Also add a lot of old-style multipolygon tests
(closes #4009)

modules/osm/multipolygon.js

@@ -6,7 +6,7 @@ import { osmIsInterestingTag } from './tags';
 // For fixing up rendering of multipolygons with tags on the outer member.
 // https://github.com/openstreetmap/iD/issues/613
 export function osmIsSimpleMultipolygonOuterMember(entity, graph) {
-    if (entity.type !== 'way')
+    if (entity.type !== 'way' || Object.keys(entity.tags).filter(osmIsInterestingTag).length === 0)
         return false;
 
     var parents = graph.parentRelations(entity);
@@ -52,7 +52,14 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) {
         }
     }
 
-    return outerMember && graph.hasEntity(outerMember.id);
+    if (!outerMember)
+        return false;
+
+    var outerEntity = graph.hasEntity(outerMember.id);
+    if (!outerEntity || !Object.keys(outerEntity.tags).filter(osmIsInterestingTag).length)
+        return false;
+
+    return outerEntity;
 }
 
 

test/spec/osm/multipolygon.js

@@ -1,7 +1,100 @@
+describe('iD.osmIsSimpleMultipolygonOuterMember', function() {
+    it('returns the parent relation of a simple multipolygon outer', function() {
+        var outer = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: outer.id, role: 'outer'}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation);
+    });
+
+    it('returns the parent relation of a simple multipolygon outer, assuming role outer if unspecified', function() {
+        var outer = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: outer.id}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation);
+    });
+
+    it('returns false if entity is not a way', function() {
+        var outer = iD.Node({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: outer.id, role: 'outer'}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
+    });
+
+    it('returns false if entity does not have interesting tags', function() {
+        var outer = iD.Way({tags: {'tiger:reviewed':'no'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: outer.id, role: 'outer'}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
+    });
+
+    it('returns false if entity does not have a parent relation', function() {
+        var outer = iD.Way({tags: {'natural':'wood'}}),
+            graph = iD.Graph([outer]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
+    });
+
+    it('returns false if the parent is not a multipolygon', function() {
+        var outer = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'route'},
+                members: [{id: outer.id, role: 'outer'}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
+    });
+
+    it('returns false if the parent has interesting tags', function() {
+        var outer = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {natural: 'wood', type: 'multipolygon'},
+                members: [{id: outer.id, role: 'outer'}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.be.false;
+    });
+
+    it('returns the parent relation of a simple multipolygon outer, ignoring uninteresting parent tags', function() {
+        var outer = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {'tiger:reviewed':'no', type: 'multipolygon'},
+                members: [{id: outer.id, role: 'outer'}]}),
+            graph = iD.Graph([outer, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer, graph)).to.equal(relation);
+    });
+
+    it('returns false if the parent has multiple outer ways', function() {
+        var outer1 = iD.Way({tags: {'natural':'wood'}}),
+            outer2 = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: outer1.id, role: 'outer'}, {id: outer2.id, role: 'outer'}]}),
+            graph = iD.Graph([outer1, outer2, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer1, graph)).to.be.false;
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer2, graph)).to.be.false;
+    });
+
+    it('returns false if the parent has multiple outer ways, assuming role outer if unspecified', function() {
+        var outer1 = iD.Way({tags: {'natural':'wood'}}),
+            outer2 = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: outer1.id}, {id: outer2.id}]}),
+            graph = iD.Graph([outer1, outer2, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer1, graph)).to.be.false;
+        expect(iD.osmIsSimpleMultipolygonOuterMember(outer2, graph)).to.be.false;
+    });
+
+    it('returns false if the entity is not an outer', function() {
+        var inner = iD.Way({tags: {'natural':'wood'}}),
+            relation = iD.Relation({tags: {type: 'multipolygon'},
+                members: [{id: inner.id, role: 'inner'}]}),
+            graph = iD.Graph([inner, relation]);
+        expect(iD.osmIsSimpleMultipolygonOuterMember(inner, graph)).to.be.false;
+    });
+});
+
+
 describe('iD.osmSimpleMultipolygonOuterMember', function() {
     it('returns the outer member of a simple multipolygon', function() {
         var inner = iD.Way(),
-            outer = iD.Way(),
+            outer = iD.Way({tags: {'natural':'wood'}}),
             relation = iD.Relation({tags: {type: 'multipolygon'}, members: [
                 {id: outer.id, role: 'outer'},
                 {id: inner.id, role: 'inner'}]
@@ -14,8 +107,8 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() {
 
     it('returns falsy for a complex multipolygon', function() {
         var inner = iD.Way(),
-            outer1 = iD.Way(),
-            outer2 = iD.Way(),
+            outer1 = iD.Way({tags: {'natural':'wood'}}),
+            outer2 = iD.Way({tags: {'natural':'wood'}}),
             relation = iD.Relation({tags: {type: 'multipolygon'}, members: [
                 {id: outer1.id, role: 'outer'},
                 {id: outer2.id, role: 'outer'},
@@ -36,7 +129,7 @@ describe('iD.osmSimpleMultipolygonOuterMember', function() {
             }),
             graph = iD.Graph([way, relation]);
 
-        expect(iD.osmSimpleMultipolygonOuterMember(way, graph)).to.be.undefined;
+        expect(iD.osmSimpleMultipolygonOuterMember(way, graph)).not.to.be.ok;
     });
 });
 

test/spec/svg/lines.js

@@ -56,27 +56,31 @@ describe('iD.svgLines', function () {
         var a = iD.Node({loc: [1, 1]}),
             b = iD.Node({loc: [2, 2]}),
             c = iD.Node({loc: [3, 3]}),
-            w = iD.Way({tags: {natural: 'wood'}, nodes: [a.id, b.id, c.id, a.id]}),
+            w = iD.Way({id: 'w-1', tags: {natural: 'wood'}, nodes: [a.id, b.id, c.id, a.id]}),
             r = iD.Relation({members: [{id: w.id}], tags: {type: 'multipolygon'}}),
             graph = iD.Graph([a, b, c, w, r]);
 
         surface.call(iD.svgLines(projection, context), graph, [w], all);
 
-        expect(surface.select('.stroke').classed('tag-natural-wood')).to.be.true;
+        expect(surface.select('.stroke.w-1').classed('tag-natural-wood')).to.equal(true, 'outer tag-natural-wood true');
+        expect(surface.select('.stroke.w-1').classed('old-multipolygon')).to.equal(true, 'outer old-multipolygon true');
     });
 
     it('adds stroke classes for the tags of the outer way of multipolygon with tags on the outer way', function() {
         var a = iD.Node({loc: [1, 1]}),
             b = iD.Node({loc: [2, 2]}),
             c = iD.Node({loc: [3, 3]}),
-            o = iD.Way({tags: {natural: 'wood'}, nodes: [a.id, b.id, c.id, a.id]}),
-            i = iD.Way({nodes: [a.id, b.id, c.id, a.id]}),
+            o = iD.Way({id: 'w-1', nodes: [a.id, b.id, c.id, a.id], tags: {natural: 'wood'}}),
+            i = iD.Way({id: 'w-2', nodes: [a.id, b.id, c.id, a.id]}),
             r = iD.Relation({members: [{id: o.id, role: 'outer'}, {id: i.id, role: 'inner'}], tags: {type: 'multipolygon'}}),
             graph = iD.Graph([a, b, c, o, i, r]);
 
-        surface.call(iD.svgLines(projection, context), graph, [i], all);
+        surface.call(iD.svgLines(projection, context), graph, [i, o], all);
 
-        expect(surface.select('.stroke').classed('tag-natural-wood')).to.be.true;
+        expect(surface.select('.stroke.w-1').classed('tag-natural-wood')).to.equal(true, 'outer tag-natural-wood true');
+        expect(surface.select('.stroke.w-1').classed('old-multipolygon')).to.equal(true, 'outer old-multipolygon true');
+        expect(surface.select('.stroke.w-2').classed('tag-natural-wood')).to.equal(true, 'inner tag-natural-wood true');
+        expect(surface.select('.stroke.w-2').classed('old-multipolygon')).to.equal(false, 'inner old-multipolygon false');
     });
 
     describe('z-indexing', function() {