Framework/DateTimePicker: Set element's value to Unix timestamp after the input is considered valid

refs #4440

library/Icinga/Web/Form/Element/DateTimePicker.php

@@ -28,14 +28,16 @@
 
 namespace Icinga\Web\Form\Element;
 
-use \DateTime;
-use \DateTimeZone;
 use \Exception;
-use Zend_Form_Element_Xhtml;
+use \Zend_Form_Element_Xhtml;
-use Icinga\Application\Icinga;
+use \Icinga\Application\Icinga;
+use \Icinga\Util\DateTimeFactory;
 
 /**
- * Datetime form element
+ * Datetime form element which returns the input as Unix timestamp after the input has been proven valid. Utilizes
+ * DateTimeFactory to ensure time zone awareness
+ *
+ * @see isValid()
  */
 class DateTimePicker extends Zend_Form_Element_Xhtml
 {
@@ -45,41 +47,6 @@ class DateTimePicker extends Zend_Form_Element_Xhtml
      */
     public $helper = 'formDateTime';
 
-    /**
-     * Time zone
-     * @var mixed
-     */
-    private $timeZone;
-
-    /**
-     * Getter for the time zone
-     *
-     * If the time zone has never been set, the user's time zone is returned
-     *
-     * @return  DateTimeZone
-     * @see     setTimeZone()
-     */
-    public function getTimeZone()
-    {
-        $timeZone = $this->timeZone;
-        if ($timeZone === null) {
-            $timeZone = Icinga::app()->getFrontController()->getRequest()->getUser()->getTimeZone();
-        }
-        return $timeZone;
-    }
-
-    /**
-     * Setter for the time zone
-     *
-     * @param   DateTimeZone    $timeZone
-     * @return  self
-     */
-    public function setTimeZone(DateTimeZone $timeZone)
-    {
-        $this->timeZone = $timeZone;
-        return $this;
-    }
-
     /**
      * Finds whether a variable is a Unix timestamp
      *
@@ -93,36 +60,20 @@ class DateTimePicker extends Zend_Form_Element_Xhtml
             && ($timestamp >= ~PHP_INT_MAX);
     }
 
-    /**
-     * Retrieve element value as unix timestamp respecting the user's timezone
-     *
-     * @param   mixed   $timeZone
-     * @return  int
-     */
-    public function getValue()
-    {
-        $valueFiltered = parent::getValue();
-        if ($this->isUnixTimestamp($valueFiltered)) {
-            // Using the Unix timestamp format to construct a new DateTime
-            $valueFiltered = '@' . $valueFiltered;
-        }
-        $dt = new DateTime($valueFiltered, $this->getTimeZone());
-        return $dt->getTimestamp();
-    }
-
     /**
      * Validate filtered date/time strings
      *
-     * Expects formats that the php date parser understands
+     * Expects formats that the php date parser understands. Sets element value as Unix timestamp if the input is
+     * considered valid. Utilizes DateTimeFactory to ensure time zone awareness
      *
      * @param   string  $value
      * @param   mixed   $context
      * @return  bool
-     * @see     DateTime::__construct()
+     * @see     \Icinga\Util\DateTimeFactory::create()
      */
     public function isValid($value, $context = null)
     {
-        if (!parent::isValid(parent::getValue(), $context)) {
+        if (!parent::isValid($value, $context)) {
             return false;
         }
 
@@ -139,7 +90,7 @@ class DateTimePicker extends Zend_Form_Element_Xhtml
         }
 
         try {
-            new DateTime($value);
+            $dt = DateTimeFactory::create($value);
         } catch (Exception $e) {
             $this->addErrorMessage(
                 _(
@@ -150,6 +101,8 @@ class DateTimePicker extends Zend_Form_Element_Xhtml
             return false;
         }
 
+        $this->setValue($dt->getTimestamp());
+
         return true;
     }
 }

test/php/library/Icinga/Web/Form/Element/DateTimePickerTest.php

@@ -3,13 +3,17 @@
 namespace Test\Icinga\Web\Form\Element;
 
 require_once 'Zend/Form/Element/Xhtml.php';
-require_once __DIR__ . '/../../../../../../../library/Icinga/Application/Icinga.php';
+require_once realpath(__DIR__ . '/../../../../../../../library/Icinga/Application/Icinga.php');
-require_once __DIR__ . '/../../../../../../../library/Icinga/Web/Form/Element/DateTimePicker.php';
+require_once realpath(__DIR__ . '/../../../../../../../library/Icinga/Web/Form/Element/DateTimePicker.php');
+require_once realpath(__DIR__ . '/../../../../../../../library/Icinga/Util/ConfigAwareFactory.php');
+require_once realpath(__DIR__ . '/../../../../../../../library/Icinga/Util/DateTimeFactory.php');
 
 use \DateTimeZone;
-use Icinga\Web\Form\Element\DateTimePicker;
+use \PHPUnit_Framework_TestCase;
+use \Icinga\Web\Form\Element\DateTimePicker;
+use \Icinga\Util\DateTimeFactory;
 
-class DateTimeTest extends \PHPUnit_Framework_TestCase
+class DateTimeTest extends PHPUnit_Framework_TestCase
 {
     public function setUp()
     {
@@ -18,13 +22,19 @@ class DateTimeTest extends \PHPUnit_Framework_TestCase
         date_default_timezone_set('UTC');
     }
 
+    /**
+     * Test that DateTimePicker::isValid() returns false if the input is not valid in terms of being a date/time string
+     * or a timestamp
+     *
+     * Utilizes singleton DateTimeFactory
+     *
+     * @backupStaticAttributes enabled
+     */
     public function testValidateInvalidInput()
     {
+        DateTimeFactory::setConfig(array('timezone' => new DateTimeZone('UTC')));
+
         $dt = new DateTimePicker('foo');
-        // Set element's timezone else it'll try to load the time zone from the user
-        // which requires Icinga::app() to be bootstrapped which is not the case
-        // within tests - so without a ProgrammingError will be thrown
-        $dt->setTimeZone(new DateTimeZone('UTC'));
 
         $this->assertFalse(
             $dt->isValid('bar'),
@@ -36,10 +46,19 @@ class DateTimeTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    /**
+     * Test that DateTimePicker::isValid() returns true if the input is valid in terms of being a date/time string
+     * or a timestamp
+     *
+     * Utilizes singleton DateTimeFactory
+     *
+     * @backupStaticAttributes enabled
+     */
     public function testValidateValidInput()
     {
+        DateTimeFactory::setConfig(array('timezone' => new DateTimeZone('UTC')));
+
         $dt = new DateTimePicker('foo');
-        $dt->setTimeZone(new DateTimeZone('UTC'));
 
         $this->assertTrue(
             $dt->isValid('2013-07-12 08:03:43'),
@@ -55,11 +74,24 @@ class DateTimeTest extends \PHPUnit_Framework_TestCase
         );
     }
 
-    public function testGetValueReturnsUnixTimestamp()
+    /**
+     * Test that DateTimePicker::getValue() returns a timestamp after a call to isValid considers the input as correct
+     *
+     * Utilizes singleton DateTimeFactory
+     *
+     * @backupStaticAttributes enabled
+     */
+    public function testGetValueReturnsUnixTimestampAfterSuccessfulIsValidCall()
     {
+        DateTimeFactory::setConfig(array('timezone' => new DateTimeZone('UTC')));
+
         $dt = new DateTimePicker('foo');
-        $dt->setTimeZone(new DateTimeZone('UTC'))
+        $dt->setValue('2013-07-12 08:03:43');
-            ->setValue('2013-07-12 08:03:43');
+
+        $this->assertTrue(
+            $dt->isValid($dt->getValue()),
+            'Using a valid date/time string must not fail'
+        );
 
         $this->assertEquals(
             $dt->getValue(),
@@ -69,17 +101,30 @@ class DateTimeTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    /**
+     * Test that DateTimePicker::getValue() returns a timestamp respecting the given non-UTC time zone after a call to
+     * isValid considers the input as correct
+     *
+     * Utilizes singleton DateTimeFactory
+     *
+     * @backupStaticAttributes enabled
+     */
     public function testGetValueIsTimeZoneAware()
     {
+        DateTimeFactory::setConfig(array('timezone' => new DateTimeZone('Europe/Berlin')));
+
         $dt = new DateTimePicker('foo');
-        $dt->setTimeZone(new DateTimeZone('Europe/Berlin'))
+        $dt->setValue('2013-07-12 08:03:43');
-            ->setValue('2013-07-12 08:03:43');
+
+        $this->assertTrue(
+            $dt->isValid($dt->getValue()),
+            'Using a valid date/time string must not fail'
+        );
 
         $this->assertEquals(
             $dt->getValue(),
             1373609023,
-            'getValue did not return the correct Unix timestamp according to the given date/time '
+            'getValue did not return the correct Unix timestamp according to the given date/time string and time zone'
-                . 'string'
         );
     }
 }