don't reverse side tag of highway=cyclist_waiting_aid features, fixes #10128

CHANGELOG.md

@@ -53,6 +53,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 * Local photos: Fix bug which prevented the last image from being removed from the map when removed from the list
 * Fix wrong mouse cursor on "foreign link" field buttons (for example in the Mapillary or Wikimedia Commons fields) ([#9992], thanks [@ramith-kulal])
 * Don't show duplicates of notes when they lie exactly on special locations like null island (0.0,0.0)
+* Preserve `side` tag of `highway=cyclist_waiting_aid` features when reversing its way ([#10128])
 #### :earth_asia: Localization
 #### :hourglass: Performance
 #### :mortar_board: Walkthrough / Help
@@ -74,6 +75,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
 [#10062]: https://github.com/openstreetmap/iD/pull/10062
 [#10066]: https://github.com/openstreetmap/iD/pull/10066
 [#10074]: https://github.com/openstreetmap/iD/issues/10074
+[#10128]: https://github.com/openstreetmap/iD/issues/10128 
 [id-tagging-schema#1076]: https://github.com/openstreetmap/id-tagging-schema/pull/1076
 [@ramith-kulal]: https://github.com/ramith-kulal
 [@mangerlahn]: https://github.com/mangerlahn

modules/actions/reverse.js

@@ -42,6 +42,13 @@ export function actionReverse(entityID, options) {
         forwards: 'backward',
         backwards: 'forward',
     };
+    // tags whose values should not be reversed when certain other tags are also present
+    // https://github.com/openstreetmap/iD/issues/10128
+    const valueReplacementsExceptions = {
+        'side': [
+            {highway: 'cyclist_waiting_aid'}
+        ]
+    };
     var roleReplacements = {
         forward: 'backward',
         backward: 'forward',
@@ -85,7 +92,7 @@ export function actionReverse(entityID, options) {
     }
 
 
-    function reverseValue(key, value, includeAbsolute) {
+    function reverseValue(key, value, includeAbsolute, allTags) {
         if (ignoreKey.test(key)) return value;
 
         // Turn lanes are left/right to key (not way) direction - #5674
@@ -116,6 +123,16 @@ export function actionReverse(entityID, options) {
             }).join(';');
         }
 
+        if (valueReplacementsExceptions[key] && valueReplacementsExceptions[key].some(exceptionTags =>
+            Object.keys(exceptionTags).every(k => {
+                const v = exceptionTags[k];
+                return allTags[k] && (v === '*' || allTags[k] === v);
+            })
+        )) {
+            // don't reverse, for example, side=left/right on highway=cyclist_waiting_aid features
+            // see https://github.com/openstreetmap/iD/issues/10128
+            return value;
+        }
         return valueReplacements[value] || value;
     }
 
@@ -128,7 +145,7 @@ export function actionReverse(entityID, options) {
 
             var tags = {};
             for (var key in node.tags) {
-                tags[reverseKey(key)] = reverseValue(key, node.tags[key], node.id === entityID);
+                tags[reverseKey(key)] = reverseValue(key, node.tags[key], node.id === entityID, node.tags);
             }
             graph = graph.replace(node.update({tags: tags}));
         }
@@ -142,7 +159,7 @@ export function actionReverse(entityID, options) {
         var role;
 
         for (var key in way.tags) {
-            tags[reverseKey(key)] = reverseValue(key, way.tags[key]);
+            tags[reverseKey(key)] = reverseValue(key, way.tags[key], false, way.tags);
         }
 
         graph.parentRelations(way).forEach(function(relation) {
@@ -175,7 +192,7 @@ export function actionReverse(entityID, options) {
 
         for (var key in entity.tags) {
             var value = entity.tags[key];
-            if (reverseKey(key) !== key || reverseValue(key, value, true) !== value) {
+            if (reverseKey(key) !== key || reverseValue(key, value, true, entity.tags) !== value) {
                 return false;
             }
         }

test/spec/actions/reverse.js

@@ -588,6 +588,20 @@ describe('iD.actionReverse', function () {
             var target = graph.entity(node2.id);
             expect(target.tags['traffic_signals:direction']).to.eql('empty');
         });
-    });
 
+        it('preserves the value of the side tag of a cycling waiting aid', function () {
+            var node1 = iD.osmNode();
+            var node2 = iD.osmNode({tags: {
+                'highway': 'cyclist_waiting_aid',
+                'direction': 'forward',
+                'side': 'right'
+            }});
+            var node3 = iD.osmNode();
+            var way = iD.osmWay({nodes: [node1.id, node2.id, node3.id]});
+            var graph = iD.actionReverse(way.id)(iD.coreGraph([node1, node2, node3, way]));
+            var target = graph.entity(node2.id);
+            expect(target.tags.direction).to.eql('backward');
+            expect(target.tags.side).to.eql('right');
+        });
+    });
 });