ParabolInc / parabol

Free online agile retrospective meeting tool
https://www.parabol.co/
Other
1.91k stars 331 forks source link

Standups: Backend Recurrence MVP Implementation Spec #6888

Closed jmtaber129 closed 2 years ago

jmtaber129 commented 2 years ago

See https://github.com/ParabolInc/parabol/issues/6852 for other discussion and some alternatives considered.

High-level overview

We store state about recurring meetings/activities in a new postgres table, MeetingSeries, which contains metadata about its collection of meetings, and how these meetings recur. We introduce a new mutation, processRecurrence, which uses the recurrence information in MeetingSeries to idempotently open and close meetings accordingly. We then run this new mutation on a 5-minute interval to keep actual meeting state in-sync with what the meeting series' recurrence specifies.

Spec

Data model

MeetingSeries postgres table

NewMeeting rethink table

New field:

Recurrence format

We should use the RFC-2445 iCal spec, though we won't use all of its features. The rrule library supports this spec, and supports date generation and parsing+serialization.

For MVP, we should hard-code the recurrence to use the following rule when creating meeting series:

Note that this does not include enough information to determine duration - let's assume all meetings end 24 hours after they start for the MVP, then we can add duration (including the duration column) as a follow-up.

processRecurrence mutation

This mutation does two things in the MVP:

Closing meetings is simple - find any open meetings where now is after their scheduledEndTime, and close them.

Creating new meetings is a bit more complicated:

Important note: this mutation is idempotent - if this mutation is run twice consecutively, the second execution should have no effect. This should increase robustness, and make debugging, etc. a bit easier.

Starting/ending recurrence

(note: editing recurrence rules is out-of-scope for MVP) How recurring/non-recurring meetings are represented:

Setting a meeting to be recurring for the first time:

Cancelling recurrence:

Restarting recurrence:

mattkrick commented 2 years ago

Lookin good!

Thoughts:

let's assume all meetings end 24 hours after they start for the MVP

there could be dragons here, so we should explore & have a plan so the time between MVP & GA is as small as possible. Like you say, we've got a couple options:

Set the scheduledEndTime of any open meetings to null Set the meeting's scheduledEndTime to 24 hours from now (or when the recurrence rule says this meeting should end, or when the next meeting should begin, or something else that makes sense)

IMO this behavior is unexpected. I'd expect a meeting series to start new meeting, but not modify existing ones. If they're already created, users should be able to let the meeting expire at its current scheduledEndTime, or they can end it manually.

Pause vs. Cancel

jmtaber129 commented 2 years ago

@mattkrick

Updated the spec to address most of these, with the exception of a few:

IMO a series should have 0 or 1 meetings open at any given time, more than 1 & you'll get users asking themselves "do I log my stuff in this meeting or that one?" processRecurrence should guarantee this.

I'm not sure we want that. For example, if a team wants their standups open for e.g. 36 hours so people can add their response for the previous day, while also having today's open. I think the meeting title (which will include the date) should make it clear which meeting is which.

IMO this behavior is unexpected. I'd expect a meeting series to start new meeting, but not modify existing ones. If they're already created, users should be able to let the meeting expire at its current scheduledEndTime, or they can end it manually.

I don't think this is an accurate assessment w.r.t how this would happen in the UI. I.e. if I'm enabling recurrence from within a meeting, I'd expect the current meeting to become part of a meeting series (including being scheduled an end time), and if I'm canceling/pausing recurrence from within a meeting, I'd expect the current meeting to no longer have any recurrence artifacts (including no longer having a scheduled end time). In fact, I think it would be unexpected if I canceled recurrence for a meeting, and the meeting still ended automatically, with no way to prevent it from ending.

Re: Pause vs. cancel The main thinking is that someone may cancel recurrence for a meeting, realize they didn't want to do that, and then re-enable recurrence, which should be part of the same meeting series. Keep in mind that the concept of a "meeting series" is somewhat hidden from the user - they just see a meeting that is or isn't recurring - so forcing users to create a new meeting just to restart recurrence isn't a great experience. Additionally, I think it makes sense to keep each meeting associated with only one meeting series over its lifetime, since allowing meetings to jump from one series to another seems error-prone once we start adding more complex features (e.g. navigating to other meetings within a meeting's series).

Specifically as for pausing vs. canceling, I think this is a distinction that's outside the scope of the MVP.

BartoszJarocki commented 2 years ago

@jmtaber129 I'm closing this as we're likely star a new discussion when moving forward with recurrence