8000 bug #10567 [Form] Disabled validation/violation mapping of unsubmitte… · symfony/symfony@bafced6 · GitHub
[go: up one dir, main page]
Skip to content

Commit bafced6

Browse files
fabpotfabpot
committed
bug #10567 [Form] Disabled validation/violation mapping of unsubmitted forms (webmozart)
This PR was merged into the 2.5-dev branch. Discussion ---------- [Form] Disabled validation/violation mapping of unsubmitted forms | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #9998 | License | MIT | Doc PR | - This PR fixes submissions of PATCH form in the case that any of the non-submitted forms causes an error (such as not-null). Commits ------- 9dfebd5 [Form] Disabled violation mapping of unsubmitted forms
2 parents 95f8e43 + 9dfebd5 commit bafced6

File tree

5 files changed

+73
-29
lines changed

5 files changed

+73
-29
lines changed

‎src/Symfony/Component/Form/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ CHANGELOG
1111
* [BC BREAK] added two optional parameters to FormInterface::getErrors() and
1212
changed the method to return a Symfony\Component\Form\FormErrorIterator
1313
instance instead of an array
14+
* errors mapped to unsubmitted forms are discarded now
1415

1516
2.4.0
1617
-----

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

Lines changed: 4 additions & 1 deletion
8000
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,9 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
294294
*/
295295
private function acceptsErrors(FormInterface $form)
296296
{
297-
return $this->allowNonSynchronized || $form->isSynchronized();
297+
// Ignore non-submitted forms. This happens, for example, in PATCH
298+
// requests.
299+
// https://github.com/symfony/symfony/pull/10567
300+
return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
298301
}
299302
}

‎src/Symfony/Component/Form/Form.php

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -556,8 +556,10 @@ public function submit($submittedData, $clearMissing = true)
556556
}
557557

558558
foreach ($this->children as $name => $child) {
559-
if (array_key_exists($name, $submittedData) || $clearMissing) {
560-
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
559+
$isSubmitted = array_key_exists($name, $submittedData);
560+
561+
if ($isSubmitted || $clearMissing) {
562+
$child->submit($isSubmitted ? $submittedData[$name] : null, $clearMissing);
561563
unset($submittedData[$name]);
562564

563565
if (null !== $this->clickedButton) {
@@ -739,18 +741,12 @@ public function isValid()
739741
return false;
740742
}
741743

742-
if (count($this->errors) > 0) {
743-
return false;
744-
}
745-
746744
if ($this->isDisabled()) {
747745
return true;
748746
}
749747

750-
foreach ($this->children as $child) {
751-
if ($child->isSubmitted() && !$child->isValid()) {
752-
return false;
753-
}
748+
if (< 6D40 span class=pl-en>count($this->getErrors(true)) > 0) {
749+
return false;
754750
}
755751

756752
return true;

‎src/Symfony/Component/Form/Tests/CompoundFormTest.php

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,6 @@ public function testInvalidIfChildIsInvalid()
5151
$this->assertFalse($this->form->isValid());
5252
}
5353

54-
public function testValidIfChildIsNotSubmitted()
55-
{
56-
$this->form->add($this->getBuilder('firstName')->getForm());
57-
$this->form->add($this->getBuilder('lastName')->getForm());
58-
59-
$this->form->submit(array(
60-
'firstName' => 'Bernhard',
61-
));
62-
63-
// "lastName" is not "valid" because it was not submitted. This happens
64-
// for example in PATCH requests. The parent form should still be
65-
// considered valid.
66-
67-
$this->assertTrue($this->form->isValid());
68-
}
69-
7054
public function testDisabledFormsValidEvenIfChildrenInvalid()
7155
{
7256
$form = $this->getBuilder('person')

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

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ public function testMapToFormInheritingParentDataIfDataDoesNotMatch()
125125
$parent->add($child);
126126
$child->add($grandChild);
127127

128+
$parent->submit(array());
129+
128130
$this->mapper->mapViolation($violation, $parent);
129131

130132
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
@@ -150,6 +152,8 @@ public function testFollowDotRules()
150152
$child->add($grandChild);
151153
$grandChild->add($grandGrandChild);
152154

155+
$parent->submit(array());
156+
153157
$this->mapper->mapViolation($violation, $parent);
154158

155159
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
@@ -170,7 +174,7 @@ public function testAbortMappingIfNotSynchronized()
170174
$parent->add($child);
171175
$child->add($grandChild);
172176

173-
// submit to invoke the transformer and mark the form unsynchronized
177+
// invoke the transformer and mark the form unsynchronized
174178
$parent->submit(array());
175179

176180
$this->mapper->mapViolation($violation, $parent);
@@ -194,7 +198,7 @@ public function testAbortDotRuleMappingIfNotSynchronized()
194198
$parent->add($child);
195199
$child->add($grandChild);
196200

197-
// submit to invoke the transformer and mark the form unsynchronized
201+
// invoke the transformer and mark the form unsynchronized
198202
$parent->submit(array());
199203

200204
$this->mapper->mapViolation($violation, $parent);
@@ -204,6 +208,54 @@ public function testAbortDotRuleMappingIfNotSynchronized()
204208
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
205209
}
206210

211+
public function testAbortMappingIfNotSubmitted()
212+
{
213+
$violation = $this->getConstraintViolation('children[address].data.street');
214+
$parent = $this->getForm('parent');
215+
$child = $this->getForm('address', 'address');
216+
$grandChild = $this->getForm('street' , 'street');
217+
218+
$parent->add($child);
219+
$child->add($grandChild);
220+
221+
// Disable automatic submission of missing fields
222+
$parent->submit(array(), false);
223+
$child->submit(array(), false);
224+
225+
// $grandChild is not submitted
226+
227+
$this->mapper->mapViolation($violation, $parent);
228+
229+
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
230+
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
231+
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
232+
}
233+
234+
public function testAbortDotRuleMappingIfNotSubmitted()
235+
{
236+
$violation = $this->getConstraintViolation('data.address');
237+
$parent = $this->getForm('parent');
238+
$child = $this->getForm('address', 'address', null, array(
239+
'.' => 'street',
240+
));
241+
$grandChild = $this->getForm('street');
242+
243+
$parent->add($child);
244+
$child->add($grandChild);
245+
246+
// Disable automatic submission of missing fields
247+
$parent->submit(array(), false);
248+
$child->submit(array(), false);
249+
250+
// $grandChild is not submitted
251+
252+
$this->mapper->mapViolation($violation, $parent);
253+
254+
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
255+
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
256+
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
257+
}
258+
207259
public function provideDefaultTests()
208260
{
209261
// The mapping must be deterministic! If a child has the property path "[street]",
@@ -743,6 +795,8 @@ public function testDefaultErrorMapping($target, $childName, $childPath, $grandC
743795
$parent->add($child);
744796
$child->add($grandChild);
745797

798+
$parent->submit(array());
799+
746800
$this->mapper->mapViolation($violation, $parent);
747801

748802
if (self::LEVEL_0 === $target) {
@@ -1211,6 +1265,8 @@ public function testCustomDataErrorMapping($target, $mapFrom, $mapTo, $childName
12111265
$parent->add($distraction);
12121266
}
12131267

1268+
$parent->submit(array());
1269+
12141270
$this->mapper->mapViolation($violation, $parent);
12151271

12161272
if ($target !== self::LEVEL_0) {
@@ -1396,6 +1452,8 @@ public function testCustomFormErrorMapping($target, $mapFrom, $mapTo, $errorName
13961452
$parent->add($errorChild);
13971453
$child->add($grandChild);
13981454

1455+
$parent->submit(array());
1456+
13991457
$this->mapper->mapViolation($violation, $parent);
14001458

14011459
if (self::LEVEL_0 === $target) {
@@ -1459,6 +1517,8 @@ public function testErrorMappingForFormInheritingParentData($target, $childName,
14591517
$parent->add($child);
14601518
$child->add($grandChild);
14611519

1520+
$parent->submit(array());
1521+
14621522
$this->mapper->mapViolation($violation, $parent);
14631523

14641524
if (self::LEVEL_0 === $target) {

0 commit comments

Comments
 (0)
0