diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php @@ -94,7 +94,7 @@ // 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); + $is_future = ($this->getSeriesEditMode() == self::MODE_FUTURE); $fields = array( id(new PhabricatorTextEditField()) @@ -162,7 +162,7 @@ ->setConduitTypeDescription(pht('New event host.')) ->setSingleValue($object->getHostPHID()), id(new PhabricatorDatasourceEditField()) - ->setIsHidden($hide_invitees) + ->setIsHidden($is_future) ->setKey('inviteePHIDs') ->setAliases(array('invite', 'invitee', 'invitees', 'inviteePHID')) ->setLabel(pht('Invitees')) @@ -193,8 +193,16 @@ ->setConduitDescription(pht('Change the event icon.')) ->setConduitTypeDescription(pht('New event icon.')) ->setValue($object->getIcon()), + + // NOTE: We're being a little sneaky here. This field is hidden and + // always has the value "true", so it makes the event recurring when you + // submit a form which contains the field. Then we put the the field on + // the "recurring" page in the "Make Recurring" dialog to simplify the + // workflow. This is still normal, explicit field from the perspective + // of the API. + id(new PhabricatorBoolEditField()) - ->setIsHidden($object->getIsRecurring()) + ->setIsHidden(true) ->setKey('isRecurring') ->setLabel(pht('Recurring')) ->setOptions(pht('One-Time Event'), pht('Recurring Event')) @@ -203,7 +211,7 @@ ->setDescription(pht('One time or recurring event.')) ->setConduitDescription(pht('Make the event recurring.')) ->setConduitTypeDescription(pht('Mark the event as a recurring event.')) - ->setValue($object->getIsRecurring()), + ->setValue(true), id(new PhabricatorSelectEditField()) ->setKey('frequency') ->setLabel(pht('Frequency')) @@ -295,35 +303,35 @@ protected function willApplyTransactions($object, array $xactions) { $viewer = $this->getViewer(); - $this->rawTransactions = $this->cloneTransactions($xactions); $is_parent = $object->isParentEvent(); $is_child = $object->isChildEvent(); $is_future = ($this->getSeriesEditMode() === self::MODE_FUTURE); + // Figure out which transactions we can apply to the whole series of events. + // Some transactions (like comments) can never be bulk applied. + $inherited_xactions = array(); + foreach ($xactions as $xaction) { + $modular_type = $xaction->getModularType(); + if (!($modular_type instanceof PhabricatorCalendarEventTransactionType)) { + continue; + } + + $inherited_edit = $modular_type->isInheritedEdit(); + if ($inherited_edit) { + $inherited_xactions[] = $xaction; + } + } + $this->rawTransactions = $this->cloneTransactions($inherited_xactions); + $must_fork = ($is_child && $is_future) || ($is_parent && !$is_future); + // We don't need to fork when editing a parent event if none of the edits + // can transfer to child events. For example, commenting on a parent is + // fine. if ($is_parent && !$is_future) { - // We don't necessarily need to fork if whatever we're editing is not - // inherited by children. For example, we can add a comment to the parent - // event without needing to fork. Test all the stuff we're doing and see - // if anything is actually inherited. - - $inherited_edit = false; - foreach ($xactions as $xaction) { - $modular_type = $xaction->getTransactionImplementation(); - if ($modular_type instanceof PhabricatorCalendarEventTransactionType) { - $inherited_edit = $modular_type->isInheritedEdit(); - if ($inherited_edit) { - break; - } - } - } - - // Nothing the user is trying to do requires us to fork, so we can just - // apply the changes normally. - if (!$inherited_edit) { + if (!$inherited_xactions) { $must_fork = false; } } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -935,10 +935,14 @@ $datetime->newAbsoluteDateTime()->toDictionary()); } - public function setUntilDateTime(PhutilCalendarDateTime $datetime) { - return $this->setParameter( - 'untilDateTime', - $datetime->newAbsoluteDateTime()->toDictionary()); + public function setUntilDateTime(PhutilCalendarDateTime $datetime = null) { + if ($datetime) { + $value = $datetime->newAbsoluteDateTime()->toDictionary(); + } else { + $value = null; + } + + return $this->setParameter('untilDateTime', $value); } public function setRecurrenceRule(PhutilCalendarRecurrenceRule $rrule) { diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php --- a/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php @@ -5,8 +5,17 @@ abstract protected function getInvalidDateMessage(); + public function isInheritedEdit() { + return false; + } + public function generateNewValue($object, $value) { $editor = $this->getEditor(); + + if ($value->isDisabled()) { + return null; + } + return $value->newPhutilDateTime() ->setIsAllDay($editor->getNewIsAllDay()) ->newAbsoluteDateTime() diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php --- a/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventIconTransaction.php @@ -13,6 +13,14 @@ $object->setIcon($value); } + public function shouldHide() { + if ($this->isCreateTransaction()) { + return true; + } + + return false; + } + public function getTitle() { $old = $this->getIconLabel($this->getOldValue()); $new = $this->getIconLabel($this->getNewValue()); diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php --- a/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventRecurringTransaction.php @@ -13,6 +13,17 @@ return (int)$value; } + public function isInheritedEdit() { + return false; + } + + public function shouldHide() { + // This event isn't interesting on its own, and is accompanied by an + // "alice set this event to repeat weekly." event in normal circumstances + // anyway. + return true; + } + public function applyInternalEffects($object, $value) { $object->setIsRecurring($value); } diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php --- a/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php +++ b/src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php @@ -23,25 +23,41 @@ $actor = $this->getActor(); $editor = $this->getEditor(); - $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); - $datetime->setIsAllDay($editor->getNewIsAllDay()); - - $object->setUntilDateTime($datetime); + if ($value) { + $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value); + $datetime->setIsAllDay($editor->getNewIsAllDay()); + $object->setUntilDateTime($datetime); + } else { + $object->setUntilDateTime(null); + } } public function getTitle() { - return pht( - '%s changed this event to repeat until %s.', - $this->renderAuthor(), - $this->renderNewDate()); + if ($this->getNewValue()) { + return pht( + '%s changed this event to repeat until %s.', + $this->renderAuthor(), + $this->renderNewDate()); + } else { + return pht( + '%s changed this event to repeat forever.', + $this->renderAuthor()); + } } public function getTitleForFeed() { - return pht( - '%s changed %s to repeat until %s.', - $this->renderAuthor(), - $this->renderObject(), - $this->renderNewDate()); + if ($this->getNewValue()) { + return pht( + '%s changed %s to repeat until %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderNewDate()); + } else { + return pht( + '%s changed %s to repeat forever.', + $this->renderAuthor(), + $this->renderObject()); + } } protected function getInvalidDateMessage() { diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -1073,10 +1073,21 @@ break; default: - return pht( - '%s edited this %s.', - $this->renderHandleLink($author_phid), - $this->getApplicationObjectTypeName()); + // In developer mode, provide a better hint here about which string + // we're missing. + $developer_mode = 'phabricator.developer-mode'; + $is_developer = PhabricatorEnv::getEnvConfig($developer_mode); + if ($is_developer) { + return pht( + '%s edited this object (transaction type "%s").', + $this->renderHandleLink($author_phid), + $this->getTransactionType()); + } else { + return pht( + '%s edited this %s.', + $this->renderHandleLink($author_phid), + $this->getApplicationObjectTypeName()); + } } } @@ -1615,6 +1626,10 @@ 'editable by the transaction author.'); } + public function getModularType() { + return null; + } + /* -( PhabricatorDestructibleInterface )----------------------------------- */ diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -7,6 +7,10 @@ abstract public function getBaseTransactionClass(); + public function getModularType() { + return $this->getTransactionImplementation(); + } + final protected function getTransactionImplementation() { if (!$this->implementation) { $this->implementation = $this->newTransactionImplementation();