merged branch stloyd/datetime_fixes (PR #1485)

Commits
-------

3917ed7 Revert "* DateType, DateTimeType, TimeType: - a bit changed readability"
c85b815 Fixed few issues with Date and Time:

Discussion
----------

[Form] Fixed few issues with Date and Time

Fixed few issues with Date and Time:

* TimeType:
  - seconds are no longer populated if "with_seconds" = false
  - "widget = text" is now properly rendered (closes #1480)
* DateTimeToStringTransformer:
  - fixed using not default "format" (probably fix #1183)
* DateType, DateTimeType, TimeType:
  - fixed "input = datetime" and test covered

src/Symfony/Component/Form/Extension/Core/DataTransformer/DateTimeToStringTransformer.php

@@ -95,7 +95,12 @@ class DateTimeToStringTransformer extends BaseDateTimeTransformer
         }
 
         try {
-            $dateTime = new \DateTime(sprintf("%s %s", $value, $this->outputTimezone));
+            $dateTime = new \DateTime($value, new \DateTimeZone($this->outputTimezone));
+
+            // Force value to be in same format as given to transform
+            if ($value !== $dateTime->format($this->format)) {
+                $dateTime = new \DateTime($dateTime->format($this->format), new \DateTimeZone($this->outputTimezone));
+            }
 
             if ($this->inputTimezone !== $this->outputTimezone) {
                 $dateTime->setTimeZone(new \DateTimeZone($this->inputTimezone));

src/Symfony/Component/Form/Extension/Core/Type/TimeType.php

@@ -28,51 +28,67 @@ class TimeType extends AbstractType
      */
     public function buildForm(FormBuilder $builder, array $options)
     {
+        $parts  = array('hour', 'minute');
+        $format = 'H:i:00';
+        if ($options['with_seconds']) {
+            $format  = 'H:i:s';
+            $parts[] = 'second';
+        }
+
         if ($options['widget'] === 'single_text') {
-            $builder->appendClientTransformer(new DateTimeToStringTransformer($options['data_timezone'], $options['user_timezone'], 'H:i:s'));
-        } else if ($options['widget'] === 'choice') {
-            if (is_array($options['empty_value'])) {
-                $options['empty_value'] = array_merge(array('hour' => null, 'minute' => null, 'second' => null), $options['empty_value']);
-            } else {
-                $options['empty_value'] = array('hour' => $options['empty_value'], 'minute' => $options['empty_value'], 'second' => $options['empty_value']);
-            }
+            $builder->appendClientTransformer(new DateTimeToStringTransformer($options['data_timezone'], $options['user_timezone'], $format));
+        } else {
+            $hourOptions = $minuteOptions = $secondOptions = array();
 
-            $builder
-                ->add('hour', $options['widget'], array(
+            if ($options['widget'] === 'choice') {
+                if (is_array($options['empty_value'])) {
+                    $options['empty_value'] = array_merge(array('hour' => null, 'minute' => null, 'second' => null), $options['empty_value']);
+                } else {
+                    $options['empty_value'] = array('hour' => $options['empty_value'], 'minute' => $options['empty_value'], 'second' => $options['empty_value']);
+                }
+
+                // Only pass a subset of the options to children
+                $hourOptions = array(
                     'choice_list' => new PaddedChoiceList(
                         array_combine($options['hours'], $options['hours']), 2, '0', STR_PAD_LEFT
                     ),
                     'empty_value' => $options['empty_value']['hour'],
                     'required' => $options['required'],
-                ))
-                ->add('minute', $options['widget'], array(
+                );
+                $minuteOptions = array(
                     'choice_list' => new PaddedChoiceList(
                         array_combine($options['minutes'], $options['minutes']), 2, '0', STR_PAD_LEFT
                     ),
                     'empty_value' => $options['empty_value']['minute'],
                     'required' => $options['required'],
-                ))
+                );
+
+                if ($options['with_seconds']) {
+                    $secondOptions = array(
+                        'choice_list' => new PaddedChoiceList(
+                            array_combine($options['seconds'], $options['seconds']), 2, '0', STR_PAD_LEFT
+                        ),
+                        'empty_value' => $options['empty_value']['second'],
+                        'required' => $options['required'],
+                    );
+                }
+            }
+
+            $builder
+                ->add('hour', $options['widget'], $hourOptions)
+                ->add('minute', $options['widget'], $minuteOptions)
             ;
 
             if ($options['with_seconds']) {
-                $builder->add('second', $options['widget'], array(
-                    'choice_list' => new PaddedChoiceList(
-                        array_combine($options['seconds'], $options['seconds']), 2, '0', STR_PAD_LEFT
-                    ),
-                    'empty_value' => $options['empty_value']['second'],
-                    'required' => $options['required'],
-                ));
+                $builder->add('second', $options['widget'], $secondOptions);
             }
-        }
 
-        $parts = array('hour', 'minute');
-        if ($options['with_seconds']) {
-            $parts[] = 'second';
+            $builder->appendClientTransformer(new DateTimeToArrayTransformer($options['data_timezone'], $options['user_timezone'], $parts, $options['widget'] === 'text'));
         }
 
         if ($options['input'] === 'string') {
             $builder->appendNormTransformer(new ReversedTransformer(
-                new DateTimeToStringTransformer($options['data_timezone'], $options['data_timezone'], 'H:i:s')
+                new DateTimeToStringTransformer($options['data_timezone'], $options['data_timezone'], $format)
             ));
         } else if ($options['input'] === 'timestamp') {
             $builder->appendNormTransformer(new ReversedTransformer(
@@ -84,15 +100,6 @@ class TimeType extends AbstractType
             ));
         }
 
-        if ($options['widget'] !== 'single_text') {
-            $builder->appendClientTransformer(new DateTimeToArrayTransformer(
-                $options['data_timezone'],
-                $options['user_timezone'],
-                $parts,
-                $options['widget'] === 'text'
-            ));
-        }
-
         $builder
             ->setAttribute('widget', $options['widget'])
             ->setAttribute('with_seconds', $options['with_seconds'])

tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php

@@ -1512,6 +1512,37 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testTimeText()
+    {
+        $form = $this->factory->createNamed('time', 'na&me', '04:05:06', array(
+            'property_path' => 'name',
+            'input' => 'string',
+            'widget' => 'text',
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./input
+            [@type="text"]
+            [@id="na&me_hour"]
+            [@name="na&me[hour]"]
+            [@value="04"]
+            [@size="1"]
+            [@required="required"]
+        /following-sibling::input
+            [@type="text"]
+            [@id="na&me_minute"]
+            [@name="na&me[minute]"]
+            [@value="05"]
+            [@size="1"]
+            [@required="required"]
+    ]
+    [count(./input)=2]
+'
+        );
+    }
+
     public function testTimeSingleText()
     {
         $form = $this->factory->createNamed('time', 'na&me', '04:05:06', array(
@@ -1524,7 +1555,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
 '/input
     [@type="text"]
     [@name="na&me"]
-    [@value="04:05:06"]
+    [@value="04:05:00"]
 '
         );
     }

tests/Symfony/Tests/Component/Form/Extension/Core/DataTransformer/DateTimeToStringTransformerTest.php

@@ -29,6 +29,20 @@ class DateTimeToStringTransformerTest extends DateTimeTestCase
         $this->assertEquals($output, $transformer->transform($input));
     }
 
+    /**
+     * @dataProvider getFormatAndDateTime
+     */
+    public function testTransformRandomFormat($format, $datetime)
+    {
+        $transformer = new DateTimeToStringTransformer('UTC', 'UTC', $format);
+
+        $input = new \DateTime($datetime);
+        $output = clone $input;
+        $output->setTimezone(new \DateTimeZone('UTC'));
+
+        $this->assertEquals($output->format($format), $transformer->transform($input));
+    }
+
     public function testTransform_empty()
     {
         $transformer = new DateTimeToStringTransformer();
@@ -40,7 +54,7 @@ class DateTimeToStringTransformerTest extends DateTimeTestCase
     {
         $transformer = new DateTimeToStringTransformer('Asia/Hong_Kong', 'America/New_York', 'Y-m-d H:i:s');
 
-        $input = new \DateTime('2010-02-03 04:05:06 America/New_York');
+        $input = new \DateTime('2010-02-03 12:05:06 America/New_York');
         $output = $input->format('Y-m-d H:i:s');
         $input->setTimezone(new \DateTimeZone('Asia/Hong_Kong'));
 
@@ -66,6 +80,31 @@ class DateTimeToStringTransformerTest extends DateTimeTestCase
         $this->assertDateTimeEquals($output, $reverseTransformer->reverseTransform($input, null));
     }
 
+    /**
+     * @dataProvider getFormatAndDateTime
+     */
+    public function testReverseTransformRandomFormat($format, $datetime)
+    {
+        $reverseTransformer = new DateTimeToStringTransformer('UTC', 'UTC', $format);
+
+        $dateTime = new \DateTime($datetime);
+        $input = $dateTime->format($format);
+
+        $this->assertDateTimeEquals($dateTime, $reverseTransformer->reverseTransform($input, null));
+    }
+
+    public function getFormatAndDateTime()
+    {
+        return array(
+            array('Y-m-d H:i:s', '2010-02-03 04:05:06 UTC'),
+            array('Y-m-d H:i:00', '2010-02-03 04:05:00 UTC'),
+            array('Y-m-d', '2010-02-03 UTC'),
+            array('d-m-Y', '03-02-2010 UTC'),
+            array('H:i:s', '04:05:06 UTC'),
+            array('H:i:00', '04:05:00 UTC'),
+        );
+    }
+
     public function testReverseTransform_empty()
     {
         $reverseTransformer = new DateTimeToStringTransformer();

tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTimeTypeTest.php

@@ -136,7 +136,6 @@ class DateTimeTypeTest extends LocalizedTestCase
             'user_timezone' => 'Pacific/Tahiti',
             'date_widget' => 'choice',
             'time_widget' => 'choice',
-            // don't do this test with DateTime, because it leads to wrong results!
             'input' => 'string',
             'with_seconds' => true,
         ));
@@ -161,6 +160,26 @@ class DateTimeTypeTest extends LocalizedTestCase
         $this->assertEquals($dateTime->format('Y-m-d H:i:s'), $form->getData());
     }
 
+    public function testSubmit_differentTimezonesDateTime()
+    {
+        $form = $this->factory->create('datetime', null, array(
+            'data_timezone' => 'America/New_York',
+            'user_timezone' => 'Pacific/Tahiti',
+            'widget' => 'single_text',
+            'input' => 'datetime',
+        ));
+
+        $dateTime = new \DateTime('2010-06-02 03:04:05 America/New_York');
+
+        $form->bind('2010-06-02 03:04:05');
+
+        $outputTime = new \DateTime('2010-06-02 03:04:00 Pacific/Tahiti');
+        $outputTime->setTimezone(new \DateTimeZone('America/New_York'));
+
+        $this->assertDateTimeEquals($outputTime, $form->getData());
+        $this->assertEquals('2010-06-02 03:04:00', $form->getClientData());
+    }
+
     public function testSubmit_stringSingleText()
     {
         $form = $this->factory->create('datetime', null, array(

tests/Symfony/Tests/Component/Form/Extension/Core/Type/DateTypeTest.php

@@ -183,7 +183,6 @@ class DateTypeTest extends LocalizedTestCase
         $form = $this->factory->create('date', null, array(
             'data_timezone' => 'America/New_York',
             'user_timezone' => 'Pacific/Tahiti',
-            // don't do this test with DateTime, because it leads to wrong results!
             'input' => 'string',
             'widget' => 'single_text',
         ));
@@ -193,6 +192,23 @@ class DateTypeTest extends LocalizedTestCase
         $this->assertEquals('01.06.2010', $form->getClientData());
     }
 
+    public function testSetData_differentTimezonesDateTime()
+    {
+        $form = $this->factory->create('date', null, array(
+            'data_timezone' => 'America/New_York',
+            'user_timezone' => 'Pacific/Tahiti',
+            'input' => 'datetime',
+            'widget' => 'single_text',
+        ));
+
+        $dateTime = new \DateTime('2010-06-02 America/New_York');
+
+        $form->setData($dateTime);
+
+        $this->assertDateTimeEquals($dateTime, $form->getData());
+        $this->assertEquals('01.06.2010', $form->getClientData());
+    }
+
     public function testIsYearWithinRangeReturnsTrueIfWithin()
     {
         $this->markTestIncomplete('Needs to be reimplemented using validators');

tests/Symfony/Tests/Component/Form/Extension/Core/Type/TimeTypeTest.php

@@ -91,17 +91,27 @@ class TimeTypeTest extends LocalizedTestCase
             'minute' => '4',
         );
 
-        $data = array(
-            'hour' => '3',
-            'minute' => '4',
-        );
-
         $form->bind($input);
 
-        $this->assertEquals($data, $form->getData());
+        $this->assertEquals($input, $form->getData());
         $this->assertEquals($input, $form->getClientData());
     }
 
+    public function testSubmit_datetimeSingleText()
+    {
+        $form = $this->factory->create('time', null, array(
+            'data_timezone' => 'UTC',
+            'user_timezone' => 'UTC',
+            'input' => 'datetime',
+            'widget' => 'single_text',
+        ));
+
+        $form->bind('03:04:05');
+
+        $this->assertEquals(new \DateTime('03:04:00 UTC'), $form->getData());
+        $this->assertEquals('03:04:00', $form->getClientData());
+    }
+
     public function testSubmit_arraySingleText()
     {
         $form = $this->factory->create('time', null, array(
@@ -144,21 +154,6 @@ class TimeTypeTest extends LocalizedTestCase
         $this->assertEquals('03:04:05', $form->getClientData());
     }
 
-    public function testSubmit_datetimeSingleText()
-    {
-        $form = $this->factory->create('time', null, array(
-            'data_timezone' => 'UTC',
-            'user_timezone' => 'UTC',
-            'input' => 'datetime',
-            'widget' => 'single_text',
-        ));
-
-        $form->bind('03:04:05');
-
-        $this->assertEquals(new \DateTime('03:04:05 UTC'), $form->getData());
-        $this->assertEquals('03:04:05', $form->getClientData());
-    }
-
     public function testSubmit_stringSingleText()
     {
         $form = $this->factory->create('time', null, array(
@@ -168,12 +163,10 @@ class TimeTypeTest extends LocalizedTestCase
             'widget' => 'single_text',
         ));
 
-        $time = '03:04:05';
-
-        $form->bind($time);
+        $form->bind('03:04:05');
 
-        $this->assertEquals($time, $form->getData());
-        $this->assertEquals($time, $form->getClientData());
+        $this->assertEquals('03:04:00', $form->getData());
+        $this->assertEquals('03:04:00', $form->getClientData());
     }
 
     public function testSetData_withSeconds()
@@ -194,25 +187,52 @@ class TimeTypeTest extends LocalizedTestCase
     {
         $form = $this->factory->create('time', null, array(
             'data_timezone' => 'America/New_York',
-            'user_timezone' => 'Pacific/Tahiti',
-            // don't do this test with DateTime, because it leads to wrong results!
+            'user_timezone' => 'Asia/Hong_Kong',
             'input' => 'string',
             'with_seconds' => true,
         ));
 
-        $dateTime = new \DateTime('03:04:05 America/New_York');
+        $dateTime = new \DateTime('12:04:05');
+        $dateTime->setTimezone(new \DateTimeZone('America/New_York'));
 
         $form->setData($dateTime->format('H:i:s'));
 
-        $dateTime = clone $dateTime;
-        $dateTime->setTimezone(new \DateTimeZone('Pacific/Tahiti'));
+        $outputTime = clone $dateTime;
+        $outputTime->setTimezone(new \DateTimeZone('Asia/Hong_Kong'));
+
+        $displayedData = array(
+            'hour' => (int)$outputTime->format('H'),
+            'minute' => (int)$outputTime->format('i'),
+            'second' => (int)$outputTime->format('s')
+        );
+
+        $this->assertEquals($displayedData, $form->getClientData());
+    }
+
+    public function testSetData_differentTimezonesDateTime()
+    {
+        $form = $this->factory->create('time', null, array(
+            'data_timezone' => 'America/New_York',
+            'user_timezone' => 'Asia/Hong_Kong',
+            'input' => 'datetime',
+            'with_seconds' => true,
+        ));
+
+        $dateTime = new \DateTime('12:04:05');
+        $dateTime->setTimezone(new \DateTimeZone('America/New_York'));
+
+        $form->setData($dateTime);
+
+        $outputTime = clone $dateTime;
+        $outputTime->setTimezone(new \DateTimeZone('Asia/Hong_Kong'));
 
         $displayedData = array(
-            'hour' => (int)$dateTime->format('H'),
-            'minute' => (int)$dateTime->format('i'),
-            'second' => (int)$dateTime->format('s')
+            'hour' => (int)$outputTime->format('H'),
+            'minute' => (int)$outputTime->format('i'),
+            'second' => (int)$outputTime->format('s')
         );
 
+        $this->assertDateTimeEquals($dateTime, $form->getData());
         $this->assertEquals($displayedData, $form->getClientData());
     }