laravel / nova-issues

554 stars 34 forks source link

Lens count request doesn't contain the container property #6491

Closed mostafa-rz closed 3 months ago

mostafa-rz commented 3 months ago

Description:

The Nova class src/Http/Requests/LensCountRequest.php inherits from the Laravel class src/Illuminate/Foundation/Http/FormRequest.php. The Laravel FormRequest has a container property that is not nullable. The container is null when a count request is sent to the backend.

The reason is LensCountRequest()->toQuery() method recreates the request by LensRequest::createFrom($this) method. This results in a new request object but without the container. You can see the usage of this method in Laravel FormRequestServiceProvider(). After they called the FormRequest::createFrom(), they also set the container and redirectors again. Please consider this in LensCountRequest()->toQuery() so the newly created request contains the container property.

/**
 * Transform the request into a query.
 *
 * @return \Illuminate\Database\Eloquent\Builder
 */
public function toQuery()
{
    return tap($this->lens()->query(LensRequest::createFrom($this), $this->newSearchQuery()), function ($query) {
        if (! $query instanceof Builder) {
            throw new LensCountException('Lens must return an Eloquent query instance in order to count lens resources.');
        }
    });
}

Detailed steps to reproduce the issue on a fresh Nova installation:

  1. make a lens
  2. make a filter for the lens
  3. Inside the apply() method of the filter check the $request when the count request is reach there
  4. The $request object doesn't contain the container
crynobone commented 3 months ago

Laravel Framework isn't 100% strict type, while the doc block may say @var Illuminate\Contracts\Container\Container if you convert it to strict type it needs to be defined as protected Container|null $container since the parameter is not initiated within the constructor.

mostafa-rz commented 3 months ago

@crynobone It is not type-hinted but it doesn't mean that you can break the parent class functionality. If it is a FormRequest(), it must be possible to validate it by the Laravel Form Request Validation. When you remove the container, the Laravel authorize() method fails, because it needs the container.