diff --git a/data/core.yaml b/data/core.yaml index e6f4dd7e1..bc75054af 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1211,7 +1211,7 @@ en: tip: Crossing bridges should use different layers. bridge-bridge_connectable: tip: Crossing bridges should be connected or use different layers. - deprecated_tags: + deprecated_tag: message: '{feature} has outdated tags: {tags}' tip: Some tags become deprecated over time and should be replaced. disconnected_way: @@ -1224,15 +1224,15 @@ en: many_deletions: message: "Deleting {n} features: {p} nodes, {l} lines, {a} areas, and {r} relations." tip: Only redundant or nonexistent features should be deleted. + missing_tag: + message: "{feature} has no descriptive tags." + tip: Features must have tags that define what they are. old_multipolygon: message: "{multipolygon} has misplaced tags." tip: Multipolygons should be tagged on their relation, not their outer way. tag_suggests_area: message: 'Feature is a line, but the tag "{tag}" suggests it should be an area.' tip: Areas must have connected endpoints. - untagged_feature: - message: "{feature} has no descriptive tags." - tip: Features must have tags that define what they are. fix: add_connection_vertex: title: Connect the features diff --git a/dist/locales/en.json b/dist/locales/en.json index 50f5b58d7..9c4edf2a0 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1479,7 +1479,7 @@ "tip": "Crossing bridges should be connected or use different layers." } }, - "deprecated_tags": { + "deprecated_tag": { "message": "{feature} has outdated tags: {tags}", "tip": "Some tags become deprecated over time and should be replaced." }, @@ -1497,6 +1497,10 @@ "message": "Deleting {n} features: {p} nodes, {l} lines, {a} areas, and {r} relations.", "tip": "Only redundant or nonexistent features should be deleted." }, + "missing_tag": { + "message": "{feature} has no descriptive tags.", + "tip": "Features must have tags that define what they are." + }, "old_multipolygon": { "message": "{multipolygon} has misplaced tags.", "tip": "Multipolygons should be tagged on their relation, not their outer way." @@ -1505,10 +1509,6 @@ "message": "Feature is a line, but the tag \"{tag}\" suggests it should be an area.", "tip": "Areas must have connected endpoints." }, - "untagged_feature": { - "message": "{feature} has no descriptive tags.", - "tip": "Features must have tags that define what they are." - }, "fix": { "add_connection_vertex": { "title": "Connect the features", diff --git a/modules/core/validator.js b/modules/core/validator.js index 1ccce6aec..870c18e96 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -100,13 +100,16 @@ export function coreValidator(context) { // other validations require feature to be tagged if (!runValidation('missing_tag')) return issues; if (entity.type === 'way') { + + runValidation('crossing_ways'); + // only check for disconnected way if no almost junctions if (runValidation('almost_junction')) { runValidation('disconnected_way'); } else { ranValidations.add('disconnected_way'); } - runValidation('crossing_ways'); + runValidation('tag_suggests_area'); } // run all validations not yet run manually diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js index 178e0ea56..8ff3b636a 100644 --- a/modules/validations/almost_junction.js +++ b/modules/validations/almost_junction.js @@ -106,6 +106,8 @@ export function validationAlmostJunction() { return null; } + var type = 'almost_junction'; + var validation = function(endHighway, context) { var graph = context.graph(); var tree = context.history().tree(); @@ -144,7 +146,7 @@ export function validationAlmostJunction() { })); } issues.push(new validationIssue({ - type: 'almost_junction', + type: type, severity: 'warning', message: t('issues.almost_junction.message', { feature: utilDisplayLabel(endHighway, context), @@ -164,7 +166,7 @@ export function validationAlmostJunction() { return issues; }; - validation.type = 'almost_junction'; + validation.type = type; return validation; } diff --git a/modules/validations/crossing_ways.js b/modules/validations/crossing_ways.js index 053afe927..89c04a560 100644 --- a/modules/validations/crossing_ways.js +++ b/modules/validations/crossing_ways.js @@ -240,12 +240,13 @@ export function validationCrossingWays() { return edgeCrossInfos; } + var type = 'crossing_ways'; + var validation = function(entity, context) { var graph = context.graph(); var tree = context.history().tree(); - // create one issue per crossing point - var issues = []; + var waysToCheck = _flattenDeep(_map([entity], function(entity) { if (!getFeatureTypeForTags(entity.tags)) { return []; @@ -268,103 +269,110 @@ export function validationCrossingWays() { } return []; })); + var crossings = waysToCheck.reduce(function(array, way) { return array.concat(findCrossingsByWay(way, graph, tree)); }, []); - for (var j in crossings) { - var crossing = crossings[j]; - - // use the entities with the tags that define the feature type - var entities = crossing.ways.sort(function(entity1, entity2) { - var type1 = getFeatureTypeForCrossingCheck(entity1, graph); - var type2 = getFeatureTypeForCrossingCheck(entity2, graph); - if (type1 === type2) { - return utilDisplayLabel(entity1, context) > utilDisplayLabel(entity2, context); - } else if (type1 === 'waterway') { - return true; - } else if (type2 === 'waterway') { - return false; - } - return type1 < type2; - }); - entities = _map(entities, function(way) { - return getFeatureWithFeatureTypeTagsForWay(way, graph); - }); - - var connectionTags = tagsForConnectionNodeIfAllowed(entities[0], entities[1]); - - var crossingTypeID; - if (hasTag(entities[0].tags, 'tunnel') && hasTag(entities[1].tags, 'tunnel')) { - crossingTypeID = 'tunnel-tunnel'; - if (connectionTags) { - crossingTypeID += '_connectable'; - } - } - else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) { - crossingTypeID = 'bridge-bridge'; - if (connectionTags) { - crossingTypeID += '_connectable'; - } - } - else { - crossingTypeID = crossing.featureTypes.sort().join('-'); - } - - var messageDict = { - feature: utilDisplayLabel(entities[0], context), - feature2: utilDisplayLabel(entities[1], context) - }; - - var fixes = []; - if (connectionTags) { - fixes.push(new validationIssueFix({ - title: t('issues.fix.add_connection_vertex.title'), - onClick: function() { - var loc = this.issue.coordinates; - var ways = this.issue.info.ways; - var connectionTags = this.issue.info.connectionTags; - - context.perform( - function actionConnectCrossingWays(graph) { - var projection = context.projection; - - var node = osmNode({ loc: loc, tags: connectionTags }); - graph = graph.replace(node); - - var way0 = graph.entity(ways[0].id); - var choice0 = geoChooseEdge(graph.childNodes(way0), projection(loc), projection); - var edge0 = [way0.nodes[choice0.index - 1], way0.nodes[choice0.index]]; - graph = actionAddMidpoint({loc: loc, edge: edge0}, node)(graph); - - var way1 = graph.entity(ways[1].id); - var choice1 = geoChooseEdge(graph.childNodes(way1), projection(loc), projection); - var edge1 = [way1.nodes[choice1.index - 1], way1.nodes[choice1.index]]; - graph = actionAddMidpoint({loc: loc, edge: edge1}, node)(graph); - - return graph; - }, - t('issues.fix.add_connection_vertex.undo_redo') - ); - } - })); - } - - issues.push(new validationIssue({ - type: 'crossing_ways', - severity: 'warning', - message: t('issues.crossing_ways.message', messageDict), - tooltip: t('issues.crossing_ways.'+crossingTypeID+'.tip'), - entities: entities, - info: { ways: crossing.ways, connectionTags: connectionTags }, - coordinates: crossing.cross_point, - fixes: fixes - })); - } + var issues = []; + crossings.forEach(function(crossing) { + issues.push(createIssue(crossing, context)); + }); return issues; }; - validation.type = 'crossing_ways'; + function createIssue(crossing, context) { + + var graph = context.graph(); + + // use the entities with the tags that define the feature type + var entities = crossing.ways.sort(function(entity1, entity2) { + var type1 = getFeatureTypeForCrossingCheck(entity1, graph); + var type2 = getFeatureTypeForCrossingCheck(entity2, graph); + if (type1 === type2) { + return utilDisplayLabel(entity1, context) > utilDisplayLabel(entity2, context); + } else if (type1 === 'waterway') { + return true; + } else if (type2 === 'waterway') { + return false; + } + return type1 < type2; + }); + entities = _map(entities, function(way) { + return getFeatureWithFeatureTypeTagsForWay(way, graph); + }); + + var connectionTags = tagsForConnectionNodeIfAllowed(entities[0], entities[1]); + + var crossingTypeID; + if (hasTag(entities[0].tags, 'tunnel') && hasTag(entities[1].tags, 'tunnel')) { + crossingTypeID = 'tunnel-tunnel'; + if (connectionTags) { + crossingTypeID += '_connectable'; + } + } + else if (hasTag(entities[0].tags, 'bridge') && hasTag(entities[1].tags, 'bridge')) { + crossingTypeID = 'bridge-bridge'; + if (connectionTags) { + crossingTypeID += '_connectable'; + } + } + else { + crossingTypeID = crossing.featureTypes.sort().join('-'); + } + + var messageDict = { + feature: utilDisplayLabel(entities[0], context), + feature2: utilDisplayLabel(entities[1], context) + }; + + var fixes = []; + if (connectionTags) { + fixes.push(new validationIssueFix({ + title: t('issues.fix.add_connection_vertex.title'), + onClick: function() { + var loc = this.issue.coordinates; + var ways = this.issue.info.ways; + var connectionTags = this.issue.info.connectionTags; + + context.perform( + function actionConnectCrossingWays(graph) { + var projection = context.projection; + + var node = osmNode({ loc: loc, tags: connectionTags }); + graph = graph.replace(node); + + var way0 = graph.entity(ways[0].id); + var choice0 = geoChooseEdge(graph.childNodes(way0), projection(loc), projection); + var edge0 = [way0.nodes[choice0.index - 1], way0.nodes[choice0.index]]; + graph = actionAddMidpoint({loc: loc, edge: edge0}, node)(graph); + + var way1 = graph.entity(ways[1].id); + var choice1 = geoChooseEdge(graph.childNodes(way1), projection(loc), projection); + var edge1 = [way1.nodes[choice1.index - 1], way1.nodes[choice1.index]]; + graph = actionAddMidpoint({loc: loc, edge: edge1}, node)(graph); + + return graph; + }, + t('issues.fix.add_connection_vertex.undo_redo') + ); + } + })); + } + + return new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.crossing_ways.message', messageDict), + tooltip: t('issues.crossing_ways.'+crossingTypeID+'.tip'), + entities: entities, + info: { ways: crossing.ways, connectionTags: connectionTags }, + coordinates: crossing.cross_point, + fixes: fixes + }); + } + + validation.type = type; return validation; } diff --git a/modules/validations/deprecated_tag.js b/modules/validations/deprecated_tag.js index d9fa39cf1..741307f7e 100644 --- a/modules/validations/deprecated_tag.js +++ b/modules/validations/deprecated_tag.js @@ -15,6 +15,8 @@ import { export function validationDeprecatedTag() { + var type = 'deprecated_tag'; + var validation = function(change, context) { var issues = []; var deprecatedTagsArray = change.deprecatedTags(); @@ -24,10 +26,10 @@ export function validationDeprecatedTag() { var tagsLabel = utilTagText({ tags: deprecatedTags.old }); var featureLabel = utilDisplayLabel(change, context); issues.push(new validationIssue({ - type: 'deprecated_tags', + type: type, severity: 'warning', - message: t('issues.deprecated_tags.message', { feature: featureLabel, tags: tagsLabel }), - tooltip: t('issues.deprecated_tags.tip'), + message: t('issues.deprecated_tag.message', { feature: featureLabel, tags: tagsLabel }), + tooltip: t('issues.deprecated_tag.tip'), entities: [change], hash: tagsLabel, info: { @@ -80,7 +82,7 @@ export function validationDeprecatedTag() { return issues; }; - validation.type = 'deprecated_tag'; + validation.type = type; return validation; } diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js index ef26ebecb..0a8b3d0ae 100644 --- a/modules/validations/disconnected_way.js +++ b/modules/validations/disconnected_way.js @@ -29,6 +29,7 @@ export function validationDisconnectedWay() { }); } + var type = 'disconnected_way'; var validation = function(entity, context) { var issues = []; @@ -37,7 +38,7 @@ export function validationDisconnectedWay() { var entityLabel = utilDisplayLabel(entity, context); issues.push(new validationIssue({ - type: 'disconnected_way', + type: type, severity: 'warning', message: t('issues.disconnected_way.highway.message', { highway: entityLabel }), tooltip: t('issues.disconnected_way.highway.tip'), @@ -81,7 +82,7 @@ export function validationDisconnectedWay() { return issues; }; - validation.type = 'disconnected_way'; + validation.type = type; return validation; } diff --git a/modules/validations/generic_name.js b/modules/validations/generic_name.js index a69b810f9..406ae61e6 100644 --- a/modules/validations/generic_name.js +++ b/modules/validations/generic_name.js @@ -41,6 +41,7 @@ export function validationGenericName(context) { return false; } + var type = 'generic_name'; var validation = function(entity) { var issues = []; @@ -48,7 +49,7 @@ export function validationGenericName(context) { if (generic) { var preset = utilPreset(entity, context); issues.push(new validationIssue({ - type: 'generic_name', + type: type, severity: 'warning', message: t('issues.generic_name.message', {feature: preset.name(), name: generic}), tooltip: t('issues.generic_name.tip'), @@ -73,7 +74,7 @@ export function validationGenericName(context) { return issues; }; - validation.type = 'generic_name'; + validation.type = type; return validation; } diff --git a/modules/validations/many_deletions.js b/modules/validations/many_deletions.js index cf68530f1..a1b2baed2 100644 --- a/modules/validations/many_deletions.js +++ b/modules/validations/many_deletions.js @@ -7,6 +7,8 @@ export function validationManyDeletions() { var threshold = 100; + var type = 'many_deletions'; + var validation = function(changes, context) { var issues = []; var nodes = 0, ways = 0, areas = 0, relations = 0; @@ -19,7 +21,7 @@ export function validationManyDeletions() { }); if (changes.deleted.length > threshold) { issues.push(new validationIssue({ - type: 'many_deletions', + type: type, severity: 'warning', message: t( 'issues.many_deletions.message', @@ -33,7 +35,7 @@ export function validationManyDeletions() { return issues; }; - validation.type = 'many_deletions'; + validation.type = type; validation.inputType = 'changes'; return validation; diff --git a/modules/validations/missing_tag.js b/modules/validations/missing_tag.js index 022dc3b92..9b2939b91 100644 --- a/modules/validations/missing_tag.js +++ b/modules/validations/missing_tag.js @@ -20,6 +20,8 @@ export function validationMissingTag() { return keys.length > 0; } + var type = 'missing_tag'; + var validation = function(entity, context) { var types = ['point', 'line', 'area', 'relation']; var issues = []; @@ -30,10 +32,10 @@ export function validationMissingTag() { !(hasDescriptiveTags(entity) || entity.hasParentRelations(graph))) { var entityLabel = utilDisplayLabel(entity, context); issues.push(new validationIssue({ - type: 'missing_tag', + type: type, severity: 'error', - message: t('issues.untagged_feature.message', {feature: entityLabel}), - tooltip: t('issues.untagged_feature.tip'), + message: t('issues.missing_tag.message', {feature: entityLabel}), + tooltip: t('issues.missing_tag.tip'), entities: [entity], fixes: [ new validationIssueFix({ @@ -56,7 +58,7 @@ export function validationMissingTag() { return issues; }; - validation.type = 'missing_tag'; + validation.type = type; return validation; } diff --git a/modules/validations/old_multipolygon.js b/modules/validations/old_multipolygon.js index 0ae3d9512..7cc30e71b 100644 --- a/modules/validations/old_multipolygon.js +++ b/modules/validations/old_multipolygon.js @@ -14,6 +14,8 @@ import { export function validationOldMultipolygon() { + var type = 'old_multipolygon'; + var validation = function(entity, context) { var issues = []; @@ -33,7 +35,7 @@ export function validationOldMultipolygon() { if (multipolygon && outerWay) { var multipolygonLabel = utilDisplayLabel(multipolygon, context); issues.push(new validationIssue({ - type: 'old_multipolygon', + type: type, severity: 'warning', message: t('issues.old_multipolygon.message', { multipolygon: multipolygonLabel }), tooltip: t('issues.old_multipolygon.tip'), @@ -61,7 +63,7 @@ export function validationOldMultipolygon() { return issues; }; - validation.type = 'old_multipolygon'; + validation.type = type; return validation; } diff --git a/modules/validations/tag_suggests_area.js b/modules/validations/tag_suggests_area.js index 916af06b9..d442c2d6d 100644 --- a/modules/validations/tag_suggests_area.js +++ b/modules/validations/tag_suggests_area.js @@ -37,6 +37,8 @@ export function validationTagSuggestsArea() { return false; } + var type = 'tag_suggests_area'; + var validation = function(entity, context) { var issues = []; var graph = context.graph(); @@ -46,7 +48,7 @@ export function validationTagSuggestsArea() { if (suggestingTags) { var tagText = utilTagText({ tags: suggestingTags }); issues.push(new validationIssue({ - type: 'tag_suggests_area', + type: type, severity: 'warning', message: t('issues.tag_suggests_area.message', { tag: tagText }), tooltip: t('issues.tag_suggests_area.tip'), @@ -73,7 +75,7 @@ export function validationTagSuggestsArea() { return issues; }; - validation.type = 'tag_suggests_area'; + validation.type = type; return validation; } diff --git a/test/spec/validations/deprecated_tag.js b/test/spec/validations/deprecated_tag.js index f4126469d..6881092b5 100644 --- a/test/spec/validations/deprecated_tag.js +++ b/test/spec/validations/deprecated_tag.js @@ -45,7 +45,7 @@ describe('iD.validations.deprecated_tag', function () { var issues = validate(); expect(issues).to.have.lengthOf(1); var issue = issues[0]; - expect(issue.type).to.eql('deprecated_tags'); + expect(issue.type).to.eql('deprecated_tag'); expect(issue.severity).to.eql('warning'); expect(issue.entities).to.have.lengthOf(1); expect(issue.entities[0].id).to.eql('w-1');