dingo / api

A RESTful API package for the Laravel and Lumen frameworks.
BSD 3-Clause "New" or "Revised" License
9.33k stars 1.25k forks source link

Elasticquent pagination and fractal #709

Closed drauta closed 8 years ago

drauta commented 8 years ago

Hi, I'm using elasticquent/Elasticquent to do search queries. If you do a search on the model, add pagination and apply a transformer, in the fractal adapter from dingo makes a call to inexistent function:

Line 89 on dingo/api/src/Transformer/Adapter/Fractal.php:

if (($response instanceof EloquentCollection || $response instanceof IlluminatePaginator) && $this->eagerLoading) {
            $eagerLoads = $this->mergeEagerLoads($transformer, $this->fractal->getRequestedIncludes());
            $response->load($eagerLoads);
        }

The call to $response->load() fails because the collection is an instance of Illuminate\Support\Collection instead of Illuminate\Database\Eloquent\Collection.

In order to solve this I added a condition to the if:

if (($response instanceof EloquentCollection || $response instanceof IlluminatePaginator) && $this->eagerLoading && method_exists($response->getCollection(), 'load')) {
            $eagerLoads = $this->mergeEagerLoads($transformer, $this->fractal->getRequestedIncludes());
            $response->load($eagerLoads);
        }

In summary: when you do pagination on a Illuminate\Support\Collection the fractal paginator fails.

Thank you!!

jasonlewis commented 8 years ago

Okay no worries I'll need to account for that as well. Cheers.

drauta commented 8 years ago

The workaround I posted yestarday fails in some cases, this not:

if ( ($response instanceof EloquentCollection || ($response instanceof IlluminatePaginator &&
method_exists($response->getCollection(), 'load') ) ) && $this->eagerLoading ) 
{
    $eagerLoads = $this->mergeEagerLoads($transformer, $this->fractal->getRequestedIncludes());
    $response->load($eagerLoads);
}
jasonlewis commented 8 years ago

Indeed. No worries I'll have something up when I can.

drauta commented 8 years ago

Thank you!

andreygrehov commented 8 years ago

This is indeed an issue for larger projects, where not only an eloquent collections are used, but hand-made collections of entity objects.

jasonlewis commented 8 years ago

This should've been fixed with cda184a566d9b68f47d4edff51b43ba8f56bbc3c so that it only attempts to eager load Eloquent collections.