fix(editor): handle bad entity and pattern IDs in editor URL

fixes #99

Author: landonreed

lib/editor/actions/active.js

@@ -73,7 +73,15 @@ export function setActiveEntity (feedSourceId, component, entity, subComponent,
  *
  * FIXME: There is too much going on in this action.
  */
-export function setActiveGtfsEntity (feedSourceId, component, entityId, subComponent, subEntityId, subSubComponent, subSubEntityId) {
+export function setActiveGtfsEntity (
+  feedSourceId,
+  component,
+  entityId,
+  subComponent,
+  subEntityId,
+  subSubComponent,
+  subSubEntityId
+) {
   return function (dispatch, getState) {
     // TODO: Figure out a good way to handle route changes without confirm window
     // when there are unsaved changes to the active entity (or subentity).
@@ -93,6 +101,7 @@ export function setActiveGtfsEntity (feedSourceId, component, entityId, subCompo
       // Feed source has changed. Clear GTFS content
       // FIXME: Do we need to clear GTFS content if namespace changes?
       // FIXME: This should be covered by GtfsEditor#componentWillReceiveProps
+      console.warn(`Feed source ID has changed ${prevFeedSourceId} to ${feedSourceId}. Clearing GTFS content.`)
       dispatch(clearGtfsContent())
     }
     if (editSettings.editGeometry) {

lib/editor/actions/editor.js

@@ -212,10 +212,10 @@ export function newGtfsEntity (feedSourceId, component, props, save, refetch = t
     } else {
       dispatch(createGtfsEntity(feedSourceId, component, props))
       if (props && 'routeId' in props) {
-        // console.log('setting after create')
+        // Set active entity for trip pattern
         dispatch(setActiveGtfsEntity(feedSourceId, 'route', props.routeId, component, ENTITY.NEW_ID))
       } else {
-        console.log('setting after create', feedSourceId, component, ENTITY.NEW_ID)
+        // Set active entity for all other basic types (e.g., routes, stops, etc.).
         dispatch(setActiveGtfsEntity(feedSourceId, component, ENTITY.NEW_ID))
       }
     }

lib/editor/containers/ActiveGtfsEditor.js

@@ -1,7 +1,6 @@
 import {connect} from 'react-redux'
 
 import GtfsEditor from '../components/GtfsEditor'
-import {ENTITY} from '../constants'
 import {fetchFeedSourceAndProject} from '../../manager/actions/feeds'
 import {
   fetchTripPatternsForRoute,
@@ -45,37 +44,32 @@ import {updateUserMetadata} from '../../manager/actions/user'
 import {findProjectByFeedSource} from '../../manager/util'
 import {setTutorialHidden} from '../../manager/actions/ui'
 import {getControlPoints, getValidationErrors} from '../selectors'
-import {getTableById} from '../util/gtfs'
+import {getTableById, getIdsFromParams} from '../util/gtfs'
 
 const mapStateToProps = (state, ownProps) => {
   const {
     feedSourceId,
     activeComponent,
     subComponent,
     subSubComponent,
-    // activeEntityId,
-    // subEntityId,
     activeSubSubEntity
   } = ownProps.routeParams
-  // TODO: Figure out a better way to parse an integer from react-router route
-  // params
   // Cast IDs to integers to match data type of fields returned by the SQL database
-  const activeEntityId = typeof ownProps.routeParams.activeEntityId !== 'undefined' ? +ownProps.routeParams.activeEntityId : undefined
-  const subEntityId = typeof ownProps.routeParams.subEntityId !== 'undefined' ? +ownProps.routeParams.subEntityId : undefined
+  const {activeEntityId, subEntityId} = getIdsFromParams(ownProps.routeParams)
   const {data, editSettings: editSettingsState, mapState} = state.editor
   const {present: editSettings} = editSettingsState
   const {active, tables, tripPatterns, status} = data
   // FIXME: entityId is now a non-string line number and somewhere the number is
   // being cast to a string.
-  const activeEntity =
-    active.entity && active.entity.id === activeEntityId
+  const activeEntity = active.entity && active.entity.id === activeEntityId
+    ? active.entity
+    : active.entity && activeComponent === 'feedinfo'
       ? active.entity
-      : active.entity && activeComponent === 'feedinfo' ? active.entity : null
+      : null
   const activePattern = active.subEntity
   const entityEdited = active.edited
   const patternEdited = Boolean(active.patternEdited)
   const {controlPoints, patternSegments: patternCoordinates} = getControlPoints(state)
-  // console.log('pattern segments', patternCoordinates)
   const tableView = ownProps.location.query && ownProps.location.query.table === 'true'
   // Active set of entities (e.g., stops, routes, agency)
   const entities = activeComponent && getTableById(tables, activeComponent)
@@ -86,6 +80,7 @@ const mapStateToProps = (state, ownProps) => {
   // find the containing project
   const project = findProjectByFeedSource(state.projects.all, feedSourceId)
   const feedSource = project && project.feedSources.find(fs => fs.id === feedSourceId)
+  const namespace = feedSource && feedSource.editorNamespace
 
   const feedInfo = getTableById(tables, 'feedinfo')[0]
   const patternStop = active.patternStop || {}
@@ -100,6 +95,7 @@ const mapStateToProps = (state, ownProps) => {
     feedSource,
     entities,
     feedSourceId,
+    namespace,
     feedInfo,
     entityEdited,
     tableView,
@@ -133,16 +129,10 @@ const mapDispatchToProps = (dispatch, ownProps) => {
     activeComponent,
     subComponent,
     subSubComponent,
-    // activeEntityId,
-    // subEntityId,
     activeSubSubEntity
   } = ownProps.routeParams
-  // console.log(ownProps.routeParams)
-  // TODO: Figure out a better way to parse an integer from react-router route
-  // params
-  // FIXME: handle "new" entityId route
-  const activeEntityId = typeof ownProps.routeParams.activeEntityId !== 'undefined' ? +ownProps.routeParams.activeEntityId : undefined
-  const subEntityId = typeof ownProps.routeParams.subEntityId !== 'undefined' ? +ownProps.routeParams.subEntityId : undefined
+  // Parse IDs from route params
+  const {activeEntityId, subEntityId} = getIdsFromParams(ownProps.routeParams)
   const props = {
     component: activeComponent,
     newId: activeEntityId,
@@ -171,18 +161,15 @@ const mapDispatchToProps = (dispatch, ownProps) => {
       }
     },
     onComponentUpdate: (prevProps, newProps) => {
-      // handle back button presses by re-setting active gtfs entity
-      if (prevProps.activeEntityId !== ENTITY.NEW_ID &&
-        (prevProps.activeComponent !== newProps.activeComponent ||
+      // Detect push changes to URL (e.g., back button or direct link) and update
+      // active table/entity accordingly.
+      if (prevProps.activeComponent !== newProps.activeComponent ||
         prevProps.activeEntityId !== newProps.activeEntityId ||
         prevProps.subComponent !== newProps.subComponent ||
         prevProps.subEntityId !== newProps.subEntityId ||
         prevProps.subSubComponent !== newProps.subSubComponent ||
-        prevProps.activeSubSubEntity !== newProps.activeSubSubEntity)
+        prevProps.activeSubSubEntity !== newProps.activeSubSubEntity
       ) {
-        console.log('handling back button', activeEntityId, subEntityId)
-        // FIXME: handle infinite loop case when, for example, entity ID is bad
-        // or is not found.
         dispatch(setActiveGtfsEntity(feedSourceId, activeComponent, activeEntityId, subComponent, subEntityId, subSubComponent, activeSubSubEntity))
       }
     },

lib/editor/util/gtfs.js

@@ -57,14 +57,21 @@ export function getTableById (tableData: any, id: string, emptyArrayOnNull: bool
   const table = tableData[tableName]
   // TODO: use Immutable List?
   // If table is null, return empty array if boolean specifies
-  // return emptyArrayOnNull
-  //   ? List(table) || []
-  //   : List(table)
   return emptyArrayOnNull
     ? table || []
     : table
 }
 
+export function getIdsFromParams (params: any) {
+  const activeEntityId = typeof params.activeEntityId !== 'undefined' && !isNaN(params.activeEntityId)
+    ? +params.activeEntityId
+    : undefined
+  const subEntityId = typeof params.subEntityId !== 'undefined' && !isNaN(params.subEntityId)
+    ? +params.subEntityId
+    : undefined
+  return {activeEntityId, subEntityId}
+}
+
 export function getKeyForId (id: string, key: string): ?string {
   const component = COMPONENT_LIST.find(c => c.id === id)
   return typeof component !== 'undefined' ? component[key] : null

lib/manager/actions/versions.js

@@ -8,6 +8,7 @@ import {SECURE_API_PREFIX} from '../../common/constants'
 import {getConfigProperty} from '../../common/util/config'
 import {uploadFile} from '../../common/util/upload-file'
 import fileDownload from '../../common/util/file-download'
+import {getKeyForId} from '../../editor/util/gtfs'
 import {getEntityGraphQLRoot, getEntityIdField, getGraphQLFieldsForEntity} from '../../gtfs/util'
 import {handleJobResponse, setErrorMessage, startJobMonitor} from './status'
 import {fetchFeedSource} from './feeds'
@@ -207,14 +208,16 @@ export function fetchGTFSEntities ({namespace, id, type, editor = false, replace
         }
       }
     `
-    dispatch(fetchingGTFSEntities({namespace, id, type, editor, query, patternId}))
+    dispatch(fetchingGTFSEntities({namespace, id, type, editor, query}))
     // If fetching for the editor, cast id to int for csv_line field
     return dispatch(fetchGraphQL({
       query,
       variables: {namespace, [entityIdField]: editor ? +id : id}
     }))
       .then(data => {
         dispatch(receiveGTFSEntities({namespace, id, component: type, data, editor, replaceNew}))
+        const patternId = getState().editor.data.active.subEntityId
+        checkEntityIdValidity(data, type, id, patternId)
         if (replaceNew) {
           // FIXME Verify this is working properly.
           const {feedSourceId} = getState().editor.data.active
@@ -231,6 +234,33 @@ export function fetchGTFSEntities ({namespace, id, type, editor = false, replace
   }
 }
 
+/**
+ * Check that entity and pattern IDs are valid and update URL if not.
+ */
+function checkEntityIdValidity (data, type, id, patternId) {
+  const tableName = getKeyForId(type, 'tableName')
+  const entity = data.feed[tableName][0]
+  const pathParts = window.location.pathname.split('/')
+  let entityNotFound = false
+  if (!entity) {
+    console.warn(`No ${type} entity found for id=${id}. Removing id from URL.`)
+    entityNotFound = true
+    while (pathParts[pathParts.length - 1] !== type) pathParts.pop()
+  } else if (type === 'route' && patternId) {
+    const pattern = entity.tripPatterns.find(p => p.id === patternId)
+    if (!pattern) {
+      console.warn(`No pattern entity found for id=${patternId}. Removing id from URL.`)
+      entityNotFound = true
+      pathParts.pop()
+    }
+  }
+  if (entityNotFound) {
+    // If there was no entity found for the requested ID, remove the ID
+    // from the URL
+    browserHistory.push(pathParts.join('/'))
+  }
+}
+
 export function fetchValidationErrors ({feedVersion, errorType, limit, offset}) {
   return function (dispatch, getState) {
     dispatch(fetchingValidationErrors({feedVersion, errorType, limit, offset}))