Merge pull request #259 from opentripplanner/batch-routing-fixes

Get error prop from state; skip sort if batch disabled

__tests__/reducers/__snapshots__/create-otp-reducer.js.snap

@@ -50,7 +50,7 @@ Object {
     "filter": "ALL",
     "sort": Object {
       "direction": "ASC",
-      "type": "BEST",
+      "type": null,
     },
   },
   "location": Object {

lib/components/form/error-message.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 import { connect } from 'react-redux'
 import TripTools from '../narrative/trip-tools'
 
-import { getActiveSearch } from '../../util/state'
+import { getActiveErrors } from '../../util/state'
 
 class ErrorMessage extends Component {
   static propTypes = {
@@ -45,9 +45,8 @@ class ErrorMessage extends Component {
 // connect to the redux store
 
 const mapStateToProps = (state, ownProps) => {
-  const activeSearch = getActiveSearch(state.otp)
   return {
-    error: activeSearch && activeSearch.response && activeSearch.response.error,
+    error: getActiveErrors(state.otp)[0],
     currentQuery: state.otp.currentQuery,
     errorMessages: state.otp.config.errorMessages
   }

lib/components/mobile/results-screen.js

@@ -16,7 +16,12 @@ import MobileNavigationBar from './navigation-bar'
 import { MobileScreens, setMobileScreen } from '../../actions/ui'
 import { setUseRealtimeResponse } from '../../actions/narrative'
 import { clearActiveSearch } from '../../actions/form'
-import { getActiveItineraries, getActiveSearch, getRealtimeEffects } from '../../util/state'
+import {
+  getActiveErrors,
+  getActiveItineraries,
+  getActiveSearch,
+  getRealtimeEffects
+} from '../../util/state'
 
 const LocationContainer = styled.div`
   font-weight: 300;
@@ -239,7 +244,7 @@ const mapStateToProps = (state, ownProps) => {
   return {
     query: state.otp.currentQuery,
     realtimeEffects,
-    error: response && response.error,
+    error: getActiveErrors(state.otp)[0],
     resultCount:
       response
         ? activeSearch.query.routingType === 'ITINERARY'

lib/components/narrative/narrative-routing-results.js

@@ -6,7 +6,11 @@ import Loading from './loading'
 import TabbedItineraries from './tabbed-itineraries'
 import ErrorMessage from '../form/error-message'
 
-import { getActiveSearch, getActiveItineraries } from '../../util/state'
+import {
+  getActiveErrors,
+  getActiveItineraries,
+  getActiveSearch
+} from '../../util/state'
 import { setMainPanelContent } from '../../actions/ui'
 
 class NarrativeRoutingResults extends Component {
@@ -47,7 +51,7 @@ const mapStateToProps = (state, ownProps) => {
   const pending = activeSearch ? Boolean(activeSearch.pending) : false
   return {
     mainPanelContent: state.otp.ui.mainPanelContent,
-    error: activeSearch && activeSearch.response && activeSearch.response.error,
+    error: getActiveErrors(state.otp)[0],
     itineraries: getActiveItineraries(state.otp),
     pending,
     routingType: activeSearch && activeSearch.query.routingType

lib/reducers/create-otp-reducer.js

@@ -6,6 +6,7 @@ import coreUtils from '@opentripplanner/core-utils'
 
 import { MainPanelContent, MobileScreens } from '../actions/ui'
 import {getTimestamp} from '../util/state'
+import {isBatchRoutingEnabled} from '../util/itinerary'
 
 const { isTransit, getTransitModes } = coreUtils.itinerary
 const { matchLatLon } = coreUtils.map
@@ -161,7 +162,8 @@ export function getInitialState (userDefinedConfig, initialQuery) {
       filter: 'ALL',
       sort: {
         direction: 'ASC',
-        type: 'BEST'
+        // Only apply custom sort if batch routing is enabled.
+        type: isBatchRoutingEnabled(config) ? 'BEST' : null
       }
     },
     location: {

lib/util/itinerary.js

@@ -11,3 +11,7 @@ export function getLeafletItineraryBounds (itinerary) {
 export function getLeafletLegBounds (leg) {
   return latLngBounds(coreUtils.itinerary.getLegBounds(leg))
 }
+
+export function isBatchRoutingEnabled (otpConfig) {
+  return Boolean(otpConfig.routingTypes.find(t => t.key === 'BATCH'))
+}

lib/util/state.js

@@ -65,7 +65,7 @@ export function getActiveItineraries (otpState) {
   }
   const {filter, sort} = otpState.filter
   const {direction, type} = sort
-  return itineraries
+  const filteredItineraries = itineraries
     .filter(itinerary => {
       switch (filter) {
         case 'ALL':
@@ -91,34 +91,44 @@ export function getActiveItineraries (otpState) {
           return true
       }
     })
-    .sort((a, b) => {
-      switch (type) {
-        case 'WALKTIME':
-          if (direction === 'ASC') return a.walkTime - b.walkTime
-          else return b.walkTime - a.walkTime
-        case 'ARRIVALTIME':
-          if (direction === 'ASC') return a.endTime - b.endTime
-          else return b.endTime - a.endTime
-        case 'DEPARTURETIME':
-          if (direction === 'ASC') return a.startTime - b.startTime
-          else return b.startTime - a.startTime
-        case 'DURATION':
-          if (direction === 'ASC') return a.duration - b.duration
-          else return b.duration - a.duration
-        case 'COST':
-          const aTotal = getTotalFare(a, config)
-          const bTotal = getTotalFare(b, config)
-          if (direction === 'ASC') return aTotal - bTotal
-          else return bTotal - aTotal
-        default:
-          if (type !== 'BEST') console.warn(`Sort (${type}) not supported. Defaulting to BEST.`)
-          // FIXME: Fully implement default sort algorithm.
-          const aCost = calculateItineraryCost(a, config)
-          const bCost = calculateItineraryCost(b, config)
-          if (direction === 'ASC') return aCost - bCost
-          else return bCost - aCost
-      }
-    })
+  // If no sort type is provided (e.g., because batch routing is not enabled),
+  // do not sort itineraries (default sort from API response is used).
+  return !type
+    ? filteredItineraries
+    : filteredItineraries.sort((a, b) => sortItineraries(type, direction, a, b, config))
+}
+
+/**
+ * Array sort function for itineraries (in batch routing context) that attempts
+ * to sort based on the type/direction specified.
+ */
+function sortItineraries (type, direction, a, b, config) {
+  switch (type) {
+    case 'WALKTIME':
+      if (direction === 'ASC') return a.walkTime - b.walkTime
+      else return b.walkTime - a.walkTime
+    case 'ARRIVALTIME':
+      if (direction === 'ASC') return a.endTime - b.endTime
+      else return b.endTime - a.endTime
+    case 'DEPARTURETIME':
+      if (direction === 'ASC') return a.startTime - b.startTime
+      else return b.startTime - a.startTime
+    case 'DURATION':
+      if (direction === 'ASC') return a.duration - b.duration
+      else return b.duration - a.duration
+    case 'COST':
+      const aTotal = getTotalFare(a, config)
+      const bTotal = getTotalFare(b, config)
+      if (direction === 'ASC') return aTotal - bTotal
+      else return bTotal - aTotal
+    default:
+      if (type !== 'BEST') console.warn(`Sort (${type}) not supported. Defaulting to BEST.`)
+      // FIXME: Fully implement default sort algorithm.
+      const aCost = calculateItineraryCost(a, config)
+      const bCost = calculateItineraryCost(b, config)
+      if (direction === 'ASC') return aCost - bCost
+      else return bCost - aCost
+  }
 }
 
 /**