Merge pull request #418 from opentripplanner/improve-modestr-extraction

Use url query params if response lacks OTP data
opentripplanner · Jul 16, 2021 · a0b6f21 · a0b6f21
2 parents f590d24 + a46c56c
commit a0b6f21
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 171 deletions.
diff --git a/__tests__/actions/__snapshots__/api.js.snap b/__tests__/actions/__snapshots__/api.js.snap
@@ -38,6 +38,7 @@ Array [
         "error": [TypeError: Cannot read property 'trackRecent' of undefined],
         "requestId": "abcd1239",
         "searchId": "abcd1234",
+        "url": "http://mock-host.com:80/api/plan?fromPlace=Origin%20%2812%2C34%29%3A%3A12%2C34&toPlace=Destination%20%2834%2C12%29%3A%3A34%2C12&mode=WALK%2CTRANSIT&ignoreRealtimeUpdates=false",
       },
       "type": "ROUTING_ERROR",
     },
@@ -66,6 +67,7 @@ Array [
         "error": [Error: Received error from server],
         "requestId": "abcd1240",
         "searchId": "abcd1237",
+        "url": "http://mock-host.com:80/api/plan?fromPlace=Origin%20%2812%2C34%29%3A%3A12%2C34&toPlace=Destination%20%2834%2C12%29%3A%3A34%2C12&mode=WALK%2CTRANSIT&ignoreRealtimeUpdates=false",
       },
       "type": "ROUTING_ERROR",
     },
@@ -111,6 +113,7 @@ Array [
         "error": [TypeError: Cannot read property 'trackRecent' of undefined],
         "requestId": "abcd1236",
         "searchId": "abcd1234",
+        "url": "http://mock-host.com:80/api/plan?fromPlace=Origin%20%2812%2C34%29%3A%3A12%2C34&toPlace=Destination%20%2834%2C12%29%3A%3A34%2C12&mode=WALK%2CTRANSIT&ignoreRealtimeUpdates=false",
       },
       "type": "ROUTING_ERROR",
     },

diff --git a/lib/actions/api.js b/lib/actions/api.js
@@ -124,8 +124,8 @@ export function routingQuery (searchId = null, updateSearchInReducer = false) {
     return Promise.all(iterations.map((injectedParams, i) => {
       const requestId = randId()
       // fetch a realtime route
-      const query = constructRoutingQuery(state, false, injectedParams)
-      const realTimeFetch = fetch(query, getOtpFetchOptions(state))
+      const url = constructRoutingQuery(state, false, injectedParams)
+      const realTimeFetch = fetch(url, getOtpFetchOptions(state))
         .then(getJsonAndCheckResponse)
         .then(json => {
           const dispatchedRoutingResponse = dispatch(routingResponse({
@@ -143,12 +143,12 @@ export function routingQuery (searchId = null, updateSearchInReducer = false) {
             if (!isStoredPlace(to)) {
               dispatch(rememberPlace({ location: formatRecentPlace(to), type: 'recent' }))
             }
-            dispatch(rememberSearch(formatRecentSearch(query, state)))
+            dispatch(rememberSearch(formatRecentSearch(url, state)))
           }
           return dispatchedRoutingResponse
         })
         .catch(error => {
-          dispatch(routingError({ error, requestId, searchId }))
+          dispatch(routingError({ error, requestId, searchId, url }))
         })
       // Update OTP URL params if a new search. In other words, if we're
       // performing a search based on query params taken from the URL after a back
@@ -328,84 +328,6 @@ export function findStop (params) {
   )
 }
 
-// TODO: Optionally substitute GraphQL queries? Note: this is not currently
-// possible because gtfsdb (the alternative transit index used by TriMet) does not
-// support GraphQL queries.
-// export function findStop (params) {
-//   const query = `
-// query stopQuery($stopId: [String]) {
-//   stops (ids: $stopId) {
-//     id: gtfsId
-//     code
-//     name
-//     url
-//     lat
-//     lon
-//     stoptimesForPatterns {
-//       pattern {
-//         id: semanticHash
-//         route {
-//           id: gtfsId
-//           longName
-//           shortName
-//           sortOrder
-//         }
-//       }
-//       stoptimes {
-//         scheduledArrival
-//         realtimeArrival
-//         arrivalDelay
-//         scheduledDeparture
-//         realtimeDeparture
-//         departureDelay
-//         timepoint
-//         realtime
-//         realtimeState
-//         serviceDay
-//         headsign
-//       }
-//     }
-//   }
-// }
-// `
-//   return createGraphQLQueryAction(
-//     query,
-//     { stopId: params.stopId },
-//     findStopResponse,
-//     findStopError,
-//     {
-//       // find stop should not be throttled since it can make quite frequent
-//       // updates when fetching stop times for a stop
-//       noThrottle: true,
-//       serviceId: 'stops',
-//       rewritePayload: (payload) => {
-//         // convert pattern array to ID-mapped object
-//         const patterns = []
-//         const { stoptimesForPatterns, ...stop } = payload.data.stops[0]
-//         stoptimesForPatterns.forEach(obj => {
-//           const { pattern, stoptimes: stopTimes } = obj
-//           // It's possible that not all stop times for a pattern will share the
-//           // same headsign, but this is probably a minor edge case.
-//           const headsign = stopTimes[0]
-//             ? stopTimes[0].headsign
-//             : pattern.route.longName
-//           const patternIndex = patterns.findIndex(p =>
-//             p.headsign === headsign && pattern.route.id === p.route.id)
-//           if (patternIndex === -1) {
-//             patterns.push({ ...pattern, headsign, stopTimes })
-//           } else {
-//             patterns[patternIndex].stopTimes.push(...stopTimes)
-//           }
-//         })
-//         return {
-//           ...stop,
-//           patterns
-//         }
-//       }
-//     }
-//   )
-// }
-
 // Single trip lookup query
 
 export const findTripResponse = createAction('FIND_TRIP_RESPONSE')
@@ -558,45 +480,6 @@ export function findRoutes (params) {
   )
 }
 
-// export function findRoutes (params) {
-//   const query = `
-// {
-//   routes {
-//     id: gtfsId
-//     color
-//     longName
-//     shortName
-//     mode
-//     type
-//     desc
-//     bikesAllowed
-//     sortOrder
-//     textColor
-//     url
-//     agency {
-//       id: gtfsId
-//       name
-//       url
-//     }
-//   }
-// }
-//   `
-//   return createGraphQLQueryAction(
-//     query,
-//     {},
-//     findRoutesResponse,
-//     findRoutesError,
-//     {
-//       serviceId: 'routes',
-//       rewritePayload: (payload) => {
-//         const routes = {}
-//         payload.data.routes.forEach(rte => { routes[rte.id] = rte })
-//         return routes
-//       }
-//     }
-//   )
-// }
-
 // Patterns for Route lookup query
 // TODO: replace with GraphQL query for route => patterns => geometry
 const findPatternsForRouteResponse = createAction('FIND_PATTERNS_FOR_ROUTE_RESPONSE')
@@ -673,51 +556,6 @@ export function findGeometryForPattern (params) {
   )
 }
 
-// export function findRoute (params) {
-//   const query = `
-//   query routeQuery($routeId: [String]) {
-//     routes (ids: $routeId) {
-//       id: gtfsId
-//       patterns {
-//         id: semanticHash
-//         directionId
-//         headsign
-//         name
-//         semanticHash
-//         geometry {
-//           lat
-//           lon
-//         }
-//       }
-//     }
-//   }
-//   `
-//   return createGraphQLQueryAction(
-//     query,
-//     { routeId: params.routeId },
-//     findPatternsForRouteResponse,
-//     findPatternsForRouteError,
-//     {
-//       rewritePayload: (payload) => {
-//         // convert pattern array to ID-mapped object
-//         const patterns = {}
-//         payload.data.routes[0].patterns.forEach(ptn => {
-//           patterns[ptn.id] = {
-//             routeId: params.routeId,
-//             patternId: ptn.id,
-//             geometry: ptn.geometry
-//           }
-//         })
-//
-//         return {
-//           routeId: params.routeId,
-//           patterns
-//         }
-//       }
-//     }
-//   )
-// }
-
 // TNC ETA estimate lookup query
 
 export const transportationNetworkCompanyEtaResponse = createAction('TNC_ETA_RESPONSE')

diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js
@@ -299,7 +299,8 @@ function createOtpReducer (config) {
               response: {
                 $push: [{
                   error: action.payload.error,
-                  requestId
+                  requestId,
+                  url: action.payload.url
                 }]
               }
             }

diff --git a/lib/util/state.js b/lib/util/state.js
@@ -3,6 +3,7 @@ import isEqual from 'lodash.isequal'
 import memoize from 'lodash.memoize'
 import moment from 'moment'
 import hash from 'object-hash'
+import qs from 'qs'
 import { createSelector } from 'reselect'
 
 import { MainPanelContent } from '../actions/ui'
@@ -93,16 +94,19 @@ export function getResponsesWithErrors (state) {
     response.forEach(res => {
       if (res) {
         if (res.error) {
+          const params = qs.parse(res.url)
+          const modeStr = res.requestParameters?.mode || params.mode || ''
           let msg = res.error.msg || 'An error occurred while planning a trip'
           // include the modes if doing batch routing
-          if (showModes && res.requestParameters?.mode) {
-            const mode = humanReadableMode(res.requestParameters.mode)
+          if (showModes && modeStr) {
+            const mode = humanReadableMode(modeStr)
             msg = `No trip found for ${mode}. ${msg.replace(/^No trip found. /, '')}`
           }
           tripPlanningErrors.push({
             id: res.error.id,
-            modes: res.requestParameters?.mode?.split(','),
-            msg
+            modes: modeStr.split(','),
+            msg,
+            url: res.url
           })
         }
         const feedWideRentalErrors = getFeedWideRentalErrors(res)