Recurring event 1351 bug fix #1395
Conversation
|
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes. |
There was a problem hiding this comment.
Amazing work @vanessavun 🎉
Users will finally be able to create and update meeting times with a duration of 30 minutes!
Loved that refactor of the gross switch statement and well done going above and beyond with the testing 💪
Feel free to merge whenever you like!
| @@ -39,7 +39,8 @@ const EditMeetingTimes = ({ | |||
| }; | |||
| } | |||
|
|
|||
| if (values.description) { | |||
| //if there is a description or a blank description | |||
| if (values.description || !values.description) { | |||
There was a problem hiding this comment.
Help us understand why the if statement is necessary here? Wouldn't this conditional just always evaluate to true ?
There was a problem hiding this comment.
Yes, it needs to always be true so the description is carried over with the update. When there is a blank description, the original conditional, if (values.description), will be falsy so the blank description doesn't get updated. I guess there is a better way to rewrite this file in which we do a catch all to update all the values like:
theUpdatedEvent = { ...theUpdatedEvent, **value name: updated value** };
| // Create the endTime by adding seconds based on the selected duration to the timestamp and converting it back to date | ||
| if(startTimeDate && duration){ | ||
| return new Date(startTimeDate.getTime() + (Number(duration)*3600000)) | ||
| } else { | ||
| throw new Error('Error: Cannot calculate endTime.') | ||
| } | ||
| }; |
|
Will take a peak at this tomorrow but it looks super clean from a quick skim! |
There was a problem hiding this comment.
Removed 30+ lines of code of hard-coded cases 💯
|
Seriously great work figuring this one out. Well done :) |
Fixes #1351
What changes did you make and why did you make them?
addDurationToTime.jsfile which had 8 hardcoded switch statements that did not include a statement for any "0.5" inputted time. The file now has one line of the same formula and any duration will return a new Date object. Any invalid inputs will result an Error.addDurationToTime.jsfile to ensure that the formula works.Screenshots of Proposed Changes Of The Website (if any, please do not screenshot code changes)
Visuals before and after for blank descriptions
Visuals before and after for half-hour meetings