From badde3d583287d4a0b54e3067247a311c1c86847 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Fri, 4 Oct 2019 11:06:27 +0200 Subject: [PATCH] Add warning for features with names that equal values in their `not:name` tag (close #6411) --- data/core.yaml | 13 +++- dist/locales/en.json | 17 ++++- modules/validations/index.js | 4 +- .../{generic_name.js => suspicious_name.js} | 70 +++++++++++++++++-- test/index.html | 2 +- .../{generic_name.js => suspicious_name.js} | 41 +++++++++-- 6 files changed, 126 insertions(+), 21 deletions(-) rename modules/validations/{generic_name.js => suspicious_name.js} (61%) rename test/spec/validations/{generic_name.js => suspicious_name.js} (63%) diff --git a/data/core.yaml b/data/core.yaml index 216531433..44fbebb8a 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1413,10 +1413,8 @@ en: tip: 'Find features with "fixme" tags' reference: 'A "fixme" tag indicates that a mapper has requested help with a feature.' generic_name: - title: Suspicious Names message: '{feature} has the suspicious name "{name}"' message_language: '{feature} has the suspicious name "{name}" in {language}' - tip: "Find features with generic or suspicious names" reference: "Names should be the actual, on-the-ground names of features." incompatible_source: title: Suspicious Sources @@ -1425,6 +1423,9 @@ en: feature: message: '{feature} lists Google as a data source' reference: "Google products are proprietary and must not be used as references." + incorrect_name: + message: '{feature} has the mistaken name "{name}"' + message_language: '{feature} has the mistaken name "{name}" in {language}' invalid_format: title: Invalid Formatting tip: Find tags with unexpected formats @@ -1475,6 +1476,9 @@ en: reference: "Sensitive data like personal phone numbers should not be tagged." contact: message: '{feature} might be tagged with private contact information' + suspicious_name: + title: Suspicious Names + tip: "Find features with generic or suspicious names" tag_suggests_area: message: '{feature} should be a closed area based on the tag "{tag}"' reference: "Areas must have connected endpoints." @@ -1543,8 +1547,9 @@ en: remove_from_relation: title: Remove from relation remove_generic_name: - title: Remove the name annotation: Removed a generic name. + remove_mistaken_name: + annotation: Removed a mistaken name. remove_private_info: annotation: Removed private information. remove_proprietary_data: @@ -1554,6 +1559,8 @@ en: annotation: Removed tag. remove_tags: title: Remove the tags + remove_the_name: + title: Remove the name reposition_features: title: Reposition the features reverse_feature: diff --git a/dist/locales/en.json b/dist/locales/en.json index d95cc845c..e94b4fd7b 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1747,10 +1747,8 @@ "reference": "A \"fixme\" tag indicates that a mapper has requested help with a feature." }, "generic_name": { - "title": "Suspicious Names", "message": "{feature} has the suspicious name \"{name}\"", "message_language": "{feature} has the suspicious name \"{name}\" in {language}", - "tip": "Find features with generic or suspicious names", "reference": "Names should be the actual, on-the-ground names of features." }, "incompatible_source": { @@ -1763,6 +1761,10 @@ "reference": "Google products are proprietary and must not be used as references." } }, + "incorrect_name": { + "message": "{feature} has the mistaken name \"{name}\"", + "message_language": "{feature} has the mistaken name \"{name}\" in {language}" + }, "invalid_format": { "title": "Invalid Formatting", "tip": "Find tags with unexpected formats", @@ -1829,6 +1831,10 @@ "message": "{feature} might be tagged with private contact information" } }, + "suspicious_name": { + "title": "Suspicious Names", + "tip": "Find features with generic or suspicious names" + }, "tag_suggests_area": { "message": "{feature} should be a closed area based on the tag \"{tag}\"", "reference": "Areas must have connected endpoints." @@ -1926,9 +1932,11 @@ "title": "Remove from relation" }, "remove_generic_name": { - "title": "Remove the name", "annotation": "Removed a generic name." }, + "remove_mistaken_name": { + "annotation": "Removed a mistaken name." + }, "remove_private_info": { "annotation": "Removed private information." }, @@ -1942,6 +1950,9 @@ "remove_tags": { "title": "Remove the tags" }, + "remove_the_name": { + "title": "Remove the name" + }, "reposition_features": { "title": "Reposition the features" }, diff --git a/modules/validations/index.js b/modules/validations/index.js index 224f349ef..345a0c034 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -3,14 +3,14 @@ export { validationCloseNodes } from './close_nodes'; export { validationCrossingWays } from './crossing_ways'; export { validationDisconnectedWay } from './disconnected_way'; export { validationFixmeTag } from './fixme_tag'; -export { validationGenericName } from './generic_name'; +export { validationFormatting } from './invalid_format'; export { validationImpossibleOneway } from './impossible_oneway'; export { validationIncompatibleSource } from './incompatible_source'; -export { validationFormatting } from './invalid_format'; export { validationMaprules } from './maprules'; export { validationMismatchedGeometry } from './mismatched_geometry'; export { validationMissingRole } from './missing_role'; export { validationMissingTag } from './missing_tag'; export { validationOutdatedTags } from './outdated_tags'; export { validationPrivateData } from './private_data'; +export { validationSuspiciousName } from './suspicious_name'; export { validationUnsquareWay } from './unsquare_way'; diff --git a/modules/validations/generic_name.js b/modules/validations/suspicious_name.js similarity index 61% rename from modules/validations/generic_name.js rename to modules/validations/suspicious_name.js index 931fd9eec..33153f5a4 100644 --- a/modules/validations/generic_name.js +++ b/modules/validations/suspicious_name.js @@ -6,8 +6,8 @@ import { validationIssue, validationIssueFix } from '../core/validation'; import { actionChangeTags } from '../actions/change_tags'; -export function validationGenericName() { - var type = 'generic_name'; +export function validationSuspiciousName() { + var type = 'suspicious_name'; // known list of generic names (e.g. "bar") var discardNamesRegexes = filters.discardNames.map(function(discardName) { @@ -52,6 +52,7 @@ export function validationGenericName() { function makeGenericNameIssue(entityId, nameKey, genericName, langCode) { return new validationIssue({ type: type, + subtype: 'generic_name', severity: 'warning', message: function(context) { var entity = context.hasEntity(this.entityIds[0]); @@ -68,7 +69,7 @@ export function validationGenericName() { fixes: [ new validationIssueFix({ icon: 'iD-operation-delete', - title: t('issues.fix.remove_generic_name.title'), + title: t('issues.fix.remove_the_name.title'), onClick: function(context) { var entityId = this.issue.entityIds[0]; var entity = context.entity(entityId); @@ -93,6 +94,51 @@ export function validationGenericName() { } } + function makeIncorrectNameIssue(entityId, nameKey, incorrectName, langCode) { + return new validationIssue({ + type: type, + subtype: 'not_name', + severity: 'warning', + message: function(context) { + var entity = context.hasEntity(this.entityIds[0]); + if (!entity) return ''; + var preset = utilPreset(entity, context); + var langName = langCode && languageName(langCode); + return t('issues.incorrect_name.message' + (langName ? '_language' : ''), + { feature: preset.name(), name: incorrectName, language: langName } + ); + }, + reference: showReference, + entityIds: [entityId], + hash: nameKey + '=' + incorrectName, + fixes: [ + new validationIssueFix({ + icon: 'iD-operation-delete', + title: t('issues.fix.remove_the_name.title'), + onClick: function(context) { + var entityId = this.issue.entityIds[0]; + var entity = context.entity(entityId); + var tags = Object.assign({}, entity.tags); // shallow copy + delete tags[nameKey]; + context.perform( + actionChangeTags(entityId, tags), + t('issues.fix.remove_mistaken_name.annotation') + ); + } + }) + ] + }); + + function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(t('issues.generic_name.reference')); + } + } + var validation = function checkGenericName(entity) { // a generic name is okay if it's a known brand or entity @@ -100,15 +146,25 @@ export function validationGenericName() { var issues = []; + var notNames = (entity.tags['not:name'] || '').split(';'); + for (var key in entity.tags) { var m = key.match(/^name(?:(?::)([a-zA-Z_-]+))?$/); if (!m) continue; - var value = entity.tags[key]; - if (isGenericName(value, entity.tags)) { - var langCode = null; - if (m.length >=2) langCode = m[1]; + var langCode = m.length >= 2 ? m[1] : null; + var value = entity.tags[key]; + if (notNames.length) { + for (var i in notNames) { + var notName = notNames[i]; + if (value === notName) { + issues.push(makeIncorrectNameIssue(entity.id, key, value, langCode)); + continue; + } + } + } + if (isGenericName(value, entity.tags)) { issues.push(makeGenericNameIssue(entity.id, key, value, langCode)); } } diff --git a/test/index.html b/test/index.html index 571ca9107..378346730 100644 --- a/test/index.html +++ b/test/index.html @@ -155,7 +155,6 @@ - @@ -163,6 +162,7 @@ +