Skip to content

[5.4] Set data key when testing file uploads in nested array#18954

Merged
taylorotwell merged 3 commits into
laravel:5.4from
RDelorier:5.4
Apr 28, 2017
Merged

[5.4] Set data key when testing file uploads in nested array#18954
taylorotwell merged 3 commits into
laravel:5.4from
RDelorier:5.4

Conversation

@RDelorier

@RDelorier RDelorier commented Apr 26, 2017

Copy link
Copy Markdown
Contributor

Fix a bug that causes data keys to be overridden when testing a nested file upload array.

Example of bug in action:

routes/web.php

Route::post('test', function () {
    Validator::make(request()->all(), [
        'foo' => 'required'
    ])->validate();
});

tests/Feature/ExampleTest.php

<?php

namespace Tests\Feature;

use Tests\TestCase;
use Illuminate\Http\UploadedFile;

class ExampleTest extends TestCase
{
    public function testExample()
    {
        $this->postJson('test', [
            'foo' => 'bar',
            'images' => [UploadedFile::fake()->image('image.jpg')]
        ])->assertStatus(200);
    }
}

Test result:

Expected status code 200 but received 422.
Failed asserting that false is true.

This pr sets the value in the data which fixes the bug.

fixes #18671

@themsaid

themsaid commented Apr 27, 2017

Copy link
Copy Markdown
Member

This is wrong, using unset($data[$key]) here will unset every single key with a value of type array, even if the field is not an array of files.

if (is_array($value)) {
$files[$key] = $this->extractFilesFromDataArray($value);

unset($data[$key]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this with $data[$key] = $value;, I think it'll fix your issue.

@RDelorier

Copy link
Copy Markdown
Contributor Author

Woops, good catch @themsaid

@RDelorier RDelorier changed the title [5.4] Remove data key when testing file uploads in nested array [5.4] Set data key when testing file uploads in nested array Apr 28, 2017
@taylorotwell taylorotwell merged commit a102a9f into laravel:5.4 Apr 28, 2017
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