From ef88e1c7ca59fcb92f883f419a4a59910e0c18e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ky=E2=84=93e=20Hensel?= <k-yle@users.noreply.github.com>
Date: Mon, 17 Feb 2025 21:46:42 +1100
Subject: [PATCH] fix destination_sign relations not updated when splitting a
 way (#10646)

---
 modules/actions/split.js   | 10 ++++++--
 modules/osm/relation.js    |  5 +++-
 test/spec/actions/split.js | 50 ++++++++++++++++++++------------------
 test/spec/osm/relation.js  | 26 ++++++++++++++++++++
 4 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/modules/actions/split.js b/modules/actions/split.js
index 6c39ae441..65089874e 100644
--- a/modules/actions/split.js
+++ b/modules/actions/split.js
@@ -172,7 +172,10 @@ export function actionSplit(nodeIds, newWayIds) {
             // 2. Splitting a VIA way - `wayB` remains in relation as a VIA way
             if (relation.hasFromViaTo()) {
                 var f = relation.memberByRole('from');
-                var v = relation.membersByRole('via');
+                var v = [
+                    ...relation.membersByRole('via'),
+                    ...relation.membersByRole('intersection'),
+                ];
                 var t = relation.memberByRole('to');
                 var i;
 
@@ -461,7 +464,10 @@ export function actionSplit(nodeIds, newWayIds) {
                 for (const parentRelation of parentRelations) {
                     if (parentRelation.hasFromViaTo()) {
                         // turn restrictions: via members must be loaded
-                        const vias = parentRelation.membersByRole('via');
+                        const vias = [
+                            ...parentRelation.membersByRole('via'),
+                            ...parentRelation.membersByRole('intersection'),
+                        ];
                         if (!vias.every(via => graph.hasEntity(via.id))) {
                             return 'parent_incomplete';
                         }
diff --git a/modules/osm/relation.js b/modules/osm/relation.js
index 66d50269b..6e5c8e18f 100644
--- a/modules/osm/relation.js
+++ b/modules/osm/relation.js
@@ -262,7 +262,10 @@ const prototype = {
     hasFromViaTo: function() {
         return (
             this.members.some(function(m) { return m.role === 'from'; }) &&
-            this.members.some(function(m) { return m.role === 'via'; }) &&
+            this.members.some((m) =>
+                m.role === 'via' ||
+                (m.role === 'intersection' && this.tags.type === 'destination_sign')
+            ) &&
             this.members.some(function(m) { return m.role === 'to'; })
         );
     },
diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js
index 10597209d..c2c9d186f 100644
--- a/test/spec/actions/split.js
+++ b/test/spec/actions/split.js
@@ -1673,7 +1673,9 @@ describe('iD.actionSplit', function () {
         });
 
 
-        ['restriction', 'restriction:bus', 'manoeuvre'].forEach(function (type) {
+        ['restriction', 'restriction:bus', 'manoeuvre', 'destination_sign'].forEach(function (type) {
+            const viaRole = type === 'destination_sign' ? 'intersection' : 'via';
+
             describe('type = ' + type, function () {
                 var a = iD.osmNode({id: 'a', loc: [0, 0]});
                 var b = iD.osmNode({id: 'b', loc: [1, 0]});
@@ -1694,7 +1696,7 @@ describe('iD.actionSplit', function () {
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '-', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]})
                 ]);
 
@@ -1710,7 +1712,7 @@ describe('iD.actionSplit', function () {
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '~', role: 'from', type: 'way'},
                         {id: '-', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]})
                 ]);
 
@@ -1726,7 +1728,7 @@ describe('iD.actionSplit', function () {
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '-', role: 'from', type: 'way'},
                         {id: '-', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]})
                 ]);
 
@@ -1747,7 +1749,7 @@ describe('iD.actionSplit', function () {
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '-', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: '|', role: 'via', type: 'way'}
+                        {id: '|', role: viaRole, type: 'way'}
                     ]})
                 ]);
 
@@ -1768,7 +1770,7 @@ describe('iD.actionSplit', function () {
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '~', role: 'from', type: 'way'},
                         {id: '-', role: 'to', type: 'way'},
-                        {id: '|', role: 'via', type: 'way'}
+                        {id: '|', role: viaRole, type: 'way'}
                     ]})
                 ]);
 
@@ -1788,7 +1790,7 @@ describe('iD.actionSplit', function () {
                     iD.osmWay({id: '‖', nodes: ['f', 'd']}),
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '|', role: 'from', type: 'way'},
-                        {id: '-', role: 'via', type: 'way'},
+                        {id: '-', role: viaRole, type: 'way'},
                         {id: '‖', role: 'to', type: 'way'}
                     ]})
                 ]);
@@ -1805,7 +1807,7 @@ describe('iD.actionSplit', function () {
                     iD.osmRelation({id: 'r', tags: {type: type}, members: [
                         {id: '-', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]})
                 ]);
 
@@ -1821,7 +1823,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '=', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -1837,7 +1839,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '-', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -1853,7 +1855,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '~', role: 'from', type: 'way'},
                         {id: '=', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -1869,7 +1871,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '~', role: 'from', type: 'way'},
                         {id: '-', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -1885,7 +1887,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '=', role: 'from', type: 'way'},
                         {id: '=', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -1901,7 +1903,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '-', role: 'from', type: 'way'},
                         {id: '-', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -1921,7 +1923,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '=', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: '|', role: 'via', type: 'way'}
+                        {id: '|', role: viaRole, type: 'way'}
                     ]);
                 });
 
@@ -1941,7 +1943,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '-', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: '|', role: 'via', type: 'way'}
+                        {id: '|', role: viaRole, type: 'way'}
                     ]);
                 });
 
@@ -1961,7 +1963,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '~', role: 'from', type: 'way'},
                         {id: '=', role: 'to', type: 'way'},
-                        {id: '|', role: 'via', type: 'way'}
+                        {id: '|', role: viaRole, type: 'way'}
                     ]);
                 });
 
@@ -1981,7 +1983,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '~', role: 'from', type: 'way'},
                         {id: '-', role: 'to', type: 'way'},
-                        {id: '|', role: 'via', type: 'way'}
+                        {id: '|', role: viaRole, type: 'way'}
                     ]);
                 });
 
@@ -2000,8 +2002,8 @@ describe('iD.actionSplit', function () {
 
                     expect(graph.entity('r').members).to.eql([
                         {id: '|', role: 'from', type: 'way'},
-                        {id: '-', role: 'via', type: 'way'},
-                        {id: '=', role: 'via', type: 'way'},
+                        {id: '-', role: viaRole, type: 'way'},
+                        {id: '=', role: viaRole, type: 'way'},
                         {id: '‖', role: 'to', type: 'way'}
                     ]);
                 });
@@ -2021,8 +2023,8 @@ describe('iD.actionSplit', function () {
 
                     expect(graph.entity('r').members).to.eql([
                         {id: '|', role: 'from', type: 'way'},
-                        {id: '-', role: 'via', type: 'way'},
-                        {id: '=', role: 'via', type: 'way'},
+                        {id: '-', role: viaRole, type: 'way'},
+                        {id: '=', role: viaRole, type: 'way'},
                         {id: '‖', role: 'to', type: 'way'}
                     ]);
                 });
@@ -2039,7 +2041,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '-', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
 
@@ -2055,7 +2057,7 @@ describe('iD.actionSplit', function () {
                     expect(graph.entity('r').members).to.eql([
                         {id: '=', role: 'from', type: 'way'},
                         {id: '~', role: 'to', type: 'way'},
-                        {id: 'd', role: 'via', type: 'node'}
+                        {id: 'd', role: viaRole, type: 'node'}
                     ]);
                 });
             });
diff --git a/test/spec/osm/relation.js b/test/spec/osm/relation.js
index e8f2b15af..9741ca19e 100644
--- a/test/spec/osm/relation.js
+++ b/test/spec/osm/relation.js
@@ -262,6 +262,32 @@ describe('iD.osmRelation', function () {
             });
             expect(r.hasFromViaTo()).to.be.false;
         });
+
+        it('returns true if the `intersection` role is used instead of `via` for destination signs', () => {
+            const r = iD.osmRelation({
+                id: 'r',
+                tags: { type: 'destination_sign' },
+                members: [
+                    { role: 'from', id: 'f', type: 'way' },
+                    { role: 'intersection', id: 'v', type: 'node' },
+                    { role: 'to', id: 't', type: 'way' },
+                ]
+            });
+            expect(r.hasFromViaTo()).to.be.true;
+        });
+
+        it('returns false if the `intersection` role is used on anything other than a destination sign', () => {
+            const r = iD.osmRelation({
+                id: 'r',
+                tags: { type: 'restriction' },
+                members: [
+                    { role: 'from', id: 'f', type: 'way' },
+                    { role: 'intersection', id: 'v', type: 'node' },
+                    { role: 'to', id: 't', type: 'way' },
+                ]
+            });
+            expect(r.hasFromViaTo()).to.be.false;
+        });
     });
 
     describe('#isRestriction', function () {