Fix datepicker to show correct error messages on Calendar event edit view

Summary: Ref T8024, Fix datepicker to show correct error messages on Calendar event edit view.

Test Plan: Edit Calendar event, set blank title, and valid new end time, error page should correctly point out invalid title, but reflec the updated time.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T8024

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

src/__phutil_library_map__.php

@@ -114,6 +114,7 @@ phutil_register_library_map(array(
     'AphrontFormChooseButtonControl' => 'view/form/control/AphrontFormChooseButtonControl.php',
     'AphrontFormControl' => 'view/form/control/AphrontFormControl.php',
     'AphrontFormDateControl' => 'view/form/control/AphrontFormDateControl.php',
+    'AphrontFormDateControlValue' => 'view/form/control/AphrontFormDateControlValue.php',
     'AphrontFormDividerControl' => 'view/form/control/AphrontFormDividerControl.php',
     'AphrontFormFileControl' => 'view/form/control/AphrontFormFileControl.php',
     'AphrontFormMarkupControl' => 'view/form/control/AphrontFormMarkupControl.php',
@@ -3360,6 +3361,7 @@ phutil_register_library_map(array(
     'AphrontFormChooseButtonControl' => 'AphrontFormControl',
     'AphrontFormControl' => 'AphrontView',
     'AphrontFormDateControl' => 'AphrontFormControl',
+    'AphrontFormDateControlValue' => 'Phobject',
     'AphrontFormDividerControl' => 'AphrontFormControl',
     'AphrontFormFileControl' => 'AphrontFormControl',
     'AphrontFormMarkupControl' => 'AphrontFormControl',

src/applications/calendar/controller/PhabricatorCalendarEventEditController.php

@@ -18,24 +18,14 @@ final class PhabricatorCalendarEventEditController
     $user = $request->getUser();
     $user_phid = $user->getPHID();
     $error_name = true;
+    $error_start_date = true;
+    $error_end_date = true;
     $validation_exception = null;
 
-    $start_time = id(new AphrontFormDateControl())
-      ->setUser($user)
-      ->setName('start')
-      ->setLabel(pht('Start'))
-      ->setInitialTime(AphrontFormDateControl::TIME_START_OF_DAY);
-
-    $end_time = id(new AphrontFormDateControl())
-      ->setUser($user)
-      ->setName('end')
-      ->setLabel(pht('End'))
-      ->setInitialTime(AphrontFormDateControl::TIME_END_OF_DAY);
-
     if ($this->isCreate()) {
       $event = PhabricatorCalendarEvent::initializeNewCalendarEvent($user);
-      $end_value = $end_time->readValueFromRequest($request);
+      $end_value = AphrontFormDateControlValue::newFromEpoch($user, time());
-      $start_value = $start_time->readValueFromRequest($request);
+      $start_value = AphrontFormDateControlValue::newFromEpoch($user, time());
       $submit_label = pht('Create');
       $page_title = pht('Create Event');
       $redirect = 'created';
@@ -56,8 +46,13 @@ final class PhabricatorCalendarEventEditController
         return new Aphront404Response();
       }
 
-      $end_time->setValue($event->getDateTo());
+      $end_value = AphrontFormDateControlValue::newFromEpoch(
-      $start_time->setValue($event->getDateFrom());
+        $user,
+        $event->getDateTo());
+      $start_value = AphrontFormDateControlValue::newFromEpoch(
+        $user,
+        $event->getDateFrom());
+
       $submit_label = pht('Update');
       $page_title   = pht('Update Event');
 
@@ -76,7 +71,6 @@ final class PhabricatorCalendarEventEditController
       $cancel_uri = '/'.$event->getMonogram();
     }
 
-    $errors = array();
     $name = $event->getName();
     $description = $event->getDescription();
     $type = $event->getStatus();
@@ -90,8 +84,13 @@ final class PhabricatorCalendarEventEditController
       $xactions = array();
       $name = $request->getStr('name');
       $type = $request->getInt('status');
-      $start_value = $start_time->readValueFromRequest($request);
+
-      $end_value = $end_time->readValueFromRequest($request);
+      $start_value = AphrontFormDateControlValue::newFromRequest(
+        $request,
+        'start');
+      $end_value = AphrontFormDateControlValue::newFromRequest(
+        $request,
+        'end');
       $description = $request->getStr('description');
       $subscribers = $request->getArr('subscribers');
       $edit_policy = $request->getStr('editPolicy');
@@ -107,86 +106,72 @@ final class PhabricatorCalendarEventEditController
         }
       }
 
-      if ($start_time->getError()) {
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-        $errors[] = pht('Invalid start time; reset to default.');
+        ->setTransactionType(
-      }
+          PhabricatorCalendarEventTransaction::TYPE_NAME)
-      if ($end_time->getError()) {
+        ->setNewValue($name);
-        $errors[] = pht('Invalid end time; reset to default.');
-      }
-      if (!$errors) {
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
-            PhabricatorCalendarEventTransaction::TYPE_NAME)
-          ->setNewValue($name);
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
+        ->setTransactionType(
-            PhabricatorCalendarEventTransaction::TYPE_START_DATE)
+          PhabricatorCalendarEventTransaction::TYPE_START_DATE)
-          ->setNewValue($start_value);
+        ->setNewValue($start_value);
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
+        ->setTransactionType(
-            PhabricatorCalendarEventTransaction::TYPE_END_DATE)
+          PhabricatorCalendarEventTransaction::TYPE_END_DATE)
-          ->setNewValue($end_value);
+        ->setNewValue($end_value);
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
+        ->setTransactionType(
-            PhabricatorCalendarEventTransaction::TYPE_STATUS)
+          PhabricatorCalendarEventTransaction::TYPE_STATUS)
-          ->setNewValue($type);
+        ->setNewValue($type);
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
+        ->setTransactionType(
-            PhabricatorTransactions::TYPE_SUBSCRIBERS)
+          PhabricatorTransactions::TYPE_SUBSCRIBERS)
-          ->setNewValue(array('=' => array_fuse($subscribers)));
+        ->setNewValue(array('=' => array_fuse($subscribers)));
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
+        ->setTransactionType(
-            PhabricatorCalendarEventTransaction::TYPE_INVITE)
+          PhabricatorCalendarEventTransaction::TYPE_INVITE)
-          ->setNewValue($new_invitees);
+        ->setNewValue($new_invitees);
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(
+        ->setTransactionType(
-            PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION)
+          PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION)
-          ->setNewValue($description);
+        ->setNewValue($description);
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
+        ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
-          ->setNewValue($request->getStr('viewPolicy'));
+        ->setNewValue($request->getStr('viewPolicy'));
 
-        $xactions[] = id(new PhabricatorCalendarEventTransaction())
+      $xactions[] = id(new PhabricatorCalendarEventTransaction())
-          ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY)
+        ->setTransactionType(PhabricatorTransactions::TYPE_EDIT_POLICY)
-          ->setNewValue($request->getStr('editPolicy'));
+        ->setNewValue($request->getStr('editPolicy'));
 
-        $editor = id(new PhabricatorCalendarEventEditor())
+      $editor = id(new PhabricatorCalendarEventEditor())
-          ->setActor($user)
+        ->setActor($user)
-          ->setContentSourceFromRequest($request)
+        ->setContentSourceFromRequest($request)
-          ->setContinueOnNoEffect(true);
+        ->setContinueOnNoEffect(true);
 
-        try {
+      try {
-          $xactions = $editor->applyTransactions($event, $xactions);
+        $xactions = $editor->applyTransactions($event, $xactions);
-          $response = id(new AphrontRedirectResponse());
+        $response = id(new AphrontRedirectResponse());
-          return $response->setURI('/E'.$event->getID());
+        return $response->setURI('/E'.$event->getID());
-        } catch (PhabricatorApplicationTransactionValidationException $ex) {
+      } catch (PhabricatorApplicationTransactionValidationException $ex) {
-          $validation_exception = $ex;
+        $validation_exception = $ex;
-          $error_name = $ex
+        $error_name = $ex->getShortMessage(
-            ->getShortMessage(PhabricatorCalendarEventTransaction::TYPE_NAME);
+            PhabricatorCalendarEventTransaction::TYPE_NAME);
+        $error_start_date = $ex->getShortMessage(
+            PhabricatorCalendarEventTransaction::TYPE_START_DATE);
+        $error_end_date = $ex->getShortMessage(
+            PhabricatorCalendarEventTransaction::TYPE_END_DATE);
 
-          $event->setViewPolicy($view_policy);
-          $event->setEditPolicy($edit_policy);
-        }
-      } else {
         $event->setViewPolicy($view_policy);
         $event->setEditPolicy($edit_policy);
       }
     }
 
-    $error_view = null;
-    if ($errors) {
-      $error_view = id(new PHUIInfoView())
-        ->setTitle(pht('Status can not be set!'))
-        ->setErrors($errors);
-    }
-
     $name = id(new AphrontFormTextControl())
       ->setLabel(pht('Name'))
       ->setName('name')
@@ -199,6 +184,20 @@ final class PhabricatorCalendarEventEditController
       ->setValue($type)
       ->setOptions($event->getStatusOptions());
 
+    $start_control = id(new AphrontFormDateControl())
+      ->setUser($user)
+      ->setName('start')
+      ->setLabel(pht('Start'))
+      ->setError($error_start_date)
+      ->setValue($start_value);
+
+    $end_control = id(new AphrontFormDateControl())
+      ->setUser($user)
+      ->setName('end')
+      ->setLabel(pht('End'))
+      ->setError($error_end_date)
+      ->setValue($end_value);
+
     $description = id(new AphrontFormTextAreaControl())
       ->setLabel(pht('Description'))
       ->setName('description')
@@ -235,8 +234,8 @@ final class PhabricatorCalendarEventEditController
       ->setUser($user)
       ->appendChild($name)
       ->appendChild($status_select)
-      ->appendChild($start_time)
+      ->appendChild($start_control)
-      ->appendChild($end_time)
+      ->appendChild($end_control)
       ->appendControl($view_policies)
       ->appendControl($edit_policies)
       ->appendControl($subscribers)
@@ -261,7 +260,6 @@ final class PhabricatorCalendarEventEditController
 
     $form_box = id(new PHUIObjectBoxView())
       ->setHeaderText($page_title)
-      ->setFormErrors($errors)
       ->setForm($form);
 
     $crumbs = $this->buildApplicationCrumbs();

src/applications/calendar/editor/PhabricatorCalendarEventEditor.php

@@ -83,14 +83,15 @@ final class PhabricatorCalendarEventEditor
     PhabricatorApplicationTransaction $xaction) {
     switch ($xaction->getTransactionType()) {
       case PhabricatorCalendarEventTransaction::TYPE_NAME:
-      case PhabricatorCalendarEventTransaction::TYPE_START_DATE:
-      case PhabricatorCalendarEventTransaction::TYPE_END_DATE:
       case PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION:
       case PhabricatorCalendarEventTransaction::TYPE_CANCEL:
       case PhabricatorCalendarEventTransaction::TYPE_INVITE:
         return $xaction->getNewValue();
       case PhabricatorCalendarEventTransaction::TYPE_STATUS:
         return (int)$xaction->getNewValue();
+      case PhabricatorCalendarEventTransaction::TYPE_START_DATE:
+      case PhabricatorCalendarEventTransaction::TYPE_END_DATE:
+        return $xaction->getNewValue()->getEpoch();
     }
 
     return parent::getCustomTransactionNewValue($object, $xaction);
@@ -204,6 +205,19 @@ final class PhabricatorCalendarEventEditor
           $errors[] = $error;
         }
         break;
+      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/view/form/control/AphrontFormDateControl.php

@@ -10,6 +10,7 @@ final class AphrontFormDateControl extends AphrontFormControl {
   private $valueYear;
   private $valueTime;
   private $allowNull;
+  private $continueOnInvalidDate = false;
 
   public function setAllowNull($allow_null) {
     $this->allowNull = $allow_null;
@@ -84,6 +85,16 @@ final class AphrontFormDateControl extends AphrontFormControl {
   }
 
   public function setValue($epoch) {
+    if ($epoch instanceof AphrontFormDateControlValue) {
+      $this->continueOnInvalidDate = true;
+      $this->valueYear  = $epoch->getValueYear();
+      $this->valueMonth = $epoch->getValueMonth();
+      $this->valueDay   = $epoch->getValueDay();
+      $this->valueTime  = $epoch->getValueTime();
+
+      return parent::setValue($epoch->getEpoch());
+    }
+
     $result = parent::setValue($epoch);
 
     if ($epoch === null) {
@@ -165,7 +176,7 @@ final class AphrontFormDateControl extends AphrontFormControl {
   protected function renderInput() {
 
     $disabled = null;
-    if ($this->getValue() === null) {
+    if ($this->getValue() === null && !$this->continueOnInvalidDate) {
       $this->setValue($this->getInitialValue());
       if ($this->allowNull) {
         $disabled = 'disabled';

src/view/form/control/AphrontFormDateControlValue.php

@@ -0,0 +1,97 @@
+<?php
+
+final class AphrontFormDateControlValue extends Phobject {
+
+  private $valueDay;
+  private $valueMonth;
+  private $valueYear;
+  private $valueTime;
+
+  private $viewer;
+  private $zone;
+
+  public function getValueDay() {
+    return $this->valueDay;
+  }
+
+  public function getValueMonth() {
+    return $this->valueMonth;
+  }
+
+  public function getValueYear() {
+    return $this->valueYear;
+  }
+
+  public function getValueTime() {
+    return $this->valueTime;
+  }
+
+  public function isValid() {
+    return ($this->getEpoch() !== null);
+  }
+
+  public static function newFromRequest($request, $key) {
+    $value = new AphrontFormDateControlValue();
+    $value->viewer = $request->getViewer();
+
+    $value->valueDay = $request->getInt($key.'_d');
+    $value->valueMonth = $request->getInt($key.'_m');
+    $value->valueYear = $request->getInt($key.'_y');
+    $value->valueTime = $request->getStr($key.'_t');
+
+    return $value;
+  }
+
+  public static function newFromEpoch(PhabricatorUser $viewer, $epoch) {
+    $value = new AphrontFormDateControlValue();
+    $value->viewer = $viewer;
+    $readable = $value->formatTime($epoch, 'Y!m!d!g:i A');
+    $readable = explode('!', $readable, 4);
+
+    $value->valueYear  = $readable[0];
+    $value->valueMonth = $readable[1];
+    $value->valueDay   = $readable[2];
+    $value->valueTime  = $readable[3];
+
+    return $value;
+  }
+
+  private function formatTime($epoch, $format) {
+    return phabricator_format_local_time(
+      $epoch,
+      $this->viewer,
+      $format);
+  }
+
+  public function getEpoch() {
+    $year = $this->valueYear;
+    $month = $this->valueMonth;
+    $day = $this->valueDay;
+    $time = $this->valueTime;
+    $zone = $this->getTimezone();
+
+    if (!strlen($time)) {
+      return null;
+    }
+
+    try {
+      $date = new DateTime("{$year}-{$month}-{$day} {$time}", $zone);
+      $value = $date->format('U');
+    } catch (Exception $ex) {
+      $value = null;
+    }
+    return $value;
+  }
+
+  private function getTimezone() {
+    if ($this->zone) {
+      return $this->zone;
+    }
+
+    $viewer_zone = $this->viewer->getTimezoneIdentifier();
+    $this->zone = new DateTimeZone($viewer_zone);
+    return $this->zone;
+  }
+
+
+}