Skip to content

[Form] Introduce validation events for forms #37641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Form] Introduce validation events for forms #37641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/Symfony/Component/Form/Extension/Validator/Event/PostValidateEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form\Extension\Validator\Event;

use Symfony\Component\Form\FormEvent;

/**
* This event is dispatched after validation completes.
*
* In this stage, the form will return a correct value to Form::isValid() and allow for
* further working with the form data.
*/
final class PostValidateEvent extends FormEvent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just asking, why not

Suggested change
final class PostValidateEvent extends FormEvent
final class PostValidationEvent extends FormEvent

?
same for PreValidateEvent and PreValidationEvent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've got a point. I'm not entirely convinced by "validate" either, but I'm open for ideas. Any opinions or objections to the suggested wording?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first i see the listener is called validationListener, thus suggest using validationEvent
but then, i saw the method is called validateForm, so maybe the word validate is ok

(i have in mind the workflow event logic, where event are named after the method) :)

{
public function isFormValid(): bool
{
return $this->getForm()->isValid();
}
}
29 changes: 29 additions & 0 deletions src/Symfony/Component/Form/Extension/Validator/Event/PreValidateEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form\Extension\Validator\Event;

use Symfony\Component\Form\FormEvent;

/**
* This event is dispatched before validation begins.
*
* In this stage the model and view data may have been denormalized. Otherwise the form
* is desynchronized because transformation failed during submission.
*
* It can be used to fetch data after denormalization.
*
* The event attaches the current view data. To know whether this is the renormalized data
* or the invalid request data, call Form::isSynchronized() first.
*/
final class PreValidateEvent extends FormEvent
{
}
22 changes: 21 additions & 1 deletion src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@

namespace Symfony\Component\Form\Extension\Validator\EventListener;

use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Form\Extension\Validator\Constraints\Form;
use Symfony\Component\Form\Extension\Validator\Event\PostValidateEvent;
use Symfony\Component\Form\Extension\Validator\Event\PreValidateEvent;
use Symfony\Component\Form\Extension\Validator\FormValidationEvents;
use Symfony\Component\Form\Extension\Validator\ViolationMapper\ViolationMapperInterface;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\FormEvents;
Expand All @@ -27,6 +31,8 @@ class ValidationListener implements EventSubscriberInterface

private $violationMapper;

private $eventDispatcher;

/**
* {@inheritdoc}
*/
Expand All @@ -35,17 +41,27 @@ public static function getSubscribedEvents()
return [FormEvents::POST_SUBMIT => 'validateForm'];
}

public function __construct(ValidatorInterface $validator, ViolationMapperInterface $violationMapper)
public function __construct(ValidatorInterface $validator, ViolationMapperInterface $violationMapper, ?EventDispatcherInterface $eventDispatcher = null)
{
$this->validator = $validator;
$this->violationMapper = $violationMapper;

if (!$this->eventDispatcher) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be the following?

Suggested change
if (!$this->eventDispatcher) {
if (!$eventDispatcher) {

$this->eventDispatcher is never set at this point.

@trigger_error(sprintf('The "$eventDispatcher" argument to the "%s" constructor will be required in Symfony 6.0.', self::class));
}

$this->eventDispatcher = $eventDispatcher;
}

public function validateForm(FormEvent $event)
{
$form = $event->getForm();

if ($form->isRoot()) {
if ($this->eventDispatcher) {
$this->eventDispatcher->dispatch(new PreValidateEvent($event->getForm(), $event->getData()), FormValidationEvents::PRE_VALIDATE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any reason for these events to be root-only?

Copy link
Contributor Author

@alcaeus alcaeus Jul 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validation is only triggered for root forms, hence the checks here. The validator then traverses the form and its data.

Copy link
Contributor

@ro0NL ro0NL Jul 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given i have a custom form type listening to VALIDATE events

now, im nesting this form type somewhere else ...all of a sudden the listener isnt called anymore on the child type.

Im not sure that's expected, given validator service maps violations back to underlying form types, so each type has a PRE/POST validation knowledge imho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the issue is that this is form validation. Form validation is only triggered on root forms. You always have to attach validation listeners to the form that is being validated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

form validation bubbles down.

put different, why shouldnt i be able to listen to POST_VALIDATE in a child form to check for some synchronized errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Form validation does not bubble down. Validation does, in that the form validation metadata contains a Traverse constraint, which triggers validation for fields, child forms, and the entity. The POST_SUBMIT event for any non-root forms does not trigger validation, which is why there should be no form.(pre|post)_validate event

Copy link
Contributor

@ro0NL ro0NL Jul 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm yes, i agree. Im a bit worried the difference is too subtle, compared to current post-submit with prio >0.

Is there any reason to prefer the new events actually? other then cosmetics.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am kind of interested in this discussion - because my Bundle does exactly this. It works quite nicely.

The most important parts for me - and my project - were - it has to bubble down - because I want to be able to run code inside of child forms whenever the whole form was successfully validated.

And inside of the $event variable, you have to be able to access the current form (current child) and the root form. Otherwise, it would not be possible to detect what data to use for some action like uploading an image or persisting an entity etc.

Like I said. My bundle works like a charm. I like to see this inside the Symfony core (form component).
But if these success metrics are not given, I will stick to my bundle ;).

Anyways. I like to see that you guys put effort into this.

}

// Form groups are validated internally (FormValidator). Here we don't set groups as they are retrieved into the validator.
foreach ($this->validator->validate($form) as $violation) {
// Allow the "invalid" constraint to be put onto
Expand All @@ -54,6 +70,10 @@ public function validateForm(FormEvent $event)

$this->violationMapper->mapViolation($violation, $form, $allowNonSynchronized);
}

if ($this->eventDispatcher) {
$this->eventDispatcher->dispatch(new PostValidateEvent($event->getForm(), $event->getData()), FormValidationEvents::POST_VALIDATE);
}
}
}
}
32 changes: 32 additions & 0 deletions src/Symfony/Component/Form/Extension/Validator/FormValidationEvents.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Form\Extension\Validator;

final class FormValidationEvents
{
/**
* @see Event\PreValidateEvent
* @Event(Event\PreValidateEvent::class)
*/
const PRE_VALIDATE = 'form.pre_validate';

/**
* This event is dispatched after validation completes.
*
* In this stage, the form will return a correct value to Form::isValid() and allow for
* further working with the form data.
*
* @see Event\PostValidateEvent
* @Event(Event\PostValidateEvent::class)
*/
const POST_VALIDATE = 'form.post_validate';
}
38 changes: 35 additions & 3 deletions ...Symfony/Component/Form/Tests/Extension/Validator/EventListener/ValidationListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper;
use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint;
use Symfony\Component\Form\Extension\Validator\EventListener\ValidationListener;
use Symfony\Component\Form\Extension\Validator\FormValidationEvents;
use Symfony\Component\Form\Extension\Validator\ViolationMapper\ViolationMapper;
use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormBuilder;
Expand Down Expand Up @@ -67,21 +68,26 @@ protected function setUp(): void
$this->dispatcher = new EventDispatcher();
$this->factory = (new FormFactoryBuilder())->getFormFactory();
$this->validator = Validation::createValidator();
$this->listener = new ValidationListener($this->validator, new ViolationMapper());
$this->listener = new ValidationListener($this->validator, new ViolationMapper(), $this->dispatcher);
$this->message = 'Message';
$this->messageTemplate = 'Message template';
$this->params = ['foo' => 'bar'];
}

private function createForm($name = '', $compound = false)
private function createForm($name = '', $compound = false, ?object $listener = null)
{
$config = new FormBuilder($name, null, new EventDispatcher(), (new FormFactoryBuilder())->getFormFactory());
$config = new FormBuilder($name, null, $this->dispatcher, (new FormFactoryBuilder())->getFormFactory());
$config->setCompound($compound);

if ($compound) {
$config->setDataMapper(new PropertyPathMapper());
}

if ($listener) {
$config->addEventListener(FormValidationEvents::PRE_VALIDATE, [$listener, 'preValidate']);
$config->addEventListener(FormValidationEvents::POST_VALIDATE, [$listener, 'postValidate']);
}

return new Form($config);
}

Expand Down Expand Up @@ -136,6 +142,32 @@ public function testValidateWithEmptyViolationList()

$this->assertTrue($form->isValid());
}

public function testEventsAreDispatched()
{
$data = ['foo' => 'bar'];

$mock = $this->getMockBuilder('\stdClass')
->setMethods(['preValidate', 'postValidate'])
->getMock();
$mock->expects($this->once())
->method('preValidate')
->with($this->callback(function ($event) use ($data) {
return $data === $event->getData();
}));
$mock->expects($this->once())
->method('postValidate')
->with($this->callback(function ($event) use ($data) {
return $data === $event->getData();
}));

$form = $this->createForm('', false, $mock);
$form->submit($data);

$this->listener->validateForm(new FormEvent($form, $data));

$this->assertTrue($form->isValid());
}
}

class SubmittedNotSynchronizedForm extends Form
Expand Down