From 64194d35f33db067c9ce83616c4ab6c6555ca4dc Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Thu, 10 Mar 2022 11:09:53 -0800 Subject: [PATCH 1/8] refactor(actions/ui): change full route data check --- lib/actions/ui.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 1b0622ee4..50a769c54 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -174,9 +174,9 @@ export function matchContentToUrl(location) { case 'route': if (id) { // This is a bit of a hack to check if the route details have been grabbed - // agencyName will only be populated if route details are repsent + // bikesAllowed will only be populated if route details are repsent // Moving away from manual requests should help resolve this - if (!state.otp.transitIndex?.routes?.[id]?.agencyName) { + if (!state.otp.transitIndex?.routes?.[id]?.bikesAllowed) { dispatch(findRoute({ routeId: id })) } // Check for pattern "submatch" From e0bc80dc3dcc9b84645df4d5399b03c8de1788c9 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Thu, 10 Mar 2022 13:06:39 -0800 Subject: [PATCH 2/8] refactor: address pr feedback --- lib/actions/api.js | 44 +++++++++++++++++------------- lib/reducers/create-otp-reducer.js | 2 +- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/lib/actions/api.js b/lib/actions/api.js index ada905e72..4f7eeef4f 100644 --- a/lib/actions/api.js +++ b/lib/actions/api.js @@ -25,9 +25,6 @@ const { getRoutingParams, getTripOptionsFromQuery, getUrlParams } = coreUtils.query const { randId } = coreUtils.storage -// set actions variable that can be set to v2Actions if v2 server is set -let actions = v1Actions - // Generic API actions /* @@ -91,6 +88,24 @@ function getActiveItinerary(state) { return activeItinerary } +/** + * Dispatches a method from either v1actions or v2actions, depending on + * which version of OTP is specified in the config. + * @param {*} methodName the method to execute + * @param {...any} params varargs of params to send to the action + */ +function executeOTPVersionSpecificAction(methodName, ...params) { + return function (dispatch, getState) { + const state = getState() + const { api } = state.otp.config + return dispatch( + api.v2 + ? v2Actions[methodName](...params) + : v1Actions[methodName](...params) + ) + } +} + /** * Send a routing query to the OTP backend. * @@ -248,7 +263,6 @@ function getOtpFetchOptions(state, includeToken = false) { let apiBaseUrl, apiKey, token const { api, persistence } = state.otp.config - if (api.v2) actions = v2Actions if (persistence && persistence.otp_middleware) { // Prettier does not understand the syntax on this line // eslint-disable-next-line prettier/prettier @@ -350,7 +364,8 @@ export function vehicleRentalQuery( errorAction = vehicleRentalError, options = {} ) { - return actions.vehicleRentalQuery( + return executeOTPVersionSpecificAction( + 'vehicleRentalQuery', params, responseAction, errorAction, @@ -363,7 +378,7 @@ export const findStopResponse = createAction('FIND_STOP_RESPONSE') export const findStopError = createAction('FIND_STOP_ERROR') export function fetchStopInfo(stop) { - return actions.fetchStopInfo(stop) + return executeOTPVersionSpecificAction('fetchStopInfo', stop) } // Single trip lookup query @@ -372,7 +387,7 @@ export const findTripResponse = createAction('FIND_TRIP_RESPONSE') export const findTripError = createAction('FIND_TRIP_ERROR') export function findTrip(params) { - return actions.findTrip(params) + return executeOTPVersionSpecificAction('findTrip', params) } // Stops for trip query @@ -545,20 +560,11 @@ export const findRouteResponse = createAction('FIND_ROUTE_RESPONSE') export const findRouteError = createAction('FIND_ROUTE_ERROR') export function findRoute(params) { - return function (dispatch, getState) { - const state = getState() - const { api } = state.otp.config - // Because findRoute is sometimes called from strange places, - // we must explicitly check for V2 as we can not rely on - // the check having already happened - if (api.v2) actions = v2Actions - - return dispatch(actions.findRoute(params)) - } + return executeOTPVersionSpecificAction('findRoute', params) } export function findPatternsForRoute(params) { - return actions.findPatternsForRoute(params) + return executeOTPVersionSpecificAction('findPatternsForRoute', params) } // Geometry for Pattern lookup query @@ -768,7 +774,7 @@ export const receivedVehiclePositionsError = createAction( ) export function getVehiclePositionsForRoute(routeId) { - return actions.getVehiclePositionsForRoute(routeId) + return executeOTPVersionSpecificAction('getVehiclePositionsForRoute', routeId) } const throttledUrls = {} diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js index a6d9a1a78..c574b8614 100644 --- a/lib/reducers/create-otp-reducer.js +++ b/lib/reducers/create-otp-reducer.js @@ -727,7 +727,7 @@ function createOtpReducer(config) { return update(state, { transitIndex: { routes: { - [action.payload.routeId || 'null']: { + [action?.payload?.routeId || 'null']: { patterns: { [action.payload.patternId]: { stops: { $set: patternStops } From d2507aa982ecca23a82fb1964c5e7a7099b81d88 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Thu, 10 Mar 2022 13:32:01 -0800 Subject: [PATCH 3/8] refactor(actions/apiv*): address pr comments --- lib/actions/apiV1.js | 28 +++++++++++++--------------- lib/actions/apiV2.js | 33 ++++++++++++++++----------------- 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/lib/actions/apiV1.js b/lib/actions/apiV1.js index 62388041f..e255bd863 100644 --- a/lib/actions/apiV1.js +++ b/lib/actions/apiV1.js @@ -54,9 +54,8 @@ export function vehicleRentalQuery( return createQueryAction(endpoint, responseAction, errorAction, options) } -function findNearbyAmenities(params, stopId) { +function findNearbyAmenities({ lat, lon, radius = 300 }, stopId) { return function (dispatch, getState) { - const { lat, lon, radius } = params const bounds = L.latLng(lat, lon).toBounds(radius) const { lat: low, lng: left } = bounds.getSouthWest() const { lat: up, lng: right } = bounds.getNorthEast() @@ -109,8 +108,8 @@ function findNearbyAmenities(params, stopId) { } } -export function findStop(params) { - return createQueryAction( +export const findStop = (params) => + createQueryAction( `index/stops/${params.stopId}`, findStopResponse, findStopError, @@ -123,19 +122,18 @@ export function findStop(params) { serviceId: 'stops' } ) -} const fetchStopInfo = (stop) => async function (dispatch, getState) { await dispatch(findStop({ stopId: stop.stopId })) const state = getState() - const { nearbyRadius: radius } = state.otp.config.stopViewer + const { nearbyRadius } = state.otp?.config?.stopViewer const fetchedStop = state.otp.transitIndex.stops?.[stop?.stopId] // TODO: stop not found message if (!fetchedStop) return const { lat, lon } = fetchedStop - if (radius > 0) { + if (nearbyRadius > 0) { dispatch( findNearbyStops( { @@ -143,12 +141,14 @@ const fetchStopInfo = (stop) => includeStopTimes: true, lat, lon, - radius + nearbyRadius }, stop.stopId ) ) - dispatch(findNearbyAmenities({ lat, lon, radius }, stop.stopId)) + dispatch( + findNearbyAmenities({ lat, lon, radius: nearbyRadius }, stop.stopId) + ) dispatch(zoomToStop(fetchedStop)) } } @@ -168,8 +168,8 @@ const getVehiclePositionsForRoute = (routeId) => } ) -export function findPatternsForRoute(params) { - return createQueryAction( +export const findPatternsForRoute = (params) => + createQueryAction( `index/routes/${params.routeId}/patterns?includeGeometry=true`, findPatternsForRouteResponse, findPatternsForRouteError, @@ -205,10 +205,9 @@ export function findPatternsForRoute(params) { } } ) -} -export function findRoute(params) { - return createQueryAction( +export const findRoute = (params) => + createQueryAction( `index/routes/${params.routeId}`, findRouteResponse, findRouteError, @@ -221,7 +220,6 @@ export function findRoute(params) { } } ) -} export default { fetchStopInfo, diff --git a/lib/actions/apiV2.js b/lib/actions/apiV2.js index 270bb5b6d..a2b2cf6ce 100644 --- a/lib/actions/apiV2.js +++ b/lib/actions/apiV2.js @@ -1,3 +1,5 @@ +import clone from 'clone' + import { createQueryAction, findGeometryForTrip, @@ -105,14 +107,14 @@ const findTrip = (params) => } ) -export function vehicleRentalQuery( +export const vehicleRentalQuery = ( params, responseAction, errorAction, options -) { +) => // TODO: ErrorsByNetwork is missing - return createGraphQLQueryAction( + createGraphQLQueryAction( `{ rentalVehicles { vehicleId @@ -153,7 +155,6 @@ export function vehicleRentalQuery( } } ) -} const stopTimeGraphQLQuery = ` stopTimes: stoptimesForPatterns(numberOfDepartures: 3) { @@ -411,14 +412,13 @@ const fetchStopInfo = (stop) => { ) } -const getVehiclePositionsForRoute = () => { - return function (dispatch, getState) { +const getVehiclePositionsForRoute = () => + function (dispatch, getState) { console.warn('OTP2 does not yet support vehicle positions for route!') } -} -export function findRoute(params) { - return function (dispatch, getState) { +export const findRoute = (params) => + function (dispatch, getState) { const { routeId } = params if (!routeId) return @@ -481,8 +481,9 @@ export function findRoute(params) { const { route } = payload?.data if (!route) return + const newRoute = clone(route) const routePatterns = {} - route.patterns.forEach((pattern) => { + newRoute.patterns.forEach((pattern) => { const patternStops = pattern.stops.map((stop) => { const color = stop.routes?.length > 0 && `#${stop.routes[0].color}` @@ -496,20 +497,19 @@ export function findRoute(params) { stops: patternStops } }) - route.patterns = routePatterns + newRoute.patterns = routePatterns // TODO: avoid explicit behavior shift like this - route.v2 = true + newRoute.v2 = true - return route + return newRoute } } ) ) } -} -export function findPatternsForRoute(params) { - return function (dispatch, getState) { +export const findPatternsForRoute = (params) => + function (dispatch, getState) { const state = getState() const { routeId } = params const route = state?.otp?.transitIndex?.routes?.[routeId] @@ -521,7 +521,6 @@ export function findPatternsForRoute(params) { return dispatch(findRoute(params)) } } -} export default { fetchStopInfo, From f57035336853e6aad5443594f8d65b5031f48539 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Thu, 10 Mar 2022 13:35:35 -0800 Subject: [PATCH 4/8] refactor(actions/api): make v2 field optional --- lib/actions/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/actions/api.js b/lib/actions/api.js index 4f7eeef4f..429edc397 100644 --- a/lib/actions/api.js +++ b/lib/actions/api.js @@ -99,7 +99,7 @@ function executeOTPVersionSpecificAction(methodName, ...params) { const state = getState() const { api } = state.otp.config return dispatch( - api.v2 + api?.v2 ? v2Actions[methodName](...params) : v1Actions[methodName](...params) ) From 85b2db7d7a9dd6b2bfde26f7dfbad97ae2de9151 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Thu, 10 Mar 2022 13:42:06 -0800 Subject: [PATCH 5/8] refactor(actions/apiv2): fetch nearby scooter networks --- lib/actions/apiV2.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/actions/apiV2.js b/lib/actions/apiV2.js index a2b2cf6ce..d090a0bf2 100644 --- a/lib/actions/apiV2.js +++ b/lib/actions/apiV2.js @@ -256,6 +256,9 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { id lat lon + ...on vehicleRentalStation { + network + } } distance } @@ -268,6 +271,9 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { id lat lon + ...on RentalVehicle { + network + } } distance } @@ -309,7 +315,7 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { lat: edge.node?.place?.lat, lon: edge.node?.place?.lon, name: edge.node?.place?.id || '', - networks: ['null'] + networks: [edge.node?.place?.network || 'null'] } }) }, From a500cbe779f68c61c9e9739c30b640641fdb2b1a Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Mon, 14 Mar 2022 08:28:48 -0700 Subject: [PATCH 6/8] refactor: address pr feedback --- lib/actions/api.js | 14 +++++++------- lib/actions/ui.js | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/actions/api.js b/lib/actions/api.js index 429edc397..b1eadcece 100644 --- a/lib/actions/api.js +++ b/lib/actions/api.js @@ -94,7 +94,7 @@ function getActiveItinerary(state) { * @param {*} methodName the method to execute * @param {...any} params varargs of params to send to the action */ -function executeOTPVersionSpecificAction(methodName, ...params) { +function executeOTPAction(methodName, ...params) { return function (dispatch, getState) { const state = getState() const { api } = state.otp.config @@ -364,7 +364,7 @@ export function vehicleRentalQuery( errorAction = vehicleRentalError, options = {} ) { - return executeOTPVersionSpecificAction( + return executeOTPAction( 'vehicleRentalQuery', params, responseAction, @@ -378,7 +378,7 @@ export const findStopResponse = createAction('FIND_STOP_RESPONSE') export const findStopError = createAction('FIND_STOP_ERROR') export function fetchStopInfo(stop) { - return executeOTPVersionSpecificAction('fetchStopInfo', stop) + return executeOTPAction('fetchStopInfo', stop) } // Single trip lookup query @@ -387,7 +387,7 @@ export const findTripResponse = createAction('FIND_TRIP_RESPONSE') export const findTripError = createAction('FIND_TRIP_ERROR') export function findTrip(params) { - return executeOTPVersionSpecificAction('findTrip', params) + return executeOTPAction('findTrip', params) } // Stops for trip query @@ -560,11 +560,11 @@ export const findRouteResponse = createAction('FIND_ROUTE_RESPONSE') export const findRouteError = createAction('FIND_ROUTE_ERROR') export function findRoute(params) { - return executeOTPVersionSpecificAction('findRoute', params) + return executeOTPAction('findRoute', params) } export function findPatternsForRoute(params) { - return executeOTPVersionSpecificAction('findPatternsForRoute', params) + return executeOTPAction('findPatternsForRoute', params) } // Geometry for Pattern lookup query @@ -774,7 +774,7 @@ export const receivedVehiclePositionsError = createAction( ) export function getVehiclePositionsForRoute(routeId) { - return executeOTPVersionSpecificAction('getVehiclePositionsForRoute', routeId) + return executeOTPAction('getVehiclePositionsForRoute', routeId) } const throttledUrls = {} diff --git a/lib/actions/ui.js b/lib/actions/ui.js index 50a769c54..ba82b8365 100644 --- a/lib/actions/ui.js +++ b/lib/actions/ui.js @@ -174,7 +174,7 @@ export function matchContentToUrl(location) { case 'route': if (id) { // This is a bit of a hack to check if the route details have been grabbed - // bikesAllowed will only be populated if route details are repsent + // bikesAllowed will only be populated if route details are present // Moving away from manual requests should help resolve this if (!state.otp.transitIndex?.routes?.[id]?.bikesAllowed) { dispatch(findRoute({ routeId: id })) From e8ff07d4c60bdb564eeb79b4e4fb28486c1da907 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Wed, 16 Mar 2022 07:22:29 -0700 Subject: [PATCH 7/8] refactor(otp-reducer): avoid crash on incomplete nearby scooter response --- lib/reducers/create-otp-reducer.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js index c574b8614..ff92ee4b7 100644 --- a/lib/reducers/create-otp-reducer.js +++ b/lib/reducers/create-otp-reducer.js @@ -605,6 +605,8 @@ function createOtpReducer(config) { } case 'FIND_NEARBY_AMENITIES_RESPONSE': const { stopId, ...amenities } = action.payload + if (!stopId || !amenities) return state + return update(state, { transitIndex: { stops: { From 24adf0b688525050b8c29e7cd68a1d1248698679 Mon Sep 17 00:00:00 2001 From: miles-grant-ibigroup Date: Fri, 18 Mar 2022 08:08:53 -0700 Subject: [PATCH 8/8] refactor(api_v2): stability improvements --- lib/actions/apiV2.js | 24 +++++++++++++++++------- lib/reducers/create-otp-reducer.js | 3 +++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/actions/apiV2.js b/lib/actions/apiV2.js index d090a0bf2..67aab4695 100644 --- a/lib/actions/apiV2.js +++ b/lib/actions/apiV2.js @@ -75,7 +75,8 @@ const findTrip = (params) => tripBikesAllowed: bikesAllowed stops { - gtfsId: id + id: gtfsId + stopId: gtfsId code name lat @@ -256,7 +257,7 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { id lat lon - ...on vehicleRentalStation { + ...on VehicleRentalStation { network } } @@ -315,7 +316,9 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { lat: edge.node?.place?.lat, lon: edge.node?.place?.lon, name: edge.node?.place?.id || '', - networks: [edge.node?.place?.network || 'null'] + networks: [edge.node?.place?.network || 'null'], + x: edge.node?.place?.lon, + y: edge.node?.place?.lat } }) }, @@ -328,7 +331,9 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { lat: edge.node?.place?.lat, lon: edge.node?.place?.lon, name: edge.node?.place?.id || '', - networks: ['null'] + networks: ['null'], + x: edge.node?.place?.lon, + y: edge.node?.place?.lat } } ), @@ -342,7 +347,9 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { lat: edge.node?.place?.lat, lon: edge.node?.place?.lon, name: edge.node?.place?.id || '', - networks: ['null'] + networks: [edge.node?.place?.network || 'null'], + x: edge.node?.place?.lon, + y: edge.node?.place?.lat } }) } @@ -354,7 +361,10 @@ const findNearbyAmenities = ({ lat, lon, radius = 300, stopId }) => { const fetchStopInfo = (stop) => { const { stopId } = stop - if (!stopId) return {} + if (!stopId) + return function (dispatch, getState) { + console.warn("No stopId passed, can't fetch stop!") + } return createGraphQLQueryAction( `{ @@ -373,7 +383,7 @@ const fetchStopInfo = (stop) => { return findStopError(payload.errors) } const { stop } = payload?.data - if (!stop) return findStopError() + if (!stop || !stop.lat || !stop.lon) return findStopError() // Fetch nearby stops and amenities // TODO: add radius from config diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js index ff92ee4b7..ce02fa8f0 100644 --- a/lib/reducers/create-otp-reducer.js +++ b/lib/reducers/create-otp-reducer.js @@ -487,6 +487,8 @@ function createOtpReducer(config) { config: { autoPlan: { $set: action.payload.autoPlan } } }) case 'SET_MAP_CENTER': + if (isNaN(action.payload.lat) || isNaN(action.payload.lon)) return state + return update(state, { config: { map: { @@ -740,6 +742,7 @@ function createOtpReducer(config) { } }) case 'FIND_STOP_TIMES_FOR_TRIP_RESPONSE': + if (!action.payload.stopTimes) return state return update(state, { transitIndex: { trips: {