Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

[Form] Fixed: errors are not mapped to unsynchronized forms anymore

  • Loading branch information...
commit c9c49006633a725f2e967fbb975950f9820c0d74 1 parent c8b61d5
@webmozart webmozart authored
View
1  src/Symfony/Component/Form/CHANGELOG.md
@@ -73,6 +73,7 @@ CHANGELOG
* added option "mapped" which should be used instead of setting "property_path" to false
* "data_class" now *must* be set if a form maps to an object and should be left empty otherwise
* improved error mapping on forms
+ * errors are not mapped to unsynchronized forms anymore
* changed Form constructor to accept a single `FormConfigInterface` object
* changed argument order in the FormBuilder constructor
* deprecated Form methods
View
2  src/Symfony/Component/Form/Extension/Core/Type/FormType.php
@@ -47,6 +47,7 @@ public function buildForm(FormBuilder $builder, array $options)
// BC compatibility, when "property_path" could be false
->setPropertyPath(is_string($options['property_path']) ? $options['property_path'] : null)
->setMapped($options['mapped'])
+ ->setVirtual($options['virtual'])
->setAttribute('read_only', $options['read_only'])
->setAttribute('by_reference', $options['by_reference'])
->setAttribute('error_mapping', $options['error_mapping'])
@@ -58,7 +59,6 @@ public function buildForm(FormBuilder $builder, array $options)
->setAttribute('invalid_message', $options['invalid_message'])
->setAttribute('invalid_message_parameters', $options['invalid_message_parameters'])
->setAttribute('translation_domain', $options['translation_domain'])
- ->setAttribute('virtual', $options['virtual'])
->setAttribute('single_control', $options['single_control'])
->setData($options['data'])
->setDataMapper(new PropertyPathMapper())
View
1  src/Symfony/Component/Form/Extension/Validator/EventListener/DelegatingValidationListener.php
@@ -18,7 +18,6 @@
use Symfony\Component\Form\FormEvents;
use Symfony\Component\Form\Event\DataEvent;
use Symfony\Component\Form\Exception\FormException;
-use Symfony\Component\Form\Util\VirtualFormAwareIterator;
use Symfony\Component\Form\Util\PropertyPath;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ValidatorInterface;
View
19 src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php
@@ -91,11 +91,18 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
}
}
- $this->scope->addError(new FormError(
- $violation->getMessageTemplate(),
- $violation->getMessageParameters(),
- $violation->getMessagePluralization()
- ));
+ // Only add the error if the form is synchronized
+ // If the form is not synchronized, it already contains an
+ // error about being invalid. Further errors could be a result
+ // of the failed transformation and thus should not be
+ // displayed.
+ if ($this->scope->isSynchronized()) {
+ $this->scope->addError(new FormError(
+ $violation->getMessageTemplate(),
+ $violation->getMessageParameters(),
+ $violation->getMessagePluralization()
+ ));
+ }
}
/**
@@ -204,7 +211,7 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
// Process child form
$scope = $scope->get($it->current());
- if ($scope->getAttribute('virtual')) {
+ if ($scope->getConfig()->getVirtual()) {
// Form is virtual
// Cut the piece out of the property path and proceed
$propertyPathBuilder->remove($i);
View
27 src/Symfony/Component/Form/FormConfig.php
@@ -44,6 +44,11 @@ class FormConfig implements FormConfigInterface
private $mapped;
/**
+ * @var Boolean
+ */
+ private $virtual;
+
+ /**
* @var array
*/
private $types = array();
@@ -292,6 +297,14 @@ public function getMapped()
/**
* {@inheritdoc}
*/
+ public function getVirtual()
+ {
+ return $this->virtual;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
public function getTypes()
{
return $this->types;
@@ -538,6 +551,20 @@ public function setMapped($mapped)
}
/**
+ * Sets whether the form should be virtual.
+ *
+ * @param Boolean $virtual Whether the form should be virtual.
+ *
+ * @return self The configuration object.
+ */
+ public function setVirtual($virtual)
+ {
+ $this->virtual = $virtual;
+
+ return $this;
+ }
+
+ /**
* Set the types.
*
* @param array $types An array FormTypeInterface
View
11 src/Symfony/Component/Form/FormConfigInterface.php
@@ -48,6 +48,17 @@ function getPropertyPath();
function getMapped();
/**
+ * Returns whether the form should be virtual.
+ *
+ * When mapping data to the children of a form, the data mapper
+ * should ignore virtual forms and map to the children of the
+ * virtual form instead.
+ *
+ * @return Boolean Whether the form is virtual.
+ */
+ function getVirtual();
+
+ /**
* Returns the form types used to construct the form.
*
* @return array An array of {@link FormTypeInterface} instances.
View
54 src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php
@@ -12,6 +12,10 @@
namespace Symfony\Component\Form\Tests\Extension\Validator\ViolationMapper;
use Symfony\Component\Form\Extension\Validator\ViolationMapper\ViolationMapper;
+use Symfony\Component\Form\Exception\TransformationFailedException;
+use Symfony\Component\Form\CallbackTransformer;
+use Symfony\Component\Form\Form;
+use Symfony\Component\Form\FormConfig;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\Util\PropertyPath;
use Symfony\Component\Form\FormBuilder;
@@ -36,11 +40,6 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
private $dispatcher;
/**
- * @var \PHPUnit_Framework_MockObject_MockObject
- */
- private $factory;
-
- /**
* @var ViolationMapper
*/
private $mapper;
@@ -62,27 +61,27 @@ protected function setUp()
}
$this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
- $this->factory = $this->getMock('Symfony\Component\Form\FormFactoryInterface');
$this->mapper = new ViolationMapper();
$this->message = 'Message';
$this->params = array('foo' => 'bar');
}
- protected function getBuilder($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = array(), $virtual = false)
+ protected function getForm($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = array(), $virtual = false, $synchronized = true)
{
- $builder = new FormBuilder($name, $dataClass, $this->dispatcher, $this->factory);
- $builder->setPropertyPath(new PropertyPath($propertyPath ?: $name));
- $builder->setAttribute('error_mapping', $errorMapping);
- $builder->setAttribute('virtual', $virtual);
- $builder->setErrorBubbling(false);
- $builder->setMapped(true);
-
- return $builder;
- }
+ $config = new FormConfig($name, $dataClass, $this->dispatcher);
+ $config->setMapped(true);
+ $config->setVirtual($virtual);
+ $config->setPropertyPath($propertyPath);
+ $config->setAttribute('error_mapping', $errorMapping);
+
+ if (!$synchronized) {
+ $config->appendClientTransformer(new CallbackTransformer(
+ function ($normData) { return $normData; },
+ function () { throw new TransformationFailedException(); }
+ ));
+ }
- protected function getForm($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = array(), $virtual = false)
- {
- return $this->getBuilder($name, $propertyPath, $dataClass, $errorMapping, $virtual)->getForm();
+ return new Form($config);
}
/**
@@ -1338,4 +1337,21 @@ public function testVirtualFormErrorMapping($target, $childName, $childPath, $gr
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName. ' should have an error, but has none');
}
}
+
+ public function testDontMapToUnsynchronizedForms()
+ {
+ $violation = $this->getConstraintViolation('children[address].data.street');
+ $parent = $this->getForm('parent');
+ $child = $this->getForm('address', 'address', null, array(), false, false);
+
+ $parent->add($child);
+
+ // bind to invoke the transformer and mark the form unsynchronized
+ $parent->bind(array());
+
+ $this->mapper->mapViolation($violation, $parent);
+
+ $this->assertFalse($parent->hasErrors(), $parent->getName() . ' should not have an error, but has one');
+ $this->assertFalse($child->hasErrors(), $child->getName() . ' should not have an error, but has one');
+ }
}
View
14 src/Symfony/Component/Form/UnmodifiableFormConfig.php
@@ -43,6 +43,11 @@ class UnmodifiableFormConfig implements FormConfigInterface
private $mapped;
/**
+ * @var Boolean
+ */
+ private $virtual;
+
+ /**
* @var array
*/
private $types;
@@ -118,6 +123,7 @@ public function __construct(FormConfigInterface $config)
$this->name = $config->getName();
$this->propertyPath = $config->getPropertyPath();
$this->mapped = $config->getMapped();
+ $this->virtual = $config->getVirtual();
$this->types = $config->getTypes();
$this->clientTransformers = $config->getClientTransformers();
$this->normTransformers = $config->getNormTransformers();
@@ -167,6 +173,14 @@ public function getMapped()
/**
* {@inheritdoc}
*/
+ public function getVirtual()
+ {
+ return $this->virtual;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
public function getTypes()
{
return $this->types;
View
3  src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php
@@ -28,7 +28,6 @@ public function getChildren()
public function hasChildren()
{
- return $this->current()->hasAttribute('virtual')
- && $this->current()->getAttribute('virtual');
+ return $this->current()->getConfig()->getVirtual();
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.