cphalcon icon indicating copy to clipboard operation
cphalcon copied to clipboard

[BUG]: allowEmpty for form validation does not work correctly when an entity is bound

Open wurst-hans opened this issue 3 years ago • 4 comments

Using Phalcon 4.1.2 on PHP 7.4.18. Took me a while to figure it out.

Having a simple form:

$form = new Form();
$form->add(
    (new Text('name'))->addValidators([
        new Validation\Validator\Alnum([
            'message' => 'Invalid value'
            'allowEmpty' => true,
        ]),
    ])
);
$company = new Company();
$form->bind($_POST, $company);
if ($form->isValid()) {
    // do something
}

No matter if $_POST contains a value for name or not, all is fine. But I can't use allowEmpty=true in my projects, because this is also true for string value 0, which is wrong IMHO. To not skip any validations, I have to use allowEmpty=[null, ''] instead. When modifying the above code to:

$form = new Form();
$form->add(
    (new Text('name'))->addValidators([
        new Validation\Validator\Alnum([
            'message' => 'Invalid value'
            'allowEmpty' => [null,''],
        ]),
    ])
);
$form->bind($_POST, $company);
if ($form->isValid()) {
    // is not reached for all cases
}

the form validation for Alnum gives following results now:

  • $_POST['name'] = 'Acme'; = success
  • $_POST['name'] = ''; = success
  • $_POST['name'] = null; = fails
  • having name absent from $_POST = fails

When the form is not bound to the model, the validation does not fail for the previously mentioned cases. When var_dump()ing the validation object, I can see, that the property of the model is not null but &null. There seems to be any internal references. Maybe that's the reason why comparing allowEmpty values (see here) fails.

I expect, that allowEmpty = [null, ''] works for bound entities on forms too, even when the related property in entity is null.

Update: Found something. When replacing

$form->bind($_POST, $company);
if ($form->isValid()) {}

with

if ($form->isValid($_POST, $company)) {}

The error seems to be gone. What's the difference when using bind() explicitely?

wurst-hans avatar May 15 '21 18:05 wurst-hans

Probably fixed in #15772

BeMySlaveDarlin avatar Nov 07 '21 13:11 BeMySlaveDarlin

@wurst-hans

  1. Can you check with latest 5.0 build?
  2. When you call bind before isValid 2.1. bind filters all values and stores in object cache properties as this->data and this->entity 2.2. isValid uses that props, if you do not pass arguments to function, and after that calls bind. 2.3. isValid uses provided values without pre-filtering, if you pass them as arguments

BeMySlaveDarlin avatar Nov 07 '21 18:11 BeMySlaveDarlin

v5.0 is available when self-compiling from source only, correct? Sorry, then I'm not able to test right now.

But when I understand the linked pull request correct, that's a different issue. I know, that I've talked about my personal antipathy against using empty(), but that was not the matter of concern for this thread. Sorry for the misunderstanding.

For me, the problem is the concept of binding and validating data and entities on forms. And that has arisen, because I couldn't explain the behavior of allowEmpty() between using:

$form->bind($_POST, $company);
if ($form->isValid()) {}

and

if ($form->isValid($_POST, $company)) {}

Looking at source of v4.1.2, now I think I understand what's going on:

  1. when initalizing a form providing an entity, it is stored in this->entity

  2. when binding data via bind($_POST, $entity) the data is bound to the given entity but not to the stored entity in this->entity, additionally data is stored in this->data nevertheless

  3. using isValid() without parameters, the code:

  • uses data that has been used in bind() before, but binds it to this->entity now
  • validation component is called with this data but without any entity now, because isValid() was called without parameters
  1. calling isValid($_POST, $entity) causes the code to:
  • bind data to $entity (so calling extra bind() is redundant)
  • call validation component with given entity from isValid(...) call

So, to summarize, the following is a bit irritating and non-working

$form = new Form($entity);
$form->bind($_POST, $entity);
if ($form->isValid()) {
}

Currently, the only correct working example is

$form = new Form($entity);
if ($form->isValid($_POST, $entity)) {
}

But, the correct working example should be:

$form = new Form($entity);
if ($form->isValid($_POST)) {
}

So isValid() should call validation component with initially stored entity from this->entity when no explicit entity is given in function call.

wurst-hans avatar Nov 29 '21 15:11 wurst-hans

@wurst-hans you can install v5 through PECL

pecl install phalcon-5.0.0-beta1

I will try and write a couple of tests to check your case.

niden avatar Jan 05 '22 18:01 niden