Begin navigating the mess that is edits to recurring events

Summary: Ref T11804. This puts us on a path toward some kind of reasonable behavior here. Currently, cancelling recurring events makes approximately zero sense ever in any situation. Instead, give users the choice to cancel just the instance, or all future events. This is similar to Calendar.app. (Google Calendar has a third option, "All Events", which I may implement). When the user picks something, basically do that. The particulars of "do that" are messy. We have to split the series into two different series, stop the first series early, then edit the second series. Then we need to update any concrete events that are now part of the second series. This code will get less junk in the next couple of diffs (I hope?) since I need to make it apply to edits, too, but this was a little easier to get started with. Test Plan: Cancelled an instance of an event; cancelled "All future events". Both of them more or less worked in a reasonble way. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11804 Differential Revision: https://secure.phabricator.com/D16778
2024-12-22 13:30:55 +01:00 · 2016-10-31 12:47:39 -07:00 · 2016-10-31 12:47:39 -07:00 · 91089acbe5
commit 91089acbe5
parent 8e5437226f
6 changed files with 216 additions and 88 deletions
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -2049,6 +2049,7 @@ phutil_register_library_map(array(
    'PhabricatorCalendarEventEmailCommand' => 'applications/calendar/command/PhabricatorCalendarEventEmailCommand.php',
    'PhabricatorCalendarEventEndDateTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php',
    'PhabricatorCalendarEventExportController' => 'applications/calendar/controller/PhabricatorCalendarEventExportController.php',
+    'PhabricatorCalendarEventForkTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php',
    'PhabricatorCalendarEventFrequencyTransaction' => 'applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php',
    'PhabricatorCalendarEventFulltextEngine' => 'applications/calendar/search/PhabricatorCalendarEventFulltextEngine.php',
    'PhabricatorCalendarEventHeraldAdapter' => 'applications/calendar/herald/PhabricatorCalendarEventHeraldAdapter.php',
@ -6889,6 +6890,7 @@ phutil_register_library_map(array(
    'PhabricatorCalendarEventEmailCommand' => 'MetaMTAEmailTransactionCommand',
    'PhabricatorCalendarEventEndDateTransaction' => 'PhabricatorCalendarEventDateTransaction',
    'PhabricatorCalendarEventExportController' => 'PhabricatorCalendarController',
+    'PhabricatorCalendarEventForkTransaction' => 'PhabricatorCalendarEventTransactionType',
    'PhabricatorCalendarEventFrequencyTransaction' => 'PhabricatorCalendarEventTransactionType',
    'PhabricatorCalendarEventFulltextEngine' => 'PhabricatorFulltextEngine',
    'PhabricatorCalendarEventHeraldAdapter' => 'HeraldAdapter',
--- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php
+++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php
@ -32,16 +32,77 @@ final class PhabricatorCalendarEventCancelController

    $is_parent = $event->isParentEvent();
    $is_child = $event->isChildEvent();
-    $is_cancelled = $event->getIsCancelled();

-    if ($is_child) {
-      $is_parent_cancelled = $event->getParentEvent()->getIsCancelled();
-    } else {
-      $is_parent_cancelled = false;
-    }
+    $is_cancelled = $event->getIsCancelled();
+    $is_recurring = $event->getIsRecurring();

    $validation_exception = null;
    if ($request->isFormPost()) {
+
+      $targets = array();
+      if ($is_recurring) {
+        $mode = $request->getStr('mode');
+        $is_future = ($mode == 'future');
+
+        // We need to fork the event if we're cancelling just the parent, or
+        // are cancelling a child and all future events.
+        $must_fork = ($is_child && $is_future) ||
+                     ($is_parent && !$is_future);
+
+        if ($must_fork) {
+          if ($is_child) {
+            $xactions = array();
+
+            $xaction = id(new PhabricatorCalendarEventTransaction())
+              ->setTransactionType(
+                PhabricatorCalendarEventForkTransaction::TRANSACTIONTYPE)
+              ->setNewValue(true);
+
+            $editor = id(new PhabricatorCalendarEventEditor())
+              ->setActor($viewer)
+              ->setContentSourceFromRequest($request)
+              ->setContinueOnNoEffect(true)
+              ->setContinueOnMissingFields(true);
+
+            $editor->applyTransactions($event, array($xaction));
+
+            $targets[] = $event;
+          } else {
+            // TODO: This is a huge mess; we need to load or generate the
+            // first child, then fork that, then apply the event to the
+            // parent. Just bail for now.
+            throw new Exception(
+              pht(
+                'Individual edits to parent events are not yet supported '.
+                'because they are real tricky to implement.'));
+          }
+        } else {
+          $targets[] = $event;
+        }
+
+        if ($is_future) {
+          // NOTE: If you can't edit some of the future events, we just
+          // don't try to update them. This seems like it's probably what
+          // users are likely to expect.
+          $future = id(new PhabricatorCalendarEventQuery())
+            ->setViewer($viewer)
+            ->withParentEventPHIDs(array($event->getPHID()))
+            ->withUTCInitialEpochBetween($event->getUTCInitialEpoch(), null)
+            ->requireCapabilities(
+              array(
+                PhabricatorPolicyCapability::CAN_VIEW,
+                PhabricatorPolicyCapability::CAN_EDIT,
+              ))
+            ->execute();
+          foreach ($future as $future_event) {
+            $targets[] = $future_event;
+          }
+        }
+      } else {
+        $targets = array($event);
+      }
+
+      foreach ($targets as $target) {
        $xactions = array();

        $xaction = id(new PhabricatorCalendarEventTransaction())
@ -56,64 +117,67 @@ final class PhabricatorCalendarEventCancelController
          ->setContinueOnMissingFields(true);

        try {
-        $editor->applyTransactions($event, array($xaction));
-        return id(new AphrontRedirectResponse())->setURI($cancel_uri);
+          $editor->applyTransactions($target, array($xaction));
        } catch (PhabricatorApplicationTransactionValidationException $ex) {
          $validation_exception = $ex;
+          break;
+        }
+
+      }
+
+      if (!$validation_exception) {
+        return id(new AphrontRedirectResponse())->setURI($cancel_uri);
      }
    }

    if ($is_cancelled) {
-      if ($is_parent_cancelled) {
-        $title = pht('Cannot Reinstate Instance');
-        $paragraph = pht(
-          'You cannot reinstate an instance of a cancelled recurring event.');
-        $cancel = pht('Back');
-        $submit = null;
-      } else if ($is_child) {
-        $title = pht('Reinstate Instance');
-        $paragraph = pht(
-          'Reinstate this instance of this recurring event?');
-        $cancel = pht('Back');
-        $submit = pht('Reinstate Instance');
-      } else if ($is_parent) {
-        $title = pht('Reinstate Recurring Event');
-        $paragraph = pht(
-          'Reinstate all instances of this recurring event which have not '.
-          'been individually cancelled?');
-        $cancel = pht('Back');
-        $submit = pht('Reinstate Recurring Event');
-      } else {
      $title = pht('Reinstate Event');
-        $paragraph = pht('Reinstate this event?');
-        $cancel = pht('Back');
-        $submit = pht('Reinstate Event');
-      }
+      if ($is_recurring) {
+        $body = pht(
+          'This event is part of a series. Which events do you want to '.
+          'reinstate?');
+        $show_control = true;
      } else {
-      if ($is_child) {
-        $title = pht('Cancel Instance');
-        $paragraph = pht('Cancel this instance of this recurring event?');
-        $cancel = pht('Back');
-        $submit = pht('Cancel Instance');
-      } else if ($is_parent) {
-        $title = pht('Cancel Recurrin Event');
-        $paragraph = pht('Cancel this entire series of recurring events?');
-        $cancel = pht('Back');
-        $submit = pht('Cancel Recurring Event');
+        $body = pht('Reinstate this event?');
+        $show_control = false;
+      }
+      $submit = pht('Reinstate Event');
    } else {
      $title = pht('Cancel Event');
-        $paragraph = pht(
-          'Cancel this event? You can always reinstate the event later.');
-        $cancel = pht('Back');
+      if ($is_recurring) {
+        $body = pht(
+          'This event is part of a series. Which events do you want to '.
+          'cancel?');
+        $show_control = true;
+      } else {
+        $body = pht('Cancel this event?');
+        $show_control = false;
+      }
      $submit = pht('Cancel Event');
    }
-    }

-    return $this->newDialog()
+    $dialog = $this->newDialog()
      ->setTitle($title)
      ->setValidationException($validation_exception)
-      ->appendParagraph($paragraph)
-      ->addCancelButton($cancel_uri, $cancel)
+      ->appendParagraph($body)
+      ->addCancelButton($cancel_uri, pht('Back'))
      ->addSubmitButton($submit);
+
+    if ($show_control) {
+      $form = id(new AphrontFormView())
+        ->setViewer($viewer)
+        ->appendControl(
+          id(new AphrontFormSelectControl())
+            ->setLabel(pht('Cancel Events'))
+            ->setName('mode')
+            ->setOptions(
+              array(
+                'this' => pht('Only This Event'),
+                'future' => pht('All Future Events'),
+              )));
+      $dialog->appendForm($form);
+    }
+
+    return $dialog;
  }
 }
--- a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php
+++ b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php
@ -206,20 +206,8 @@ final class PhabricatorCalendarEventViewController
    $cancel_uri = $this->getApplicationURI("event/cancel/{$id}/");
    $cancel_disabled = !$can_edit;

-    if ($event->isChildEvent()) {
-      $cancel_label = pht('Cancel This Instance');
-      $reinstate_label = pht('Reinstate This Instance');
-
-      if ($event->getParentEvent()->getIsCancelled()) {
-        $cancel_disabled = true;
-      }
-    } else if ($event->isParentEvent()) {
-      $cancel_label = pht('Cancel All');
-      $reinstate_label = pht('Reinstate All');
-    } else {
    $cancel_label = pht('Cancel Event');
    $reinstate_label = pht('Reinstate Event');
-    }

    if ($event->isCancelledEvent()) {
      $curtain->addAction(
--- a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php
+++ b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php
@ -17,6 +17,8 @@ final class PhabricatorCalendarEventQuery
  private $importSourcePHIDs;
  private $importAuthorPHIDs;
  private $importUIDs;
+  private $utcInitialEpochMin;
+  private $utcInitialEpochMax;

  private $generateGhosts = false;

@ -45,6 +47,12 @@ final class PhabricatorCalendarEventQuery
    return $this;
  }

+  public function withUTCInitialEpochBetween($min, $max) {
+    $this->utcInitialEpochMin = $min;
+    $this->utcInitialEpochMax = $max;
+    return $this;
+  }
+
  public function withInvitedPHIDs(array $phids) {
    $this->inviteePHIDs = $phids;
    return $this;
@ -371,6 +379,20 @@ final class PhabricatorCalendarEventQuery
        $this->rangeEnd + phutil_units('16 hours in seconds'));
    }

+    if ($this->utcInitialEpochMin !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'event.utcInitialEpoch >= %d',
+        $this->utcInitialEpochMin);
+    }
+
+    if ($this->utcInitialEpochMax !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'event.utcInitialEpoch <= %d',
+        $this->utcInitialEpochMax);
+    }
+
    if ($this->inviteePHIDs !== null) {
      $where[] = qsprintf(
        $conn,
--- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php
+++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php
@ -165,6 +165,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
      'editPolicy' => true,
      'name' => true,
      'description' => true,
+      'isCancelled' => true,
    );

    // Read these fields from the parent event instead of this event. For
@ -204,7 +205,8 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
      ->setViewPolicy($parent->getViewPolicy())
      ->setEditPolicy($parent->getEditPolicy())
      ->setName($parent->getName())
-      ->setDescription($parent->getDescription());
+      ->setDescription($parent->getDescription())
+      ->setIsCancelled($parent->getIsCancelled());

    if ($start) {
      $start_datetime = $start;
@ -569,7 +571,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
    return $this->assertAttached($this->parentEvent);
  }

-  public function attachParentEvent($event) {
+  public function attachParentEvent(PhabricatorCalendarEvent $event = null) {
    $this->parentEvent = $event;
    return $this;
  }
@ -583,17 +585,8 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
  }

  public function isCancelledEvent() {
-    if ($this->getIsCancelled()) {
-      return true;
-    }
-
-    if ($this->isChildEvent()) {
-      if ($this->getParentEvent()->getIsCancelled()) {
-        return true;
-      }
-    }
-
-    return false;
+    // TODO: Remove this wrapper.
+    return $this->getIsCancelled();
  }

  public function renderEventDate(
--- a/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php
+++ b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php
@ -0,0 +1,59 @@
+<?php
+
+final class PhabricatorCalendarEventForkTransaction
+  extends PhabricatorCalendarEventTransactionType {
+
+  const TRANSACTIONTYPE = 'calendar.fork';
+
+  public function generateOldValue($object) {
+    return false;
+  }
+
+  public function shouldHide() {
+    // This transaction is purely an internal implementation detail which
+    // supports editing groups of events like "All Future Events".
+    return true;
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $parent = $object->getParentEvent();
+
+    $object->setInstanceOfEventPHID(null);
+    $object->attachParentEvent(null);
+
+    $rrule = $parent->newRecurrenceRule();
+    $object->setRecurrenceRule($rrule);
+
+    $until = $parent->newUntilDateTime();
+    if ($until) {
+      $object->setUntilDateTime($until);
+    }
+
+    $old_sequence_index = $object->getSequenceIndex();
+    $object->setSequenceIndex(0);
+
+    // Stop the parent event from recurring after the start date of this event.
+    $parent->setUntilDateTime($object->newStartDateTime());
+    $parent->save();
+
+    // NOTE: If we implement "COUNT" on editable events, we need to adjust
+    // the "COUNT" here and divide it up between the parent and the fork.
+
+    // Make all following children of the old parent children of this node
+    // instead.
+    $conn = $object->establishConnection('w');
+    queryfx(
+      $conn,
+      'UPDATE %T SET
+        instanceOfEventPHID = %s,
+        sequenceIndex = (sequenceIndex - %d)
+        WHERE instanceOfEventPHID = %s
+        AND utcInstanceEpoch > %d',
+      $object->getTableName(),
+      $object->getPHID(),
+      $old_sequence_index,
+      $parent->getPHID(),
+      $object->getUTCInstanceEpoch());
+  }
+
+}