Skip to content

Commit d4ac467

Browse files
committed
bug symfony#17099 [Form] Fixed violation mapping if multiple forms are using the same (or part of the same) property path (alekitto)
This PR was squashed before being merged into the 2.3 branch (closes symfony#17099). Discussion ---------- [Form] Fixed violation mapping if multiple forms are using the same (or part of the same) property path | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#5656 | License | MIT | Doc PR | Commits ------- f005c80 [Form] Fixed violation mapping if multiple forms are using the same (or part of the same) property path
2 parents a966a88 + f005c80 commit d4ac467

File tree

2 files changed

+41
-35
lines changed

2 files changed

+41
-35
lines changed

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

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,9 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form
147147
*/
148148
private function matchChild(FormInterface $form, PropertyPathIteratorInterface $it)
149149
{
150-
// Remember at what property path underneath "data"
151-
// we are looking. Check if there is a child with that
152-
// path, otherwise increase path by one more piece
150+
$target = null;
153151
$chunk = '';
154-
$foundChild = null;
155-
$foundAtIndex = 0;
152+
$foundAtIndex = null;
156153

157154
// Construct mapping rules for the given form
158155
$rules = array();
@@ -164,17 +161,11 @@ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $
164161
}
165162
}
166163

167-
// Skip forms inheriting their parent data when iterating the children
168-
$childIterator = new \RecursiveIteratorIterator(
164+
$children = iterator_to_array(new \RecursiveIteratorIterator(
169165
new InheritDataAwareIterator($form)
170-
);
171-
172-
// Make the path longer until we find a matching child
173-
while (true) {
174-
if (!$it->valid()) {
175-
return;
176-
}
166+
));
177167

168+
while ($it->valid()) {
178169
if ($it->isIndex()) {
179170
$chunk .= '['.$it->current().']';
180171
} else {
@@ -196,33 +187,27 @@ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $
196187
}
197188
}
198189

199-
// Test children unless we already found one
200-
if (null === $foundChild) {
201-
foreach ($childIterator as $child) {
202-
/* @var FormInterface $child */
203-
$childPath = (string) $child->getPropertyPath();
204-
205-
// Child found, mark as return value
206-
if ($chunk === $childPath) {
207-
$foundChild = $child;
208-
$foundAtIndex = $it->key();
209-
}
190+
/** @var FormInterface $child */
191+
foreach ($children as $key => $child) {
192+
$childPath = (string) $child->getPropertyPath();
193+
if ($childPath === $chunk) {
194+
$target = $child;
195+
$foundAtIndex = $it->key();
196+
} elseif (0 === strpos($childPath, $chunk)) {
197+
continue;
210198
}
199+
200+
unset($children[$key]);
211201
}
212202

213-
// Add element to the chunk
214203
$it->next();
204+
}
215205

216-
// If we reached the end of the path or if there are no
217-
// more matching mapping rules, return the found child
218-
if (null !== $foundChild && (!$it->valid() || count($rules) === 0)) {
219-
// Reset index in case we tried to find mapping
220-
// rules further down the path
221-
$it->seek($foundAtIndex);
222-
223-
return $foundChild;
224-
}
206+
if (null !== $foundAtIndex) {
207+
$it->seek($foundAtIndex);
225208
}
209+
210+
return $target;
226211
}
227212

228213
/**

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,4 +1474,25 @@ public function testErrorMappingForFormInheritingParentData($target, $childName,
14741474
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChildName.' should have an error, but has none');
14751475
}
14761476
}
1477+
1478+
public function testBacktrackIfSeveralSubFormsWithSamePropertyPath()
1479+
{
1480+
$violation = $this->getConstraintViolation('data.address[street]');
1481+
$parent = $this->getForm('parent');
1482+
$child1 = $this->getForm('subform1', 'address');
1483+
$child2 = $this->getForm('subform2', 'address');
1484+
$grandChild = $this->getForm('street');
1485+
1486+
$parent->add($child1);
1487+
$parent->add($child2);
1488+
$child2->add($grandChild);
1489+
1490+
$this->mapper->mapViolation($violation, $parent);
1491+
1492+
// The error occurred on the child of the second form with the same path
1493+
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
1494+
$this->assertCount(0, $child1->getErrors(), $child1->getName().' should not have an error, but has one');
1495+
$this->assertCount(0, $child2->getErrors(), $child2->getName().' should not have an error, but has one');
1496+
$this->assertEquals(array($this->getFormError()), $grandChild->getErrors(), $grandChild->getName().' should have an error, but has none');
1497+
}
14771498
}

0 commit comments

Comments
 (0)