address pr feedback

Author: miles-grant-ibigroup

lib/editor/actions/location.js

@@ -8,6 +8,7 @@ import type {GtfsLocation} from '../../types'
 
 import { receivedNewEntity, savedGtfsEntity } from './active'
 
+// FLEX TODO: The two big dispatch blocks are almost the same... Would it be possible to extract them (such as saveEntity('locationgroup', locationGroupUrl) and same for locationgroupstop)?
 export function saveLocationGroup (
   feedId: ?string,
   locationGroup: GtfsLocation,
@@ -38,11 +39,11 @@ export function saveLocationGroup (
         // Refetch entity and replace in store
         if (refetch) {
           dispatch(fetchGTFSEntities({
-            namespace,
-            id: savedEntity.id,
-            type: 'locationgroupstop',
             editor: true,
-            replaceNew: !notNew
+            id: savedEntity.id,
+            namespace,
+            replaceNew: !notNew,
+            type: 'locationgroupstop'
           }))
         } else {
           // Push new entity into store.
@@ -57,11 +58,11 @@ export function saveLocationGroup (
         // Refetch entity and replace in store
         if (refetch) {
           dispatch(fetchGTFSEntities({
-            namespace,
-            id: savedEntity.id,
-            type: 'locationgroup',
             editor: true,
-            replaceNew: !notNew
+            id: savedEntity.id,
+            namespace,
+            replaceNew: !notNew,
+            type: 'locationgroup'
           }))
         } else {
           // Push new entity into store.

lib/editor/actions/map/stopStrategies.js

@@ -253,9 +253,10 @@ export function addStopToPattern (pattern: Pattern, origStop: GtfsStop | GtfsLoc
 
       // find center to simulate stop
       const center = centerOfMass(turfPolygon([polygon]))
-      if (center.geometry.coordinates) {
-        stop.stop_lon = center.geometry.coordinates[1]
-        stop.stop_lat = center.geometry.coordinates[0]
+      const {coordinates} = center.geometry ? center.geometry : {coordinates: false}
+      if (coordinates) {
+        stop.stop_lon = coordinates[1]
+        stop.stop_lat = coordinates[0]
       }
     }
 
@@ -300,6 +301,7 @@ export function addStopToPattern (pattern: Pattern, origStop: GtfsStop | GtfsLoc
         dispatch(updatePatternStops(pattern, patternStops))
         // Only a stop should be checked, not a location
         // Otherwise, check if a shape ought to be created. Then, save.
+        // FLEX TODO: Is this working? Why 2?
         if (patternStops.length === 2 && followStreets) {
           // Create shape between stops the added stop is the second one and
           // followStreets is enabled. Otherwise, there is no need to create a

lib/editor/components/map/PatternStopsLayer.js

@@ -116,7 +116,8 @@ export default class PatternStopsLayer extends Component<Props> {
                 }
                 removeStopFromPattern={removeStopFromPattern}
                 stop={stop}
-                patternStop={patternStop} />
+                patternStop={patternStop}
+              />
             )
           }
           if (location) {
@@ -131,6 +132,7 @@ export default class PatternStopsLayer extends Component<Props> {
               key={patternStop.id}
               location={location}
               setActiveStop={setActiveStop}
+              // FLEX TODO: do we need these fallbacks? We might not.
               // fallback to index if/when id changes
               // $FlowFixMe
               ref={`${patternStop.id || patternStop.locationId || patternStop.locationGroupId}`}
@@ -139,7 +141,7 @@ export default class PatternStopsLayer extends Component<Props> {
               patternStop={patternStop} />
           }
           if (locationGroup) {
-            // 2024 TODO: support rendering location groups. will be tricky. need to
+            // FLEX TODO: support rendering location groups. will be tricky. need to
             // grab location group stop data
             return <></>
           }

lib/editor/components/pattern/EditShapePanel.js

@@ -124,7 +124,8 @@ export default class EditShapePanel extends Component<Props> {
       saveActiveGtfsEntity,
       showConfirmModal,
       stops,
-      locations, locationGroups,
+      locations,
+      locationGroups,
       updateActiveGtfsEntity,
       updatePatternGeometry
     } = this.props

lib/editor/components/pattern/PatternStopCard.js

@@ -242,7 +242,7 @@ class PatternStopCard extends Component<Props> {
     if (patternStop.locationId !== null || patternStop.locationGroupId !== null) {
       cardBackground = 'hsla(187, 84%, 87%, 0.5)'
     }
-    // $FlowFixMe flexDefaultTravelTime doens't exist on a PatternStop, which is the point of this check
+    // $FlowFixMe flexDefaultTravelTime doesn't exist on a PatternStop, which is the point of this check
     if ((!patternStop.defaultTravelTime && !patternStop.flexDefaultTravelTime) && index !== 0) {
       cardBackground = 'hsla(35, 84%, 87%, 1)'
     }

lib/editor/components/pattern/PatternStopContainer.js

@@ -137,19 +137,18 @@ class PatternStopContainer extends Component<Props, State> {
           let stop = false
           if (card.stopId !== null) {
             stop = stops.find(st => st.stop_id === card.stopId)
-            cumulativeTravelTime += card.defaultDwellTime + card.defaultTravelTime
           }
           // Since stops and locations are unified, the stop could also refer to a location
           // IDs exist in one namespace as per the spac
           if (card.locationId !== null) {
             stop = locations.find(l => l.location_id === card.locationId)
-            cumulativeTravelTime += card.defaultDwellTime + card.defaultTravelTime
           }
           if (card.locationGroupId !== null) {
             stop = locationGroups.find(lg => lg.location_group_id === card.locationGroupId)
-            cumulativeTravelTime += card.defaultDwellTime + card.defaultTravelTime
           }
 
+          cumulativeTravelTime += card.defaultDwellTime + card.defaultTravelTime
+
           return (
             // $FlowFixMe
             <PatternStopCard

lib/editor/components/pattern/TripPatternViewer.js

@@ -39,6 +39,7 @@ export default class TripPatternViewer extends Component<Props> {
     activePattern.patternStops = activePattern.patternStops.map((ps) => {
       return {
         ...ps,
+        // FLEX TODO: can we use || here?
         id: `${
           ps.stopId
             ? ps.stopId

lib/editor/components/timetable/TimetableEditor.js

@@ -199,8 +199,6 @@ export default class TimetableEditor extends Component<Props, State> {
           objectPath.set(newRow, `stopTimes.${i}`, blankStopTime)
         }
 
-        // objectPath.set(newRow, `stopTimes.${i}.stopId`, halt.stopId !== null ? halt.stopId : halt.locationId !== null ? halt.locationId : halt.locationGroupId)
-
         // only set time if timepoint set to true or null
         // if (stop.timepoint === null || stop.timepoint) {
         if (halt.stopId !== null) {

lib/editor/selectors/timetable.js

@@ -68,7 +68,7 @@ export const getTimetableColumns = createSelector(
           if (stops && !isLocation) stop = stops.find(st => st.stop_id === ps.stopId)
           if (locations && isLocation) stop = locations.find(l => l.location_id === ps.locationId)
 
-          // 2024 todo: fix location group names here?
+          // FLEX todo: fix location group names here?
           const stopName = stop ? stop.stop_name : ps.stopId ? ps.stopId : (ps.locationId ? ps.locationId : ps.locationGroupId)
           const stopId = stop && stop.stop_code
             ? stop.stop_code

lib/editor/util/map.js

@@ -819,8 +819,8 @@ export function stopToPatternStop (
     throw new Error('Stop Id not set')
   }
   return {
-    continuousDropOff: 1, // 1?
-    continuousPickup: 1, // 1?
+    continuousDropOff: 1, // FLEX TODO: Is 1 correct?
+    continuousPickup: 1, // FLEX TODO: 1 Is 1 correct?
     defaultDwellTime: 0,
     defaultTravelTime: 0,
     dropOffBookingRuleId: null,