Merge pull request #344 from opentripplanner/enhance-auto-plan

Clarify strategies for auto-plan

Author: landonreed

Diff for: __tests__/reducers/__snapshots__/create-otp-reducer.js.snap

@@ -4,7 +4,10 @@ exports[`lib > reducers > create-otp-reducer should be able to create the initia
 Object {
   "activeSearchId": 0,
   "config": Object {
-    "autoPlan": false,
+    "autoPlan": Object {
+      "default": "ONE_LOCATION_CHANGED",
+      "mobile": "BOTH_LOCATIONS_CHANGED",
+    },
     "debouncePlanTimeMs": 0,
     "homeTimezone": "America/Los_Angeles",
     "language": Object {},

Diff for: example-config.yml

@@ -15,11 +15,22 @@ api:
 #     lon: -122.71607145667079
 #     name: Oregon Zoo, Portland, OR
 
+
+### Define the strategies for how to handle auto-planning of a new trip when
+### different query parameters are changes in the form. The default config is
+### shown below, but if autoPlan is set to false, auto-plan will never occur.
+### Other strategies besides those shown below are: ANY (any changed param will
+### cause a re-plan).
+# autoPlan:
+#   mobile: BOTH_LOCATIONS_CHANGED
+#   default: ONE_LOCATION_CHANGED
+
 ### The default query parameters can be overridden be uncommenting this object.
 ### Note: the override values must be valid values within otp-ui's query-params.js
 # defaultQueryParams:
 #   maxWalkDistance: 3219 # 2 miles in meters
 
+
 ### The persistence setting is used to enable the storage of places (home, work),
 ### recent searches/places, user overrides, and favorite stops.
 ### Pick the strategy that best suits your needs.

Diff for: lib/actions/form.js

@@ -78,48 +78,50 @@ export function parseUrlQueryString (params = getUrlParams()) {
 let debouncedPlanTrip // store as variable here, so it can be reused.
 let lastDebouncePlanTimeMs
 
+/**
+ * This action is dispatched when a change between the old query and new query
+ * is detected. It handles checks for whether the trip should be replanned
+ * (based on autoPlan strategies) as well as updating the UI state (esp. for
+ * mobile).
+ */
 export function formChanged (oldQuery, newQuery) {
   return function (dispatch, getState) {
     const otpState = getState().otp
+    const { config, currentQuery, ui } = otpState
+    const { autoPlan, debouncePlanTimeMs } = config
     const isMobile = coreUtils.ui.isMobile()
-
+    const {
+      fromChanged,
+      oneLocationChanged,
+      shouldReplanTrip,
+      toChanged
+    } = checkShouldReplanTrip(autoPlan, isMobile, oldQuery, newQuery)
     // If departArrive is set to 'NOW', update the query time to current
-    if (otpState.currentQuery && otpState.currentQuery.departArrive === 'NOW') {
-      dispatch(settingQueryParam({ time: moment().format(coreUtils.time.OTP_API_TIME_FORMAT) }))
+    if (currentQuery.departArrive === 'NOW') {
+      const now = moment().format(coreUtils.time.OTP_API_TIME_FORMAT)
+      dispatch(settingQueryParam({ time: now }))
     }
-
-    // Determine if either from/to location has changed
-    const fromChanged = !isEqual(oldQuery.from, newQuery.from)
-    const toChanged = !isEqual(oldQuery.to, newQuery.to)
-
     // Only clear the main panel if a single location changed. This prevents
     // clearing the panel on load if the app is focused on a stop viewer but a
     // search query should also be visible.
-    const oneLocationChanged = (fromChanged && !toChanged) || (!fromChanged && toChanged)
     if (oneLocationChanged) {
       dispatch(setMainPanelContent(null))
     }
-
-    // Clear the current search and return to search screen on mobile when
-    // either location changes only if not currently on welcome screen (otherwise
-    // when the current position is auto-set the screen will change unexpectedly).
-    if (
-      isMobile &&
-      (fromChanged || toChanged) &&
-      otpState.ui.mobileScreen !== MobileScreens.WELCOME_SCREEN
-    ) {
-      dispatch(clearActiveSearch())
-      dispatch(setMobileScreen(MobileScreens.SEARCH_FORM))
-    }
-
-    // Check whether a trip should be auto-replanned
-    const { autoPlan, debouncePlanTimeMs } = otpState.config
-    const updatePlan =
-      autoPlan ||
-      (!isMobile && oneLocationChanged) || // TODO: make autoplan configurable at the parameter level?
-      (isMobile && fromChanged && toChanged)
-    if (updatePlan && queryIsValid(otpState)) { // trip plan should be made
-      // check if debouncing function needs to be (re)created
+    if (!shouldReplanTrip) {
+      // If not replanning the trip, clear the current search when either
+      // location changes.
+      if (fromChanged || toChanged) {
+        dispatch(clearActiveSearch())
+        // Return to search screen on mobile only if not currently on welcome
+        // screen (otherwise when the current position is auto-set the screen
+        // will change unexpectedly).
+        if (ui.mobileScreen !== MobileScreens.WELCOME_SCREEN) {
+          dispatch(setMobileScreen(MobileScreens.SEARCH_FORM))
+        }
+      }
+    } else if (queryIsValid(otpState)) {
+      // If replanning trip and query is valid,
+      // check if debouncing function needs to be (re)created.
       if (!debouncedPlanTrip || lastDebouncePlanTimeMs !== debouncePlanTimeMs) {
         debouncedPlanTrip = debounce(() => dispatch(routingQuery()), debouncePlanTimeMs)
         lastDebouncePlanTimeMs = debouncePlanTimeMs
@@ -128,3 +130,50 @@ export function formChanged (oldQuery, newQuery) {
     }
   }
 }
+
+/**
+ * Check if the trip should be replanned based on the auto plan strategy,
+ * whether the mobile view is active, and the old/new queries. Response type is
+ * an object containing various booleans.
+ */
+export function checkShouldReplanTrip (autoPlan, isMobile, oldQuery, newQuery) {
+  // Determine if either from/to location has changed
+  const fromChanged = !isEqual(oldQuery.from, newQuery.from)
+  const toChanged = !isEqual(oldQuery.to, newQuery.to)
+  const oneLocationChanged = (fromChanged && !toChanged) || (!fromChanged && toChanged)
+  // Check whether a trip should be auto-replanned
+  const strategy = isMobile && autoPlan?.mobile
+    ? autoPlan?.mobile
+    : autoPlan?.default
+  const shouldReplanTrip = evaluateAutoPlanStrategy(
+    strategy,
+    fromChanged,
+    toChanged,
+    oneLocationChanged
+  )
+  return {
+    fromChanged,
+    oneLocationChanged,
+    shouldReplanTrip,
+    toChanged
+  }
+}
+
+/**
+ * Shorthand method to evaluate auto plan strategy. It is assumed that this is
+ * being called within the context of the `formChanged` action, so presumably
+ * some query param has already changed. If further checking of query params is
+ * needed, additional strategies should be added.
+ */
+const evaluateAutoPlanStrategy = (strategy, fromChanged, toChanged, oneLocationChanged) => {
+  switch (strategy) {
+    case 'ONE_LOCATION_CHANGED':
+      if (oneLocationChanged) return true
+      break
+    case 'BOTH_LOCATIONS_CHANGED':
+      if (fromChanged && toChanged) return true
+      break
+    case 'ANY': return true
+    default: return false
+  }
+}

Diff for: lib/reducers/create-otp-reducer.js

@@ -66,7 +66,10 @@ function validateInitialState (initialState) {
  */
 export function getInitialState (userDefinedConfig) {
   const defaultConfig = {
-    autoPlan: false,
+    autoPlan: {
+      mobile: 'BOTH_LOCATIONS_CHANGED',
+      default: 'ONE_LOCATION_CHANGED'
+    },
     debouncePlanTimeMs: 0,
     language: {},
     transitOperators: [],