Page MenuHomePhabricator

Editing a recurring event overrides all past instances
Closed, ResolvedPublic

Description

Upstream bug report: https://secure.phabricator.com/T11909


On 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 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

daniel created this task.Nov 21 2016, 6:39 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptNov 21 2016, 6:39 PM
daniel triaged this task as High priority.Nov 21 2016, 6:40 PM

Setting to high prio, since this issue apparently causes data loss.
Not UBN, because i guess recurring phab events are rarely used.

looking into it...

mmodell claimed this task.Nov 21 2016, 8:44 PM
mmodell added a subscriber: jcrespo.

How much data was lost? We can probably restore the past descriptions from a snapshot with the help of a dba. @jcrespo?

Which table(s) are we looking at here?

Here's a query that returns the past events that were affected. I do not know which ones of these actually had edited titles/descriptions. Probably most.

https://phabricator.wikimedia.org/calendar/query/XJjy54s3PEFf/

This seems like a pretty nasty upstream bug. If you can confirm the problem, should we file it somewhere?

I am not really sure I am looking into the right table here (learning phabricator schema as we go!) but I can see that the following query:

select FROM_UNIXTIME(dateCreated),Description from calendar_event where name='Weekly ArchCom Planning Meeting' order by dateCreated desc limit 3\G

Despite of the time the event was created, it always has the same description for all the 18 rows it finds.
Example of 3 events:
Is that what you are saying it is broken or is that description expected to be the same for all the events? @mmodell can you provide some more details db-wise about what is you think it is broken?

MariaDB MISC m3 localhost phabricator_calendar > select FROM_UNIXTIME(dateCreated),Description from calendar_event where name='Weekly ArchCom Planning Meeting' order by dateCreated desc limit 3\G
*************************** 1. row ***************************
FROM_UNIXTIME(dateCreated): 2016-11-21 18:05:07
               Description: The weekly ArchCom planning meeting is described on [mw:ArchComPlanning](https://www.mediawiki.org/wiki/ArchComPlanning).  This meeting is typically held in the hour //before// the regular #ArchCom-RFC weekly meetings (E66)
 - Location:  Usually video call, with reserved room in SF.  See GCal invite
 - Time: Weekly, Wednesday 20:00 UTC (1pm [PDT](https://en.wikipedia.org/wiki/America/Los_Angeles), 22:00 [CEST](https://en.wikipedia.org/wiki/Central_European_Summer_Time))
  - This time is the standard as of this writing (in March 2016), but may change as we make adjustments to accommodate daylight savings/summer time adjustments for the participants.  By default, we intend for this meeting to be held Wednesday 1pm [San Francisco time](https://en.wikipedia.org/wiki/America/Los_Angeles).
 - Agenda/Minutes:
  - On wiki: [Architecture_committee/2016](https://www.mediawiki.org/wiki/Architecture_committee/2016)-xx-xx
  - Template: [Architecture_committee/MeetingNoteTemplate](https://www.mediawiki.org/wiki/Architecture_committee/MeetingNoteTemplate)
  - Important meeting guidelines/links:
    - [Architecture_committee/Planning meetings (ArchComPlanning)](https://www.mediawiki.org/wiki/ArchComPlanning)
    - [Governance (proposed)](https://www.mediawiki.org/wiki/Requests_for_comment/Governance) - description of "final comment period" in use
    - [Architecture_committee/Status](https://www.mediawiki.org/wiki/Architecture_committee/Status)
    - [#archcom-has-shepherd workboard](https://phabricator.wikimedia.org/project/board/1892/)

Minutes of most of these meetings:
https://www.mediawiki.org/wiki/Architecture_committee/Minutes

| 13:00 PT ArchCom Planning Meetings | [upcoming](https://phabricator.wikimedia.org/calendar/query/DlzGwrbxMLu9/) |  [all since 2016-03-30](https://phabricator.wikimedia.org/calendar/query/M4fMy2PZOBQQ/) |
| 14:00 PT ArchCom-RFC Meetings | [upcoming](https://phabricator.wikimedia.org/calendar/query/ShzbHT6BPGCE/) | [all since 2015-09-09](https://phabricator.wikimedia.org/calendar/query/XJjy54s3PEFf/)
*************************** 2. row ***************************
FROM_UNIXTIME(dateCreated): 2016-10-06 01:10:08
               Description: The weekly ArchCom planning meeting is described on [mw:ArchComPlanning](https://www.mediawiki.org/wiki/ArchComPlanning).  This meeting is typically held in the hour //before// the regular #ArchCom-RFC weekly meetings (E66)
 - Location:  Usually video call, with reserved room in SF.  See GCal invite
 - Time: Weekly, Wednesday 20:00 UTC (1pm [PDT](https://en.wikipedia.org/wiki/America/Los_Angeles), 22:00 [CEST](https://en.wikipedia.org/wiki/Central_European_Summer_Time))
  - This time is the standard as of this writing (in March 2016), but may change as we make adjustments to accommodate daylight savings/summer time adjustments for the participants.  By default, we intend for this meeting to be held Wednesday 1pm [San Francisco time](https://en.wikipedia.org/wiki/America/Los_Angeles).
 - Agenda/Minutes:
  - On wiki: [Architecture_committee/2016](https://www.mediawiki.org/wiki/Architecture_committee/2016)-xx-xx
  - Template: [Architecture_committee/MeetingNoteTemplate](https://www.mediawiki.org/wiki/Architecture_committee/MeetingNoteTemplate)
  - Important meeting guidelines/links:
    - [Architecture_committee/Planning meetings (ArchComPlanning)](https://www.mediawiki.org/wiki/ArchComPlanning)
    - [Governance (proposed)](https://www.mediawiki.org/wiki/Requests_for_comment/Governance) - description of "final comment period" in use
    - [Architecture_committee/Status](https://www.mediawiki.org/wiki/Architecture_committee/Status)
    - [#archcom-has-shepherd workboard](https://phabricator.wikimedia.org/project/board/1892/)

Minutes of most of these meetings:
https://www.mediawiki.org/wiki/Architecture_committee/Minutes

| 13:00 PT ArchCom Planning Meetings | [upcoming](https://phabricator.wikimedia.org/calendar/query/DlzGwrbxMLu9/) |  [all since 2016-03-30](https://phabricator.wikimedia.org/calendar/query/M4fMy2PZOBQQ/) |
| 14:00 PT ArchCom-RFC Meetings | [upcoming](https://phabricator.wikimedia.org/calendar/query/ShzbHT6BPGCE/) | [all since 2015-09-09](https://phabricator.wikimedia.org/calendar/query/XJjy54s3PEFf/)
*************************** 3. row ***************************
FROM_UNIXTIME(dateCreated): 2016-10-06 01:06:59
               Description: The weekly ArchCom planning meeting is described on [mw:ArchComPlanning](https://www.mediawiki.org/wiki/ArchComPlanning).  This meeting is typically held in the hour //before// the regular #ArchCom-RFC weekly meetings (E66)
 - Location:  Usually video call, with reserved room in SF.  See GCal invite
 - Time: Weekly, Wednesday 20:00 UTC (1pm [PDT](https://en.wikipedia.org/wiki/America/Los_Angeles), 22:00 [CEST](https://en.wikipedia.org/wiki/Central_European_Summer_Time))
  - This time is the standard as of this writing (in March 2016), but may change as we make adjustments to accommodate daylight savings/summer time adjustments for the participants.  By default, we intend for this meeting to be held Wednesday 1pm [San Francisco time](https://en.wikipedia.org/wiki/America/Los_Angeles).
 - Agenda/Minutes:
  - On wiki: [Architecture_committee/2016](https://www.mediawiki.org/wiki/Architecture_committee/2016)-xx-xx
  - Template: [Architecture_committee/MeetingNoteTemplate](https://www.mediawiki.org/wiki/Architecture_committee/MeetingNoteTemplate)
  - Important meeting guidelines/links:
    - [Architecture_committee/Planning meetings (ArchComPlanning)](https://www.mediawiki.org/wiki/ArchComPlanning)
    - [Governance (proposed)](https://www.mediawiki.org/wiki/Requests_for_comment/Governance) - description of "final comment period" in use
    - [Architecture_committee/Status](https://www.mediawiki.org/wiki/Architecture_committee/Status)
    - [#archcom-has-shepherd workboard](https://phabricator.wikimedia.org/project/board/1892/)

Minutes of most of these meetings:
https://www.mediawiki.org/wiki/Architecture_committee/Minutes

| 13:00 PT ArchCom Planning Meetings | [upcoming](https://phabricator.wikimedia.org/calendar/query/DlzGwrbxMLu9/) |  [all since 2016-03-30](https://phabricator.wikimedia.org/calendar/query/M4fMy2PZOBQQ/) |
| 14:00 PT ArchCom-RFC Meetings | [upcoming](https://phabricator.wikimedia.org/calendar/query/ShzbHT6BPGCE/) | [all since 2015-09-09](https://phabricator.wikimedia.org/calendar/query/XJjy54s3PEFf/)
3 rows in set (0.00 sec)

Despite of the time the event was created, it always has the same description for all the 18 rows it finds.
Example of 3 events:
Is that what you are saying it is broken or is that description expected to be the same for all the events?

Yes, exactly. A recurring event has a "base" event that is copied for every "instance" of the recurring event, until that instance is edited and changed. You can think of it as prototype inheritance. Most of the past events should have a description that says what that session was about, with a link to the RFC, and an RFC number in the title.

When I updated the prototype, I chose "Edit all future Events". But it overrode *all* instances, including *past* events.

Actually, the choice "edit only this event" makes no sense for the base event (the prototype). Maybe that's where the confusion arises.

mmodell updated the task description. (Show Details)Nov 22 2016, 8:08 PM
mmodell moved this task from Backlog to Reported Upstream on the Upstream board.
mmodell moved this task from To Triage to Misc on the Phabricator board.

So from upstream, it seems that phabricator is just behaving the same way as other calendar apps and the language about "future events" is just potentially confusing. It means all future events with respect to the event you are editing, not with respect to the current time.

see https://secure.phabricator.com/T11909#201469

I think phab behavior is misleading at best. I commented to that effect on the upstream ticket.

@Marostegui any chance of recovering the lost titles and descriptions?

I was working with Manuel on this, and he recovered a copy of the database from 2016-10-05 01:00:02, previous one exist. In order to help with this issue, I need

  • From @daniel: the exact time where this happened and what data was deleted.
  • From @mmodell: please guide us through the phabricator tables to find what has been lost. I do not expect an exact row number, but there are 58 databases on the backup, and I do not know where to start (database/table):

{P4503}

We cannot just recover everything, or an entire table, as usually things will break, but we can check individual lost entries.

If it's easier, this data can also be recovered completely from the live phabricator_calendar.calendar_eventtransaction table, which stores the old and new values.

See upstream https://secure.phabricator.com/T8637 for a vaguely similar issue and script.

The records to revert will have @daniel's user PHID as the authorPHID, and a dateCreated within a few seconds of the time of his edit (which looks like it was Mon, Nov 21, 10:11 AM).

The oldValue field stores the old value in JSON, the objectPHID is which event was edited.

I can provide a template script for performing the revert, but I'm not sure if that's less work than just reverting from the snapshot.

@jcrespo & @Marostegui:

The calendar event phid for E66 is PHID-CEVT-sggwinpwrtfsppo7pbqd and the timestamp for the edit is 1479751874.

All of the affected records can be found with this query:

select * from calendar_event where instanceOfEventPHID="PHID-CEVT-sggwinpwrtfsppo7pbqd";

If you would like, then I can write a script based on what @epriestley suggested above, or we can restore the 65 records from the snapshot using that query. I'm not sure how difficult it is to restore from the snapshot. It would not be too difficult for me to adapt @epriestley's script and restore from the transaction log. @jcrespo: Should I try that first that first? It's a holiday weekend so I likely won't be able to get to it until monday.

BTW, thanks @epriestley for responding here. Once again you exceed all expectations for helpful support ;)

This query selects the phid of the event instance, the oldValue and the newValue from for the events in question:

    SELECT ev.phid, tr.oldValue, tr.newValue
      FROM calendar_event ev
INNER JOIN calendar_eventtransaction tr ON (ev.phid = tr.objectPHID)
     WHERE ev.instanceOfEventPHID='PHID-CEVT-sggwinpwrtfsppo7pbqd' 
       AND tr.transactionType='calendar.description' 
       AND tr.dateModified>1479751850
       AND tr.dateModified<1479751900

I believe that restoring the descriptions is just a matter of updating the description of each calendar_event record matching the phid returned, setting it to the oldValue referenced in the result from that query.

Actually it should be possible to do this with an UPDATE query, I'm just a bit rusty on my SQL so I don't really want to try it on a live database.

this is the update query which I am not confident enough to run on live database:

    UPDATE calendar_event ev
INNER JOIN calendar_eventtransaction tr
        ON ev.`phid` = tr.`objectPHID`
       SET ev.`description`          = tr.`oldValue`
     WHERE ev.`instanceOfEventPHID`  = 'PHID-CEVT-sggwinpwrtfsppo7pbqd' 
       AND tr.`transactionType`      = 'calendar.description' 
       AND tr.`dateModified`         > 1479751850
       AND tr.`dateModified`         < 1479751900;
daniel lowered the priority of this task from High to Medium.Nov 25 2016, 1:10 AM

Oh wow, this is turning into an actual rescue mission. Note that this is not critical. It would be nice to have the agendas of past sessions back, but please don't treat this as urgent if you have better things to do.

I really appreciate all the help, thank you!

I'm taking this down to normal prio. My main concern was understanding the issue to avoid more accidental data loss.

I can provide a template script for performing the revert, but I'm not sure if that's less work than just reverting from the snapshot.

Thanks for offering this! I actually think this would be safer than reverting the whole snapshot as I am completely not familiar with the data model phabricator uses, so the less we can touch, the safest probably.

Seeing that @mmodell did a nice job getting the initial query, maybe it can be added or converted into that script that you were proposing?

Thank you @jcrespo @mmodell @epriestley and @daniel for all the help here!!! Really appreciate all the inputs and ideas!

Here's a starting point for doing the revert with a script.

  • Put it in phabricator/.
  • Customize the $actor_phid and date ranges at the top of the file.
  • Run it with php -f undo_event_edits.php.

It will identify title and description edits made by the specified user to events in that time window, and prompt you to undo each one. Here's what a prompt looks like:

>>> Found edit of: E252 Another Group Event Zoinks!
 PRE-EDIT NAME: Another Group Event
POST-EDIT NAME: Another Group Event Zoinks!
  CURRENT NAME: Another Group Event Zoinks!


    Revert this edit? [y/N] y

Applying changes...
Done.

That means that before the current edit we're considering reverting, the event name was "Another Group Event". After the edit, the name was changed to "Another Group Event Zoinks!". The "Current Name" is the value of the object in the database today (e.g., possibly after additional edits which were made later).

If you revert the edit by answering "y" to the prompt, the transaction will be applied in reverse and the "PRE-EDIT NAME" will be restored. The prompt for description changes is similar.

(You won't be prompted to revert edits if reverting would have no effect: for example, if the edit was already reverted by an earlier run of the script, or someone manually fixed it at some point.)

That query also seems reasonable to me, except that oldValue is JSON and description is plain text, so you'll get some extra quotes and escaping if you just copy the fields over. You'd have to clean that up separately I think unless there are some JSON functions in MySQL that I don't know about.

undo_event_edits.php
<?php

// Put the PHID of the user who performed the bad edit here. You can get this
// by running `user.query` from the Conduit web API.
$actor_phid = 'PHID-USER-cvfydnwadpdj7vdon36z';

// Specify the start and end time of the bad edit here. We'll ignore changes
// outside this time range. By default, this is looking at changes in the last
// 24 hours.
$min_date = time() - (24 * 60 * 60);
$max_date = time();


// --- End of Configuration ------------------------------------------------- //

require_once 'scripts/__init_script__.php';

$viewer = PhabricatorUser::getOmnipotentUser();

// Find all the event edits the user has ever performed.
$xactions = id(new PhabricatorCalendarEventTransactionQuery())
  ->setViewer($viewer)
  ->withAuthorPHIDs(array($actor_phid))
  ->withTransactionTypes(
    array(
      // We're only looking for title and description edits.
      PhabricatorCalendarEventNameTransaction::TRANSACTIONTYPE,
      PhabricatorCalendarEventDescriptionTransaction::TRANSACTIONTYPE,
    ))
  ->execute();

// Remove edits which did not occur in the specified time range.
foreach ($xactions as $key => $xaction) {
  $created = $xaction->getDateCreated();
  if ($created < $min_date || $created > $max_date) {
    unset($xactions[$key]);
  }
}

if (!$xactions) {
  echo "Found nothing to revert.\n";
}

// Group remaining edits by which event they affected.
$xaction_groups = mgroup($xactions, 'getObjectPHID');
foreach ($xaction_groups as $event_phid => $xactions) {
  $event = id(new PhabricatorCalendarEventQuery())
    ->setViewer($viewer)
    ->withPHIDs(array($event_phid))
    ->executeOne();
  if (!$event) {
    continue;
  }

  $monogram = $event->getMonogram();
  $name = $event->getName();

  $apply = array();
  foreach ($xactions as $xaction) {
    $old = $xaction->getOldValue();
    $new = $xaction->getNewValue();

    switch ($xaction->getTransactionType()) {
      case PhabricatorCalendarEventNameTransaction::TRANSACTIONTYPE:
        $current = $event->getName();
        break;
      case PhabricatorCalendarEventDescriptionTransaction::TRANSACTIONTYPE:
        $current = $event->getDescription();
        break;
    }

    // If reverting this edit would not do anything, skip this edit. Maybe
    // we already reverted it, or maybe a user manually undid it.
    if ($current == $old) {
      continue;
    }

    echo ">>> Found edit of: {$monogram} {$name}\n";
    switch ($xaction->getTransactionType()) {
      case PhabricatorCalendarEventNameTransaction::TRANSACTIONTYPE:
        echo " PRE-EDIT NAME: ".$old."\n";
        echo "POST-EDIT NAME: ".$new."\n";
        echo "  CURRENT NAME: ".$current."\n";
        break;
      case PhabricatorCalendarEventDescriptionTransaction::TRANSACTIONTYPE:
        echo " PRE-EDIT DESCRIPTION:\n".$old."\n\n";
        echo "POST-EDIT DESCRIPTION:\n".$new."\n\n";
        echo "  CURRENT DESCRIPTION:\n".$current."\n\n";
        break;
    }

    if (phutil_console_confirm(pht('Revert this edit?'))) {
      $apply[] = id(new PhabricatorCalendarEventTransaction())
        ->setTransactionType($xaction->getTransactionType())
        ->setNewValue($xaction->getOldValue());
    }
  }

  if (!$apply) {
    echo "No changes to apply, continuing.\n";
  }

  echo "Applying changes...\n";

  $content_source = PhabricatorContentSource::newForSource(
      PhabricatorUnitTestContentSource::SOURCECONST);

  $editor = id(new PhabricatorCalendarEventEditor())
    ->setActor($viewer)
    ->setActingAsPHID($actor_phid)
    ->setContinueOnMissingFields(true)
    ->setContinueOnNoEffect(true)
    ->setContentSource($content_source);

  $editor->applyTransactions($event, $apply);
}

echo "Done.\n";

@epriestley amazing! Thanks a lot for that!!
@mmodell @daniel let me know if you guys want me to take a backup (and if possible from which tables) before you attempt to run the script

Thank you all!

Ok I've reverted the edits using the script provided by epriestley.

@daniel, does everything look ok?

daniel closed this task as Resolved.Dec 9 2016, 10:49 AM

@mmodell yes, looks great, thank you!