-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Form Validation and Entity are not playing well together #14203
Labels
Comments
Jeckerson
added
4.1.1
The issues we want to solve in the 4.1.1 release
and removed
4.1.0
labels
Dec 14, 2020
Jeckerson
added
5.0
The issues we want to solve in the 5.0 release
and removed
4.1.1
The issues we want to solve in the 4.1.1 release
labels
Mar 26, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 2, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 2, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 2, 2021
5 tasks
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 3, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 4, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 4, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 4, 2021
5 tasks
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
BeMySlaveDarlin
pushed a commit
to BeMySlaveDarlin/cphalcon
that referenced
this issue
Nov 7, 2021
Resolved in #15752 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
If you pass both data and entity parameters to
$form->isValid()
, even though you expect$validation->getValue($field)
to fallback to data if entity does not have that value (like From does), it doesn't happen. Validation will not try to read value from any other data source if entity is present.For an example if you implemented CSRF check through validation, that wont work. That is, unless you have some sick, twisted desire to have a CSRF getter in your entities.
If you split
dateOfBirth
into three fields, glue them back and set to entity after validation, and you attach i.e. presence validators to those fields, the form will still fail validation if you didn't implement getters for those three fields in your entity. That's shame because it forces you to polute the entity with getters you don't need as we can utilize form's getters for displaying calculated values i.e.$form->getMonth()
.cphalcon/phalcon/validation.zep
Lines 463 to 524 in 6518334
Another thing that is giving me troubles is the fact that
$form->isValid($data, $entity)
automatically calls$this->bind($data, $entity)
but doesn't accept whitelist parameter, forcing you to call$form->isValid($data)
and then$form->bind($data, $entity)
, which in turn makes the entity unavailable to validators because$form->isValid()
passes entity to validation class only if it was provided to the function directly, it does not pass entity from$this->_entity
as a fallback.So my proposal is following:
Rewrite
Phalcon\Validation::getValue()
method so that it tries to retrieve value from the array as well, before it gives up when it can't find one in the entity. But it should not write back filtered values if they came from an array while entity is present, as validation is unaware of whitelisting.Rewrite
Phalcon\Forms\Form::isValid()
to accept whitelist as a third parameter which will be passed ontoPhalcon\Forms\Form::bind()
The text was updated successfully, but these errors were encountered: