Page MenuHomePhabricator

Event availability doesn't account for all day events
Closed, ResolvedPublic

Description

The code, that calculates user availability based on events he will be attending (see https://secure.phabricator.com/diffusion/P/browse/master/src/applications/people/query/PhabricatorPeopleQuery.php;46a225c7b1cba077979b59166211011cf939cdce$397) is presuming, that event length is shorter then queried interval (from today and 3 days long). Because of that events, that either "start before queried range" or "end after queried range" (including all day events) never affect user availability.

IMO the event range should be checked like this "events user is attending are ongoing within queried interval".

Event Timeline

aik099 raised the priority of this task from to Needs Triage.
aik099 updated the task description. (Show Details)
aik099 added a project: Calendar.
aik099 added a subscriber: aik099.
lpriestley triaged this task as Normal priority.

The problem was that all-day events were not in the correct timezone (we store them as UTC, but then convert them and pretend they're not UTC). Another problem that I discovered was that we weren't generating ghost events to see if any of them fit inside the "3 day availability window". There is one thing left here, and that's to figure out how to avoid getting a rawResultLimit of 0 - placeholder T8613

I think there may be a different/better fix to D13354, or maybe I'm not remembering why we made some decisions.

Specifically, all-day events seem to work OK for me, it's just an issue with recurring events. This is because we don't generate ghosts in the people query.

However, my expectation is that we don't need to: it should be impossible to RSVP "Attending" to a ghost, so the only events a user can possibly be attending should be real events. This isn't the way Calendar currently works, though -- when you RSVP to a ghost, you actually RSVP to the root event. So it's impossible to set yourself as attending this week's Book Club Meeting but not next week's Book Club Meeting.

Maybe I'm misremembering a product decision, but that seems incorrect -- I would expect RSVP'ing to apply to individual recurrences, not to the root event. So I say I'm attending Book Club on April 7, specifically, not that I'm attending all Book Club meetings for all time. I should also be able to note that I won't be able to make it to the meeting on April 14th, but currently can't.

(If we want to make it easier to say "I regularly attend this event", we could add some option later to the RSVP dialog that lets you quickly RSVP to multiple recurrences, maybe?)

If we change the RSVP behavior so that RSVP'ing to an instance really RSVP's to that instance, that should fix this issue as a side effect, since we won't need to generate ghosts. Does that make sense? Am I totally misremembering something?

Not sure what to say about this, because the problem I was experiencing originally wasn't with recurring events, but with all day events.

As for recurring events doing like done in Google Calendar could be a solution: when you create a recurring event you generate real events for each recurrence but remember original event that created recurring sequence. Then through updating any event in the sequence you ask if change needs to be applied to this event only or also to all further recurrences after the changed one.

This way ghost event concept completely goes away before all events would be in a double linked list (each recurrence knows which recurrence comes before and after it) and they can be iterated over.

I believe there's no actual issue here outside of the RSVP/instancing issue and other issues resolved or tracked elsewhere. If there is, file a new report -- with reproduction steps -- after Calendar leaves prototype.