8000 [Form] Fixed forms not to be marked invalid if their children are alr… · symfony/symfony@4909bc3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4909bc3

Browse files
webmozartwebmozart
committed
[Form] Fixed forms not to be marked invalid if their children are already marked invalid
1 parent 9c38e76 commit 4909bc3

File tree

2 files changed

+59
-11
lines changed

2 files changed

+59
-11
lines changed

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

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,35 @@ public function validate($form, Constraint $constraint)
8080
}
8181
}
8282
} else {
83-
$clientDataAsString = is_scalar($form->getViewData())
84-
? (string) $form->getViewData()
85-
: gettype($form->getViewData());
83+
$childrenSynchronized = true;
8684

87-
// Mark the form with an error if it is not synchronized
88-
$this->context->addViolation(
89-
$config->getOption('invalid_message'),
90-
array_replace(array('{{ value }}' => $clientDataAsString), $config->getOption('invalid_message_parameters')),
91-
$form->getViewData(),
92-
null,
93-
Form::ERR_INVALID
94-
);
85+
foreach ($form as $child) {
86+
if (!$child->isSynchronized()) {
87+
$childrenSynchronized = false;
88+
break;
89+
}
90+
}
91+
92+
// Mark the form with an error if it is not synchronized BUT all
93+
// of its children are synchronized. If any child is not
94+
// synchronized, an error is displayed there already and showing
95+
// a second error in its parent form is pointless, or worse, may
96+
// lead to duplicate errors if error bubbling is enabled on the
97+
// child.
98+
// See also https://github.com/symfony/symfony/issues/4359
99+
if ($childrenSynchronized) {
100+
$clientDataAsString = is_scalar($form->getViewData())
101+
? (string) $form->getViewData()
102+
: gettype($form->getViewData());
103+
104+
$this->context->addViolation(
105+
$config->getOption('invalid_message'),
106+
array_replace(array('{{ value }}' => $clientDataAsString), $config->getOption('invalid_message_parameters')),
107+
$form->getViewData(),
108+
null,
109+
Form::ERR_INVALID
110+
);
111+
}
95112
}
96113

97114
// Mark the form with an error if it contains extra fields

‎src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,37 @@ function () { throw new TransformationFailedException(); }
250250
$this->validator->validate($form, new Form());
251251
}
252252

253+
// https://github.com/symfony/symfony/issues/4359
254+
public function testDontMarkInvalidIfAnyChildIsNotSynchronized()
255+
{
256+
$context = $this->getExecutionContext();
257+
$object = $this->getMock('\stdClass');
258+
259+
$failingTransformer = new CallbackTransformer(
260+
function ($data) { return $data; },
261+
function () { throw new TransformationFailedException(); }
262+
);
263+
264+
$form = $this->getBuilder('name', '\stdClass')
265+
->setData($object)
266+< AC76 div class="diff-text-inner"> ->addViewTransformer($failingTransformer)
267+
->setCompound(true)
268+
->setDataMapper($this->getDataMapper())
269+
->add(
270+
$this->getBuilder('child')
271+
->addViewTransformer($failingTransformer)
272+
)
273+
->getForm();
274+
275+
// Launch transformer
276+
$form->bind(array('child' => 'foo'));
277+
278+
$this->validator->initialize($context);
279+
$this->validator->validate($form, new Form());
280+
281+
$this->assertCount(0, $context->getViolations());
282+
}
283+
253284
public function testHandleCallbackValidationGroups()
254285
{
255286
$context = $this->getExecutionContext();

0 commit comments

Comments
 (0)
0