When users edit recurring events, prompt to "Edit This Event" or "Edit All Future Events"

Summary:
Fixes T11804. This probably isn't perfect but seems to work fairly reasonably and not be as much of a weird nonsense mess like the old behavior was.

When a user edits a recurring event, we ask them what they're trying to do. Then we more or less do that.

Test Plan:
  - Edited an event in the middle of a series.
  - Edited the first event in a series.
  - Edited "just this" and "all future" events in various places in a series.
  - Edited normal events.
  - Cancelled various events.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11804

Differential Revision: https://secure.phabricator.com/D16782

src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php

@@ -48,40 +48,8 @@ final class PhabricatorCalendarEventCancelController
         // are cancelling a child and all future events.
         $must_fork = ($is_child && $is_future) ||
                      ($is_parent && !$is_future);
-
         if ($must_fork) {
-          if ($is_child) {
-            $fork_target = $event;
-          } else {
-            if ($event->isValidSequenceIndex($viewer, 1)) {
-              $next_event = id(new PhabricatorCalendarEventQuery())
-                ->setViewer($viewer)
-                ->withInstanceSequencePairs(
-                  array(
-                    array($event->getPHID(), 1),
-                  ))
-                ->requireCapabilities(
-                  array(
-                    PhabricatorPolicyCapability::CAN_VIEW,
-                    PhabricatorPolicyCapability::CAN_EDIT,
-                  ))
-                ->executeOne();
-
-              if (!$next_event) {
-                $next_event = $event->newStub($viewer, 1);
-              }
-
-              $fork_target = $next_event;
-            } else {
-              // This appears to be a "recurring" event with no valid
-              // instances: for example, its "until" date is before the second
-              // instance would occur. This can happen if we already forked the
-              // event or if users entered silly stuff. Just edit the event
-              // directly without forking anything.
-              $fork_target = null;
-            }
-          }
-
+          $fork_target = $event->loadForkTarget($viewer);
           if ($fork_target) {
             $xactions = array();
 
@@ -101,26 +69,7 @@ final class PhabricatorCalendarEventCancelController
         }
 
         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.
-
-          // NOTE: This only affects events that are currently in the same
-          // series, not all events that were ever in the original series.
-          // We could use series PHIDs instead of parent PHIDs to affect more
-          // events if this turns out to be counterintuitive. Other
-          // applications differ in their behavior.
-
-          $future = id(new PhabricatorCalendarEventQuery())
-            ->setViewer($viewer)
-            ->withParentEventPHIDs(array($event->getPHID()))
-            ->withUTCInitialEpochBetween($event->getUTCInitialEpoch(), null)
-            ->requireCapabilities(
-              array(
-                PhabricatorPolicyCapability::CAN_VIEW,
-                PhabricatorPolicyCapability::CAN_EDIT,
-              ))
-            ->execute();
+          $future = $event->loadFutureEvents($viewer);
           foreach ($future as $future_event) {
             $targets[] = $future_event;
           }

src/applications/calendar/controller/PhabricatorCalendarEventEditController.php

@@ -6,6 +6,9 @@ final class PhabricatorCalendarEventEditController
   public function handleRequest(AphrontRequest $request) {
     $viewer = $this->getViewer();
 
+    $engine = id(new PhabricatorCalendarEventEditEngine())
+      ->setController($this);
+
     $id = $request->getURIData('id');
     if ($id) {
       $event = id(new PhabricatorCalendarEventQuery())
@@ -16,11 +19,57 @@ final class PhabricatorCalendarEventEditController
       if ($response) {
         return $response;
       }
+
+      $cancel_uri = $event->getURI();
+
+      $page = $request->getURIData('pageKey');
+      if ($page == 'recurring') {
+        if ($event->isChildEvent()) {
+          return $this->newDialog()
+            ->setTitle(pht('Series Event'))
+            ->appendParagraph(
+              pht(
+                'This event is an instance in an event series. To change '.
+                'the behavior for the series, edit the parent event.'))
+            ->addCancelButton($cancel_uri);
+        }
+      } else if ($event->getIsRecurring()) {
+        $mode = $request->getStr('mode');
+        if (!$mode) {
+          $form = id(new AphrontFormView())
+            ->setViewer($viewer)
+            ->appendControl(
+              id(new AphrontFormSelectControl())
+                ->setLabel(pht('Edit Events'))
+                ->setName('mode')
+                ->setOptions(
+                  array(
+                    PhabricatorCalendarEventEditEngine::MODE_THIS
+                      => pht('Edit Only This Event'),
+                    PhabricatorCalendarEventEditEngine::MODE_FUTURE
+                      => pht('Edit All Future Events'),
+                  )));
+
+          return $this->newDialog()
+            ->setTitle(pht('Edit Event'))
+            ->appendParagraph(
+              pht(
+                'This event is part of a series. Which events do you '.
+                'want to edit?'))
+            ->appendForm($form)
+            ->addSubmitButton(pht('Continue'))
+            ->addCancelButton($cancel_uri)
+            ->setDisableWorkflowOnSubmit(true);
+
+        }
+
+        $engine
+          ->addContextParameter('mode', $mode)
+          ->setSeriesEditMode($mode);
+      }
     }
 
-    return id(new PhabricatorCalendarEventEditEngine())
-      ->setController($this)
-      ->buildResponse();
+    return $engine->buildResponse();
   }
 
 }

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

@@ -147,12 +147,8 @@ final class PhabricatorCalendarEventViewController
 
     $edit_uri = "event/edit/{$id}/";
     $edit_uri = $this->getApplicationURI($edit_uri);
-
-    if ($event->isChildEvent()) {
-      $edit_label = pht('Edit This Instance');
-    } else {
-      $edit_label = pht('Edit Event');
-    }
+    $is_recurring = $event->getIsRecurring();
+    $edit_label = pht('Edit Event');
 
     $curtain = $this->newCurtainView($event);
 
@@ -163,7 +159,7 @@ final class PhabricatorCalendarEventViewController
           ->setIcon('fa-pencil')
           ->setHref($edit_uri)
           ->setDisabled(!$can_edit)
-          ->setWorkflow(!$can_edit));
+          ->setWorkflow(!$can_edit || $is_recurring));
     }
 
     $recurring_uri = "{$edit_uri}page/recurring/";

src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php

@@ -5,6 +5,21 @@ final class PhabricatorCalendarEventEditEngine
 
   const ENGINECONST = 'calendar.event';
 
+  private $rawTransactions;
+  private $seriesEditMode = self::MODE_THIS;
+
+  const MODE_THIS = 'this';
+  const MODE_FUTURE = 'future';
+
+  public function setSeriesEditMode($series_edit_mode) {
+    $this->seriesEditMode = $series_edit_mode;
+    return $this;
+  }
+
+  public function getSeriesEditMode() {
+    return $this->seriesEditMode;
+  }
+
   public function getEngineName() {
     return pht('Calendar Events');
   }
@@ -77,6 +92,10 @@ final class PhabricatorCalendarEventEditEngine
       $frequency = null;
     }
 
+    // At least for now, just hide "Invitees" when editing all future events.
+    // This may eventually deserve a more nuanced approach.
+    $hide_invitees = ($this->getSeriesEditMode() == self::MODE_FUTURE);
+
     $fields = array(
       id(new PhabricatorTextEditField())
         ->setKey('name')
@@ -143,6 +162,7 @@ final class PhabricatorCalendarEventEditEngine
         ->setConduitTypeDescription(pht('New event host.'))
         ->setSingleValue($object->getHostPHID()),
       id(new PhabricatorDatasourceEditField())
+        ->setIsHidden($hide_invitees)
         ->setKey('inviteePHIDs')
         ->setAliases(array('invite', 'invitee', 'invitees', 'inviteePHID'))
         ->setLabel(pht('Invitees'))
@@ -273,4 +293,77 @@ final class PhabricatorCalendarEventEditEngine
     );
   }
 
+  protected function willApplyTransactions($object, array $xactions) {
+    $viewer = $this->getViewer();
+    $this->rawTransactions = $xactions;
+
+    $is_parent = $object->isParentEvent();
+    $is_child = $object->isChildEvent();
+    $is_future = ($this->getSeriesEditMode() === self::MODE_FUTURE);
+
+    $must_fork = ($is_child && $is_future) ||
+                 ($is_parent && !$is_future);
+
+    if ($must_fork) {
+      $fork_target = $object->loadForkTarget($viewer);
+      if ($fork_target) {
+        $fork_xaction = id(new PhabricatorCalendarEventTransaction())
+          ->setTransactionType(
+            PhabricatorCalendarEventForkTransaction::TRANSACTIONTYPE)
+          ->setNewValue(true);
+
+        if ($fork_target->getPHID() == $object->getPHID()) {
+          // We're forking the object itself, so just slip it into the
+          // transactions we're going to apply.
+          array_unshift($xactions, $fork_xaction);
+        } else {
+          // Otherwise, we're forking a different object, so we have to
+          // apply that separately.
+          $this->applyTransactions($fork_target, array($fork_xaction));
+        }
+      }
+    }
+
+    return $xactions;
+  }
+
+  protected function didApplyTransactions($object, array $xactions) {
+    $viewer = $this->getViewer();
+
+    if ($this->getSeriesEditMode() !== self::MODE_FUTURE) {
+      return;
+    }
+
+    $targets = $object->loadFutureEvents($viewer);
+    if (!$targets) {
+      return;
+    }
+
+    foreach ($targets as $target) {
+      $apply = clone $this->rawTransactions;
+      $this->applyTransactions($target, $apply);
+    }
+  }
+
+  private function applyTransactions($target, array $xactions) {
+    $viewer = $this->getViewer();
+
+    // TODO: This isn't the most accurate source we could use, but this mode
+    // is web-only for now.
+    $content_source = PhabricatorContentSource::newForSource(
+      PhabricatorWebContentSource::SOURCECONST);
+
+    $editor = id(new PhabricatorCalendarEventEditor())
+      ->setActor($viewer)
+      ->setContentSource($content_source)
+      ->setContinueOnNoEffect(true)
+      ->setContinueOnMissingFields(true);
+
+    try {
+      $editor->applyTransactions($target, $xactions);
+    } catch (PhabricatorApplicationTransactionValidationException $ex) {
+      // Just ignore any issues we run into.
+    }
+  }
+
 }

src/applications/calendar/storage/PhabricatorCalendarEvent.php

@@ -154,6 +154,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
       ->setSequenceIndex($sequence)
       ->setIsRecurring(true)
       ->attachParentEvent($this)
+      ->attachImportSource(null)
       ->setAllDayDateFrom(0)
       ->setAllDayDateTo(0)
       ->setDateFrom(0)
@@ -1060,6 +1061,69 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     return $this;
   }
 
+  public function loadForkTarget(PhabricatorUser $viewer) {
+    if (!$this->getIsRecurring()) {
+      // Can't fork an event which isn't recurring.
+      return null;
+    }
+
+    if ($this->isChildEvent()) {
+      // If this is a child event, this is the fork target.
+      return $this;
+    }
+
+    if (!$this->isValidSequenceIndex($viewer, 1)) {
+      // This appears to be a "recurring" event with no valid instances: for
+      // example, its "until" date is before the second instance would occur.
+      // This can happen if we already forked the event or if users entered
+      // silly stuff. Just edit the event directly without forking anything.
+      return null;
+    }
+
+
+    $next_event = id(new PhabricatorCalendarEventQuery())
+      ->setViewer($viewer)
+      ->withInstanceSequencePairs(
+        array(
+          array($this->getPHID(), 1),
+        ))
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->executeOne();
+
+    if (!$next_event) {
+      $next_event = $this->newStub($viewer, 1);
+    }
+
+    return $next_event;
+  }
+
+  public function loadFutureEvents(PhabricatorUser $viewer) {
+    // 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.
+
+    // NOTE: This only affects events that are currently in the same
+    // series, not all events that were ever in the original series.
+    // We could use series PHIDs instead of parent PHIDs to affect more
+    // events if this turns out to be counterintuitive. Other
+    // applications differ in their behavior.
+
+    return id(new PhabricatorCalendarEventQuery())
+      ->setViewer($viewer)
+      ->withParentEventPHIDs(array($this->getPHID()))
+      ->withUTCInitialEpochBetween($this->getUTCInitialEpoch(), null)
+      ->requireCapabilities(
+        array(
+          PhabricatorPolicyCapability::CAN_VIEW,
+          PhabricatorPolicyCapability::CAN_EDIT,
+        ))
+      ->execute();
+  }
+
 
 /* -(  Markup Interface  )--------------------------------------------------- */
 

src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php

@@ -33,7 +33,14 @@ final class PhabricatorCalendarEventForkTransaction
     $object->setSequenceIndex(0);
 
     // Stop the parent event from recurring after the start date of this event.
-    $parent->setUntilDateTime($object->newStartDateTime());
+    // Since the "until" time is inclusive, rewind it by one second. We could
+    // figure out the previous instance's time instead or use a COUNT, but this
+    // seems simpler as long as it doesn't cause any issues.
+    $until_cutoff = $object->newStartDateTime()
+      ->newRelativeDateTime('-PT1S')
+      ->newAbsoluteDateTime();
+
+    $parent->setUntilDateTime($until_cutoff);
     $parent->save();
 
     // NOTE: If we implement "COUNT" on editable events, we need to adjust

src/applications/transactions/editengine/PhabricatorEditEngine.php

@@ -996,9 +996,12 @@ abstract class PhabricatorEditEngine
         ->setContinueOnNoEffect(true);
 
       try {
+        $xactions = $this->willApplyTransactions($object, $xactions);
 
         $editor->applyTransactions($object, $xactions);
 
+        $this->didApplyTransactions($object, $xactions);
+
         return $this->newEditResponse($request, $object, $xactions);
       } catch (PhabricatorApplicationTransactionValidationException $ex) {
         $validation_exception = $ex;
@@ -2176,6 +2179,14 @@ abstract class PhabricatorEditEngine
     return $page_map[$selected_key];
   }
 
+  protected function willApplyTransactions($object, array $xactions) {
+    return $xactions;
+  }
+
+  protected function didApplyTransactions($object, array $xactions) {
+    return;
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */