From 8e5437226fe9424c46ae700b628d2fd021d55314 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 31 Oct 2016 11:51:31 -0700
Subject: [PATCH] Make calendar intepret all-day dates in a more consistent way

Summary:
In ICS, an event on "Nov 1" starts on "2016-11-01" and ends on "2016-11-02".

This is convenient for computers, but this isn't what users expect to enter in date controls. They expect to enter "nov 1" to "Nov 1" for a one-day, all-day event. This is consistent with other applications.

Store the value the user entered, but treat it as the first second of the next day when actually using it if the event is an all day event.

Test Plan:
Mucked around with multi-day all-day events, recurring all-day events, imports, etc. Couldn't catch any weird/unintuitive stuff anymore offhand.

(Previously, entering "Nov 1" to "Nov 2" created a one-day event, which was unclear.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16777
---
 .../PhabricatorCalendarEventEditEngine.php    |  2 +-
 .../storage/PhabricatorCalendarEvent.php      | 21 ++++++++++++++++++-
 ...ricatorCalendarEventEndDateTransaction.php |  2 +-
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php
index 441c3c5de5..4457f60fd8 100644
--- a/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php
+++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php
@@ -119,7 +119,7 @@ final class PhabricatorCalendarEventEditEngine
         ->setDescription(pht('End time of the event.'))
         ->setConduitDescription(pht('Change the end time of the event.'))
         ->setConduitTypeDescription(pht('New event end time.'))
-        ->setValue($object->getEndDateTimeEpoch()),
+        ->setValue($object->newEndDateTimeForEdit()->getEpoch()),
       id(new PhabricatorBoolEditField())
         ->setKey('cancelled')
         ->setOptions(pht('Active'), pht('Cancelled'))
diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php
index 1410c0e106..ef7a808a61 100644
--- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php
+++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php
@@ -857,7 +857,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     return $this->newStartDateTime()->getEpoch();
   }
 
-  public function newEndDateTime() {
+  public function newEndDateTimeForEdit() {
     $datetime = $this->getParameter('endDateTime');
     if ($datetime) {
       return $this->newDateTimeFromDictionary($datetime);
@@ -867,6 +867,25 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     return $this->newDateTimeFromEpoch($epoch);
   }
 
+  public function newEndDateTime() {
+    $datetime = $this->newEndDateTimeForEdit();
+
+    // If this is an all day event, we move the end date time forward to the
+    // first second of the following day. This is consistent with what users
+    // expect: an all day event from "Nov 1" to "Nov 1" lasts the entire day.
+    if ($this->getIsAllDay()) {
+      $datetime = $datetime
+        ->newAbsoluteDateTime()
+        ->setHour(0)
+        ->setMinute(0)
+        ->setSecond(0)
+        ->newRelativeDateTime('P1D')
+        ->newAbsoluteDateTime();
+    }
+
+    return $datetime;
+  }
+
   public function getEndDateTimeEpoch() {
     return $this->newEndDateTime()->getEpoch();
   }
diff --git a/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php b/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php
index a0d127c32f..8d66176ea8 100644
--- a/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php
+++ b/src/applications/calendar/xaction/PhabricatorCalendarEventEndDateTransaction.php
@@ -7,7 +7,7 @@ final class PhabricatorCalendarEventEndDateTransaction
 
   public function generateOldValue($object) {
     // TODO: Upgrade this.
-    return $object->getEndDateTimeEpoch();
+    return $object->newEndDateTimeForEdit()->getEpoch();
   }
 
   public function applyInternalEffects($object, $value) {