merged branch Romain-Geissler/read-only-form-datamapper-fix (PR #4280)

Commits
-------

47605f6 [Form][DataMapper] Do not update form to data when form is read only

Discussion
----------

[Form] [DataMapper] Read only form datamapper fix

The current 2.0.13 ``Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper`` enables to overwrite data from form values, no matter the form fields are read only or not.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

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

by travisbot at 2012-05-14T15:50:02Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1328279) (merged 47605f63 into 72b2f698).

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

by bschussek at 2012-05-14T18:06:41Z

Forms don't bind when they are read only, so why is this change necessary?

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

by stof at 2012-05-14T19:29:45Z

@bschussek A read-only child will not be bound but the setter will still be called on the parent object for this field (with the old value), making it mandatory to define setters for read-only fields.

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

by Romain-Geissler at 2012-05-14T19:43:11Z

In my case, the property is still set through a setter even if the field for this property is read only. The problem is the setter is not called with the legacy value it held, but with the value given by the form. In my case the value is transformed from a string to an object by a ``DataMapper``, which returns ``null`` for an empty string/value. Thus, the setter is called with ``null`` instead of the previous non ``null`` value (and not always the same) it held.

This PR just prevent the setter for an object property marked as read only in the form definition from being called.

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

by bschussek at 2012-05-15T08:20:28Z

Ok, 👍 then

src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php

@@ -77,7 +77,7 @@ class PropertyPathMapper implements DataMapperInterface
 
     public function mapFormToData(FormInterface $form, &$data)
     {
-        if ($form->getAttribute('property_path') !== null && $form->isSynchronized()) {
+        if ($form->getAttribute('property_path') !== null && $form->isSynchronized() && !$form->isReadOnly()) {
             $propertyPath = $form->getAttribute('property_path');
 
             // If the data is identical to the value in $data, we are

tests/Symfony/Tests/Component/Form/Extension/Core/DataMapper/PropertyPathMapperTest.php

@@ -34,7 +34,7 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase
         $this->propertyPath = null;
     }
 
-    private function getForm(PropertyPath $propertyPath = null)
+    private function getForm(PropertyPath $propertyPath = null, $synchronized = true, $readOnly = false)
     {
         $form = $this->getMock('Symfony\Tests\Component\Form\FormInterface');
 
@@ -43,6 +43,14 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase
             ->with('property_path')
             ->will($this->returnValue($propertyPath));
 
+        $form->expects($this->any())
+            ->method('isSynchronized')
+            ->will($this->returnValue($synchronized));
+
+        $form->expects($this->any())
+            ->method('isReadOnly')
+            ->will($this->returnValue($readOnly));
+
         return $form;
     }
 
@@ -87,4 +95,16 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase
 
         $this->mapper->mapDataToForm(null, $form);
     }
+
+    public function testMapFormToDataIgnoresReadOnlyForm()
+    {
+        $data = new \stdClass();
+
+        $form = $this->getForm($this->propertyPath, true, true);
+
+        $this->propertyPath->expects($this->never())
+            ->method('setValue');
+
+        $this->mapper->mapFormToData($form, $data);
+    }
 }