Skip to content
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

[5.2] Fix invalid() and valid() methods. (issue #14646) #14651

Merged
merged 8 commits into from
Aug 12, 2016
Merged

[5.2] Fix invalid() and valid() methods. (issue #14646) #14651

merged 8 commits into from
Aug 12, 2016

Conversation

carloscarucce
Copy link
Contributor

@carloscarucce carloscarucce commented Aug 5, 2016

invalid() and valid() methods where returning empty arrays when validating multi-dimensional data array

Carlos Alberto Bertholdo Carucce added 5 commits August 5, 2016 20:46
Get updates from laravel
invalid() and valid() methods where returning empty arrays
Issue link: #14646
Fixing style
@GrahamCampbell GrahamCampbell changed the title Fix invalid() and valid() methods. (issue #14646) [5.2] Fix invalid() and valid() methods. (issue #14646) Aug 6, 2016
@taylorotwell
Copy link
Member

@themsaid can you look at this? I can't follow what this is doing.

@themsaid
Copy link
Member

themsaid commented Aug 7, 2016

Take a look at this test @taylorotwell

    public function testInvalidMethod()
    {
        $trans = $this->getRealTranslator();

        $v = new Validator($trans,
            [
                ['name' => 'John'],
                ['name' => null],
                ['name' => '']
            ],
            [
                '*.name' => 'required',
            ]);

        $this->assertEquals($v->invalid(), [
            1 => ['name' => null],
            2 => ['name' => '']
        ]);
    }

The problem is with implicit attributes, currently to get the invalid attributes we do:

return array_diff_key($this->data, $this->messages()->toArray());

But in that case the data would look like:

[
    0 => ['name' => 'John'],
    1 => ['name' => null],
    2 => ['name' => '']
]

While the messages array:

[
    '1.name' => [...],
    '2.name' => [...],
]

This will lead to no intersection between keys, resulting an empty array out of invalid()

In the PR he's checking the key that comes before the first dot (0.name, persons.3.name, etc...) and check for intersection based on that first key since it's the one the $data array knows about.

@carloscarucce
Copy link
Contributor Author

Exactly :)

@themsaid
Copy link
Member

themsaid commented Aug 7, 2016

Maybe you need to add this test @xxnoobmanxx and provide something similar for valid() as well.

    public function testInvalidMethod()
    {
        $trans = $this->getRealTranslator();

        $v = new Validator($trans,
            [
                ['name' => 'John'],
                ['name' => null],
                ['name' => '']
            ],
            [
                '*.name' => 'required',
            ]);

        $this->assertEquals($v->invalid(), [
            1 => ['name' => null],
            2 => ['name' => '']
        ]);

        $v = new Validator($trans,
            [
                'name' => ''
            ],
            [
                'name' => 'required',
            ]);

        $this->assertEquals($v->invalid(), [
            'name' => ''
        ]);
    }

@carloscarucce
Copy link
Contributor Author

@themsaid Done.

@taylorotwell taylorotwell merged commit aba8491 into laravel:5.2 Aug 12, 2016
tillkruss pushed a commit to tillkruss/framework that referenced this pull request Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants