merged branch helmer/readonly_fix (PR #3258)

Commits
-------

8321593 [Form] DRYed ChoiceType
0753cee [Form] Fixed read_only attribute for expanded fields

Discussion
----------

[Form] Fixed read_only attribute for expanded fields

Expanded choice widgets lose the knowledge of read_only attribute value.

Fixes bug introduced by #3193

---------------------------------------------------------------------------

by helmer at 2012-02-02T16:24:50Z

Please hold before merging, @bschussek had some thoughts about my changes in ``ChoiceType``, waiting for feedback.

---------------------------------------------------------------------------

by bschussek at 2012-02-02T16:33:12Z

I'm fine with the refactoring then, but please split it into two commits at least. The changes in ChoiceType have nothing in common with the actual issue here.

---------------------------------------------------------------------------

by helmer at 2012-02-02T19:40:39Z

Tests added.

---------------------------------------------------------------------------

by bschussek at 2012-02-03T10:14:32Z

Great, thanks.

@fabpot 👍

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

@@ -190,21 +190,23 @@ private function addSubFields(FormBuilder $builder, array $choiceViews, array $o
             if (is_array($choiceView)) {
                 // Flatten groups
                 $this->addSubFields($builder, $choiceView, $options);
-            } elseif ($options['multiple']) {
-                $builder->add((string) $i, 'checkbox', array(
-                    'value' => $choiceView->getValue(),
-                    'label' => $choiceView->getLabel(),
-                    // The user can check 0 or more checkboxes. If required
-                    // is true, he is required to check all of them.
-                    'required' => false,
-                    'translation_domain' => $options['translation_domain'],
-                ));
             } else {
-                $builder->add((string) $i, 'radio', array(
+                $choiceOpts = array(
                     'value' => $choiceView->getValue(),
                     'label' => $choiceView->getLabel(),
                     'translation_domain' => $options['translation_domain'],
-                ));
+                );
+
+                if ($options['multiple']) {
+                    $choiceType = 'checkbox';
+                    // The user can check 0 or more checkboxes. If required
+                    // is true, he is required to check all of them.
+                    $choiceOpts['required'] = false;
+                } else {
+                    $choiceType = 'radio';
+                }
+
+                $builder->add((string) $i, $choiceType, $choiceOpts);
             }
         }
     }

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

@@ -73,6 +73,7 @@ public function buildForm(FormBuilder $builder, array $options)
     public function buildView(FormView $view, FormInterface $form)
     {
         $name = $form->getName();
+        $readOnly = $form->getAttribute('read_only');
 
         if ($view->hasParent()) {
             if ('' === $name) {
@@ -86,6 +87,9 @@ public function buildView(FormView $view, FormInterface $form)
                 $id = $name;
                 $fullName = $name;
             }
+
+            // Complex fields are read-only if themselves or their parent is.
+            $readOnly = $readOnly || $view->getParent()->get('read_only');
         } else {
             $id = $name;
             $fullName = $name;
@@ -106,9 +110,9 @@ public function buildView(FormView $view, FormInterface $form)
             ->set('id', $id)
             ->set('name', $name)
             ->set('full_name', $fullName)
+            ->set('read_only', $readOnly)
             ->set('errors', $form->getErrors())
             ->set('value', $form->getClientData())
-            ->set('read_only', $form->getAttribute('read_only'))
             ->set('disabled', $form->isDisabled())
             ->set('required', $form->isRequired())
             ->set('max_length', $form->getAttribute('max_length'))

src/Symfony/Component/Form/FormBuilder.php

@@ -184,7 +184,7 @@ public function getData()
     }
 
     /**
-     * Set whether the form is disabled
+     * Set whether the form is disabled.
      *
      * @param Boolean $disabled Whether the form is disabled
      *

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

@@ -150,6 +150,33 @@ public function testPassIdAndNameToViewWithGrandParent()
         $this->assertEquals('parent[child][grand_child]', $view['child']['grand_child']->get('full_name'));
     }
 
+    public function testNonReadOnlyFieldWithReadOnlyParentBeingReadOnly()
+    {
+        $parent = $this->factory->createNamed('field', 'parent', null, array('read_only' => true));
+        $child  = $this->factory->createNamed('field', 'child');
+        $view   = $parent->add($child)->createView();
+
+        $this->assertTrue($view['child']->get('read_only'));
+    }
+
+    public function testReadOnlyFieldWithNonReadOnlyParentBeingReadOnly()
+    {
+        $parent = $this->factory->createNamed('field', 'parent');
+        $child  = $this->factory->createNamed('field', 'child', null, array('read_only' => true));
+        $view   = $parent->add($child)->createView();
+
+        $this->assertTrue($view['child']->get('read_only'));
+    }
+
+    public function testNonReadOnlyFieldWithNonReadOnlyParentBeingNonReadOnly()
+    {
+        $parent = $this->factory->createNamed('field', 'parent');
+        $child  = $this->factory->createNamed('field', 'child');
+        $view   = $parent->add($child)->createView();
+
+        $this->assertFalse($view['child']->get('read_only'));
+    }
+
     public function testPassMaxLengthToView()
     {
         $form = $this->factory->create('field', null, array('max_length' => 10));