From 91089acbe56b0833e9a973a17722ef2c57e20b81 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 31 Oct 2016 12:47:39 -0700 Subject: [PATCH] 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 --- src/__phutil_library_map__.php | 2 + ...abricatorCalendarEventCancelController.php | 186 ++++++++++++------ ...PhabricatorCalendarEventViewController.php | 16 +- .../query/PhabricatorCalendarEventQuery.php | 22 +++ .../storage/PhabricatorCalendarEvent.php | 19 +- ...habricatorCalendarEventForkTransaction.php | 59 ++++++ 6 files changed, 216 insertions(+), 88 deletions(-) create mode 100644 src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d91e588a16..a7d250eba3 100644 --- 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', diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php index fbf7f9d45e..9f4e0e508e 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventCancelController.php @@ -32,88 +32,152 @@ 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()) { - $xactions = array(); - $xaction = id(new PhabricatorCalendarEventTransaction()) - ->setTransactionType( - PhabricatorCalendarEventCancelTransaction::TRANSACTIONTYPE) - ->setNewValue(!$is_cancelled); + $targets = array(); + if ($is_recurring) { + $mode = $request->getStr('mode'); + $is_future = ($mode == 'future'); - $editor = id(new PhabricatorCalendarEventEditor()) - ->setActor($viewer) - ->setContentSourceFromRequest($request) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true); + // 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); - try { - $editor->applyTransactions($event, array($xaction)); + 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()) + ->setTransactionType( + PhabricatorCalendarEventCancelTransaction::TRANSACTIONTYPE) + ->setNewValue(!$is_cancelled); + + $editor = id(new PhabricatorCalendarEventEditor()) + ->setActor($viewer) + ->setContentSourceFromRequest($request) + ->setContinueOnNoEffect(true) + ->setContinueOnMissingFields(true); + + try { + $editor->applyTransactions($target, array($xaction)); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + $validation_exception = $ex; + break; + } + + } + + if (!$validation_exception) { return id(new AphrontRedirectResponse())->setURI($cancel_uri); - } catch (PhabricatorApplicationTransactionValidationException $ex) { - $validation_exception = $ex; } } 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'); + $title = 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 { - $title = pht('Reinstate Event'); - $paragraph = pht('Reinstate this event?'); - $cancel = pht('Back'); - $submit = pht('Reinstate Event'); + $body = pht('Reinstate this event?'); + $show_control = false; } + $submit = pht('Reinstate Event'); } 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'); + $title = pht('Cancel Event'); + if ($is_recurring) { + $body = pht( + 'This event is part of a series. Which events do you want to '. + 'cancel?'); + $show_control = true; } else { - $title = pht('Cancel Event'); - $paragraph = pht( - 'Cancel this event? You can always reinstate the event later.'); - $cancel = pht('Back'); - $submit = pht('Cancel Event'); + $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; } } diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php b/src/applications/calendar/controller/PhabricatorCalendarEventViewController.php index a67bc8b741..e0d17d55a7 100644 --- 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'); - } + $cancel_label = pht('Cancel Event'); + $reinstate_label = pht('Reinstate Event'); if ($event->isCancelledEvent()) { $curtain->addAction( diff --git a/src/applications/calendar/query/PhabricatorCalendarEventQuery.php b/src/applications/calendar/query/PhabricatorCalendarEventQuery.php index 5c6b34c9fb..a6ac2e4ffc 100644 --- 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, diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index ef7a808a61..3459601b0a 100644 --- 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( diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php new file mode 100644 index 0000000000..ad7692dafe --- /dev/null +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventForkTransaction.php @@ -0,0 +1,59 @@ +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()); + } + +}