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

…eady marked invalid
symfony · Nov 8, 2012 · 4909bc3 · 4909bc3
1 parent 9c38e76
commit 4909bc3
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 11 deletions.
diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
@@ -80,18 +80,35 @@ public function validate($form, Constraint $constraint)
                 }
             }
         } 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

diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php
@@ -250,6 +250,37 @@ function () { throw new TransformationFailedException(); }
         $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();