[Form] Introduce validation events #47210

Seb33300 · 2022-08-06T10:45:55Z

Q	A
Branch?	6.2
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #47046
License	MIT
Doc PR	symfony/symfony-docs#17229

Inspired from #38479 (@alcaeus, I coud not reuse your commit because you deleted your branch)

With some changes:

I removed the BC break
form.pre_validate and form.post_validate events are triggered individually to all Form items, not only the to the root form (same as all others existing form events)

Regarding the second point, for example, when working with nested forms, if the form.post_validate event is attached to a child form:

$event->getForm()->isValid() will return the validity for the current form item only (the nested one)
If you want to check if the entire form is valid form a nested form, you will have to check the root form: $event->getForm()->getRoot()->isValid()

Usage example:

Thy are working in the same way as existing form events work.

class MyFormType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        $builder
            ->addEventListener(ValidatorFormEvents::PRE_VALIDATE, function (FormEvent $event) {
                // ...
            })
            ->addEventListener(ValidatorFormEvents::POST_VALIDATE, function (FormEvent $event) {
                $event->getForm()->isValid(); // Check if current form item is valid
                $event->getForm()->getRoot()->isValid(); // Check if entire form is valid
            })
        ;
    }
}

Why are those events required?
Can't we just add a POST_SUBMIT event with a lower priority than the validation event?
Yes, this solution can work, but ONLY on the root form.
Because the validation of the form is triggered only by the root form, and events are triggered individually on all form items, starting from ancestors and ending by the root form.
This means, we currently have no way to know if a nested form is valid when attaching a POST_SUBMIT event on it.

Seb33300 · 2022-08-06T11:15:42Z

In case you want to review, I saw you participated to the previous PR
@d-ph @silasjoisten

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php

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

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php

d-ph · 2022-08-07T09:55:13Z

Hi.

Just wanted to say that I'm happy to see this feature gaining traction, and looking forward to seeing it in the stock Symfony code. Unfortunately, it's unlikely I'd review the PR in depth, however I did skim through the change, and it looks good to me.

May I ask a quick question though, just to double-check: Given the following case:

A root form foo listening on the postValidate event.
A child form fooChild also listenting on the postValidate event.
The form foo is being submitted.

Then:

fooChild's postValidate is going to be called first, and then foo's postValidate is going to be called second. I.e. The new events are dispatched recursively, deepest child form first?
If fooChild adds a new FormError on itself in the postValidate listener, the foo's postValidate listener will be able to see that there are validation errors by calling either: $event->getForm()->isValid(), or $event->getForm()->get('fooChild')->isValid()?

Thanks.

Btw Seb33300 search for "->idValid(" on this page and consider correcting the typos, because it takes a few seconds to conclude that it's a typo. Cheers.

Seb33300 · 2022-08-07T14:38:36Z

@d-ph typo fixed. Thanks.

POST_VALIDATE event happens AFTER the form validation has been processed ON ALL form items.

Yes, child form event will be dispatched first. The root form event will be the last one to be dispatched. (same order as others form events)
Yes, you can add a new error to the form using $event->getForm()->addError() inside this event. If you do so, the form will be considered invalid after that. When the event will be propagated to parent form, the parent form will also be considered as invalid. Because a form is valid only if all his children are valid.

yceruto

Thanks for this proposal. I've now a better idea about the problem to solve after reading your code and description. I left you one comment to improve the implementation based on your approach and the other two are mainly consistency-related concerns.

I'm wondering if dispatching these new events inside the FormValidator makes more sense, is more accurate, and simplifies the implementation too (as the collection of the form tree won't be needed).

Although I've to admit that I'm not entirely sure how useful this feature is or if we are opening the door to the writing of something completely detached from the Form context.

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php

yceruto · 2022-08-27T00:51:27Z

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php

        if ($form->isRoot()) {
+            $this->dispatchEvents(ValidatorFormEvents::PRE_VALIDATE);


I like the goal of this new event but the dispatching moment is a bit confusing to me. If I create a listener in a nested form type, I expect the event to be received just before the current form is validated in FormValidator and not before the whole validation process of all forms in the tree is started.

The meaning is different in this case from other events.

To be consistant with POST_VALIDATE event, I decided to dispatch it before the validation of the entire form start.
(see my other comment to know why POST_VALIDATE is dispatched after all form items are validated)

yceruto · 2022-08-27T00:51:27Z

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php

@@ -50,6 +64,24 @@ public function validateForm(FormEvent $event)

                $this->violationMapper->mapViolation($violation, $form, $allowNonSynchronized);
            }
+
+            $this->dispatchEvents(ValidatorFormEvents::POST_VALIDATE);


I understand that child forms can check easily whether their parent is valid or not, but I have the same confusion about the right moment this event is being dispatched. They are not done just after the current form is validated but when the whole validation process is finished for all forms.

This is the main purpose of this PR, to be able to know if the entire form is valid or not.
That's why the POST_VALIDATE event must be dispatched here.
If we dispatch the event just after the validation of the current item, we will only be able to know if the current item (and his parents) are valid.

Maybe we can introduce a 3rd event "VALIDATE" that could be dispatched just after the current form item is validated?

Or rename PRE/POST events by START/FINISH to avoid confusion?

Seb33300 requested review from xabbuh and yceruto as code owners August 6, 2022 10:45

carsonbot added Status: Needs Review Feature Form labels Aug 6, 2022

carsonbot added this to the 6.2 milestone Aug 6, 2022

Seb33300 force-pushed the form-validate-events branch from 64603b1 to 134ff4b Compare August 6, 2022 11:12

Seb33300 mentioned this pull request Aug 6, 2022

[Form] Adding valid & invalid form events #47046

Open

silasjoisten reviewed Aug 7, 2022

View changes

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php Show resolved Hide resolved

94noni reviewed Aug 7, 2022

View changes

src/Symfony/Component/Form/Extension/Validator/ValidatorFormEvents.php Outdated Show resolved Hide resolved

94noni reviewed Aug 7, 2022

View changes

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php Show resolved Hide resolved

Seb33300 mentioned this pull request Aug 26, 2022

[Form] Introduce validation events symfony/symfony-docs#17229

Open

yceruto reviewed Aug 27, 2022

View changes

Seb33300 added 4 commits August 27, 2022 23:38

[Form] Introduce validation events

91ac630

Fix comment

2113e82

Comments consistency

d34d434

Remove static property

e23b9d6

Seb33300 force-pushed the form-validate-events branch from 3e869d0 to e23b9d6 Compare August 27, 2022 16:40

nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022

[Form] Introduce validation events #47210

[Form] Introduce validation events #47210

Seb33300 commented Aug 6, 2022 •

edited

Seb33300 commented Aug 6, 2022

d-ph commented Aug 7, 2022 •

edited

Seb33300 commented Aug 7, 2022 •

edited

yceruto left a comment

yceruto Aug 27, 2022

Seb33300 Aug 27, 2022

yceruto Aug 27, 2022

Seb33300 Aug 27, 2022

Seb33300 Aug 27, 2022

		if ($form->isRoot()) {
		$this->dispatchEvents(ValidatorFormEvents::PRE_VALIDATE);

[Form] Introduce validation events #47210

Are you sure you want to change the base?

[Form] Introduce validation events #47210

Conversation

Seb33300 commented Aug 6, 2022 • edited

Seb33300 commented Aug 6, 2022

d-ph commented Aug 7, 2022 • edited

Seb33300 commented Aug 7, 2022 • edited

yceruto left a comment

Choose a reason for hiding this comment

yceruto Aug 27, 2022

Choose a reason for hiding this comment

Seb33300 Aug 27, 2022

Choose a reason for hiding this comment

yceruto Aug 27, 2022

Choose a reason for hiding this comment

Seb33300 Aug 27, 2022

Choose a reason for hiding this comment

Seb33300 Aug 27, 2022

Choose a reason for hiding this comment

Seb33300 commented Aug 6, 2022 •

edited

d-ph commented Aug 7, 2022 •

edited

Seb33300 commented Aug 7, 2022 •

edited