Prepare event dates for EditEngine/API

Summary:
Ref T9275. Currently, the "Start Date", "End Date", and "Recurrence End Date" transcations take a complex value (AphrontFormDateControlValue) and reduce it to an epoch.

Do this a little earlier, since the API will be much more usable if it just passes in epoch timestamps.

Events also have some logic where they rewrite the from date and to date on the actual object for all day events, then undo the changes later. Specifically, if you have an all-day event on "July 24th", the exact start and end times vary based on who is looking at it. Instead of overwriting the persistent `dateFrom` and `dateTo` properties, add separate `viewer` properties to make it easier to keep this stuff straight.

Since this means all-day events get stored in UTC, we need to query/fetch (and then discard) slightly more events. This is perfectly and much simpler to do.

The one weird "UTC" hack in here will get nuked when this moves to EditEngine properly.

Test Plan: Edited times for normal events and all-day events.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9275

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

src/applications/calendar/controller/PhabricatorCalendarEventDragController.php

@@ -29,7 +29,7 @@ final class PhabricatorCalendarEventDragController
 
     $xactions = array();
 
-    $duration = $event->getDateTo() - $event->getDateFrom();
+    $duration = $event->getDuration();
 
     $start = $request->getInt('start');
     $start_value = id(AphrontFormDateControlValue::newFromEpoch(
@@ -50,7 +50,6 @@ final class PhabricatorCalendarEventDragController
       ->setTransactionType(PhabricatorCalendarEventTransaction::TYPE_END_DATE)
       ->setNewValue($end_value);
 
-
     $editor = id(new PhabricatorCalendarEventEditor())
       ->setActor($viewer)
       ->setContinueOnMissingFields(true)

src/applications/calendar/controller/PhabricatorCalendarEventEditController.php

@@ -91,10 +91,10 @@ final class PhabricatorCalendarEventEditController
 
       $end_value = AphrontFormDateControlValue::newFromEpoch(
         $viewer,
-        $event->getDateTo());
+        $event->getViewerDateTo());
       $start_value = AphrontFormDateControlValue::newFromEpoch(
         $viewer,
-        $event->getDateFrom());
+        $event->getViewerDateFrom());
       $recurrence_end_date_value = id(clone $end_value)
         ->setOptional(true);
 
@@ -137,7 +137,17 @@ final class PhabricatorCalendarEventEditController
     $view_policy = $event->getViewPolicy();
     $space = $event->getSpacePHID();
 
+
     if ($request->isFormPost()) {
+      $is_all_day = $request->getStr('isAllDay');
+
+      if ($is_all_day) {
+        // TODO: This is a very gross temporary hack to get this working
+        // reasonably: if this is an all day event, force the viewer's
+        // timezone to UTC so the date controls get interpreted as UTC.
+        $viewer->overrideTimezoneIdentifier('UTC');
+      }
+
       $xactions = array();
       $name = $request->getStr('name');
 
@@ -159,7 +169,6 @@ final class PhabricatorCalendarEventEditController
       $space = $request->getStr('spacePHID');
       $is_recurring = $request->getStr('isRecurring') ? 1 : 0;
       $frequency = $request->getStr('frequency');
-      $is_all_day = $request->getStr('isAllDay');
       $icon = $request->getStr('icon');
 
       $invitees = $request->getArr('invitees');
@@ -192,7 +201,7 @@ final class PhabricatorCalendarEventEditController
           $xactions[] = id(new PhabricatorCalendarEventTransaction())
             ->setTransactionType(
               PhabricatorCalendarEventTransaction::TYPE_RECURRENCE_END_DATE)
-            ->setNewValue($recurrence_end_date_value);
+            ->setNewValue($recurrence_end_date_value->getEpoch());
         }
       }
 
@@ -210,12 +219,12 @@ final class PhabricatorCalendarEventEditController
         $xactions[] = id(new PhabricatorCalendarEventTransaction())
           ->setTransactionType(
             PhabricatorCalendarEventTransaction::TYPE_START_DATE)
-          ->setNewValue($start_value);
+          ->setNewValue($start_value->getEpoch());
 
         $xactions[] = id(new PhabricatorCalendarEventTransaction())
           ->setTransactionType(
             PhabricatorCalendarEventTransaction::TYPE_END_DATE)
-          ->setNewValue($end_value);
+          ->setNewValue($end_value->getEpoch());
       }
 
 

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

@@ -198,29 +198,29 @@ final class PhabricatorCalendarEventViewController
       ->setUser($viewer);
 
     if ($event->getIsAllDay()) {
-      $date_start = phabricator_date($event->getDateFrom(), $viewer);
+      $date_start = phabricator_date($event->getViewerDateFrom(), $viewer);
-      $date_end = phabricator_date($event->getDateTo(), $viewer);
+      $date_end = phabricator_date($event->getViewerDateTo(), $viewer);
 
       if ($date_start == $date_end) {
         $properties->addProperty(
           pht('Time'),
-          phabricator_date($event->getDateFrom(), $viewer));
+          phabricator_date($event->getViewerDateFrom(), $viewer));
       } else {
         $properties->addProperty(
           pht('Starts'),
-          phabricator_date($event->getDateFrom(), $viewer));
+          phabricator_date($event->getViewerDateFrom(), $viewer));
         $properties->addProperty(
           pht('Ends'),
-          phabricator_date($event->getDateTo(), $viewer));
+          phabricator_date($event->getViewerDateTo(), $viewer));
       }
     } else {
       $properties->addProperty(
         pht('Starts'),
-        phabricator_datetime($event->getDateFrom(), $viewer));
+        phabricator_datetime($event->getViewerDateFrom(), $viewer));
 
       $properties->addProperty(
         pht('Ends'),
-        phabricator_datetime($event->getDateTo(), $viewer));
+        phabricator_datetime($event->getViewerDateTo(), $viewer));
     }
 
     if ($event->getIsRecurring()) {

src/applications/calendar/editor/PhabricatorCalendarEventEditor.php

@@ -22,8 +22,6 @@ final class PhabricatorCalendarEventEditor
     array $xactions) {
 
     $actor = $this->requireActor();
-    $object->removeViewerTimezone($actor);
-
     if ($object->getIsStub()) {
       $this->materializeStub($object);
     }
@@ -151,7 +149,7 @@ final class PhabricatorCalendarEventEditor
       case PhabricatorCalendarEventTransaction::TYPE_RECURRENCE_END_DATE:
       case PhabricatorCalendarEventTransaction::TYPE_START_DATE:
       case PhabricatorCalendarEventTransaction::TYPE_END_DATE:
-        return $xaction->getNewValue()->getEpoch();
+        return $xaction->getNewValue();
     }
 
     return parent::getCustomTransactionNewValue($object, $xaction);
@@ -308,6 +306,8 @@ final class PhabricatorCalendarEventEditor
     }
 
     if ($phids) {
+      $object->applyViewerTimezone($this->getActor());
+
       $user = new PhabricatorUser();
       $conn_w = $user->establishConnection('w');
       queryfx(
@@ -344,15 +344,16 @@ final class PhabricatorCalendarEventEditor
 
     foreach ($xactions as $xaction) {
       if ($xaction->getTransactionType() == $start_date_xaction) {
-        $start_date = $xaction->getNewValue()->getEpoch();
+        $start_date = $xaction->getNewValue();
       } else if ($xaction->getTransactionType() == $end_date_xaction) {
-        $end_date = $xaction->getNewValue()->getEpoch();
+        $end_date = $xaction->getNewValue();
       } else if ($xaction->getTransactionType() == $recurrence_end_xaction) {
         $recurrence_end = $xaction->getNewValue();
       } else if ($xaction->getTransactionType() == $is_recurrence_xaction) {
         $is_recurring = $xaction->getNewValue();
       }
     }
+
     if ($start_date > $end_date) {
       $type = PhabricatorCalendarEventTransaction::TYPE_END_DATE;
       $errors[] = new PhabricatorApplicationTransactionValidationError(
@@ -399,20 +400,6 @@ final class PhabricatorCalendarEventEditor
           $errors[] = $error;
         }
         break;
-      case PhabricatorCalendarEventTransaction::TYPE_RECURRENCE_END_DATE:
-      case PhabricatorCalendarEventTransaction::TYPE_START_DATE:
-      case PhabricatorCalendarEventTransaction::TYPE_END_DATE:
-        foreach ($xactions as $xaction) {
-          $date_value = $xaction->getNewValue();
-          if (!$date_value->isValid()) {
-            $errors[] = new PhabricatorApplicationTransactionValidationError(
-              $type,
-              pht('Invalid'),
-              pht('Invalid date.'),
-              $xaction);
-          }
-        }
-        break;
     }
 
     return $errors;

src/applications/calendar/query/PhabricatorCalendarEventQuery.php

@@ -90,7 +90,7 @@ final class PhabricatorCalendarEventQuery
   protected function getPagingValueMap($cursor, array $keys) {
     $event = $this->loadCursorObject($cursor);
     return array(
-      'start' => $event->getDateFrom(),
+      'start' => $event->getViewerDateFrom(),
       'id' => $event->getID(),
     );
   }
@@ -121,7 +121,7 @@ final class PhabricatorCalendarEventQuery
     foreach ($events as $key => $event) {
       $sequence_start = 0;
       $sequence_end = null;
-      $duration = $event->getDateTo() - $event->getDateFrom();
+      $duration = $event->getDuration();
       $end = null;
 
       $instance_of = $event->getInstanceOfEventPHID();
@@ -137,9 +137,10 @@ final class PhabricatorCalendarEventQuery
         $frequency = $event->getFrequencyUnit();
         $modify_key = '+1 '.$frequency;
 
-        if ($this->rangeBegin && $this->rangeBegin > $event->getDateFrom()) {
+        if (($this->rangeBegin !== null) &&
+            ($this->rangeBegin > $event->getViewerDateFrom())) {
           $max_date = $this->rangeBegin - $duration;
-          $date = $event->getDateFrom();
+          $date = $event->getViewerDateFrom();
           $datetime = PhabricatorTime::getDateTimeFromEpoch($date, $viewer);
 
           while ($date < $max_date) {
@@ -151,7 +152,7 @@ final class PhabricatorCalendarEventQuery
 
           $start = $this->rangeBegin;
         } else {
-          $start = $event->getDateFrom() - $duration;
+          $start = $event->getViewerDateFrom() - $duration;
         }
 
         $date = $start;
@@ -199,9 +200,9 @@ final class PhabricatorCalendarEventQuery
 
         if ($raw_limit) {
           if (count($events) >= $raw_limit) {
-            $events = msort($events, 'getDateFrom');
+            $events = msort($events, 'getViewerDateFrom');
             $events = array_slice($events, 0, $raw_limit, true);
-            $enforced_end = last($events)->getDateFrom();
+            $enforced_end = last($events)->getViewerDateFrom();
           }
         }
       }
@@ -303,18 +304,22 @@ final class PhabricatorCalendarEventQuery
         $this->phids);
     }
 
+    // NOTE: The date ranges we query for are larger than the requested ranges
+    // because we need to catch all-day events. We'll refine this range later
+    // after adjusting the visible range of events we load.
+
     if ($this->rangeBegin) {
       $where[] = qsprintf(
         $conn,
         'event.dateTo >= %d OR event.isRecurring = 1',
-        $this->rangeBegin);
+        $this->rangeBegin - phutil_units('16 hours in seconds'));
     }
 
     if ($this->rangeEnd) {
       $where[] = qsprintf(
         $conn,
         'event.dateFrom <= %d',
-        $this->rangeEnd);
+        $this->rangeEnd + phutil_units('16 hours in seconds'));
     }
 
     if ($this->inviteePHIDs !== null) {
@@ -399,8 +404,8 @@ final class PhabricatorCalendarEventQuery
     $viewer = $this->getViewer();
 
     foreach ($events as $key => $event) {
-      $event_start = $event->getDateFrom();
+      $event_start = $event->getViewerDateFrom();
-      $event_end = $event->getDateTo();
+      $event_end = $event->getViewerDateTo();
 
       if ($range_start && $event_end < $range_start) {
         unset($events[$key]);
@@ -466,7 +471,7 @@ final class PhabricatorCalendarEventQuery
       }
     }
 
-    $events = msort($events, 'getDateFrom');
+    $events = msort($events, 'getViewerDateFrom');
 
     return $events;
   }

src/applications/calendar/query/PhabricatorCalendarEventSearchEngine.php

@@ -311,11 +311,10 @@ final class PhabricatorCalendarEventSearchEngine
         $item->addAttribute($attending);
       }
 
-      if (strlen($event->getDuration()) > 0) {
+      if ($event->getDuration()) {
         $duration = pht(
           'Duration: %s',
-          $event->getDuration());
+          $event->getDisplayDuration());
-
         $item->addIcon('none', $duration);
       }
 
@@ -370,7 +369,9 @@ final class PhabricatorCalendarEventSearchEngine
       $viewer_is_invited = $status->getIsUserInvited($viewer->getPHID());
 
       $event = new AphrontCalendarEventView();
-      $event->setEpochRange($status->getDateFrom(), $status->getDateTo());
+      $event->setEpochRange(
+        $status->getViewerDateFrom(),
+        $status->getViewerDateTo());
       $event->setIsAllDay($status->getIsAllDay());
       $event->setIcon($status->getIcon());
 
@@ -434,7 +435,9 @@ final class PhabricatorCalendarEventSearchEngine
       $event = new AphrontCalendarEventView();
       $event->setCanEdit($can_edit);
       $event->setEventID($status->getID());
-      $event->setEpochRange($status->getDateFrom(), $status->getDateTo());
+      $event->setEpochRange(
+        $status->getViewerDateFrom(),
+        $status->getViewerDateTo());
       $event->setIsAllDay($status->getIsAllDay());
       $event->setIcon($status->getIcon());
       $event->setViewerIsInvited($viewer_is_invited);
@@ -553,10 +556,10 @@ final class PhabricatorCalendarEventSearchEngine
     $viewer = $this->requireViewer();
 
     $from_datetime = PhabricatorTime::getDateTimeFromEpoch(
-      $event->getDateFrom(),
+      $event->getViewerDateFrom(),
       $viewer);
     $to_datetime = PhabricatorTime::getDateTimeFromEpoch(
-      $event->getDateTo(),
+      $event->getViewerDateTo(),
       $viewer);
 
     $from_date_formatted = $from_datetime->format('Y m d');
@@ -566,23 +569,23 @@ final class PhabricatorCalendarEventSearchEngine
       if ($from_date_formatted == $to_date_formatted) {
         return pht(
           '%s, All Day',
-          phabricator_date($event->getDateFrom(), $viewer));
+          phabricator_date($event->getViewerDateFrom(), $viewer));
       } else {
         return pht(
           '%s - %s, All Day',
-          phabricator_date($event->getDateFrom(), $viewer),
+          phabricator_date($event->getViewerDateFrom(), $viewer),
-          phabricator_date($event->getDateTo(), $viewer));
+          phabricator_date($event->getViewerDateTo(), $viewer));
       }
     } else if ($from_date_formatted == $to_date_formatted) {
       return pht(
         '%s - %s',
-        phabricator_datetime($event->getDateFrom(), $viewer),
+        phabricator_datetime($event->getViewerDateFrom(), $viewer),
-        phabricator_time($event->getDateTo(), $viewer));
+        phabricator_time($event->getViewerDateTo(), $viewer));
     } else {
       return pht(
         '%s - %s',
-        phabricator_datetime($event->getDateFrom(), $viewer),
+        phabricator_datetime($event->getViewerDateFrom(), $viewer),
-        phabricator_datetime($event->getDateTo(), $viewer));
+        phabricator_datetime($event->getViewerDateTo(), $viewer));
     }
   }
 }

src/applications/calendar/storage/PhabricatorCalendarEvent.php

@@ -41,7 +41,9 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
 
   private $parentEvent = self::ATTACHABLE;
   private $invitees = self::ATTACHABLE;
-  private $appliedViewer;
+
+  private $viewerDateFrom;
+  private $viewerDateTo;
 
   // Frequency Constants
   const FREQUENCY_DAILY = 'daily';
@@ -157,7 +159,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     $date_time->modify($modify_key);
     $date = $date_time->format('U');
 
-    $duration = $parent->getDateTo() - $parent->getDateFrom();
+    $duration = $this->getDuration();
 
     $this
       ->setDateFrom($date)
@@ -192,76 +194,51 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     return $ghost;
   }
 
+  public function getViewerDateFrom() {
+    if ($this->viewerDateFrom === null) {
+      throw new PhutilInvalidStateException('applyViewerTimezone');
+    }
+
+    return $this->viewerDateFrom;
+  }
+
+  public function getViewerDateTo() {
+    if ($this->viewerDateTo === null) {
+      throw new PhutilInvalidStateException('applyViewerTimezone');
+    }
+
+    return $this->viewerDateTo;
+  }
+
   public function applyViewerTimezone(PhabricatorUser $viewer) {
-    if ($this->appliedViewer) {
-      throw new Exception(pht('Viewer timezone is already applied!'));
-    }
-
-    $this->appliedViewer = $viewer;
-
     if (!$this->getIsAllDay()) {
-      return $this;
+      $this->viewerDateFrom = $this->getDateFrom();
-    }
+      $this->viewerDateTo = $this->getDateTo();
-
+    } else {
       $zone = $viewer->getTimeZone();
 
-
+      $this->viewerDateFrom = $this->getDateEpochForTimeZone(
-    $this->setDateFrom(
-      $this->getDateEpochForTimeZone(
         $this->getDateFrom(),
-        new DateTimeZone('Pacific/Kiritimati'),
+        new DateTimeZone('UTC'),
         'Y-m-d',
         null,
-        $zone));
+        $zone);
 
-    $this->setDateTo(
+      $this->viewerDateTo = $this->getDateEpochForTimeZone(
-      $this->getDateEpochForTimeZone(
         $this->getDateTo(),
-        new DateTimeZone('Pacific/Midway'),
+        new DateTimeZone('UTC'),
         'Y-m-d 23:59:00',
-        '-1 day',
-        $zone));
-
-    return $this;
-  }
-
-
-  public function removeViewerTimezone(PhabricatorUser $viewer) {
-    if (!$this->appliedViewer) {
-      throw new Exception(pht('Viewer timezone is not applied!'));
-    }
-
-    if ($viewer->getPHID() != $this->appliedViewer->getPHID()) {
-      throw new Exception(pht('Removed viewer must match applied viewer!'));
-    }
-
-    $this->appliedViewer = null;
-
-    if (!$this->getIsAllDay()) {
-      return $this;
-    }
-
-    $zone = $viewer->getTimeZone();
-
-    $this->setDateFrom(
-      $this->getDateEpochForTimeZone(
-        $this->getDateFrom(),
-        $zone,
-        'Y-m-d',
         null,
-        new DateTimeZone('Pacific/Kiritimati')));
+        $zone);
-
+    }
-    $this->setDateTo(
-      $this->getDateEpochForTimeZone(
-        $this->getDateTo(),
-        $zone,
-        'Y-m-d',
-        '+1 day',
-        new DateTimeZone('Pacific/Midway')));
 
     return $this;
   }
 
+  public function getDuration() {
+    return $this->getDateTo() - $this->getDateFrom();
+  }
+
   private function getDateEpochForTimeZone(
     $epoch,
     $src_zone,
@@ -281,12 +258,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
   }
 
   public function save() {
-    if ($this->appliedViewer) {
-      throw new Exception(
-        pht(
-          'Can not save event with viewer timezone still applied!'));
-    }
-
     if (!$this->mailKey) {
       $this->mailKey = Filesystem::readRandomCharacters(20);
     }
@@ -298,13 +269,13 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
    * Get the event start epoch for evaluating invitee availability.
    *
    * When assessing availability, we pretend events start earlier than they
-   * really. This allows us to mark users away for the entire duration of a
+   * really do. This allows us to mark users away for the entire duration of a
    * series of back-to-back meetings, even if they don't strictly overlap.
    *
    * @return int Event start date for availability caches.
    */
   public function getDateFromForCache() {
-    return ($this->getDateFrom() - phutil_units('15 minutes in seconds'));
+    return ($this->getViewerDateFrom() - phutil_units('15 minutes in seconds'));
   }
 
   protected function getConfiguration() {
@@ -456,8 +427,8 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     return false;
   }
 
-  public function getDuration() {
+  public function getDisplayDuration() {
-    $seconds = $this->dateTo - $this->dateFrom;
+    $seconds = $this->getDuration();
     $minutes = round($seconds / 60, 1);
     $hours = round($minutes / 60, 3);
     $days = round($hours / 24, 2);

src/applications/people/controller/PhabricatorPeopleProfileViewController.php

@@ -189,41 +189,44 @@ final class PhabricatorPeopleProfileViewController
     $range_start = $midnight->format('U');
     $range_end = $week_end->format('U');
 
-    $query = id(new PhabricatorCalendarEventQuery())
+    $events = id(new PhabricatorCalendarEventQuery())
       ->setViewer($viewer)
       ->withDateRange($range_start, $range_end)
       ->withInvitedPHIDs(array($user->getPHID()))
-      ->withIsCancelled(false);
+      ->withIsCancelled(false)
+      ->execute();
 
-    $statuses = $query->execute();
+    $event_views = array();
-    $phids = mpull($statuses, 'getUserPHID');
+    foreach ($events as $event) {
-    $events = array();
+      $viewer_is_invited = $event->getIsUserInvited($viewer->getPHID());
-
-    foreach ($statuses as $status) {
-      $viewer_is_invited = $status->getIsUserInvited($user->getPHID());
 
       $can_edit = PhabricatorPolicyFilter::hasCapability(
         $viewer,
-        $status,
+        $event,
         PhabricatorPolicyCapability::CAN_EDIT);
 
-      $event = id(new AphrontCalendarEventView())
+      $epoch_min = $event->getViewerDateFrom();
+      $epoch_max = $event->getViewerDateTo();
+
+      $event_view = id(new AphrontCalendarEventView())
         ->setCanEdit($can_edit)
-        ->setEventID($status->getID())
+        ->setEventID($event->getID())
-        ->setEpochRange($status->getDateFrom(), $status->getDateTo())
+        ->setEpochRange($epoch_min, $epoch_max)
-        ->setIsAllDay($status->getIsAllDay())
+        ->setIsAllDay($event->getIsAllDay())
-        ->setIcon($status->getIcon())
+        ->setIcon($event->getIcon())
         ->setViewerIsInvited($viewer_is_invited)
-        ->setName($status->getName())
+        ->setName($event->getName())
-        ->setURI($status->getURI());
+        ->setURI($event->getURI());
-      $events[] = $event;
+
+      $event_views[] = $event_view;
     }
 
-    $events = msort($events, 'getEpochStart');
+    $event_views = msort($event_views, 'getEpochStart');
+
     $day_view = id(new PHUICalendarWeekView())
       ->setViewer($viewer)
       ->setView('week')
-      ->setEvents($events)
+      ->setEvents($event_views)
       ->setWeekLength(3)
       ->render();
 

src/applications/people/query/PhabricatorPeopleQuery.php

@@ -413,6 +413,13 @@ final class PhabricatorPeopleQuery
     foreach ($rebuild as $phid => $user) {
       $events = idx($map, $phid, array());
 
+      // We loaded events with the omnipotent user, but want to shift them
+      // into the user's timezone before building the cache because they will
+      // be unavailable during their own local day.
+      foreach ($events as $event) {
+        $event->applyViewerTimezone($user);
+      }
+
       $cursor = $min_range;
       if ($events) {
         // Find the next time when the user has no meetings. If we move forward
@@ -420,7 +427,7 @@ final class PhabricatorPeopleQuery
         while (true) {
           foreach ($events as $event) {
             $from = $event->getDateFromForCache();
-            $to = $event->getDateTo();
+            $to = $event->getViewerDateTo();
             if (($from <= $cursor) && ($to > $cursor)) {
               $cursor = $to;
               continue 2;