From 96298f2836da0f1948105bbb6c4c2298619ddc8b Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 19 Jan 2021 13:15:16 -0500 Subject: [PATCH] Preserve `name` value if this preset shows `brand` or `operator` field This also fixes the logic for calculating whether the preset shows a `brand` or `operator` field - it needs to use `fields()` to actually resolve the fields, as these fields can be inherited from another preset. This also includes a change to match "primary" names before "alternate" names (aka the "Baby Gap" / "Gap" problem) --- modules/ui/fields/input.js | 10 +--- modules/ui/fields/localized.js | 21 ++++---- modules/validations/outdated_tags.js | 80 ++++++++++++++++++---------- 3 files changed, 63 insertions(+), 48 deletions(-) diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index 7904679ad..76fe41fac 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -43,15 +43,6 @@ export function uiFieldText(field, context) { var entity = context.graph().hasEntity(entityID); if (!entity) return false; - var which = field.id; // 'brand', 'network', 'operator', 'flag' - - // If the value was already edited manually then unlock and allow further editing - var base = context.graph().base().entities[_entityIDs[0]]; - if (base) { - var hasOriginalValue = entity.tags[which] && entity.tags[which] === base.tags[which]; - if (!hasOriginalValue) return false; - } - // Features linked to Wikidata are likely important and should be protected if (entity.tags.wikidata) return true; @@ -59,6 +50,7 @@ export function uiFieldText(field, context) { var isSuggestion = preset && preset.suggestion; // Lock the field if there is a value and a companion `*:wikidata` value + var which = field.id; // 'brand', 'network', 'operator', 'flag' return isSuggestion && !!entity.tags[which] && !!entity.tags[which + ':wikidata']; }); diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index 413a8ffe3..7ef3c3f23 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -90,33 +90,30 @@ export function uiFieldLocalized(field, context) { var entity = context.graph().hasEntity(entityID); if (!entity) return false; - // If the value was already edited manually then unlock and allow further editing - var base = context.graph().base().entities[_entityIDs[0]]; - if (base) { - var hasOriginalValue = entity.tags.name && entity.tags.name === base.tags.name; - if (!hasOriginalValue) return false; - } - // Features linked to Wikidata are likely important and should be protected if (entity.tags.wikidata) return true; // Assume the name has already been confirmed if its source has been researched if (entity.tags['name:etymology:wikidata']) return true; - // Lock the name if this is a suggestion preset that assigns the name + // Lock the `name` if this is a suggestion preset that assigns the name, + // and the preset does not display a `brand` or `operator` field. + // (For presets like hotels, car dealerships, post offices, the `name` should remain editable) + // see also similar logic in `outdated_tags.js` var preset = presetManager.match(entity, context.graph()); if (preset) { var isSuggestion = preset.suggestion; - var showsBrandField = preset.originalFields.some(function(d) { return d.id === 'brand'; }); - var showsOperatorField = preset.originalFields.some(function(d) { return d.id === 'operator'; }); + var fields = preset.fields(); + var showsBrandField = fields.some(function(d) { return d.id === 'brand'; }); + var showsOperatorField = fields.some(function(d) { return d.id === 'operator'; }); var setsName = preset.addTags.name; var setsBrandWikidata = preset.addTags['brand:wikidata']; var setsOperatorWikidata = preset.addTags['operator:wikidata']; - return isSuggestion && setsName && ( + return (isSuggestion && setsName && ( (setsBrandWikidata && !showsBrandField) || (setsOperatorWikidata && !showsOperatorField) - ); + )); } return false; diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index f4be2e629..722823a36 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -99,25 +99,23 @@ export function validationOutdatedTags() { // Returns true if this tag key is a "namelike" tag that the NSI matcher would have indexed.. - function isNamelike(k) { - const namePatterns = [ - /^(flag:)?name$/i, // e.g. `name`, `flag:name` - /^(brand|country|flag|operator|network|subject)$/i, - /^\w+_name$/i, // e.g. `alt_name`, `short_name` - /^(name|brand|country|flag|operator|network|subject):\w+$/i, // e.g. `name:en`, `name:ru` - /^\w+_name:\w+$/i // e.g. `alt_name:en`, `short_name:ru` - ]; + function isNamelike(osmkey, which) { + // There are a few exceptions to the namelike regexes. + // Usually a tag suffix contains a language code like `name:en`, `name:ru` + // but we want to exclude things like `operator:type`, `name:etymology`, etc.. + const notNames = /:(colour|type|left|right|etymology|pronunciation|wikipedia)$/i; - return namePatterns.some(pattern => { - if (!pattern.test(k)) return false; // k is not a name tag, skip + let namePatterns; + if (which === 'primary') { + namePatterns = [ /^(flag:)?name(:\w+)?$/i ]; // `name`, `flag:name` (poss. w/ `:lang` suffix) + } else { + namePatterns = [ + /^(brand|country|flag|operator|network|subject)(:\w+)?$/i, // `brand`, `operator` (poss. w/ `:lang` suffix) + /^\w+_name(:\w+)?$/i, // `alt_name`, `short_name` (poss. w/ `:lang` suffix) + ]; + } - // There are a few exceptions to the namelike regexes. - // Usually a tag suffix contains a language code like `name:en`, `name:ru` - // but we want to exclude things like `operator:type`, `name:etymology`, etc.. - if (/:(colour|type|left|right|etymology|pronunciation|wikipedia)$/.test(k)) return false; - - return true; - }); + return namePatterns.some(regex => regex.test(osmkey) && !notNames.test(osmkey)); } @@ -216,12 +214,16 @@ export function validationOutdatedTags() { loc = entity.extent(graph).center(); } if (!names) { // collect names for this feature only once - names = Object.keys(newTags) - .map(k => isNamelike(k) ? newTags[k] : null) - .filter(Boolean); + names = []; + let osmkeys = Object.keys(newTags); + for (let j = 0; j < osmkeys.length; j++) { + const k = osmkeys[j]; + if (isNamelike(k, 'primary')) names.unshift(newTags[k]); // primary names first + if (isNamelike(k, 'alternate')) names.push(newTags[k]); // alternate names last + } + if (foundQID) names.push(foundQID); // matcher will recognize the QID as an alternate name too - if (foundQID) names.unshift(foundQID); // matcher will recognize the QID as a name too - names = utilArrayUniq(names); + names = utilArrayUniq(names).filter(Boolean); } // Try each namelike value @@ -250,12 +252,36 @@ export function validationOutdatedTags() { // We are keeping the match at this point subtype = 'noncanonical_brand'; + // Keys that we don't want NSI to overwrite. + let keepKeys = [/^building$/i, /^flag:name$/i, /^takeaway$/i]; + + // Don't overwrite a `name` tag if this preset shows a `brand` or `operator` field. + // (For presets like hotels, car dealerships, post offices, the `name` should be left alone) + // see also similar logic in `localized.js` + const nsiPreset = presetManager.matchTags(item.tags, 'point'); // (the actual geometry doesn't matter) + if (nsiPreset) { + const fields = nsiPreset.fields(); + const showsBrandField = fields.some(d => d.id === 'brand'); + const showsOperatorField = fields.some(d => d.id === 'operator'); + const setsName = item.tags.name; + const setsBrandWikidata = item.tags['brand:wikidata']; + const setsOperatorWikidata = item.tags['operator:wikidata']; + + if (setsName && ( + (setsBrandWikidata && showsBrandField) || + (setsOperatorWikidata && showsOperatorField) + )) { + keepKeys.push(/^name(:\w+)?$/i); // `name`, `name:en`, etc + } + } + // Preserve some tag values that we don't want NSI to overwrite. - const keepTags = ['building', 'flag:name', 'takeaway'] - .reduce((acc, k) => { - if (newTags[k]) acc[k] = newTags[k]; - return acc; - }, {}); + let keepTags = {}; + Object.keys(newTags).forEach(k => { + if (keepKeys.some(pattern => pattern.test(k))) { + keepTags[k] = newTags[k]; + } + }); // Replace the primary tags with what's in NSI ("amenity", "craft", "shop", "man_made", "route", etc) nsiKeys.forEach(k => delete newTags[k]);