symfony · fabpot · Mar 31, 2014 · Mar 28, 2014
diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md
@@ -11,6 +11,7 @@ CHANGELOG
  * [BC BREAK] added two optional parameters to FormInterface::getErrors() and
    changed the method to return a Symfony\Component\Form\FormErrorIterator
    instance instead of an array
+ * errors mapped to unsubmitted forms are discarded now
 
 2.4.0
 -----

diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php
@@ -294,6 +294,9 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
      */
     private function acceptsErrors(FormInterface $form)
     {
-        return $this->allowNonSynchronized || $form->isSynchronized();
+        // Ignore non-submitted forms. This happens, for example, in PATCH
+        // requests.
+        // https://github.com/symfony/symfony/pull/10567
+        return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
     }
 }
diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php
@@ -556,8 +556,10 @@ public function submit($submittedData, $clearMissing = true)
                 }
 
                 foreach ($this->children as $name => $child) {
-                    if (array_key_exists($name, $submittedData) || $clearMissing) {
-                        $child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
+                    $isSubmitted = array_key_exists($name, $submittedData);
+
+                    if ($isSubmitted || $clearMissing) {
+                        $child->submit($isSubmitted ? $submittedData[$name] : null, $clearMissing);
                         unset($submittedData[$name]);
 
                         if (null !== $this->clickedButton) {
@@ -739,18 +741,12 @@ public function isValid()
             return false;
         }
 
-        if (count($this->errors) > 0) {
-            return false;
-        }
-
         if ($this->isDisabled()) {
             return true;
         }
 
-        foreach ($this->children as $child) {
-            if ($child->isSubmitted() && !$child->isValid()) {
-                return false;
-            }
+        if (count($this->getErrors(true)) > 0) {
+            return false;
         }
 
         return true;

diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php
@@ -51,22 +51,6 @@ public function testInvalidIfChildIsInvalid()
         $this->assertFalse($this->form->isValid());
     }
 
-    public function testValidIfChildIsNotSubmitted()
-    {
-        $this->form->add($this->getBuilder('firstName')->getForm());
-        $this->form->add($this->getBuilder('lastName')->getForm());
-
-        $this->form->submit(array(
-            'firstName' => 'Bernhard',
-        ));
-
-        // "lastName" is not "valid" because it was not submitted. This happens
-        // for example in PATCH requests. The parent form should still be
-        // considered valid.
-
-        $this->assertTrue($this->form->isValid());
-    }
-
     public function testDisabledFormsValidEvenIfChildrenInvalid()
     {
         $form = $this->getBuilder('person')

diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php
@@ -125,6 +125,8 @@ public function testMapToFormInheritingParentDataIfDataDoesNotMatch()
         $parent->add($child);
         $child->add($grandChild);
 
+        $parent->submit(array());
+
         $this->mapper->mapViolation($violation, $parent);
 
         $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
@@ -150,6 +152,8 @@ public function testFollowDotRules()
         $child->add($grandChild);
         $grandChild->add($grandGrandChild);
 
+        $parent->submit(array());
+
         $this->mapper->mapViolation($violation, $parent);
 
         $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
@@ -170,7 +174,7 @@ public function testAbortMappingIfNotSynchronized()
         $parent->add($child);
         $child->add($grandChild);
 
-        // submit to invoke the transformer and mark the form unsynchronized
+        // invoke the transformer and mark the form unsynchronized
         $parent->submit(array());
 
         $this->mapper->mapViolation($violation, $parent);
@@ -194,7 +198,7 @@ public function testAbortDotRuleMappingIfNotSynchronized()
         $parent->add($child);
         $child->add($grandChild);
 
-        // submit to invoke the transformer and mark the form unsynchronized
+        // invoke the transformer and mark the form unsynchronized
         $parent->submit(array());
 
         $this->mapper->mapViolation($violation, $parent);
@@ -204,6 +208,54 @@ public function testAbortDotRuleMappingIfNotSynchronized()
         $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
     }
 
+    public function testAbortMappingIfNotSubmitted()
+    {
+        $violation = $this->getConstraintViolation('children[address].data.street');
+        $parent = $this->getForm('parent');
+        $child = $this->getForm('address', 'address');
+        $grandChild = $this->getForm('street' , 'street');
+
+        $parent->add($child);
+        $child->add($grandChild);
+
+        // Disable automatic submission of missing fields
+        $parent->submit(array(), false);
+        $child->submit(array(), false);
+
+        // $grandChild is not submitted
+
+        $this->mapper->mapViolation($violation, $parent);
+
+        $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
+        $this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
+        $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
+    }
+
+    public function testAbortDotRuleMappingIfNotSubmitted()
+    {
+        $violation = $this->getConstraintViolation('data.address');
+        $parent = $this->getForm('parent');
+        $child = $this->getForm('address', 'address', null, array(
+            '.' => 'street',
+        ));
+        $grandChild = $this->getForm('street');
+
+        $parent->add($child);
+        $child->add($grandChild);
+
+        // Disable automatic submission of missing fields
+        $parent->submit(array(), false);
+        $child->submit(array(), false);
+
+        // $grandChild is not submitted
+
+        $this->mapper->mapViolation($violation, $parent);
+
+        $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
+        $this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
+        $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
+    }
+
     public function provideDefaultTests()
     {
         // The mapping must be deterministic! If a child has the property path "[street]",
@@ -743,6 +795,8 @@ public function testDefaultErrorMapping($target, $childName, $childPath, $grandC
         $parent->add($child);
         $child->add($grandChild);
 
+        $parent->submit(array());
+
         $this->mapper->mapViolation($violation, $parent);
 
         if (self::LEVEL_0 === $target) {
@@ -1211,6 +1265,8 @@ public function testCustomDataErrorMapping($target, $mapFrom, $mapTo, $childName
             $parent->add($distraction);
         }
 
+        $parent->submit(array());
+
         $this->mapper->mapViolation($violation, $parent);
 
         if ($target !== self::LEVEL_0) {
@@ -1396,6 +1452,8 @@ public function testCustomFormErrorMapping($target, $mapFrom, $mapTo, $errorName
         $parent->add($errorChild);
         $child->add($grandChild);
 
+        $parent->submit(array());
+
         $this->mapper->mapViolation($violation, $parent);
 
         if (self::LEVEL_0 === $target) {
@@ -1459,6 +1517,8 @@ public function testErrorMappingForFormInheritingParentData($target, $childName,
         $parent->add($child);
         $child->add($grandChild);
 
+        $parent->submit(array());
+
         $this->mapper->mapViolation($violation, $parent);
 
         if (self::LEVEL_0 === $target) {