Skip to content

Commit f28eb9a

Browse files
fabpotfabpot
committed
bug #18935 [Form] Consider a violation even if the form is not submitted (egeloen)
This PR was merged into the 2.7 branch. Discussion ---------- [Form] Consider a violation even if the form is not submitted | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | yes (only for the behavior) | Deprecations? | no | Tests pass? | yes | Fixed tickets | #11493 | License | MIT | Doc PR | Hey! I'm currently implementing an API using the form component in order to validate the payload sent (in conjonction with the FOSRestBundle). Unfortunatelly, we dig into an issue about the PATCH request which don't map some of our validation rules to the form. Basically, the violations are lost in the middle of the process. ### Use case We have an entity with the following fields "type", "image" & "video". The field "type"can be either "default", "image" or "video" and then accordingly we use the appropriate field (none for the "default" type, video for the "video" type and image for the "image" type. Then, in our form, we change the validation groups according to our entity type in order to make the "image" field mandatory if the type is "image" and the same for the video field if the type is "video". ### Current behavior The current behavior (since 2.5) seems to not propages a violation to a form if this form is not submitted but in our use case, changing the field "type" via a PATCH request triggers some new validation which should be reported to end user (inform that a field (video or image) is missing in the PATCH request). ### Expected behavior The current behavior was introduced in #10567 but IMO, this update is a bug as suggested by @webmozart in #11493 (comment) Instead, the form component should still map validation errors to the form even if the field was not submitted. If the initial data is not valid, then your initial data was buggy from the beginning but the form should not accept it and instead of silently ignoring the errors, end users should be informed and fix it. WDYT? Commits ------- c483a0f [Form] Consider a violation even if the form is not submitted
2 parents bbb75fa + c483a0f commit f28eb9a

File tree

2 files changed

+9
-12
lines changed

2 files changed

+9
-12
lines changed

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

+1-4
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,6 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
275275
*/
276276
private function acceptsErrors(FormInterface $form)
277277
{
278-
// Ignore non-submitted forms. This happens, for example, in PATCH
279-
// requests.
280-
// https://github.com/symfony/symfony/pull/10567
281-
return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
278+
return $this->allowNonSynchronized || $form->isSynchronized();
282279
}
283280
}

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

+8-8
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public function testAbortDotRuleMappingIfNotSynchronized()
212212
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
213213
}
214214

215-
public function testAbortMappingIfNotSubmitted()
215+
public function testMappingIfNotSubmitted()
216216
{
217217
$violation = $this->getConstraintViolation('children[address].data.street');
218218
$parent = $this->getForm('parent');
@@ -230,12 +230,12 @@ public function testAbortMappingIfNotSubmitted()
230230

231231
$this->mapper->mapViolation($violation, $parent);
232232

233-
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
234-
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
235-
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
233+
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error');
234+
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error');
235+
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
236236
}
237237

238-
public function testAbortDotRuleMappingIfNotSubmitted()
238+
public function testDotRuleMappingIfNotSubmitted()
239239
{
240240
$violation = $this->getConstraintViolation('data.address');
241241
$parent = $this->getForm('parent');
@@ -255,9 +255,9 @@ public function testAbortDotRuleMappingIfNotSubmitted()
255255

256256
$this->mapper->mapViolation($violation, $parent);
257257

258-
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
259-
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
260-
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
258+
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error');
259+
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error');
260+
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have an error');
261261
}
262262

263263
public function provideDefaultTests()

0 commit comments

Comments
 (0)