diff --git a/CHANGELOG.md b/CHANGELOG.md index 892a53cef..03d636902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,10 +40,13 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :sparkles: Usability & Accessibility #### :white_check_mark: Validation #### :bug: Bugfixes +* Fix bug which made it impossible to change an object's preset from a sub-preset to the respective parents preset (e.g. from Driveway to Service Road) ([#9372]) #### :hourglass: Performance #### :rocket: Presets #### :hammer: Development +[#9372]: https://github.com/openstreetmap/iD/issues/9372 + # 2.23.2 ##### 2022-Nov-12 diff --git a/modules/actions/change_preset.js b/modules/actions/change_preset.js index a4f7ef053..13f02596e 100644 --- a/modules/actions/change_preset.js +++ b/modules/actions/change_preset.js @@ -11,10 +11,15 @@ export function actionChangePreset(entityID, oldPreset, newPreset, skipFieldDefa if (newPreset.addTags) { preserveKeys = preserveKeys.concat(Object.keys(newPreset.addTags)); } - newPreset.fields().concat(newPreset.moreFields()) - .filter(f => f.matchGeometry(geometry)) - .map(f => f.key).filter(Boolean) - .forEach(key => preserveKeys.push(key)); + if (oldPreset && !oldPreset.id.startsWith(newPreset.id)) { + // only if old preset is not a sub-preset of the new one: + // preserve tags for which the new preset has a field + // https://github.com/openstreetmap/iD/issues/9372 + newPreset.fields().concat(newPreset.moreFields()) + .filter(f => f.matchGeometry(geometry)) + .map(f => f.key).filter(Boolean) + .forEach(key => preserveKeys.push(key)); + } } if (oldPreset) tags = oldPreset.unsetTags(tags, geometry, preserveKeys); if (newPreset) tags = newPreset.setTags(tags, geometry, skipFieldDefaults); diff --git a/test/spec/actions/change_preset.js b/test/spec/actions/change_preset.js index 8f677e2eb..84d41454d 100644 --- a/test/spec/actions/change_preset.js +++ b/test/spec/actions/change_preset.js @@ -22,4 +22,77 @@ describe('iD.actionChangePreset', function() { var action = iD.actionChangePreset(entity.id, oldPreset, null); expect(action(graph).entity(entity.id).tags).to.eql({}); }); + + // https://github.com/openstreetmap/iD/issues/8159 + it('preserves the tags of a new preset\'s addTags', function() { + var entity = iD.osmNode({tags: { + 'power': 'plant', + 'plant:source': 'coal', + 'plant:method': 'combustion', + 'plant:output:electricity': '10 MW' + }}); + var graph = iD.coreGraph([entity]); + var oldPreset = iD.presetPreset('old', {tags: { + 'power': 'plant', + 'plant:source': 'coal' + }, addTags: { + 'power': 'plant', + 'plant:source': 'coal', + 'plant:method': 'combustion', + 'plant:output:electricity': '*' + }}); + var newPreset = iD.presetPreset('new', {tags: { + 'power': 'plant', + 'plant:source': 'solar', + 'plant:method': 'photovoltaic' + }, addTags: { + 'power': 'plant', + 'plant:source': 'solar', + 'plant:method': 'photovoltaic', + 'plant:output:electricity': '*' + }}); + var action = iD.actionChangePreset(entity.id, oldPreset, newPreset); + expect(action(graph).entity(entity.id).tags).to.eql({ + 'power': 'plant', + 'plant:source': 'solar', + 'plant:method': 'photovoltaic', + 'plant:output:electricity': '10 MW' + }); + }); + + // https://github.com/openstreetmap/iD/issues/9341 + // https://github.com/openstreetmap/iD/issues/9104 + it('preserves the tags when there is a matching field in the new preset', function() { + var entity = iD.osmNode({tags: {building: 'yes'}}); + var graph = iD.coreGraph([entity]); + var oldPreset = iD.presetPreset('old', {tags: {building: 'yes'}}); + var newPreset = iD.presetPreset('new', {tags: {amenity: 'school'}, fields: ['field']}, undefined, { + field: iD.presetField('field', {key: 'building'}) + }); + var action = iD.actionChangePreset(entity.id, oldPreset, newPreset); + expect(action(graph).entity(entity.id).tags).to.eql({amenity: 'school', building: 'yes'}); + }); + + it('does not preserves the tags of a non-matching field in the new preset', function() { + var entity = iD.osmNode({tags: {building: 'yes'}, loc: [0, 0]}); + var graph = iD.coreGraph([entity]); + var oldPreset = iD.presetPreset('old', {tags: {building: 'yes'}}); + var newPreset = iD.presetPreset('new', {tags: {amenity: 'school'}, fields: ['field']}, undefined, { + field: iD.presetField('field', {key: 'building', geometry: 'area'}) + }); + var action = iD.actionChangePreset(entity.id, oldPreset, newPreset); + expect(action(graph).entity(entity.id).tags).to.eql({amenity: 'school'}); + }); + + // https://github.com/openstreetmap/iD/issues/9372 + it('does not preserve field tags when changing from a subpreset to its parent', function() { + var entity = iD.osmNode({tags: {highway: 'service', service: 'driveway'}}); + var graph = iD.coreGraph([entity]); + var oldPreset = iD.presetPreset('highway/service/driveway', {tags: {highway: 'service', service: 'driveway'}}); + var newPreset = iD.presetPreset('highway/service', {tags: {highway: 'service'}, fields: ['field']}, undefined, { + field: iD.presetField('field', {key: 'service'}) + }); + var action = iD.actionChangePreset(entity.id, oldPreset, newPreset); + expect(action(graph).entity(entity.id).tags).to.eql({highway: 'service'}); + }); });