fix: multiple retro meeting fixes #10918
Conversation
There was a problem hiding this comment.
I think there is some clarification necessary as we cannot just name the current meeting with the next starting day as that's the time when we will end it and start the next one, see also #10111 (comment)
That needs some clarification first. Feel free to separate this part from the rest, so we can move forward with it. Or you can wait till it's clarified.
|
|
||
| useEffect(() => { | ||
| if (recurrenceDays.length > 0) { | ||
| const nextDate = getNextMeetingDate(recurrenceDays, recurrenceStartTime); |
There was a problem hiding this comment.
-1 We should make use of the RRule package here. We can do something like const nextMeetingDate = rrule.after(now).
| useEffect(() => { | ||
| if (recurrenceDays.length > 0) { | ||
| const nextDate = getNextMeetingDate(recurrenceDays, recurrenceStartTime); | ||
| setRecurrenceStartTime(nextDate); |
There was a problem hiding this comment.
-1 I think what you're trying here might work. However we call this from RecurrenceTimePicker below which would not trigger this effect. It would also call the state setting function twice. I think we would need to have a separate callback to the picker instead.
| const startTime=new Date( | ||
| rrule!.dtstart.year, | ||
| rrule!.dtstart.month - 1, | ||
| rrule!.dtstart.day, | ||
| rrule!.dtstart.hour, | ||
| rrule!.dtstart.minute, | ||
| rrule!.dtstart.second | ||
| ); |
There was a problem hiding this comment.
-1 With your current changes in the client, this should just be rrule.after(new Date()).
startTeamPrompt needs the same treatment.
Actually, if we start the meeting now, then it will be closed on startTime and a new meeting opened. So this is wrong and new Date() was correct.
There was a problem hiding this comment.
Actually, if we start the meeting now, then it will be closed on startTime and a new meeting opened. So this is wrong and new Date() was correct.
But in the createMeetingSeriesTitle mutation if you pass the new Date() instead of startTime then you'll never have a recurrent meeting that begins with the date that user opted for right? It'll always be today.
There was a problem hiding this comment.
No, the next meeting in the series will be started at startTime and will get a new createMeetingSeriesTitle with the startTime.
There is no complete clarity how we can solve this issue, so it's not ideal to be picked up by you. We need to do some UI/UX work first.
There was a problem hiding this comment.
Sure then, I'll work on something else. Converting this to draft, will reopen with updated PR once we have some clarity.
Description
Fixes #9748 #10111 #10113
The Retro series card now shows the upcoming meeting name instead of the retro series name.
Also, the creation of recurrent meeting where the date was not being taken as per the selected day of the week is fixed and this also flows to the meetings which are recurrent/series as the name of the card. Updated the code to save the same in backend.
Demo
Adding screenshots.
Please let me know if things are not clear, will add a loom video.
Final checklist