merged branch bschussek/issue4359 (PR #5947)

This PR was merged into the 2.1 branch.

Commits
-------

4909bc3 [Form] Fixed forms not to be marked invalid if their children are already marked invalid

Discussion
----------

[Form] Fixed forms not to be marked invalid if their children are already marked invalid

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

This PR prevents unsynchronized forms from being marked invalid if any of their children is also unsynchronized (and thus also marked invalid). Displaying an invalid message twice does not help the user and, if used in conjunction with error bubbling, may lead to duplicate errors (see #4359).

src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php

@@ -80,18 +80,35 @@ class FormValidator extends ConstraintValidator
                 }
             }
         } else {
-            $clientDataAsString = is_scalar($form->getViewData())
-                ? (string) $form->getViewData()
-                : gettype($form->getViewData());
+            $childrenSynchronized = true;
 
-            // Mark the form with an error if it is not synchronized
-            $this->context->addViolation(
-                $config->getOption('invalid_message'),
-                array_replace(array('{{ value }}' => $clientDataAsString), $config->getOption('invalid_message_parameters')),
-                $form->getViewData(),
-                null,
-                Form::ERR_INVALID
-            );
+            foreach ($form as $child) {
+                if (!$child->isSynchronized()) {
+                    $childrenSynchronized = false;
+                    break;
+                }
+            }
+
+            // Mark the form with an error if it is not synchronized BUT all
+            // of its children are synchronized. If any child is not
+            // synchronized, an error is displayed there already and showing
+            // a second error in its parent form is pointless, or worse, may
+            // lead to duplicate errors if error bubbling is enabled on the
+            // child.
+            // See also https://github.com/symfony/symfony/issues/4359
+            if ($childrenSynchronized) {
+                $clientDataAsString = is_scalar($form->getViewData())
+                    ? (string) $form->getViewData()
+                    : gettype($form->getViewData());
+
+                $this->context->addViolation(
+                    $config->getOption('invalid_message'),
+                    array_replace(array('{{ value }}' => $clientDataAsString), $config->getOption('invalid_message_parameters')),
+                    $form->getViewData(),
+                    null,
+                    Form::ERR_INVALID
+                );
+            }
         }
 
         // Mark the form with an error if it contains extra fields

src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php

@@ -250,6 +250,37 @@ class FormValidatorTest extends \PHPUnit_Framework_TestCase
         $this->validator->validate($form, new Form());
     }
 
+    // https://github.com/symfony/symfony/issues/4359
+    public function testDontMarkInvalidIfAnyChildIsNotSynchronized()
+    {
+        $context = $this->getExecutionContext();
+        $object = $this->getMock('\stdClass');
+
+        $failingTransformer = new CallbackTransformer(
+            function ($data) { return $data; },
+            function () { throw new TransformationFailedException(); }
+        );
+
+        $form = $this->getBuilder('name', '\stdClass')
+            ->setData($object)
+            ->addViewTransformer($failingTransformer)
+            ->setCompound(true)
+            ->setDataMapper($this->getDataMapper())
+            ->add(
+                $this->getBuilder('child')
+                    ->addViewTransformer($failingTransformer)
+            )
+            ->getForm();
+
+        // Launch transformer
+        $form->bind(array('child' => 'foo'));
+
+        $this->validator->initialize($context);
+        $this->validator->validate($form, new Form());
+
+        $this->assertCount(0, $context->getViolations());
+    }
+
     public function testHandleCallbackValidationGroups()
     {
         $context = $this->getExecutionContext();