Batch action $allItems needs to be an boolean

[nieuwenhuisen]

Batch action $allItems needs to be an boolean

Exception:

Argument 4 passed to Sonata\AdminBundle\Admin\AbstractAdmin::preBatchAction() must be of the type bool, string given, called in /projects/sonata-sandbox/vendor/sonata-project/admin-bundle/src/Controller/CRUDController.php on line 452

I am targeting this branch, because there is the typehint added.

Changelog

### Changed
- Cast $allItems to an boolean to prevent typehint error.

[jordisala1991]

What about using $request->request->getBoolean('all_elements')? There is other get just above that could be changed too.

This code is a bit strange BTW, it first get the all_elements, then sets it to the request with the same value, and then get it again to assign it to a var. 🤔

[nieuwenhuisen]

$request->request->set('all_elements', $allElements); Is this needed?

[jordisala1991]

Maybe is in another parameter bag? That is the only explanation I can find for it.

[franmomu]

Taking a look at 9949121#diff-b1f1981b1ed9490b32c9bc83360655c3R396 where all this started, looks like it started using Request::get() method and it has been like that since then, but taking into account that some lines above there is this check:

if (Request::METHOD_POST !== $restMethod)

I would say we can use in this block $request->request safely (batch actions are sent through POST).

[nieuwenhuisen]

Oké is this meaning that the line $request->request->set('all_elements', $allElements); (line 377) can be removed?

[nieuwenhuisen]

Maybe this is used to allow a combination of POST and GET parameters?

[franmomu]

Maybe this is used to allow a combination of POST and GET parameters?

https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Controller/CRUDController.php#L410, apparently only POST.

[nieuwenhuisen]

But you can POST to a url for example /delete?all_elements=1

[franmomu]

Yes, but in this case:

• all_elements: https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Resources/views/CRUD/base_list.html.twig#L167
• action: https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Resources/views/CRUD/base_list.html.twig#L172
• idx:

  SonataAdminBundle/src/Resources/views/CRUD/list__batch.html.twig

  Line 15 in 82a4d6e

  <input type="checkbox" name="idx[]" value="{{ admin.id(object) }}">

I don't think they are later modified by JavaScript and seeing the <form> tag: https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Resources/views/CRUD/base_list.html.twig#L37 I would say that the URL only contains filter parameters.

And looking at how the request is built in CRUDControllerTest when using idx or all_elements.

So I think we can change this to just use $request->request.

[jordisala1991]

what does the getAlnum?

[VincentLanglet]

Alpha numeric ?

[franmomu]

    /**
     * Returns the alphabetic characters and digits of the parameter value.
     *
     * @param string $key     The parameter key
     * @param string $default The default value if the parameter key does not exist
     *
     * @return string The filtered value
     */
    public function getAlnum($key, $default = '')
    {
        return preg_replace('/[^[:alnum:]]/', '', $this->get($key, $default));
    }

Returns the alphabetic characters and digits of the parameter value.

and returns a string value. Ref: https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/HttpFoundation/ParameterBag.php#L127

[franmomu]

I think we can do this on 3.x branch.

[jordisala1991]

Thank you @nieuwenhuisen