From aa1565baf80c48aec7049f0270eeb2d2c57a3519 Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Sat, 10 Mar 2018 00:12:46 -0500
Subject: [PATCH] Avoid reordering stops and platforms in PTv2 routes (closes
 #4864)

---
 modules/actions/add_member.js   | 34 ++++++++++++++++++++++++++++-----
 test/spec/actions/add_member.js | 20 ++++++++++++++-----
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js
index 049175329..4218fffc3 100644
--- a/modules/actions/add_member.js
+++ b/modules/actions/add_member.js
@@ -10,10 +10,20 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) {
     return function action(graph) {
         var relation = graph.entity(relationId);
 
-        if ((isNaN(memberIndex) || insertPair) && member.type === 'way') {
+        // There are some special rules for Public Transport v2 routes.
+        var isPTv2 = (member.role === 'stop' || member.role === 'platform');
+
+        if ((isNaN(memberIndex) || insertPair) && member.type === 'way' && !isPTv2) {
             // Try to perform sensible inserts based on how the ways join together
             graph = addWayMember(relation, graph);
         } else {
+            // see https://wiki.openstreetmap.org/wiki/Public_transport#Service_routes
+            // Stops and Platforms for PTv2 should be ordered first.
+            // hack: We do not currently have the ability to place them in the exactly correct order.
+            if (isPTv2 && isNaN(memberIndex)) {
+                memberIndex = 0;
+            }
+
             graph = graph.replace(relation.addMember(member, memberIndex));
         }
 
@@ -26,6 +36,20 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) {
     function addWayMember(relation, graph) {
         var groups, tempWay, item, i, j, k;
 
+        // remove PTv2 stops and platforms before doing anything.
+        var PTv2members = [];
+        var members = [];
+        for (i = 0; i < relation.members.length; i++) {
+            var m = relation.members[i];
+            if (m.role === 'stop' || m.role === 'platform') {
+                PTv2members.push(m);
+            } else {
+                members.push(m);
+            }
+        }
+        relation = relation.update({ members: members });
+
+
         if (insertPair) {
             // We're adding a member that must stay paired with an existing member.
             // (This feature is used by `actionSplit`)
@@ -50,7 +74,7 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) {
             groups.way.push(member);
         }
 
-        var members = withIndex(groups.way);
+        members = withIndex(groups.way);
         var joined = osmJoinWays(members, graph);
 
         // `joined` might not contain all of the way members,
@@ -116,10 +140,10 @@ export function actionAddMember(relationId, member, memberIndex, insertPair) {
             }
         }
 
-        // Write members in the order: nodes, ways, relations
-        // This is reccomended for Public Transport routes:
+        // Put stops and platforms first, then nodes, ways, relations
+        // This is recommended for Public Transport v2 routes:
         // see https://wiki.openstreetmap.org/wiki/Public_transport#Service_routes
-        var newMembers = (groups.node || []).concat(wayMembers, (groups.relation || []));
+        var newMembers = PTv2members.concat( (groups.node || []), wayMembers, (groups.relation || []) );
 
         return graph.replace(relation.update({members: newMembers}));
 
diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js
index e27ef07e7..5b70bd54b 100644
--- a/test/spec/actions/add_member.js
+++ b/test/spec/actions/add_member.js
@@ -177,7 +177,7 @@ describe('iD.actionAddMember', function() {
             expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']);
         });
 
-        it('reorders members as node, way, relation (for Public Transport routing)', function() {
+        it('keeps stops and platforms ordered before node, way, relation (for PTv2 routes)', function() {
             var graph = iD.coreGraph([
                 iD.osmNode({id: 'a', loc: [0, 0]}),
                 iD.osmNode({id: 'b', loc: [0, 0]}),
@@ -185,17 +185,27 @@ describe('iD.actionAddMember', function() {
                 iD.osmWay({id: '-', nodes: ['a', 'b']}),
                 iD.osmWay({id: '=', nodes: ['b', 'c']}),
                 iD.osmRelation({id: 'r', members: [
-                    { id: 'n1', type: 'node', role: 'forward' },
+                    { id: 'n1', type: 'node', role: 'stop' },
+                    { id: 'w1', type: 'way', role: 'platform' },
+                    { id: 'n2', type: 'node', role: 'stop' },
+                    { id: 'w2', type: 'way', role: 'platform' },
+                    { id: 'n3', type: 'node', role: 'forward' },
+                    { id: 'n4', type: 'node', role: 'forward' },
                     { id: '-', type: 'way', role: 'forward' },
                     { id: 'r1', type: 'relation', role: 'forward' },
-                    { id: 'n2', type: 'node', role: 'forward' }
+                    { id: 'n5', type: 'node', role: 'forward' }
                 ]})
             ]);
 
             graph = iD.actionAddMember('r', { id: '=', type: 'way', role: 'forward' })(graph);
             expect(graph.entity('r').members).to.eql([
-                { id: 'n1', type: 'node', role: 'forward' },
-                { id: 'n2', type: 'node', role: 'forward' },
+                { id: 'n1', type: 'node', role: 'stop' },
+                { id: 'w1', type: 'way', role: 'platform' },
+                { id: 'n2', type: 'node', role: 'stop' },
+                { id: 'w2', type: 'way', role: 'platform' },
+                { id: 'n3', type: 'node', role: 'forward' },
+                { id: 'n4', type: 'node', role: 'forward' },
+                { id: 'n5', type: 'node', role: 'forward' },
                 { id: '-', type: 'way', role: 'forward' },
                 { id: '=', type: 'way', role: 'forward' },
                 { id: 'r1', type: 'relation', role: 'forward' }