Permalink
Browse files

improved vote reset

  • Loading branch information...
teropa committed Sep 6, 2015
1 parent 33262ce commit 84c51e763c662d10e5cd2de8a55554f1b75b40cd
Showing with 30 additions and 13 deletions.
  1. +1 −1 src/components/Voting.jsx
  2. +9 −5 src/reducer.js
  3. +20 −7 test/reducer_spec.js
@@ -18,7 +18,7 @@ export const Voting = React.createClass({
function mapStateToProps(state) {
return {
pair: state.getIn(['vote', 'pair']),
hasVoted: state.get('hasVoted'),
hasVoted: state.getIn(['myVote', 'entry']),
winner: state.get('winner')
};
}
View
@@ -5,19 +5,23 @@ function setState(state, newState) {
}
function vote(state, entry) {
const currentRound = state.getIn(['vote', 'round']);
const currentPair = state.getIn(['vote', 'pair']);
if (currentPair && currentPair.includes(entry)) {
return state.set('hasVoted', entry);
return state.set('myVote', Map({
round: currentRound,
entry
}));
} else {
return state;
}
}
function resetVote(state) {
const hasVoted = state.get('hasVoted');
const currentPair = state.getIn(['vote', 'pair'], List());
if (hasVoted && !currentPair.includes(hasVoted)) {
return state.remove('hasVoted');
const votedForRound = state.getIn(['myVote', 'round']);
const currentRound = state.getIn(['vote', 'round']);
if (votedForRound !== currentRound) {
return state.remove('myVote');
} else {
return state;
}
View
@@ -67,9 +67,10 @@ describe('reducer', () => {
}));
});
it('handles VOTE by setting hasVoted', () => {
it('handles VOTE by setting myVote', () => {
const state = fromJS({
vote: {
round: 42,
pair: ['Trainspotting', '28 Days Later'],
tally: {Trainspotting: 1}
}
@@ -79,16 +80,21 @@ describe('reducer', () => {
expect(nextState).to.equal(fromJS({
vote: {
round: 42,
pair: ['Trainspotting', '28 Days Later'],
tally: {Trainspotting: 1}
},
hasVoted: 'Trainspotting'
myVote: {
round: 42,
entry: 'Trainspotting'
}
}));
});
it('does not set hasVoted for VOTE on invalid entry', () => {
it('does not set myVote for VOTE on invalid entry', () => {
const state = fromJS({
vote: {
round: 42,
pair: ['Trainspotting', '28 Days Later'],
tally: {Trainspotting: 1}
}
@@ -98,33 +104,40 @@ describe('reducer', () => {
expect(nextState).to.equal(fromJS({
vote: {
round: 42,
pair: ['Trainspotting', '28 Days Later'],
tally: {Trainspotting: 1}
}
}));
});
it('removes hasVoted on SET_STATE if pair changes', () => {
it('removes myVote on SET_STATE if round has changed', () => {
const initialState = fromJS({
vote: {
round: 42,
pair: ['Trainspotting', '28 Days Later'],
tally: {Trainspotting: 1}
},
hasVoted: 'Trainspotting'
myVote: {
round: 42,
entry: 'Trainspotting'
}
});
const action = {
type: 'SET_STATE',
state: {
vote: {
pair: ['Sunshine', 'Slumdog Millionaire']
round: 43,
pair: ['Sunshine', 'Trainspotting']
}
}
};
const nextState = reducer(initialState, action);
expect(nextState).to.equal(fromJS({
vote: {
pair: ['Sunshine', 'Slumdog Millionaire']
round: 43,
pair: ['Sunshine', 'Trainspotting']
}
}));
});

5 comments on commit 84c51e7

@sebastienbarre

This comment has been minimized.

Show comment
Hide comment
@sebastienbarre

sebastienbarre Sep 28, 2015

Great tutorial. I felt like there was a simpler solution to improving the vote state reset, which involved only modifying vote-client/src/reducer.js. Compare the old state to the new state; if the vote pairs are different, then reset the vote on that transition.

function setState(state, newState) {
  let mergedState = state.merge(newState);
  const oldPair = state.getIn(['vote', 'pair'], List());
  const newPair = mergedState.getIn(['vote', 'pair'], List());
  if (mergedState.get('hasVoted') && !oldPair.equals(newPair)) {
    return mergedState.remove('hasVoted');
  }
  return mergedState;
}

[...]

export default function(state = Map(), action) {
  switch (action.type) {
  case 'SET_STATE':
    return setState(state, action.state);
  case 'VOTE':
    return vote(state, action.entry);
  }
  return state;
}

sebastienbarre replied Sep 28, 2015

Great tutorial. I felt like there was a simpler solution to improving the vote state reset, which involved only modifying vote-client/src/reducer.js. Compare the old state to the new state; if the vote pairs are different, then reset the vote on that transition.

function setState(state, newState) {
  let mergedState = state.merge(newState);
  const oldPair = state.getIn(['vote', 'pair'], List());
  const newPair = mergedState.getIn(['vote', 'pair'], List());
  if (mergedState.get('hasVoted') && !oldPair.equals(newPair)) {
    return mergedState.remove('hasVoted');
  }
  return mergedState;
}

[...]

export default function(state = Map(), action) {
  switch (action.type) {
  case 'SET_STATE':
    return setState(state, action.state);
  case 'VOTE':
    return vote(state, action.entry);
  }
  return state;
}
@dearfrankg

This comment has been minimized.

Show comment
Hide comment
@dearfrankg

dearfrankg Oct 20, 2015

I modified a bit...

[...]
function resetVote(state, mergedState) {
  const oldPair = state.getIn(['vote', 'pair'], List());
  const newPair = mergedState.getIn(['vote', 'pair'], List());
  if (mergedState.get('hasVoted') && !oldPair.equals(newPair)) {
    return mergedState.remove('hasVoted');
  }
  return mergedState;
}

export default function(state = Map(), action) {
  switch (action.type) {
  case 'SET_STATE':
    return resetVote(state, setState(state, action.state));
  case 'VOTE':
    return vote(state, action.voteEntry);
  }
  return state;
}

dearfrankg replied Oct 20, 2015

I modified a bit...

[...]
function resetVote(state, mergedState) {
  const oldPair = state.getIn(['vote', 'pair'], List());
  const newPair = mergedState.getIn(['vote', 'pair'], List());
  if (mergedState.get('hasVoted') && !oldPair.equals(newPair)) {
    return mergedState.remove('hasVoted');
  }
  return mergedState;
}

export default function(state = Map(), action) {
  switch (action.type) {
  case 'SET_STATE':
    return resetVote(state, setState(state, action.state));
  case 'VOTE':
    return vote(state, action.voteEntry);
  }
  return state;
}
@Neitsch

This comment has been minimized.

Show comment
Hide comment
@Neitsch

Neitsch Jan 13, 2016

@sebastienbarre @dearfrankg Your solution might not work if there is a tie on the final pair and then next is clicked.

Neitsch replied Jan 13, 2016

@sebastienbarre @dearfrankg Your solution might not work if there is a tie on the final pair and then next is clicked.

@Judahmeek

This comment has been minimized.

Show comment
Hide comment
@Judahmeek

Judahmeek replied Sep 5, 2016

@sebastienbarre @dearfrankg @Neitsch On the other hand, their alternative solution works well in combination with Exercise 3: http://teropa.info/blog/2015/09/10/full-stack-redux-tutorial.html#3-duplicate-vote-prevention

@stormqx

This comment has been minimized.

Show comment
Hide comment
@stormqx

stormqx Dec 15, 2016

@Neitsch Could you give an example in detail? I think their solution works well in this situation :)

stormqx replied Dec 15, 2016

@Neitsch Could you give an example in detail? I think their solution works well in this situation :)

Please sign in to comment.