Add sanity checks against lane operations

sapegin · Mar 3, 2016 · 0508072 · 0508072
1 parent 156b909
commit 0508072
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 4 deletions.
diff --git a/immutable_app/app/reducers/lanes.js b/immutable_app/app/reducers/lanes.js
@@ -5,20 +5,32 @@ import * as types from '../actions/lanes';
 const initialState = List();
 
 export default function lanes(state = initialState, action) {
+  let laneIndex;
+
   switch (action.type) {
     case types.CREATE_LANE:
       return state.push(action.lane);
 
     case types.UPDATE_LANE:
-      // XXX: this can crash if findIndex fails
+      laneIndex = state.findIndex(lane => lane.id === action.id);
+
+      if(laneIndex < 0) {
+        return state;
+      }
+
       return state.update(
-        state.findIndex(lane => lane.id === action.id),
+        laneIndex,
         lane => Object.assign({}, lane, action)
       );
 
     case types.DELETE_LANE:
-      // XXX: this can crash if findIndex fails
-      return state.delete(state.findIndex(lane => lane.id === action.id));
+      laneIndex = state.findIndex(lane => lane.id === action.id);
+
+      if(laneIndex < 0) {
+        return state;
+      }
+
+      return state.delete(laneIndex);
 
     case types.ATTACH_TO_LANE:
       const laneId = action.laneId;

diff --git a/immutable_app/tests/lane_reducer_test.js b/immutable_app/tests/lane_reducer_test.js
@@ -43,6 +43,28 @@ describe('LaneReducer', () => {
     assert.equal(lanes.get(0).name, updatedName);
   });
 
+  it('should not crash while updating a non-existent lane', () => {
+    const lane = {
+      id: 'foobar',
+      name: 'demo lane',
+      notes: []
+    };
+
+    let lanes = reducer(undefined, {
+      type: types.CREATE_LANE,
+      lane: lane
+    });
+    lanes = reducer(lanes, {
+      type: types.UPDATE_LANE,
+      id: lane.id + lane.id,
+      name: 'foo'
+    });
+
+    assert.equal(lanes.count(), 1);
+    assert.equal(lanes.get(0).id, lane.id);
+    assert.equal(lanes.get(0).name, lane.name);
+  });
+
   it('should delete lanes', () => {
     const lane = {
       id: 'foobar',
@@ -62,6 +84,29 @@ describe('LaneReducer', () => {
     assert.equal(lanes.count(), 0);
   });
 
+
+  it('should not crash while deleting a non-existent lane', () => {
+    const lane = {
+      id: 'foobar',
+      name: 'demo lane',
+      notes: []
+    };
+
+    let lanes = reducer(undefined, {
+      type: types.CREATE_LANE,
+      lane: lane
+    });
+    lanes = reducer(lanes, {
+      type: types.DELETE_LANE,
+      id: lane.id + lane.id,
+      name: 'foo'
+    });
+
+    assert.equal(lanes.count(), 1);
+    assert.equal(lanes.get(0).id, lane.id);
+    assert.equal(lanes.get(0).name, lane.name);
+  });
+
   it('should attach notes to lanes', () => {
     const lane = {
       id: 'foobar',