Skip to content

Commit 0508072

Browse files
committed
Add sanity checks against lane operations
1 parent 156b909 commit 0508072

File tree

2 files changed

+61
-4
lines changed

2 files changed

+61
-4
lines changed

immutable_app/app/reducers/lanes.js

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,32 @@ import * as types from '../actions/lanes';
55
const initialState = List();
66

77
export default function lanes(state = initialState, action) {
8+
let laneIndex;
9+
810
switch (action.type) {
911
case types.CREATE_LANE:
1012
return state.push(action.lane);
1113

1214
case types.UPDATE_LANE:
13-
// XXX: this can crash if findIndex fails
15+
laneIndex = state.findIndex(lane => lane.id === action.id);
16+
17+
if(laneIndex < 0) {
18+
return state;
19+
}
20+
1421
return state.update(
15-
state.findIndex(lane => lane.id === action.id),
22+
laneIndex,
1623
lane => Object.assign({}, lane, action)
1724
);
1825

1926
case types.DELETE_LANE:
20-
// XXX: this can crash if findIndex fails
21-
return state.delete(state.findIndex(lane => lane.id === action.id));
27+
laneIndex = state.findIndex(lane => lane.id === action.id);
28+
29+
if(laneIndex < 0) {
30+
return state;
31+
}
32+
33+
return state.delete(laneIndex);
2234

2335
case types.ATTACH_TO_LANE:
2436
const laneId = action.laneId;

immutable_app/tests/lane_reducer_test.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,28 @@ describe('LaneReducer', () => {
4343
assert.equal(lanes.get(0).name, updatedName);
4444
});
4545

46+
it('should not crash while updating a non-existent lane', () => {
47+
const lane = {
48+
id: 'foobar',
49+
name: 'demo lane',
50+
notes: []
51+
};
52+
53+
let lanes = reducer(undefined, {
54+
type: types.CREATE_LANE,
55+
lane: lane
56+
});
57+
lanes = reducer(lanes, {
58+
type: types.UPDATE_LANE,
59+
id: lane.id + lane.id,
60+
name: 'foo'
61+
});
62+
63+
assert.equal(lanes.count(), 1);
64+
assert.equal(lanes.get(0).id, lane.id);
65+
assert.equal(lanes.get(0).name, lane.name);
66+
});
67+
4668
it('should delete lanes', () => {
4769
const lane = {
4870
id: 'foobar',
@@ -62,6 +84,29 @@ describe('LaneReducer', () => {
6284
assert.equal(lanes.count(), 0);
6385
});
6486

87+
88+
it('should not crash while deleting a non-existent lane', () => {
89+
const lane = {
90+
id: 'foobar',
91+
name: 'demo lane',
92+
notes: []
93+
};
94+
95+
let lanes = reducer(undefined, {
96+
type: types.CREATE_LANE,
97+
lane: lane
98+
});
99+
lanes = reducer(lanes, {
100+
type: types.DELETE_LANE,
101+
id: lane.id + lane.id,
102+
name: 'foo'
103+
});
104+
105+
assert.equal(lanes.count(), 1);
106+
assert.equal(lanes.get(0).id, lane.id);
107+
assert.equal(lanes.get(0).name, lane.name);
108+
});
109+
65110
it('should attach notes to lanes', () => {
66111
const lane = {
67112
id: 'foobar',

0 commit comments

Comments
 (0)