From 0dfd0612b1a3db77fe6bb7fb1c046918170b3c6c Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Thu, 23 May 2019 15:39:15 -0400 Subject: [PATCH] For ice rinks, prefer the "sport" values "ice_hockey" and "ice_skating" instead of "hockey" and "skating" since that latter are ambiguous Don't remove other values when upgrading a tag value within a semicolon-delimited list --- data/deprecated.json | 8 ++++++++ data/presets/fields.json | 2 +- data/presets/fields/sport_ice.json | 4 ++-- modules/actions/upgrade_tags.js | 16 +++++++++++++++- test/spec/actions/upgrade_tags.js | 20 ++++++++++++++++++-- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/data/deprecated.json b/data/deprecated.json index 6b1ceff2d..f1905cecc 100644 --- a/data/deprecated.json +++ b/data/deprecated.json @@ -447,6 +447,14 @@ "old": {"leisure": "club"}, "replace": {"club": "*"} }, + { + "old": {"leisure": "ice_rink", "sport": "hockey"}, + "replace": {"leisure": "ice_rink", "sport": "ice_hockey"} + }, + { + "old": {"leisure": "ice_rink", "sport": "skating"}, + "replace": {"leisure": "ice_rink", "sport": "ice_skating"} + }, { "old": {"leisure": "social_club"}, "replace": {"club": "*"} diff --git a/data/presets/fields.json b/data/presets/fields.json index aa69b1300..793b1950a 100644 --- a/data/presets/fields.json +++ b/data/presets/fields.json @@ -337,7 +337,7 @@ "social_facility_for": {"key": "social_facility:for", "type": "combo", "label": "People Served"}, "social_facility": {"key": "social_facility", "type": "combo", "label": "Type"}, "source": {"key": "source", "type": "semiCombo", "icon": "source", "universal": true, "label": "Sources", "snake_case": false, "caseSensitive": true, "options": ["survey", "local knowledge", "gps", "aerial imagery", "streetlevel imagery"]}, - "sport_ice": {"key": "sport", "type": "semiCombo", "label": "Sports", "options": ["skating", "hockey", "multi", "curling", "ice_stock"]}, + "sport_ice": {"key": "sport", "type": "semiCombo", "label": "Sports", "options": ["ice_skating", "ice_hockey", "multi", "curling", "ice_stock"]}, "sport_racing_motor": {"key": "sport", "type": "semiCombo", "label": "Sports", "options": ["motor", "karting", "motocross"]}, "sport_racing_nonmotor": {"key": "sport", "type": "semiCombo", "label": "Sports", "options": ["bmx", "cycling", "dog_racing", "horse_racing", "running"]}, "sport": {"key": "sport", "type": "semiCombo", "label": "Sports"}, diff --git a/data/presets/fields/sport_ice.json b/data/presets/fields/sport_ice.json index dd8560bd1..014cb7021 100644 --- a/data/presets/fields/sport_ice.json +++ b/data/presets/fields/sport_ice.json @@ -3,8 +3,8 @@ "type": "semiCombo", "label": "Sports", "options": [ - "skating", - "hockey", + "ice_skating", + "ice_hockey", "multi", "curling", "ice_stock" diff --git a/modules/actions/upgrade_tags.js b/modules/actions/upgrade_tags.js index bf45f2250..418050c56 100644 --- a/modules/actions/upgrade_tags.js +++ b/modules/actions/upgrade_tags.js @@ -4,6 +4,7 @@ export function actionUpgradeTags(entityId, oldTags, replaceTags) { var entity = graph.entity(entityId); var tags = Object.assign({}, entity.tags); // shallow copy var transferValue; + var semiIndex; for (var oldTagKey in oldTags) { if (oldTags[oldTagKey] === '*') { @@ -15,6 +16,10 @@ export function actionUpgradeTags(entityId, oldTags, replaceTags) { if (vals.length === 1 || oldIndex === -1) { delete tags[oldTagKey]; } else { + if (replaceTags && replaceTags[oldTagKey]) { + // replacing a value within a semicolon-delimited value, note the index + semiIndex = oldIndex; + } vals.splice(oldIndex, 1); tags[oldTagKey] = vals.join(';'); } @@ -36,7 +41,16 @@ export function actionUpgradeTags(entityId, oldTags, replaceTags) { } else if (replaceValue === '$1') { tags[replaceKey] = transferValue; } else { - tags[replaceKey] = replaceValue; + if (tags[replaceKey] && oldTags[replaceKey] && semiIndex !== undefined) { + // don't override preexisting values + var existingVals = tags[replaceKey].split(';').filter(Boolean); + if (existingVals.indexOf(replaceValue) === -1) { + existingVals.splice(semiIndex, 0, replaceValue); + tags[replaceKey] = existingVals.join(';'); + } + } else { + tags[replaceKey] = replaceValue; + } } } } diff --git a/test/spec/actions/upgrade_tags.js b/test/spec/actions/upgrade_tags.js index 2a0e2f412..4a1af78d2 100644 --- a/test/spec/actions/upgrade_tags.js +++ b/test/spec/actions/upgrade_tags.js @@ -24,6 +24,14 @@ describe('iD.actionUpgradeTags', function () { expect(graph.entity(entity.id).tags).to.eql({ natural: 'wetland', wetland: 'marsh', name: 'Foo' }); }); + it('upgrades a tag and overrides an existing value', function () { + var oldTags = { landuse: 'wood' }, + newTags = { natural: 'wood' }, + entity = iD.Entity({ tags: { landuse: 'wood', natural: 'wetland', name: 'Foo' }}), + graph = iD.actionUpgradeTags(entity.id, oldTags, newTags)(iD.coreGraph([entity])); + expect(graph.entity(entity.id).tags).to.eql({ natural: 'wood', name: 'Foo' }); + }); + it('upgrades a tag with no replacement tags', function () { var oldTags = { highway: 'no' }, newTags = undefined, @@ -56,7 +64,7 @@ describe('iD.actionUpgradeTags', function () { expect(graph.entity(entity.id).tags).to.eql({ shop: 'supermarket', name: 'Foo' }); }); - it('upgrades a tag in a semicolon-delimited list with one other value', function () { + it('upgrades a tag from a semicolon-delimited list that has one other value', function () { var oldTags = { cuisine: 'vegan' }, newTags = { 'diet:vegan': 'yes' }, entity = iD.Entity({ tags: { cuisine: 'italian;vegan', name: 'Foo' }}), @@ -64,7 +72,7 @@ describe('iD.actionUpgradeTags', function () { expect(graph.entity(entity.id).tags).to.eql({ cuisine: 'italian', 'diet:vegan': 'yes', name: 'Foo' }); }); - it('upgrades a tag in a semicolon-delimited list with many other values', function () { + it('upgrades a tag from a semicolon-delimited list that has many other values', function () { var oldTags = { cuisine: 'vegan' }, newTags = { 'diet:vegan': 'yes' }, entity = iD.Entity({ tags: { cuisine: 'italian;vegan;regional;american', name: 'Foo' }}), @@ -72,4 +80,12 @@ describe('iD.actionUpgradeTags', function () { expect(graph.entity(entity.id).tags).to.eql({ cuisine: 'italian;regional;american', 'diet:vegan': 'yes', name: 'Foo' }); }); + it('upgrades a tag within a semicolon-delimited list without changing other values', function () { + var oldTags = { leisure: 'ice_rink', sport: 'hockey' }, + newTags = { leisure: 'ice_rink', sport: 'ice_hockey' }, + entity = iD.Entity({ tags: { leisure: 'ice_rink', sport: 'curling;hockey;multi', name: 'Foo' }}), + graph = iD.actionUpgradeTags(entity.id, oldTags, newTags)(iD.coreGraph([entity])); + expect(graph.entity(entity.id).tags).to.eql({ leisure: 'ice_rink', sport: 'curling;ice_hockey;multi', name: 'Foo' }); + }); + });