refactor(editor): fixes for deleting the last stop...

evansiroky · landonreed · commit b2507b0b9837 · 2017-09-22T13:53:27.000-04:00
…when the last stop is in a pattern with only 2 stops
diff --git a/lib/editor/actions/map/index.js b/lib/editor/actions/map/index.js
@@ -141,7 +141,7 @@ export function handleControlPointDragEnd (controlPoints, index, latlng, pattern
     }).then(result => {
       if (!result.coordinates) {
         dispatch(setErrorMessage(
-          'An error occurred while trying to recalculate the shape.  Please try again.'
+          'An error occurred while trying to recalculate the shape after dragging.  Please try again.'
         ))
       } else {
         dispatch(
diff --git a/lib/editor/actions/map/stopStrategies.js b/lib/editor/actions/map/stopStrategies.js
@@ -249,12 +249,10 @@ export function removeStopFromPattern (pattern, stop, index, controlPoints) {
         return
       }
       if (!coordinates) {
-        dispatch(setErrorMessage(
-          'An error occurred while trying to recalculate the shape.  Please try again.'
-        ))
-        return
+        shape = null
+      } else {
+        shape = {type: 'LineString', coordinates}
       }
-      shape = {type: 'LineString', coordinates}
     }
     const patternStops = [...pattern.patternStops]
     patternStops.splice(index, 1)
diff --git a/lib/editor/reducers/data.js b/lib/editor/reducers/data.js
@@ -167,7 +167,6 @@ const data = (state = defaultState, action) => {
     case 'UPDATE_ACTIVE_GTFS_ENTITY':
       switch (component) {
         case 'trippattern':
-          patternIndex = state.active.entity.tripPatterns.findIndex(p => p.id === action.entity.id)
           entity = Object.assign({}, state.active.subEntity)
           for (key in action.props) {
             entity[key] = action.props[key]
diff --git a/lib/editor/util/__tests__/__snapshots__/map.js.snap b/lib/editor/util/__tests__/__snapshots__/map.js.snap
@@ -394,6 +394,59 @@ Object {
         "type": "Feature",
       },
     },
+    Object {
+      "defaultDwellTime": 0,
+      "defaultTravelTime": 120,
+      "distance": 691.729078104771,
+      "hidden": true,
+      "id": "fyz2",
+      "permanent": true,
+      "point": Object {
+        "geometry": Object {
+          "coordinates": Array [
+            -77.92252574804179,
+            42.09235127309673,
+          ],
+          "type": "Point",
+        },
+        "properties": Object {},
+        "type": "Feature",
+      },
+      "shapeDistTraveled": 691.729078104771,
+      "stopId": "a9c9cd55-ca3f-4ce7-9a93-96f2f4eaf089",
+      "timepoint": null,
+    },
+  ],
+}
+`;
+
+exports[`editor > util > map > recalculateShape > deleting > should delete the last stop when there are control points after the last stop 1`] = `
+Object {
+  "coordinates": null,
+  "dragId": null,
+  "updatedControlPoints": Array [
+    Object {
+      "defaultDwellTime": 0,
+      "defaultTravelTime": 0,
+      "distance": 0,
+      "hidden": true,
+      "id": "2zu5",
+      "permanent": true,
+      "point": Object {
+        "geometry": Object {
+          "coordinates": Array [
+            -122.00743318,
+            37.06121322,
+          ],
+          "type": "Point",
+        },
+        "properties": Object {},
+        "type": "Feature",
+      },
+      "shapeDistTraveled": 0,
+      "stopId": "aad0f374-0f29-4300-ae05-6affbe0f1562",
+      "timepoint": null,
+    },
   ],
 }
 `;
diff --git a/lib/editor/util/__tests__/fixtures/test-control-points-with-extra-point-at-end.json b/lib/editor/util/__tests__/fixtures/test-control-points-with-extra-point-at-end.json
@@ -0,0 +1,65 @@
+[{
+    "distance": 0,
+    "id": "2zu5",
+    "permanent": true,
+    "point": {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "Point",
+        "coordinates": [-122.00743318, 37.06121322]
+      }
+    },
+    "stopId": "aad0f374-0f29-4300-ae05-6affbe0f1562",
+    "defaultTravelTime": 0,
+    "defaultDwellTime": 0,
+    "timepoint": null,
+    "shapeDistTraveled": 0,
+    "hidden": true
+  },
+  {
+    "distance": 170.7245980700509,
+    "id": "25p4",
+    "permanent": true,
+    "point": {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "Point",
+        "coordinates": [-122.00872656844231, 37.060454715231224]
+      }
+    }
+  },
+  {
+    "distance": 341.4491961401018,
+    "id": "cy46",
+    "permanent": true,
+    "point": {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "Point",
+        "coordinates": [-122.00981702504365, 37.059191439360475]
+      }
+    },
+    "stopId": "ec6a3479-9795-4480-9884-1c25851a163e",
+    "defaultTravelTime": 0,
+    "defaultDwellTime": 0,
+    "timepoint": null,
+    "shapeDistTraveled": 341.4491961401018,
+    "hidden": true
+  },
+  {
+    "distance": 358.0806866950734,
+    "id": "4s05",
+    "permanent": true,
+    "point": {
+      "type": "Feature",
+      "properties": {},
+      "geometry": {
+        "type": "Point",
+        "coordinates": [-122.009919, 37.059066]
+      }
+    }
+  }
+]
diff --git a/lib/editor/util/__tests__/map.js b/lib/editor/util/__tests__/map.js
@@ -4,98 +4,107 @@ import nock from 'nock'
 
 import {recalculateShape} from '../map'
 
-const controlPoints = require('./fixtures/test-control-points.json')
+const fixtureControlPoints = require('./fixtures/test-control-points.json')
 const followStreets = true
 const patternShape = require('./fixtures/test-pattern-shape.json')
 
 describe('editor > util > map >', () => {
   describe('recalculateShape >', () => {
-    function makeTestCase ({editType, index, mapzenResponeFile, newPoint}) {
-      return async () => {
-        // setup nock
-        if (mapzenResponeFile) {
-          nock('https://valhalla.mapzen.com')
-            .get(/^\/route\?json.*/)
-            .reply(200, require(mapzenResponeFile))
-        }
+    function makeTestCase ({
+      controlPoints = fixtureControlPoints,
+      editType,
+      index,
+      mapzenResponeFile,
+      newPoint,
+      resultCoordsCanBeNull = false,
+      title
+    }) {
+      return it(
+        title,
+        async () => {
+          // setup nock
+          if (mapzenResponeFile) {
+            nock('https://valhalla.mapzen.com')
+              .get(/^\/route\?json.*/)
+              .reply(200, require(mapzenResponeFile))
+          }
 
-        const result = await recalculateShape({
-          controlPoints,
-          editType,
-          index,
-          followStreets,
-          newPoint,
-          patternShape
-        })
+          const result = await recalculateShape({
+            controlPoints,
+            editType,
+            index,
+            followStreets,
+            newPoint,
+            patternShape
+          })
 
-        expect(result).toMatchSnapshot()
-        const {coordinates} = result
-        expect(coordinates.length).toBeGreaterThan(0)
-        coordinates.forEach(coord => {
-          expect(coord.length).toBe(2)
-          expect(Math.abs(coord[0])).toBeLessThanOrEqual(180)
-          expect(Math.abs(coord[1])).toBeLessThanOrEqual(90)
-        })
-      }
+          expect(result).toMatchSnapshot()
+          const {coordinates} = result
+          if (!resultCoordsCanBeNull) {
+            expect(coordinates.length).toBeGreaterThan(0)
+            coordinates.forEach(coord => {
+              expect(coord.length).toBe(2)
+              expect(Math.abs(coord[0])).toBeLessThanOrEqual(180)
+              expect(Math.abs(coord[1])).toBeLessThanOrEqual(90)
+            })
+          }
+        }
+      )
     }
 
     describe('updating >', () => {
-      it(
-        'should update the first stop',
-        makeTestCase({
-          editType: 'update',
-          index: 0,
-          mapzenResponeFile: './fixtures/mapzen-response-update-first-stop.json',
-          newPoint: [-77.92553, 42.08666]
-        })
-      )
+      makeTestCase({
+        editType: 'update',
+        index: 0,
+        mapzenResponeFile: './fixtures/mapzen-response-update-first-stop.json',
+        newPoint: [-77.92553, 42.08666],
+        title: 'should update the first stop'
+      })
 
-      it(
-        'should update the last stop',
-        makeTestCase({
-          editType: 'update',
-          index: 4,
-          mapzenResponeFile: './fixtures/mapzen-response-update-last-stop.json',
-          newPoint: [-77.92262, 42.09828]
-        })
-      )
+      makeTestCase({
+        editType: 'update',
+        index: 4,
+        mapzenResponeFile: './fixtures/mapzen-response-update-last-stop.json',
+        newPoint: [-77.92262, 42.09828],
+        title: 'should update the last stop'
+      })
 
-      it(
-        'should update a point in the middle of the control points',
-        makeTestCase({
-          editType: 'update',
-          index: 3,
-          mapzenResponeFile: './fixtures/mapzen-response-update-middle-control-point.json',
-          newPoint: [-77.92382, 42.09182]
-        })
-      )
+      makeTestCase({
+        editType: 'update',
+        index: 3,
+        mapzenResponeFile: './fixtures/mapzen-response-update-middle-control-point.json',
+        newPoint: [-77.92382, 42.09182],
+        title: 'should update a point in the middle of the control points'
+      })
     })
 
     describe('deleting >', () => {
-      it(
-        'should delete the first stop',
-        makeTestCase({
-          editType: 'delete',
-          index: 0
-        })
-      )
+      makeTestCase({
+        editType: 'delete',
+        index: 0,
+        title: 'should delete the first stop'
+      })
 
-      it(
-        'should delete the last stop',
-        makeTestCase({
-          editType: 'delete',
-          index: 4
-        })
-      )
+      makeTestCase({
+        editType: 'delete',
+        index: 4,
+        title: 'should delete the last stop'
+      })
 
-      it(
-        'should delete a point in the middle of the control points',
-        makeTestCase({
-          editType: 'delete',
-          index: 3,
-          mapzenResponeFile: './fixtures/mapzen-response-delete-middle-control-point.json'
-        })
-      )
+      makeTestCase({
+        controlPoints: require('./fixtures/test-control-points-with-extra-point-at-end.json'),
+        editType: 'delete',
+        index: 2,
+        resultCoordsCanBeNull: true,
+        title: 'should delete the last stop when there are control points after the last stop'
+      })
+
+      makeTestCase({
+        editType: 'delete',
+        index: 3,
+        mapzenResponeFile: './fixtures/mapzen-response-delete-middle-control-point.json',
+        title: 'should delete a point in the middle of the control points'
+      })
     })
   })
 })
diff --git a/lib/editor/util/map.js b/lib/editor/util/map.js
@@ -193,6 +193,9 @@ export async function recalculateShape ({
    * @return {Coordinate[]}
    */
   function lineSliceAlongToCoords (startDistance, endDistance) {
+    if (startDistance === 0 && endDistance === 0) {
+      return []
+    }
     return lineSliceAlong(
       lineString(patternShape.coordinates),
       startDistance,
@@ -268,22 +271,43 @@ export async function recalculateShape ({
           mToKm(controlPoints[nextToLastIndex].distance)
         )),
         dragId,
-        updatedControlPoints: controlPoints.slice(0, nextToLastIndex)
+        updatedControlPoints: controlPoints.slice(0, nextToLastIndex + 1)
       }
     } else {
       // deleting an in-between control point or stop
       if (currentControlPoint.stopId) {
-        // deleting a stop.  Recalculate shape between last 2 stops
+        // deleting a stop.  Recalculate shape between the previous and following stops
         let previousStopIdx = index - 1
         while (!controlPoints[previousStopIdx].stopId && previousStopIdx > 0) {
           previousStopIdx--
         }
         previousControlPoint = controlPoints[previousStopIdx]
 
         let followingStopIdx = index + 1
-        while (!controlPoints[followingStopIdx].stopId && followingStopIdx < controlPoints.length - 1) {
-          followingStopIdx++
+        let nextStopFound = false
+        for (; followingStopIdx < controlPoints.length; followingStopIdx++) {
+          if (controlPoints[followingStopIdx].stopId) {
+            nextStopFound = true
+            break
+          }
+        }
+
+        if (!nextStopFound) {
+          // there are no more stops after this control point!
+          // return the coordinates as far as the new last stop
+
+          // set coords to null if last stop was deleted
+          const newCoords = lineSliceAlongToCoords(
+            0,
+            mToKm(controlPoints[previousStopIdx].distance)
+          )
+          return {
+            coordinates: newCoords.length > 0 ? ensureValidCoords(newCoords) : null,
+            dragId,
+            updatedControlPoints: controlPoints.slice(0, previousStopIdx + 1)
+          }
         }
+
         followingControlPoint = controlPoints[followingStopIdx]
       }
 

Original file line number	Diff line number	Diff line change
`@@ -249,12 +249,10 @@ export function removeStopFromPattern (pattern, stop, index, controlPoints) {`
`249`	`249`	`return`
`250`	`250`	`}`
`251`	`251`	`if (!coordinates) {`
`252`		`- dispatch(setErrorMessage(`
`253`		`- 'An error occurred while trying to recalculate the shape. Please try again.'`
`254`		`- ))`
`255`		`- return`
	`252`	`+ shape = null`
	`253`	`+ } else {`
	`254`	`+ shape = {type: 'LineString', coordinates}`
`256`	`255`	`}`
`257`		`- shape = {type: 'LineString', coordinates}`
`258`	`256`	`}`
`259`	`257`	`const patternStops = [...pattern.patternStops]`
`260`	`258`	`patternStops.splice(index, 1)`