zendframework / zend-inputfilter

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

Hotfix/non iterable inputfilter value #163

Closed MadeRelevant closed 6 years ago

MadeRelevant commented 6 years ago

Fixes issue #159

Currently, when you send a non-iterable value, non-null value to a nested input filter, it incorrectly raises an InvalidArgumentException. It should report as if all underlying fields are empty, and treat the value as an empty array.

MadeRelevant commented 6 years ago

@samsonasik everything ready to merge?

samsonasik commented 6 years ago

I’m not maintainer ^^

MadeRelevant commented 6 years ago

I’m not maintainer ^^

Lol, my bad

MadeRelevant commented 6 years ago

@weierophinney ready to merge or do you need me something to change?

svycka commented 6 years ago

@MadeRelevant your test does not cover your fix see: https://coveralls.io/builds/15520340/source?filename=src%2FBaseInputFilter.php#L528

MadeRelevant commented 6 years ago

@svycka hmm, thats really weird, I ran the tests and it actually hits a breakpoint if I put it there.. Should be enough to prove it is touched and covered? When I run the tests with coverage it is for some strange reason not covered.. I have no clue why not while it reaches the breakpoint. I must be doing something wrong but not sure what, never had this before.

Any clues?

svycka commented 6 years ago

don't know but if the test fails without this fix and does not with it then it is ok

MadeRelevant commented 6 years ago

don't know but if the test fails without this fix and does not with it then it is ok

Yes it fails without it, just tested it. But still I think it is strange.. Bug in xdebug? PHPUnit reporter? Very weird..

svycka commented 6 years ago

@MadeRelevant oh I see whats wrong you have to move test to BaseInputFilterTest or add annotation like this:

    /**
     * @covers \Zend\InputFilter\BaseInputFilter
     */
    public function testNestedInputFilterShouldAllowNonArrayValueForData()
    {
// ...
weierophinney commented 6 years ago

Thanks, @MadeRelevant!