diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 63871cd16..efdf603fa 100644 --- a/modules/osm/multipolygon.js +++ b/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; } diff --git a/test/spec/osm/multipolygon.js b/test/spec/osm/multipolygon.js index e903bc1f1..e7df436e1 100644 --- a/test/spec/osm/multipolygon.js +++ b/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; }); }); diff --git a/test/spec/svg/lines.js b/test/spec/svg/lines.js index bcd268443..c1748eeff 100644 --- a/test/spec/svg/lines.js +++ b/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() {