Store more datetime information on Calendar transactions and improve rendering behaviors

Summary:
Fixes T11805. Depends on D16785. This generally tries to smooth out transactions:

  - All-day stuff now says "Nov 3" instead of "Nov 3 12:00:00 AM".
  - Fewer weird bugs / extra transactions.
  - No more silly extra "yeah, you definitely set that event time" transaction on create.

Test Plan: Edited events; changed from all-day to not-all-day and back again, viewed transaction log.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11805

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

src/applications/calendar/editor/PhabricatorCalendarEventEditor.php

@@ -3,6 +3,9 @@
 final class PhabricatorCalendarEventEditor
   extends PhabricatorApplicationTransactionEditor {
 
+  private $oldIsAllDay;
+  private $newIsAllDay;
+
   public function getEditorApplicationClass() {
     return 'PhabricatorCalendarApplication';
   }
@@ -19,13 +22,20 @@ final class PhabricatorCalendarEventEditor
     return pht('%s created %s.', $author, $object);
   }
 
-
   protected function shouldApplyInitialEffects(
     PhabricatorLiskDAO $object,
     array $xactions) {
     return true;
   }
 
+  public function getOldIsAllDay() {
+    return $this->oldIsAllDay;
+  }
+
+  public function getNewIsAllDay() {
+    return $this->newIsAllDay;
+  }
+
   protected function applyInitialEffects(
     PhabricatorLiskDAO $object,
     array $xactions) {
@@ -34,6 +44,22 @@ final class PhabricatorCalendarEventEditor
     if ($object->getIsStub()) {
       $this->materializeStub($object);
     }
+
+    // Before doing anything, figure out if the event will be an all day event
+    // or not after the edit. This affects how we store datetime values, and
+    // whether we render times or not.
+    $old_allday = $object->getIsAllDay();
+    $new_allday = $old_allday;
+    $type_allday = PhabricatorCalendarEventAllDayTransaction::TRANSACTIONTYPE;
+    foreach ($xactions as $xaction) {
+      if ($xaction->getTransactionType() != $type_allday) {
+        continue;
+      }
+      $target_alllday = (bool)$xaction->getNewValue();
+    }
+
+    $this->oldIsAllDay = $old_allday;
+    $this->newIsAllDay = $new_allday;
   }
 
   private function materializeStub(PhabricatorCalendarEvent $event) {

src/applications/calendar/xaction/PhabricatorCalendarEventDateTransaction.php

@@ -6,7 +6,11 @@ abstract class PhabricatorCalendarEventDateTransaction
   abstract protected function getInvalidDateMessage();
 
   public function generateNewValue($object, $value) {
-    return $value->getEpoch();
+    $editor = $this->getEditor();
+    return $value->newPhutilDateTime()
+      ->setIsAllDay($editor->getNewIsAllDay())
+      ->newAbsoluteDateTime()
+      ->toDictionary();
   }
 
   public function validateTransactions($object, array $xactions) {

src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php

@@ -6,20 +6,32 @@ final class PhabricatorCalendarEventEndDateTransaction
   const TRANSACTIONTYPE = 'calendar.enddate';
 
   public function generateOldValue($object) {
-    // TODO: Upgrade this.
+    $editor = $this->getEditor();
-    return $object->newEndDateTimeForEdit()->getEpoch();
+
+    return $object->newEndDateTimeForEdit()
+      ->newAbsoluteDateTime()
+      ->setIsAllDay($editor->getOldIsAllDay())
+      ->toDictionary();
   }
 
   public function applyInternalEffects($object, $value) {
     $actor = $this->getActor();
+    $editor = $this->getEditor();
+
+    $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value);
+    $datetime->setIsAllDay($editor->getNewIsAllDay());
 
-    $datetime = PhutilCalendarAbsoluteDateTime::newFromEpoch(
-      $value,
-      $actor->getTimezoneIdentifier());
-    $datetime->setIsAllDay($object->getIsAllDay());
     $object->setEndDateTime($datetime);
   }
 
+  public function shouldHide() {
+    if ($this->isCreateTransaction()) {
+      return true;
+    }
+
+    return false;
+  }
+
   public function getTitle() {
     return pht(
       '%s changed the end date for this event from %s to %s.',

src/applications/calendar/xaction/PhabricatorCalendarEventStartDateTransaction.php

@@ -6,20 +6,32 @@ final class PhabricatorCalendarEventStartDateTransaction
   const TRANSACTIONTYPE = 'calendar.startdate';
 
   public function generateOldValue($object) {
-    // TODO: Upgrade this.
+    $editor = $this->getEditor();
-    return $object->getStartDateTimeEpoch();
+
+    return $object->newStartDateTime()
+      ->newAbsoluteDateTime()
+      ->setIsAllDay($editor->getOldIsAllDay())
+      ->toDictionary();
   }
 
   public function applyInternalEffects($object, $value) {
     $actor = $this->getActor();
+    $editor = $this->getEditor();
+
+    $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value);
+    $datetime->setIsAllDay($editor->getNewIsAllDay());
 
-    $datetime = PhutilCalendarAbsoluteDateTime::newFromEpoch(
-      $value,
-      $actor->getTimezoneIdentifier());
-    $datetime->setIsAllDay($object->getIsAllDay());
     $object->setStartDateTime($datetime);
   }
 
+  public function shouldHide() {
+    if ($this->isCreateTransaction()) {
+      return true;
+    }
+
+    return false;
+  }
+
   public function getTitle() {
     return pht(
       '%s changed the start date for this event from %s to %s.',

src/applications/calendar/xaction/PhabricatorCalendarEventUntilDateTransaction.php

@@ -6,20 +6,24 @@ final class PhabricatorCalendarEventUntilDateTransaction
   const TRANSACTIONTYPE = 'calendar.recurrenceenddate';
 
   public function generateOldValue($object) {
-    // TODO: Upgrade this.
+    $editor = $this->getEditor();
-    return $object->getUntilDateTimeEpoch();
+
+    return $object->newUntilDateTime()
+      ->newAbsoluteDateTime()
+      ->setIsAllDay($editor->getOldIsAllDay())
+      ->toDictionary();
   }
 
   public function applyInternalEffects($object, $value) {
     $actor = $this->getActor();
+    $editor = $this->getEditor();
 
     // TODO: DEPRECATED.
     $object->setRecurrenceEndDate($value);
 
-    $datetime = PhutilCalendarAbsoluteDateTime::newFromEpoch(
+    $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($value);
-      $value,
+    $datetime->setIsAllDay($editor->getNewIsAllDay());
-      $actor->getTimezoneIdentifier());
+
-    $datetime->setIsAllDay($object->getIsAllDay());
     $object->setUntilDateTime($datetime);
   }
 

src/applications/transactions/storage/PhabricatorModularTransactionType.php

@@ -196,19 +196,36 @@ abstract class PhabricatorModularTransactionType
   final protected function renderDate($epoch) {
     $viewer = $this->getViewer();
 
-    $display = phabricator_datetime($epoch, $viewer);
+    // We accept either epoch timestamps or dictionaries describing a
+    // PhutilCalendarDateTime.
+    if (is_array($epoch)) {
+      $datetime = PhutilCalendarAbsoluteDateTime::newFromDictionary($epoch)
+        ->setViewerTimezone($viewer->getTimezoneIdentifier());
 
-    // When rendering to text, we explicitly render the offset from UTC to
+      $all_day = $datetime->getIsAllDay();
-    // provide context to the date: the mail may be generating with the
-    // server's settings, or the user may later refer back to it after changing
-    // timezones.
 
-    if ($this->isTextMode()) {
+      $epoch = $datetime->getEpoch();
-      $offset = $viewer->getTimeZoneOffsetInHours();
+    } else {
-      if ($offset >= 0) {
+      $all_day = false;
-        $display = pht('%s (UTC+%d)', $display, $offset);
+    }
-      } else {
+
-        $display = pht('%s (UTC-%d)', $display, abs($offset));
+    if ($all_day) {
+      $display = phabricator_date($epoch, $viewer);
+    } else {
+      $display = phabricator_datetime($epoch, $viewer);
+
+      // When rendering to text, we explicitly render the offset from UTC to
+      // provide context to the date: the mail may be generating with the
+      // server's settings, or the user may later refer back to it after
+      // changing timezones.
+
+      if ($this->isTextMode()) {
+        $offset = $viewer->getTimeZoneOffsetInHours();
+        if ($offset >= 0) {
+          $display = pht('%s (UTC+%d)', $display, $offset);
+        } else {
+          $display = pht('%s (UTC-%d)', $display, abs($offset));
+        }
       }
     }
 
@@ -262,4 +279,8 @@ abstract class PhabricatorModularTransactionType
       ->setTransaction($this->getStorage());
   }
 
+  final protected function isCreateTransaction() {
+    return $this->getStorage()->getIsCreateTransaction();
+  }
+
 }

src/view/form/control/AphrontFormDateControlValue.php

@@ -231,6 +231,32 @@ final class AphrontFormDateControlValue extends Phobject {
     return $datetime;
   }
 
+  public function newPhutilDateTime() {
+    $datetime = $this->getDateTime();
+    if (!$datetime) {
+      return null;
+    }
+
+    $all_day = !strlen($this->valueTime);
+    $zone_identifier = $this->viewer->getTimezoneIdentifier();
+
+    $result = id(new PhutilCalendarAbsoluteDateTime())
+      ->setYear((int)$datetime->format('Y'))
+      ->setMonth((int)$datetime->format('m'))
+      ->setDay((int)$datetime->format('d'))
+      ->setHour((int)$datetime->format('G'))
+      ->setMinute((int)$datetime->format('i'))
+      ->setSecond((int)$datetime->format('s'))
+      ->setTimezone($zone_identifier);
+
+    if ($all_day) {
+      $result->setIsAllDay(true);
+    }
+
+    return $result;
+  }
+
+
   private function getFormattedDateFromParts(
     $year,
     $month,