Skip to content

Commit a3f7d36

Browse files
nicolas-grekasnicolas-grekas
committed
bug #36865 [Form] validate subforms in all validation groups (xabbuh)
This PR was merged into the 3.4 branch. Discussion ---------- [Form] validate subforms in all validation groups | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36852 | License | MIT | Doc PR | Commits ------- b819d94d14 validate subforms in all validation groups
2 parents 52e1862 + 48ec675 commit a3f7d36

File tree

5 files changed

+61
-18
lines changed

5 files changed

+61
-18
lines changed

‎Extension/Validator/Constraints/FormValidator.php‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,16 @@ public function validate($form, Constraint $formConstraint)
6363
/** @var Constraint[] $constraints */
6464
$constraints = $config->getOption('constraints', []);
6565

66+
$hasChildren = $form->count() > 0;
67+
68+
if ($hasChildren && $form->isRoot()) {
69+
$this->resolvedGroups = new \SplObjectStorage();
70+
}
71+
6672
if ($groups instanceof GroupSequence) {
6773
// Validate the data, the form AND nested fields in sequence
6874
$violationsCount = $this->context->getViolations()->count();
6975
$fieldPropertyPath = \is_object($data) ? 'children[%s]' : 'children%s';
70-
$hasChildren = $form->count() > 0;
71-
$this->resolvedGroups = $hasChildren ? new \SplObjectStorage() : null;
7276

7377
foreach ($groups->groups as $group) {
7478
if ($validateDataGraph) {
@@ -86,20 +90,18 @@ public function validate($form, Constraint $formConstraint)
8690
// sequence recursively, thus some fields could fail
8791
// in different steps without breaking early enough
8892
$this->resolvedGroups[$field] = (array) $group;
89-
$validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $formConstraint);
93+
$fieldFormConstraint = new Form();
94+
$validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $fieldFormConstraint);
9095
}
9196
}
9297

9398
if ($violationsCount < $this->context->getViolations()->count()) {
9499
break;
95100
}
96101
}
97-
98-
if ($hasChildren) {
99-
// destroy storage at the end of the sequence to avoid memory leaks
100-
$this->resolvedGroups = null;
101-
}
102102
} else {
103+
$fieldPropertyPath = \is_object($data) ? 'children[%s]' : 'children%s';
104+
103105
if ($validateDataGraph) {
104106
$validator->atPath('data')->validate($data, null, $groups);
105107
}
@@ -125,6 +127,19 @@ public function validate($form, Constraint $formConstraint)
125127
}
126128
}
127129
}
130+
131+
foreach ($form->all() as $field) {
132+
if ($field->isSubmitted()) {
133+
$this->resolvedGroups[$field] = $groups;
134+
$fieldFormConstraint = new Form();
135+
$validator->atPath(sprintf($fieldPropertyPath, $field->getPropertyPath()))->validate($field, $fieldFormConstraint);
136+
}
137+
}
138+
}
139+
140+
if ($hasChildren && $form->isRoot()) {
141+
// destroy storage to avoid memory leaks
142+
$this->resolvedGroups = new \SplObjectStorage();
128143
}
129144
} elseif (!$form->isSynchronized()) {
130145
$childrenSynchronized = true;

‎Extension/Validator/ValidatorExtension.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
use Symfony\Component\Form\AbstractExtension;
1515
use Symfony\Component\Form\Extension\Validator\Constraints\Form;
16-
use Symfony\Component\Validator\Constraints\Valid;
16+
use Symfony\Component\Validator\Constraints\Traverse;
1717
use Symfony\Component\Validator\Mapping\ClassMetadata;
1818
use Symfony\Component\Validator\Validator\ValidatorInterface;
1919

@@ -37,7 +37,7 @@ public function __construct(ValidatorInterface $validator)
3737

3838
/* @var $metadata ClassMetadata */
3939
$metadata->addConstraint(new Form());
40-
$metadata->addPropertyConstraint('children', new Valid());
40+
$metadata->addConstraint(new Traverse(false));
4141

4242
$this->validator = $validator;
4343
}

‎Resources/config/validation.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66

77
<class name="Symfony\Component\Form\Form">
88
<constraint name="Symfony\Component\Form\Extension\Validator\Constraints\Form" />
9-
<property name="children">
10-
<constraint name="Valid" />
11-
</property>
9+
<constraint name="Symfony\Component\Validator\Constraints\Traverse">
10+
<option name="traverse">false</option>
11+
</constraint>
1212
</class>
1313
</constraint-mapping>

‎Tests/Extension/Validator/Constraints/FormValidatorTest.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,8 @@ public function testViolationIfExtraData()
615615

616616
$this->assertTrue($form->isSubmitted());
617617
$this->assertTrue($form->isSynchronized());
618-
$this->expectNoValidate();
618+
619+
$this->expectValidateValueAt(0, 'children[child]', $form->get('child'), new Form());
619620

620621
$this->validator->validate($form, new Form());
621622

@@ -638,7 +639,8 @@ public function testViolationFormatIfMultipleExtraFields()
638639

639640
$this->assertTrue($form->isSubmitted());
640641
$this->assertTrue($form->isSynchronized());
641-
$this->expectNoValidate();
642+
643+
$this->expectValidateValueAt(0, 'children[child]', $form->get('child'), new Form());
642644

643645
$this->validator->validate($form, new Form());
644646

‎Tests/Extension/Validator/ValidatorExtensionTest.php‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,8 @@ public function test2Dot5ValidationApi()
5454
$this->assertInstanceOf(FormConstraint::class, $metadata->getConstraints()[0]);
5555

5656
$this->assertSame(CascadingStrategy::NONE, $metadata->cascadingStrategy);
57-
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->traversalStrategy);
58-
$this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy);
59-
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy);
57+
$this->assertSame(TraversalStrategy::NONE, $metadata->traversalStrategy);
58+
$this->assertCount(0, $metadata->getPropertyMetadata('children'));
6059
}
6160

6261
public function testDataConstraintsInvalidateFormEvenIfFieldIsNotSubmitted()
@@ -138,6 +137,33 @@ public function testFieldsValidateInSequenceWithNestedGroupsArray()
138137
$this->assertInstanceOf(Length::class, $errors[1]->getCause()->getConstraint());
139138
}
140139

140+
public function testConstraintsInDifferentGroupsOnSingleField()
141+
{
142+
$form = $this->createForm(FormType::class, null, [
143+
'validation_groups' => new GroupSequence(['group1', 'group2']),
144+
])
145+
->add('foo', TextType::class, [
146+
'constraints' => [
147+
new NotBlank([
148+
'groups' => ['group1'],
149+
]),
150+
new Length([
151+
'groups' => ['group2'],
152+
'max' => 3,
153+
]),
154+
],
155+
]);
156+
$form->submit([
157+
'foo' => '[email protected]',
158+
]);
159+
160+
$errors = $form->getErrors(true);
161+
162+
$this->assertFalse($form->isValid());
163+
$this->assertCount(1, $errors);
164+
$this->assertInstanceOf(Length::class, $errors[0]->getCause()->getConstraint());
165+
}
166+
141167
private function createForm($type, $data = null, array $options = [])
142168
{
143169
$validator = Validation::createValidatorBuilder()

0 commit comments

Comments
 (0)