8000 do not validate non-submitted form fields in PATCH requests · xabbuh/symfony@c86c49c · GitHub
[go: up one dir, main page]
Skip to content

Commit c86c49c

Browse files
xabbuhxabbuh
committed
do not validate non-submitted form fields in PATCH requests
When a form field is not embedded as part of a HTTP PATCH requests, its validation constraints configured through the `constraints` option must not be evaluated. The fix from symfony#10567 achieved this by not mapping their violations to the underlying form field. This however also means that constraint violations caused by validating the whole underlying data object will never cause the form to be invalid. This breaks use cases where some constraints may, for example, depend on the value of other properties that were changed by the submitted data.
1 parent 823ccf0 commit c86c49c

File tree

4 files changed

+72
-10
lines changed

4 files changed

+72
-10
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function validate($form, Constraint $formConstraint)
4141

4242
$validator = $this->context->getValidator()->inContext($this->context);
4343

44-
if ($form->isSynchronized()) {
44+
if ($form->isSubmitted() && $form->isSynchronized()) {
4545
// Validate the form data only if transformation succeeded
4646
$groups = self::getValidationGroups($form);
4747
$data = $form->getData();
@@ -90,7 +90,7 @@ public function validate($form, Constraint $formConstraint)
9090
}
9191
}
9292
}
93-
} else {
93+
} elseif (!$form->isSynchronized()) {
9494
$childrenSynchronized = true;
9595

9696
/** @var FormInterface $child */

‎src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,6 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
273273
*/
274274
private function acceptsErrors(FormInterface $form)
275275
{
276-
// Ignore non-submitted forms. This happens, for example, in PATCH
277-
// requests.
278-
// https://github.com/symfony/symfony/pull/10567
279-
return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
276+
return $this->allowNonSynchronized || $form->isSynchronized();
280277
}
281278
}

‎src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@
1212
namespace Symfony\Component\Form\Tests\Extension\Validator;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Form\AbstractType;
1516
use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint;
1617
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
1718
use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser;
1819
use Symfony\Component\Form\Form;
20+
use Symfony\Component\Form\FormBuilderInterface;
21+
use Symfony\Component\Form\FormFactoryBuilder;
22+
use Symfony\Component\OptionsResolver\OptionsResolver;
23+
use Symfony\Component\Validator\Constraints\NotBlank;
1924
use Symfony\Component\Validator\Mapping\CascadingStrategy;
2025
use Symfony\Component\Validator\Mapping\ClassMetadata;
2126
use Symfony\Component\Validator\Mapping\TraversalStrategy;
@@ -45,4 +50,64 @@ public function test2Dot5ValidationApi()
4550
$this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy);
4651
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy);
4752
}
53+
54+
public function testDataConstraintsInvalidateFormEvenIfFieldIsNotSubmitted()
55+
{
56+
$form = $this->createForm(FooType::class);
57+
$form->submit(['baz' => 'foobar'], false);
58+
59+
$this->assertTrue($form->isSubmitted());
60+
$this->assertFalse($form->isValid());
61+
$this->assertFalse($form->get('bar')->isSubmitted());
62+
$this->assertCount(1, $form->get('bar')->getErrors());
63+
}
64+
65+
public function testFieldConstraintsDoNotInvalidFormIfFieldIsNotSubmitted()
66+
{
67+
$form = $this->createForm(FooType::class);
68+
$form->submit(['bar' => 'foobar'], false);
69+
70+
$this->assertTrue($form->isSubmitted());
71+
$this->assertTrue($form->isValid());
72+
}
73+
74+
private function createForm($type)
75+
{
76+
$validator = Validation::createValidatorBuilder()
77+
->enableAnnotationMapping()
78+
->getValidator();
79+
$formFactoryBuilder = new FormFactoryBuilder();
80+
$formFactoryBuilder->addExtension(new ValidatorExtension($validator));
81+
$formFactory = $formFactoryBuilder->getFormFactory();
82+
83+
return $formFactory->create($type);
84+
}
85+
}
86+
87+
class Foo
88+
{
89+
/**
90+
* @NotBlank()
91+
*/
92+
public $bar;
93+
94+
public $baz;
95+
}
96+
97+
class FooType extends AbstractType
98+
{
99+
public function buildForm(FormBuilderInterface $builder, array $options)
100+
{
101+
$builder
102+
->add('bar')
103+
->add('baz', null, [
104+
'constraints' => [new NotBlank()],
105+
])
106+
;
107+
}
108+
109+
public function configureOptions(OptionsResolver $resolver)
110+
{
111+
$resolver->setDefault('data_class', Foo::class);
112+
}
48113
}

‎src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ public function testAbortDotRuleMappingIfNotSynchronized()
205205
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
206206
}
207207

208-
public function testAbortMappingIfNotSubmitted()
208+
public function testMappingIfNotSubmitted()
209209
{
210210
$violation = $this->getConstraintViolation('children[address].data.street');
211211
$parent = $this->getForm('parent');
@@ -225,10 +225,10 @@ public function testAbortMappingIfNotSubmitted()
225225

226226
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
227227
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
228-
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
228+
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
229229
}
230230

231-
public function testAbortDotRuleMappingIfNotSubmitted()
231+
public function testDotRuleMappingIfNotSubmitted()
232232
{
233233
$violation = $this->getConstraintViolation('data.address');
234234
$parent = $this->getForm('parent');
@@ -250,7 +250,7 @@ public function testAbortDotRuleMappingIfNotSubmitted()
250250

251251
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
252252
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
253-
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
253+
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
254254
}
255255

256256
public function provideDefaultTests()

0 commit comments

Comments
 (0)
0