From f5995dffb02b05d0f1d7bee94904a5208c2499b5 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Thu, 3 Oct 2019 20:09:49 +0200 Subject: [PATCH 1/2] Cherry pick crossing buildings fix --- data/core.yaml | 4 ++-- dist/locales/en.json | 4 ++-- modules/ui/fields/radio.js | 4 ++-- modules/validations/crossing_ways.js | 25 +++++++------------------ 4 files changed, 13 insertions(+), 24 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 216531433..021bbc552 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1369,9 +1369,9 @@ en: building-building: reference: "Buildings should not intersect except on different layers." building-highway: - reference: "Highways crossing buildings should use bridges, tunnels, coverings, or entrances." + reference: "Highways crossing buildings should use bridges, tunnels, or different layers." building-railway: - reference: "Railways crossing buildings should use bridges or tunnels." + reference: "Railways crossing buildings should use bridges, tunnels, or different layers." building-waterway: reference: "Waterways crossing buildings should use tunnels or different layers." highway-highway: diff --git a/dist/locales/en.json b/dist/locales/en.json index d95cc845c..80cf3b440 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1682,10 +1682,10 @@ "reference": "Buildings should not intersect except on different layers." }, "building-highway": { - "reference": "Highways crossing buildings should use bridges, tunnels, coverings, or entrances." + "reference": "Highways crossing buildings should use bridges, tunnels, or different layers." }, "building-railway": { - "reference": "Railways crossing buildings should use bridges or tunnels." + "reference": "Railways crossing buildings should use bridges, tunnels, or different layers." }, "building-waterway": { "reference": "Waterways crossing buildings should use tunnels or different layers." diff --git a/modules/ui/fields/radio.js b/modules/ui/fields/radio.js index 3b8bc5a86..601be2470 100644 --- a/modules/ui/fields/radio.js +++ b/modules/ui/fields/radio.js @@ -75,10 +75,10 @@ export function uiFieldRadio(field, context) { function structureExtras(selection, tags) { - var selected = selectedKey(); + var selected = selectedKey() || tags.layer !== undefined; var type = context.presets().field(selected); var layer = context.presets().field('layer'); - var showLayer = (selected === 'bridge' || selected === 'tunnel'); + var showLayer = (selected === 'bridge' || selected === 'tunnel' || tags.layer !== undefined); var extrasWrap = selection.selectAll('.structure-extras-wrap') diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js index 08f2c7d4b..eeb4847b9 100644 --- a/modules/validations/crossing_ways.js +++ b/modules/validations/crossing_ways.js @@ -49,18 +49,12 @@ export function validationCrossingWays(context) { return hasTag(tags, 'level') || tags.highway === 'corridor'; } - function allowsStructures(featureType) { - return allowsBridge(featureType) || allowsTunnel(featureType); - } function allowsBridge(featureType) { return featureType === 'highway' || featureType === 'railway' || featureType === 'waterway'; } function allowsTunnel(featureType) { return featureType === 'highway' || featureType === 'railway' || featureType === 'waterway'; } - function canCover(featureType) { - return featureType === 'building'; - } function getFeatureTypeForCrossingCheck(way, graph) { @@ -121,20 +115,12 @@ export function validationCrossingWays(context) { } else if (allowsTunnel(featureType1) && hasTag(tags1, 'tunnel')) return true; else if (allowsTunnel(featureType2) && hasTag(tags2, 'tunnel')) return true; - if (canCover(featureType1) && canCover(featureType2)) { - if (hasTag(tags1, 'covered') && !hasTag(tags2, 'covered')) return true; - if (!hasTag(tags1, 'covered') && hasTag(tags2, 'covered')) return true; - // crossing covered features that can themselves cover must use different layers - if (hasTag(tags1, 'covered') && hasTag(tags2, 'covered') && layer1 !== layer2) return true; - } else if (canCover(featureType1) && hasTag(tags2, 'covered')) return true; - else if (canCover(featureType2) && hasTag(tags1, 'covered')) return true; - // don't flag crossing waterways and pier/highways if (featureType1 === 'waterway' && featureType2 === 'highway' && tags2.man_made === 'pier') return true; if (featureType2 === 'waterway' && featureType1 === 'highway' && tags1.man_made === 'pier') return true; - if (!allowsStructures(featureType1) && !allowsStructures(featureType2)) { - // if no structures are applicable, the layers must be different + if (featureType1 === 'building' || featureType2 === 'building') { + // for building crossings, different layers are enough if (layer1 !== layer2) return true; } return false; @@ -423,10 +409,13 @@ export function validationCrossingWays(context) { } else { useFixID = 'use_different_layers'; } - if (useFixID === 'use_different_layers') { + if (useFixID === 'use_different_layers' || + featureType1 === 'building' || + featureType2 === 'building') { fixes.push(makeChangeLayerFix('higher')); fixes.push(makeChangeLayerFix('lower')); - } else { + } + if (useFixID !== 'use_different_layers') { fixes.push(new validationIssueFix({ icon: useFixIcon, title: t('issues.fix.' + useFixID + '.title') From d3d2cbb4a254ee0cbc0d5912d11b13338fb168db Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 15 May 2019 14:56:04 -0400 Subject: [PATCH 2/2] Update code tests for building layer crossings --- test/spec/validations/crossing_ways.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/spec/validations/crossing_ways.js b/test/spec/validations/crossing_ways.js index 7e3dadc9e..5d719edac 100644 --- a/test/spec/validations/crossing_ways.js +++ b/test/spec/validations/crossing_ways.js @@ -99,7 +99,7 @@ describe('iD.validations.crossing_ways', function () { }); it('legit crossing between highway and building', function() { - createWaysWithOneCrossingPoint({ highway: 'residential', covered: 'yes' }, { building: 'yes' }); + createWaysWithOneCrossingPoint({ highway: 'residential', layer: '-1' }, { building: 'yes' }); var issues = validate(); expect(issues).to.have.lengthOf(0); }); @@ -123,7 +123,7 @@ describe('iD.validations.crossing_ways', function () { }); it('legit crossing between railway and building', function() { - createWaysWithOneCrossingPoint({ railway: 'rail', covered: 'yes' }, { building: 'yes' }); + createWaysWithOneCrossingPoint({ railway: 'rail', layer: '-1' }, { building: 'yes' }); var issues = validate(); expect(issues).to.have.lengthOf(0); }); @@ -135,13 +135,13 @@ describe('iD.validations.crossing_ways', function () { }); it('legit crossing between waterway and building', function() { - createWaysWithOneCrossingPoint({ waterway: 'river', covered: 'yes' }, { building: 'yes' }); + createWaysWithOneCrossingPoint({ waterway: 'river', layer: '-1' }, { building: 'yes' }); var issues = validate(); expect(issues).to.have.lengthOf(0); }); it('legit crossing between building and building', function() { - createWaysWithOneCrossingPoint({ building: 'yes' }, { building: 'yes', covered: 'yes' }); + createWaysWithOneCrossingPoint({ building: 'yes' }, { building: 'yes', layer: '1' }); var issues = validate(); expect(issues).to.have.lengthOf(0); });