zendframework / zend-inputfilter

InputFilter component from Zend Framework
BSD 3-Clause "New" or "Revised" License
64 stars 50 forks source link

Missing required failure message for CollectionInputFilter #94

Closed Maks3w closed 5 years ago

Maks3w commented 8 years ago

CollectionInputFilter fails without error message when is required but data is empty or not set.

n3vrax commented 7 years ago

Is this fixed? Using the latest version, and it seems indeed that the collection input filter does not have some sort of default message when required is true and an empty array is given. Need to extend the input filter for now.

tigran-m-dev commented 6 years ago

Issue is still exists. @weierophinney can you confirm this?

tigran-m-dev commented 6 years ago

Any updates on this? Problem still exists.

weierophinney commented 6 years ago

Please provide us with a minimal coffee sample that demonstrates the issue; right now, we'd be doing guess work.

tigran-m-dev commented 6 years ago

Example Input Filter

$factory     = new \Zend\InputFilter\Factory();
$inputFilter = $factory->createInputFilter(
[
    'collection-element' => [
        'type'         => \Zend\InputFilter\CollectionInputFilter::class,
        'required'     => true,
        'input_filter' => [
            'type'  => [
                'required'     => true,
                'validators' => [
                    [
                        'name'    => \Zend\Validator\Between::class,
                        'options' => [
                            'min' => 50,
                            'max' => 100,
                        ],
                    ],
                ],
            ],
            'price' => [
                'required'     => true,
                'validators' => [
                    [
                        'name'    => \Zend\Validator\Between::class,
                        'options' => [
                            'min' => 50,
                            'max' => 100,
                        ],
                    ],
                ],
            ],
        ],
    ],
]
);

First Try, when you have a problem with empty error message

$inputFilter->setData([]);
var_dump($inputFilter->isValid());
var_dump($inputFilter->getMessages());

Second Try with Valid error messages if provided some data

$inputFilter->setData([
    'collection-element' => [
        0 => [

        ],
    ],
]);
var_dump($inputFilter->isValid());
var_dump($inputFilter->getMessages());
froschdesign commented 6 years ago

Output is:

First try

boolean false

array (size=1)
  'collection-element' => 
    array (size=0)
      empty

Second try

boolean false

array (size=1)
  'collection-element' => 
    array (size=1)
      0 => 
        array (size=2)
          'type' => 
            array (size=1)
              'isEmpty' => string 'Value is required and can't be empty' (length=36)
          'price' => 
            array (size=1)
              'isEmpty' => string 'Value is required and can't be empty' (length=36)
froschdesign commented 6 years ago

No error message is generated.

Compare the Input class and the CollectionInputFilter class in this case:

https://github.com/zendframework/zend-inputfilter/blob/a2962cf00f86a58f2192ba63b572606dacbe1c20/src/Input.php#L410-L415

https://github.com/zendframework/zend-inputfilter/blob/a2962cf00f86a58f2192ba63b572606dacbe1c20/src/CollectionInputFilter.php#L171-L213

tigran-m-dev commented 6 years ago

Do you have any updates related this issue?

weierophinney commented 6 years ago

@developer-devPHP No, no updates at this time.

We actually need to address this by getting some feedback first.

InputFilters do not have a concept of required/optional by default. We tacked it on to the CollectionInputFilter (and, for the 2.8.0 release, the OptionalInputFilter, which allows marking an input filter as optional, vs. required, meaning it can validate if the data set is empty). However, when we did, the only thing we had it affect is whether or not an empty set is valid. In the case that we have an empty set, there is currently no mechanism to provide a message why.

Normally, if data is present, getMessages() will return a nested set of messages, one for each data set:

0 => [
    'foo' => [ /* array of messages */]
],
1 => [
    'bar' => [ /* array of mesages */ ]
],
/* etc. */

The problem area is that the each element of the above array is expected to be an array of key/messages pairs. How, exactly, do we represent a message indicating that a data set was empty in a way that is either consistent in structure, or easily differentiated from the existing structure?

Some thoughts off the top of my head:

We need to choose one, but I need input from users as to which one to choose.

Additionally, we need:

If anybody would like to tackle this, I'd appreciate the help.

ruzann commented 6 years ago

I have a solution for this. Please have a look into pull request #170