Framework: Multiple select with [] in name can't be crawled

[dubcanada]

Hello,

If you have a select, let's say it looks like

<select name="test[]" multiple>
    <optgroup value="Testing">
        <option value="1">Testing 1</option>
        <option value="2">Testing 2</option>
    </optgroup>
</select>

And if you try to use

$this->select([1,2], 'test[]');

It won't work, while it works in the Laravel side as soon as it goes to dom-crawler and enters the get function in FormFieldRegistry https://github.com/symfony/dom-crawler/blob/6bf50c092a5fcfb700e53addabb82ebd77625b27/FormFieldRegistry.php#L82 it breaks. The form field registry stores selects, regardless if they have a [] or not without it. So in this case if I changed it to

$this->select([1,2], 'test');

It would work on the dom-crawler, but it won't work for the Laravel CrawlerTrait. Basically if you have

$this->select([1,2], 'test[]');

In your test, and then add something like

if ($name == 'test[]') $name = 'test';

On https://github.com/symfony/dom-crawler/blob/6bf50c092a5fcfb700e53addabb82ebd77625b27/FormFieldRegistry.php#L84

It works.

So before the field names are sent to dom-crawler they need to have [] stripped.

dom-crawler version - v2.7.5
laravel foundation version - v5.1.19

[sebice]

Are there any steps done in dom-crawler to address this issue?

[GrahamCampbell]

This is the expected behaviour. [] is reserved in Laravel.

[arthurkirkosa]

@GrahamCampbell how would you go about testing an input/select that sends and array to the request?

[GrahamCampbell]

Rename it to drop the [], then you're good.

[dubcanada]

What do you mean rename it to drop the [] ? I have

$this->actingAs($user)
         ->visit('stuff')
         ->select('3', 'location[]')

How do I rename it to drop the [] on Symfony dom-crawler but keep the [] on Laravel?

If I change it to location it doesn't work.

@GrahamCampbell

[ghost]

I am still on L5.1.. however, update of https://github.com/laravel/framework/blob/5.2/src/Illuminate/Foundation/Testing/Concerns/InteractsWithPages.php#L845

with addition of:
$element = str_replace("[]", "", $element);

Would resolve the problem.

[iamthewit]

In case anyone else finds this and is struggling to write an integration test for multiselect, I managed to get it to work by doing what @spikerok said.

Taking the method @spikerok linked to : https://github.com/laravel/framework/blob/5.2/src/Illuminate/Foundation/Testing/Concerns/InteractsWithPages.php#L845

And adding in the line @spikerok said to add in:
$element = str_replace("[]", "", $element);

Gives you this:

protected function storeInput($element, $text)
    {
        $this->assertFilterProducesResults($element);
        $element = str_replace('#', '', $element);
        $element = str_replace("[]", "", $element);
        $this->inputs[$element] = $text;
        return $this;
    }

Simply copy and paste that into your test case class and you should be able to run your tests just fine.

Just incase you were wondering, my test looks like this:

$this->actingAs($adminUser)
             ->visit('/admin/project/create')
             ->type('James', 'name')
             ->select([1, 2], 'users[]')

[johnberberich]

@GrahamCampbell I don't think you understand why simply removing the "[]" from the name of your multiselect is a problem. It gets around the testing issue brought up here, but breaks how Laravel PHP handles array form inputs in the $_POST superglobal. In short, by appending "[]" to your form input name (typically a select field with multiple selections enabled), Laravel $request->input('items') hands you an array of the option values when the form is submitted; if you remove "[]", you only get one option value back, thus breaking multiselect form input retrieval and validation.

@iamthewit Is there a better solution to this problem? Manually editing core framework files breaks upgradability and is generally a bad idea.

[johnberberich]

For the time being, I've worked around this issue by submitting the form and its inputs together with InteractsWithPages@submitForm($buttonText, $inputs = [], $uploads = []). (You do this instead of calling type($text, $element), select($option, $element), press($buttonText), etc.)

Here's an example:

$this->submitForm('Submit', [
    'name' => 'Test name',
    'categories'  => [51, 52]
]);

[iamthewit]

@johnberberich I didn't edit any core framework code, I simply added the storeInput method (as per my comment above) to my test case class. This way it overrides the frameworks storeInput method.

[johnberberich]

I'm sorry, @iamthewit, I didn't read your comment thoroughly enough. I like that your solution lets you use the normal methods for filling a form, so I prefer it to mine. Thank you very much for clarifying.

[pelletiermaxime]

This is still a big problem with a quick and easy fix. I don't understand why this was closed.

[leewillis77]

I'd agree that this should be re-opened - at the minute multi-value selects can't be tested with the Laravel framework.

Attempting to fill in a form with a multi-select element, e.g.

<select multiple="" name="hills[]">
<!-- options here -->
</select>

With a test that includes:
$this->select($hill->id, 'hills[]')

Fails, with the rather cryptic:
InvalidArgumentException: Unreachable field ""

Reproduced on latest Laravel.

[themsaid]

@pelletiermaxime if you have a fix please feel free to open a Pull Request.

[pelletiermaxime]

The one liner fix a few comments above worked fine for me, but my test suite is really small, so not sure if there's any chance it breaks something. I will look at making a PR in the next few days if nobody else did.