Deprecate `AdminInterface::setRequest` and inject RequestStack instead.

[wokenlex]

"sonata-project/admin-bundle": "^3.98"

Search inside admin entities didn't initialize request before asking configureDefaultFilterValues, so if I had the multi language admin, I can't preset the locale filter:

protected function configureDefaultFilterValues(array &$filterValues) 
    {
        $filterValues['locale'] = [
            'value' => $this->request->getLocale(),
        ];
    }

it makes - Call to a member function getLocale() on null - on admin/search?q=test request.

[VincentLanglet]

You should always add a $this->hasRequest() check. And prefer $this->getRequest() instead of $this->request which will be private.

[VincentLanglet]

The admin is retrieve here: https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Block/AdminSearchBlockService.php#L143-L187 Unfortunately there is no request in the blockService.

What would be the best solution @sonata-project/contributors ? Should we add the RequestStack to the constructor ?

[VincentLanglet]

@sonata-project/contributors If we need to modify the constructor, it's a BC-break, so it might be interesting to solve this issue before the next major. WDYT ?

[franmomu]

About this issue I would say it's best if the user injects RequestStack to the Admin.

IMHO we should probably deprecate AdminInterface::getRequest and AdminInterface::hasRequest and do not expose the admin internals (this can be done for v5).

[wokenlex]

IMHO we should probably deprecate AdminInterface::getRequest and AdminInterface::hasRequest and do not expose the admin internals (this can be done for v5).

But what is ok to have most useful things near the hand in the controller. At least, in the symfony controllers you have ->getDoctrine(), ->getRequest()

[VincentLanglet]

An admin class in not a controller, it's a service.

Moreover the getDoctrine and getRequest method are defined in the controller from the time service injection wasnt this easy. I wont be surprise if one day they'll deprecate it.

As an example of the extra complexity of the adminInterface::getRequest method, we have to set the request by ourself every time. This wouldnt be require if we use the request stack.

[wokenlex]

oh… anyway, I think it's should be the same: or search service should have the request (because it's uses admin service with this not empty request), or yes, ->getRequest should be removed, because using it makes error in another service - it's just unlogical to keep it in this state. (but yes, my laziness wants just a working ->getRequest)

[VincentLanglet]

AdminInterface::getRequest might be kept as a shortcut for

$this->requestStack->getCurrentRequest()

that's debatable.

I think to solve this issue we need to

• Add a setRequestStack and getRequestStack method to the TaggedAdminInterface
• Automatically call setRequestStack in the AddDependencyCompiler
• Changing the AbstractAdminInterface::getRequest call to use the requestStack instead of an internal state.
• Deprecating AdminInterface::setRequest
• Potentially deprecating AdminInterface::getRequest in favor of AdminInterface::getRequestStack->getCurrentRequest()

Do you agree @sonata-project/contributors ? We might prefer to delay this for Sonata 5. I don't know how big will be the BC in step 3 (or if it can be considered as a bugfix instead)

Until this, the way to solve your issue is to do

protected function configureDefaultFilterValues(array &$filterValues) 
    {
        $filterValues['locale'] = [
            'value' => $this->requestStack->getCurrentRequest()->getLocale(),
        ];
    }

[VincentLanglet]

Looking at the code, the only time the request set to the admin is not the current request seems like to be here: https://github.com/sonata-project/SonataAdminBundle/blob/06b869dddac1ed74e51c8ca63bfd7da7d34ae57a/src/Block/AdminPreviewBlockService.php#L117-L119

Is there a way to avoid calling AdminInterface::setRequest here ? @franmomu @jordisala1991 ?

[VincentLanglet]

Maybe related to https://github.com/sonata-project/SonataAdminBundle/issues/5547 cc @phansys

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.