Fix 2 edit workshop bugs discovered in React class refactor #18114
Conversation
… sessions couldn't be deleted at once, and the last available facilitator couldn't be added
| filteredAvailableFacilitators = this.props.availableFacilitators.filter(availableFacilitator => { | ||
| return !this.props.facilitators.find(f => f.id === availableFacilitator.id); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Moving this out of the map on line 71 is just an optimization, not related to the bugs, but it seems like a good idea. There's really no need to recompute this available list for each row.
There was a problem hiding this comment.
👍 Good extraction. One syntax nit - could express the outer arrow function here with an expression too.
filteredAvailableFacilitators = this.props.availableFacilitators.filter(availableFacilitator =>
!this.props.facilitators.find(f => f.id === availableFacilitator.id);
);There was a problem hiding this comment.
ah right, good call 👍 Will do
| filteredAvailableFacilitators = this.props.availableFacilitators.filter(availableFacilitator => { | ||
| return !this.props.facilitators.find(f => f.id === availableFacilitator.id); | ||
| }); | ||
| } |
There was a problem hiding this comment.
👍 Good extraction. One syntax nit - could express the outer arrow function here with an expression too.
filteredAvailableFacilitators = this.props.availableFacilitators.filter(availableFacilitator =>
!this.props.facilitators.find(f => f.id === availableFacilitator.id);
);| if (i === facilitators.length - 1 && facilitators.length < MAX_FACILITATORS && | ||
| this.props.facilitators[i].id > 0 && filteredAvailableFacilitators.length > 1 ) { | ||
| this.props.facilitators[i].id > 0 && filteredAvailableFacilitators.length >= 1 ) { |
There was a problem hiding this comment.
There might be another bug here - if the placeholder id is -1, shouldn't the third condition here be this.props.facilitators[i] >= 0 not > 0?
There was a problem hiding this comment.
Rails ids start at 1, so in this case either way will work. I'd be happy to change it to >= 0 if you think that's more readable.
There was a problem hiding this comment.
Or maybe it's due to the underlying MySql auto increment, but anyway this is the behavior - our ids always start with 1.
I discovered 2 long-standing bugs :( while testing React refactors here.
1: Multiple sessions couldn't be deleted at once
Bug:
Fix:
2. the last available facilitator couldn't be added
Bug:
Fix:
It also still works correctly when none are available:
Or when no course is selected:
