codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.38k stars 1.9k forks source link

Array Validation Fails #4510

Closed luispastendev closed 3 years ago

luispastendev commented 3 years ago

hi there!

validation with more than 1 level only validates the first match.

Example:

// rule
'bar.*.foo' => [
    'rules' => 'required'
]

// data
"bar" => [
  [
    'foo' => 'baz'
  ],
  [
    'foo' => ''
  ]
]

the above validation should fails, however it passes. https://codeigniter4.github.io/userguide/libraries/validation.html#validating-keys-that-are-arrays

WinterSilence commented 3 years ago

@luispastendev it's wrong record, can you public your code?

'bar.*.foo' => [
    'rules' => 'required'
]
luispastendev commented 3 years ago

@WinterSilence you can test with this code

$validation = Services::Validation();

if(!$validation->setRules([
    'bar.*.foo' => [
        'rules' => 'required'
    ]
])->run([
    "bar" => [
        [
            'foo' => 'baz'
        ],
        [
            'foo' => ''
        ]
    ]
])){
    return $this->fail($validation->getErrors());
}

return $this->respond('ok!');

https://github.com/codeigniter4/CodeIgniter4/blob/03c5091ecc4d6e20388e617fdc75bfd2534f5233/system/Validation/Validation.php#L155

I was reviewing the code but it seems that it is only supported for selectors that end with *, but the documentation also mentions the possibility of doing something like the example above

If it is not a bug, I suppose that you should delete that text from the documentation

image

link: https://codeigniter4.github.io/userguide/libraries/validation.html#validating-keys-that-are-arrays

WinterSilence commented 3 years ago
$validation->setRules(['bar.*.foo' => 'required']);

not same to

->setRules(['bar.*.foo' => ['rules' => 'required']])
luispastendev commented 3 years ago

@WinterSilence the same result

WinterSilence commented 3 years ago

@luispastendev CI 4.1.1:

$result = \Config\Services::validation()
    ->setRules(['bar.*.foo' => 'required'])
    ->run([
        "bar" => [
            ['foo' => 'baz'],
            ['foo' => '']
        ]
    ]);
$result2 = \Config\Services::validation()
    ->setRules(['bar.*.foo' => 'required'])
    ->run([
        "bar" => [
            ['foo' => 'baz'],
            ['foo' => 'bar']
        ]
    ]);
$result3 = \Config\Services::validation()
    ->setRules(['bar.*.foo' => 'required'])
    ->run([
        "bar" => [
            ['foo2' => 'baz'],
            ['foo2' => 'bar']
        ]
    ]);
var_export([$result, $result2, $result3]); //  true, true, false
luispastendev commented 3 years ago

@WinterSilence I think the required rule should fail if an empty value is sent. taking your examples if you try the following fails and under what you explain it should return true

$result4 = \Config\Services::validation()
    ->setRules(['bar.*.foo' => 'required'])
    ->run([
        "bar" => [
            ['foo' => ''],
            ['foo' => '']
        ]
    ]);
    var_dump($result4); // currently returns false 

doc:

image

WinterSilence commented 3 years ago

@luispastendev all right, i fix my comment later

WinterSilence commented 3 years ago

@luispastendev but method really have other bug:

$result = \Config\Services::validation()
    ->setRules(['bar.*' => 'required'])
    ->run(["bar" => []]); // return true
luispastendev commented 3 years ago

@WinterSilence right! it only works if it detects something empty [''] that seems to be fixed with:

https://github.com/codeigniter4/CodeIgniter4/blob/03c5091ecc4d6e20388e617fdc75bfd2534f5233/system/Validation/Validation.php#L155

by the following:

if (is_array($value) && end($fieldNameToken) === '*' && ! empty($value))

but the other problem I do not know what others think at least reading the code I do not see an intention to support that use case