From 886430d59d145f9622d02bfe993ef6dcee499e0f Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Tue, 20 Oct 2020 15:20:35 -0400 Subject: [PATCH 1/2] fix(batch): get error prop from state; skip sort if batch disabled --- lib/components/form/error-message.js | 5 +- lib/components/mobile/results-screen.js | 9 ++- .../narrative/narrative-routing-results.js | 8 ++- lib/reducers/create-otp-reducer.js | 4 +- lib/util/itinerary.js | 4 ++ lib/util/state.js | 68 +++++++++++-------- 6 files changed, 61 insertions(+), 37 deletions(-) diff --git a/lib/components/form/error-message.js b/lib/components/form/error-message.js index 9dd6a9e61..9a9cb8cf1 100644 --- a/lib/components/form/error-message.js +++ b/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 } diff --git a/lib/components/mobile/results-screen.js b/lib/components/mobile/results-screen.js index ef3290e77..c5b8448d5 100644 --- a/lib/components/mobile/results-screen.js +++ b/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' diff --git a/lib/components/narrative/narrative-routing-results.js b/lib/components/narrative/narrative-routing-results.js index a3dcd9e4b..294080dfd 100644 --- a/lib/components/narrative/narrative-routing-results.js +++ b/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 diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js index 22c5d2a3e..c6ef88ec8 100644 --- a/lib/reducers/create-otp-reducer.js +++ b/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: { diff --git a/lib/util/itinerary.js b/lib/util/itinerary.js index 70468dbe5..b0e405be5 100644 --- a/lib/util/itinerary.js +++ b/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')) +} diff --git a/lib/util/state.js b/lib/util/state.js index 813a247d8..72dfa3114 100644 --- a/lib/util/state.js +++ b/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 + } } /** From d9931feee707aae54086f3cce651ed6afe86707e Mon Sep 17 00:00:00 2001 From: Landon Reed Date: Wed, 21 Oct 2020 08:57:11 -0400 Subject: [PATCH 2/2] test: update snapshot --- __tests__/reducers/__snapshots__/create-otp-reducer.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap b/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap index 93507dd74..69365bedb 100644 --- a/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap +++ b/__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 {