Page MenuHomePhabricator

Recurring Events: "Edit all Future Events" edits past instances as well
Closed, ResolvedPublic

Description

downstream bug report: https://phabricator.wikimedia.org/T151228

On https://phabricator.wikimedia.org/E66, I clicked "edit event". When asked, I selected "Edit all Future Events". I changed the description.

Expected outcome:

  • Future events have the new title and description
  • Past events keep their title and description

(It would be even better to have an option when changing the "base" event to only change "ghost" instances that have not been individually edited yet)

Actual outcome:

  • All instances of https://phabricator.wikimedia.org/E66, past and future, had their description and title overwritten. Description of old instances are apparently LOST.
  • Old titles are visible in the log and can be restored manually. Conversations on past events are still there.

Event Timeline

E66 is the first event in the series, so I would expect editing E66 "All Future Events" to edit every event in the series: there are no past instances. Am I misunderstanding? What's an example of a past instance?

Assuming that's just a miscommunication, I can't immediately reproduce this locally:

  • I created a weekly event starting on Dec 5th.
  • I clicked the instance on Dec 19th, clicked "Edit", selected "All Future Events", and changed the title, then saved the change.
  • Only the event I edited and future events were edited: the instances on Dec 5th and Dec 12th were not.

Screen Shot 2016-11-22 at 12.18.39 PM.png (684×337 px, 31 KB)

Oh, sorry, E66 would have been disconnected from past events by the edit. Let me look through the downstream task.

I'm still having trouble figuring out which past events were edited. Downstream has this query:

..but the only event in that result set which does not occur after E66 (with a start date of September 15, 2015) is E61, which does not seem to have been edited.

Can you give me a specific example of a past event which was incorrectly edited?

Maybe this is a misinterpretation of "All Future Events"? That means "All events which start after this event starts", not "all events which have not yet occurred, considering the current time".

Here's the language used by Calendar.app and Google Calendar. In all three interfaces, the "future" option means "events after this one", not "events after the current time if I look at a clock on the wall" -- in the case of E66, events starting on or after September 15, 2015, more than a year ago.

Screen Shot 2016-11-22 at 12.28.35 PM.png (162×554 px, 33 KB)

Screen Shot 2016-11-22 at 12.28.04 PM.png (303×757 px, 45 KB)

Generally, our behavior here is approximately the same as Calendar.app and Google Calendar. T11804 documents how various edge cases behave in those applications. I believe our behavior is no more spooky in any case I'm aware of.

We do not support a "This Event, Which Is The First In A Series of Recurring Events, And Unedited Instances Of This Event, But Not Edited Instances" mode. There is no technical reason that we can't, but neither Google Calendar nor Calendar.app allow you to make this kind of edit so I figured it would likely be more confusing than useful for users coming from those applications. We can add support for it if it's actually desirable.

I'm open to relabeling "Edit All Future Events" if another phrasing would be more clear, although this essentially the exact phrasing used by Calendar.app. I wanted to avoid using Google Calendar's "Following" phrasing, because it is ambiguous when a later-in-the-series event has been rescheduled to occur at a chronologically-earlier time: is it still a following event? Calendar.app and Google Calendar just don't let you do this (Calendar.app, at least, just seems buggy) but I believe we handle it correctly, and .ics files can certainly contain these events, and "future" seems less ambiguous than "following" in this case.

This all seems reasonable, I'm actually not 100% sure what happened so I'll try to gather a better understanding of the situation ;)

Hi @epriestley, thank you for your explanation.

I still think there is an issue here which can lead to confusion and data loss, wie no way of recovery. Let me first explain what I was trying to do, and then explain what I think what happened, and what I think should happen.

I wanted to change the default description used by events in the series based on https://phabricator.wikimedia.org/E66. As far as I understand, E66 acts as a prototyp (or template), providing defaults for when the individual instances are "materialized" later, when they are edited. Is that correct?

Based on that assumption, I went to change the "template" or "prototype" event, E66. It asked me whether I want to change future events - I said yes, because that's exactly what I was trying to do. Change the default description for FUTURE events. Not PAST events.

Maybe this is a misinterpretation of "All Future Events"? That means "All events which start after this event starts", not "all events which have not yet occurred, considering the current time".

That's not what "future" means by any dictionary I know. Ideally, the wording would be "All Events after XXX", where XXX is the actual date in question. If that is not possible, I would vote for "All events ofter this one".

Anyway. From what you said, I gather that by editing E66, I overwrite all descriptions of all past events, which had the agendas of the past meetings. Without being asked to confirm the overwrite, and without any way of recovering the description that were overwritten. Even if the wording of the initial dialog was clear, this is very dangerous. Destructive operations must either be confirmed or reversible.

I suppose I should have chosen to only edit "this" event. If I understand correctly how recurring events work, that would have actually cause all future events to change, because the future (non-materialized) instances use E66 as a template. I kind of understand this from a DB design perspective, but it's HORRIBLY misleading in the UI.

In order to edit only future events, I must NOT pick "Edit future events", but "Edit only this event" on the prototype of a series. And if I do pick "future events", I lose data about past events. This is not a good situation.

As far as I understand, E66 acts as a prototyp (or template), providing defaults for when the individual instances are "materialized" later, when they are edited. Is that correct?

Not exactly. It used to do this, because our implementation was pretty weird and working like other Calendar applications was more difficult than doing this. We now behave like Calendar.app and Google Calendar. The new model is more complex, but also a better fit for how most people use calendars, I think.

A better way to think of it might be that each event serves as a template for the next event, although this isn't perfect either.

(The real model that everything follows is some kind of interpretation of how ICS RRULE specifications are defined in RFC 5545, with various improvisations.)

I suppose I should have chosen to only edit "this" event.

That would only have edited that event, exactly, like other Calendar applications.

I think the operation you wanted was:

  • Find the next instance of the event (e.g., the one occurring next week).
  • Click "Edit Event".
  • Choose "Edit All Future Events".

That will update next week's event and all events which occur after it. It will leave events prior to next week's event untouched.

This operation is more powerful than an operation which relies on the real-world time. Suppose you want to make this edit:

"Starting next February, let's move the Monday meeting from 10AM meeting to 1PM."

Under the "real-world time" model, you must wait until the real-world time is between 10AM on the last Monday in January and 10AM on the first Monday in February to make this edit.

Under the model we and other calendars implement, you can browse to February 2017, edit the first event you want to reschedule, and use "All Future Events" to reschedule later events. You can perform this edit at any time, regardless of the current real-world wall-clock time.


It would be nice to have undo here, but undo is very difficult to implement in Phabricator. There's some discussion of why it is difficult in T11254, but one example is that your edits may have caused Herald to make a large number of additional edits, all of which we may also need to undo. I believe we'll implement undo eventually, but the project is huge and hard to justify on the grounds of minor abuse cleanup and occasional reversion of Calendar edits.

I'll rework this dialog to make it more clear what "All Future Events" means. I'd like to see more users running into issues here before putting more confirmation steps on the workflow, since I suspect mistakes with this edit may be rare, especially if we clarify the dialog. Obviously, after a bad edit it's frustrating that there weren't more guard rails, but if we add guard rails every time someone executes an operation they didn't want to we'll tend toward infinite confirmations -- see T8881 where a user made a mistake which we resolved without side effect in 22 minutes, but still wanted an additional confirmation on top of the existing confirmation, for example. In that case, no other user has made the particular mistake yet. The product is probably in a worse place if we add a extra confirmation to an operation which has an error rate of, say, 1 in 10,000. Users even dislike confirmations which probably have a much higher error rate than that, as in T7843, although the cost of mistakes there is admittedly very small.


This is small comfort, but no data has truly been lost here: the old descriptions are still available in the transaction record. However, restoring them requires writing some code, since we don't have a general "Undo" operation (that is: we essentially always have all the data we need to perform an undo, and very little data is ever destroyed, but actually reverting that data is complicated in the general case). T8637 has an older example of an undo script for batch edits against tasks; that script could likely be adapted to undo these edits. I'm can update it if it would be helpful, but it's possible that recovery from snapshots is easier for WMF operations.

Thanks @epriestley for the reply. Seems like my mental model of "prototypes" along with the misleading wording of "future" threw me off.

I agree that confirmation dialogs are annoying. I much prefer an undo option. Not having either is kind of bad, though.

Changing the wording to "Events after this" or "Subsequent events" or "Events after Nov 11" would already help a lot, I think.

Maybe it would also be sensible to not allow mass-edits to be triggered by past events. While it may be useful to tweak a single past event, I don't see much of a use case of updating all events starting from some past date. I'd argue that that is nearly always unintended.

I don't think that edit is always invalid: offhand, one reason you might want to make such an edit is so that you can add a tag to all events in a series, allowing you to query them so they can be exported to another calendar. Another case I could imagine is wanting to correct a mistake in the description of an event when you know nothing will be lost by editing future events (e.g., because you are the only user who can edit the event).

I agree that these edits are probably less common than edits of upcoming events, but I worry it would be frustrating to be unable to fix a typo you notice after a few instances of an event have already passed when you're certain the edit is safe.

Thank you! The new dialog looks excellent, it would have prevented my error.

I don't think that edit is always invalid: offhand, one reason you might want to make such an edit is so that you can add a tag to all events in a series, allowing you to query them so they can be exported to another calendar. Another case I could imagine is wanting to correct a mistake in the description of an event when you know nothing will be lost by editing future events (e.g., because you are the only user who can edit the event).

Yes I see your point, especially about the tags. But adding a tag doesn't override the title or description of old instances - or does it?! I certainly would not expect that!
Perhaps that use case is uncommon enough to allow for an extra confirmation dialog?...