fix(editor): prevent an infinite loop when sorting trips

fixes #426

Author: evansiroky

__tests__/test-utils/index.js

@@ -0,0 +1,6 @@
+// @flow
+
+export function expectArrayToMatchContents (actual: any, expected: Array<any>) {
+  expect(actual).toHaveLength(expected.length)
+  expect(actual).toEqual(expect.arrayContaining(expected))
+}

lib/editor/util/__tests__/index.js

@@ -0,0 +1,80 @@
+// @flow
+
+import clone from 'lodash/cloneDeep'
+
+import {sortAndFilterTrips} from '../'
+import {expectArrayToMatchContents} from '../../../../__tests__/test-utils'
+import {mockTripWithoutStopTimes} from '../../../mock-data'
+
+describe('editor > util > index', () => {
+  describe('sortAndFilterTrips', () => {
+    it('should sort trips where there are no stop times', () => {
+      const mockTrips = [
+        clone(mockTripWithoutStopTimes),
+        clone(mockTripWithoutStopTimes)
+      ]
+      mockTrips[0].id = 2
+      mockTrips[0].tripId = 'test-trip-id-2'
+      expectArrayToMatchContents(
+        sortAndFilterTrips(mockTrips, undefined)
+          .map(trip => trip.tripId),
+        ['test-trip-id-2', 'test-trip-id-1']
+      )
+    })
+
+    it('should sort trips where one of the trips has no stop times', () => {
+      const mockTrips = [
+        clone(mockTripWithoutStopTimes),
+        clone(mockTripWithoutStopTimes)
+      ]
+      mockTrips[0].id = 2
+      mockTrips[0].tripId = 'test-trip-id-2'
+      mockTrips[0].stopTimes.push({
+        arrivalTime: 12345,
+        departureTime: 12345,
+        id: 1,
+        shape_dist_traveled: 0,
+        stopHeadsign: '',
+        stopId: '1',
+        stopSequence: 1
+      })
+      expectArrayToMatchContents(
+        sortAndFilterTrips(mockTrips, undefined)
+          .map(trip => trip.tripId),
+        ['test-trip-id-1', 'test-trip-id-2']
+      )
+    })
+
+    it('should sort trips where both of the trips have stop times', () => {
+      const mockTrips = [
+        clone(mockTripWithoutStopTimes),
+        clone(mockTripWithoutStopTimes)
+      ]
+      mockTrips[0].id = 2
+      mockTrips[0].tripId = 'test-trip-id-2'
+      mockTrips[0].stopTimes.push({
+        arrivalTime: 12345,
+        departureTime: 12345,
+        id: 1,
+        shape_dist_traveled: 0,
+        stopHeadsign: '',
+        stopId: '1',
+        stopSequence: 1
+      })
+      mockTrips[1].stopTimes.push({
+        arrivalTime: 45678,
+        departureTime: 45678,
+        id: 1,
+        shape_dist_traveled: 0,
+        stopHeadsign: '',
+        stopId: '1',
+        stopSequence: 1
+      })
+      expectArrayToMatchContents(
+        sortAndFilterTrips(mockTrips, undefined)
+          .map(trip => trip.tripId),
+        ['test-trip-id-2', 'test-trip-id-1']
+      )
+    })
+  })
+})

lib/editor/util/index.js

@@ -45,7 +45,7 @@ export function getStopsForPattern (pattern: Pattern, stops: Array<GtfsStop>): A
 
 export function sortAndFilterTrips (
   trips: ?Array<Trip>,
-  useFrequency: boolean
+  useFrequency: ?boolean
 ): Array<Trip> {
   return trips
     ? trips
@@ -60,6 +60,9 @@ export function sortAndFilterTrips (
           // only has one frequency entry (for now).
           return a.frequencies[0].startTime - b.frequencies[0].startTime
         }
+        if (a.stopTimes.length === 0 && b.stopTimes.length === 0) return 0
+        if (a.stopTimes.length === 0) return -1
+        if (b.stopTimes.length === 0) return 1
         // There may be a case where a null value (skipped stop) appears first
         // in the list of stopTimes. In this case, we want to compare on the
         // first stopTime that exists for each pair of trips.

lib/mock-data.js

@@ -25,6 +25,23 @@ import {defaultState as defaultSignActiveState} from './signs/reducers/active'
 import {defaultState as defaultSignSignsState} from './signs/reducers/signs'
 import type {AppState} from './types/reducers'
 
+export const mockTripWithoutStopTimes = {
+  bikes_allowed: null,
+  blockId: '1',
+  directionId: 0,
+  frequencies: [],
+  id: 3,
+  pattern_id: '1',
+  route_id: 'GL',
+  service_id: 'GL-1',
+  shape_id: '19',
+  stopTimes: [],
+  tripHeadsign: 'Walla Walla',
+  tripId: 'test-trip-id-1',
+  tripShortName: null,
+  wheelchair_accessible: null
+}
+
 const defaultManagerState = {
   languages: defaultManagerLanguagesState,
   projects: defaultManagerProjectsState,

lib/types/index.js

@@ -593,11 +593,21 @@ type Frequency = {
 }
 
 export type Trip = {|
+  bikes_allowed: ?boolean,
+  blockId: string,
+  directionId: number,
   frequencies: Array<Frequency>,
   id: null | number,
+  pattern_id: string,
+  route_id: string,
+  service_id: string,
+  shape_id: string,
   stopTimes: Array<StopTime>,
+  tripHeadsign: string,
   tripId: string,
-  useFrequency: boolean
+  tripShortName: ?string,
+  useFrequency?: boolean,
+  wheelchair_accessible: ?boolean
 |}
 
 export type Entity = ScheduleException |