ソースを参照

More tests and more compatible code, with some suggestions from @helmer

stloyd 14 年 前
コミット
af4a7d77d9

+ 1 - 0
src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget.html.php

@@ -9,6 +9,7 @@
     <select
         <?php echo $view['form']->attributes() ?>
         name="<?php echo $view->escape($full_name) ?>"
+        <?php if ($required): ?> required="required"<?php endif ?>
         <?php if ($read_only): ?> disabled="disabled"<?php endif ?>
         <?php if ($multiple): ?> multiple="multiple"<?php endif ?>
     >

+ 2 - 3
src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

@@ -71,7 +71,7 @@ class ChoiceType extends AbstractType
             ->setAttribute('multiple', $options['multiple'])
             ->setAttribute('expanded', $options['expanded'])
             ->setAttribute('required', $options['required'])
-            ->setAttribute('empty_value', $options['multiple'] || $options['required'] ? null : $options['empty_value'])
+            ->setAttribute('empty_value', $options['multiple'] || $options['expanded'] ? null : $options['empty_value'])
         ;
 
         if ($options['expanded']) {
@@ -125,7 +125,6 @@ class ChoiceType extends AbstractType
     {
         $multiple = isset($options['multiple']) && $options['multiple'];
         $expanded = isset($options['expanded']) && $options['expanded'];
-        $required = isset($options['required']) && $options['required'];
 
         return array(
             'multiple'          => false,
@@ -134,7 +133,7 @@ class ChoiceType extends AbstractType
             'choices'           => array(),
             'preferred_choices' => array(),
             'empty_data'        => $multiple || $expanded ? array() : '',
-            'empty_value'       => $multiple || $required ? null : '',
+            'empty_value'       => ($multiple || $expanded) || !isset($options['empty_value']) ? null : '',
             'error_bubbling'    => false,
         );
     }

+ 80 - 6
tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php

@@ -352,6 +352,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array('separator' => '-- sep --'),
 '/select
     [@name="na&me"]
+    [@required="required"]
     [
         ./option[@value="&b"][not(@selected)][.="Choice&B"]
         /following-sibling::option[@disabled="disabled"][not(@selected)][.="-- sep --"]
@@ -375,12 +376,12 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/select
     [@name="na&me"]
+    [not(@required)]
     [
-        ./option[@value=""][.="[trans][/trans]"]
-        /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
+        ./option[@value="&a"][@selected="selected"][.="Choice&A"]
         /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
     ]
-    [count(./option)=3]
+    [count(./option)=2]
 '
         );
     }
@@ -398,12 +399,12 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/select
     [@name="na&me"]
+    [not(@required)]
     [
-        ./option[@value=""][not(@selected)][.="[trans][/trans]"]
-        /following-sibling::option[@value="&a"][not(@selected)][.="Choice&A"]
+        ./option[@value="&a"][not(@selected)][.="Choice&A"]
         /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
     ]
-    [count(./option)=3]
+    [count(./option)=2]
 '
         );
     }
@@ -422,6 +423,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/select
     [@name="na&me"]
+    [not(@required)]
     [
         ./option[@value=""][not(@selected)][.="[trans]Select&Anything&Not&Me[/trans]"]
         /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
@@ -432,6 +434,31 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testSingleChoiceRequiredWithEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'required' => true,
+            'multiple' => false,
+            'expanded' => false,
+            'empty_value' => 'Test&Me'
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me"]
+    [@required="required"]
+    [
+        ./option[@value=""][.="[trans]Test&Me[/trans]"]
+        /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
+        /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
+    ]
+    [count(./option)=3]
+'
+        );
+    }
+
     public function testSingleChoiceRequiredWithEmptyValueViaView()
     {
         $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
@@ -445,6 +472,7 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         $this->assertWidgetMatchesXpath($form->createView(), array('empty_value' => ''),
 '/select
     [@name="na&me"]
+    [@required="required"]
     [
         ./option[@value=""][.="[trans][/trans]"]
         /following-sibling::option[@value="&a"][@selected="selected"][.="Choice&A"]
@@ -508,6 +536,29 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testMultipleChoiceSkipEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', array('&a'), array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'multiple' => true,
+            'expanded' => false,
+            'empty_value' => 'Test&Me'
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/select
+    [@name="na&me[]"]
+    [@multiple="multiple"]
+    [
+        ./option[@value="&a"][@selected="selected"][.="Choice&A"]
+        /following-sibling::option[@value="&b"][not(@selected)][.="Choice&B"]
+    ]
+    [count(./option)=2]
+'
+        );
+    }
+
     public function testMultipleChoiceNonRequired()
     {
         $form = $this->factory->createNamed('choice', 'na&me', array('&a'), array(
@@ -553,6 +604,29 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testSingleChoiceExpandedSkipEmptyValue()
+    {
+        $form = $this->factory->createNamed('choice', 'na&me', '&a', array(
+            'property_path' => 'name',
+            'choices' => array('&a' => 'Choice&A', '&b' => 'Choice&B'),
+            'multiple' => false,
+            'expanded' => true,
+            'empty_value' => 'Test&Me'
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/div
+    [
+        ./input[@type="radio"][@name="na&me"][@id="na&me_&a"][@checked]
+        /following-sibling::label[@for="na&me_&a"][.="[trans]Choice&A[/trans]"]
+        /following-sibling::input[@type="radio"][@name="na&me"][@id="na&me_&b"][not(@checked)]
+        /following-sibling::label[@for="na&me_&b"][.="[trans]Choice&B[/trans]"]
+    ]
+    [count(./input)=2]
+'
+        );
+    }
+
     public function testSingleChoiceExpandedWithBooleanValue()
     {
         $form = $this->factory->createNamed('choice', 'na&me', true, array(

+ 36 - 6
tests/Symfony/Tests/Component/Form/Extension/Core/Type/ChoiceTypeTest.php

@@ -321,6 +321,17 @@ class ChoiceTypeTest extends TypeTestCase
         $this->assertTrue($view->get('required'));
     }
 
+    public function testPassNonRequiredToView()
+    {
+        $form = $this->factory->create('choice', null, array(
+            'required' => false,
+            'choices' => $this->choices,
+        ));
+        $view = $form->createView();
+
+        $this->assertFalse($view->get('required'));
+    }
+
     public function testPassMultipleToView()
     {
         $form = $this->factory->create('choice', null, array(
@@ -343,7 +354,7 @@ class ChoiceTypeTest extends TypeTestCase
         $this->assertTrue($view->get('expanded'));
     }
 
-    public function testPassEmptyValueToViewIsNull()
+    public function testNotPassedEmptyValueToViewIsNull()
     {
         $form = $this->factory->create('choice', null, array(
             'multiple' => false,
@@ -366,17 +377,36 @@ class ChoiceTypeTest extends TypeTestCase
         $this->assertEmpty($view->get('empty_value'));
     }
 
-    public function testPassEmptyValueToView()
+    /**
+     * @dataProvider getOptionsWithEmptyValue
+     */
+    public function testPassEmptyValueToView($multiple, $expanded, $required, $emptyValue, $viewValue)
     {
         $form = $this->factory->create('choice', null, array(
-            'multiple' => false,
-            'required' => false,
-            'empty_value' => 'foobar',
+            'multiple' => $multiple,
+            'expanded' => $expanded,
+            'required' => $required,
+            'empty_value' => $emptyValue,
             'choices' => $this->choices,
         ));
         $view = $form->createView();
 
-        $this->assertEquals('foobar', $view->get('empty_value'));
+        $this->assertEquals($viewValue, $view->get('empty_value'));
+    }
+
+    public function getOptionsWithEmptyValue()
+    {
+        return array(
+            array(false, false, false, 'foobar', 'foobar'),
+            array(true, false, false, 'foobar', null),
+            array(false, true, false, 'foobar', null),
+            array(false, false, true, 'foobar', 'foobar'),
+            array(false, false, true, '', ''),
+            array(false, false, true, null, null),
+            array(false, true, true, 'foobar', null),
+            array(true, true, false, 'foobar', null),
+            array(true, true, true, 'foobar', null),
+        );
     }
 
     public function testPassChoicesToView()