Skip to content

Add an other method to disable validation on Form #6461

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.

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 2 commits into from
Closed

Add an other method to disable validation on Form #6461

wants to merge 2 commits into from

Conversation

gilles-g
Copy link
Contributor

Q A
Doc fix? no
New docs? no
Applies to all
Fixed tickets -

Hi,
I think this method can be usefull for disable the group_validation.
The important question, is why set a unknow group on the group validation disable all validation, but set to false does not work?
What did you think?

@@ -735,3 +735,5 @@ all of this, use a listener::

By doing this, you may accidentally disable something more than just form
validation, since the ``POST_SUBMIT`` event may have other listeners.

An other solution is to set ``group_validation`` with an unknown group like ``disable_validation``.
Copy link
Contributor

Choose a reason for hiding this comment

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

An other -> Another

@xabbuh
Copy link
Member

xabbuh commented Apr 19, 2016

I am not happy with this suggestion. It's more a hack and will fail as soon as the group used here is introduced without thinking about using it here in a special way.

@gilles-g
Copy link
Contributor Author

Hi,
I known that it's look like a hack, but I'm not satisfied by the solution proposed by the documentation also. To disable all of this, use a listener, Caution! By doing this, you may accidentally disable something more than just form validation, since the POST_SUBMIT event may have other listeners.

What the difference between my propose and this?

Maybe we need to found a real method to disable validation, not here but an issue on the github symfony..

@weaverryan
Copy link
Member

I kind of like this, but I wonder the same thing as @Spike31 - why does setting groups to an non-existent group truly disable validation, but setting it to false doesn't. That's weird (but also out-of-scope for the docs, clearly). @Spike31 you're sure that works?

If It does work, I'd vote to only show that example. The event listener is also a hack.

But also, why are we disabling validation again? Does it hurt something to have it run?

@gilles-g
Copy link
Contributor Author

@weaverryan If you pass a non-existent group, the validator will search for asserts with that specific group, and if nothing exist so there is no validation.

We disable the validation because it's a submit in an ajax call. We just want to submit the form with a different value for the select and change the FormEvents::PRE_SET_DATA only, we don't care about the rest of the other fields in the call.

@HeahDude
Copy link
Contributor

Hi @Spike31 and thank you for this first contribution :).

To me this PR is wrong. The paragraph just above the added note says:

The reason for needing to do this is that even if you set ``group_validation``
 to ``false`` there  are still some integrity checks executed. For example
 an uploaded file will still be checked to see if it is too large and the form
 will still check to see if non-existing fields were submitted. To disable
 all of this, use a listener::

So first, I'd say this is not true about uploaded size, and the note does not change anything about that.

The max uploaded file triggers an error at the root level because it is handled by the request handler, and null is submitted, so no data is validated in such cases (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/NativeRequestHandler.php#L88). Not that children are not even submitted (because of the second argument of submit()), so no errors would be mapped on them anyway.

Second, the extra fields do trigger an error independently from the validation groups (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php#L111), so again this note does not change anything about that.

Lastly and out of scope here, "disabling" validation should be done by setting the validation_groups option to false or an empty array (which is false actually does internally). Setting it to a "random" string for this is a no-go in the docs for me.

To conclude, I propose to remove this paragraph since preventing the validation listener has no benefit:

  • handling max uploaded error is done by a request handler which can be replaced or decorated in user land,
  • validation groups and extra fields handling have theirs dedicated option for custom handling.

@javiereguiluz
Copy link
Member

Closing in favor of #8940.

weaverryan added a commit that referenced this pull request Dec 30, 2017
…guiluz)

This PR was merged into the 2.7 branch.

Discussion
----------

Reworded the note about supressing form validation

This replaces #6461 according to reviewer's comments.

@HeahDude please check if this PR is correct according to your comment here: #6461 (comment)

Commits
-------

cb823f7 Reworded the note about supressing form validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants