teamtnt / laravel-scout-tntsearch-driver

Driver for Laravel Scout search package based on https://github.com/teamtnt/tntsearch
MIT License
1.09k stars 142 forks source link

Mapping resulting collection should use newCollection() from model #342

Closed it4need closed 1 year ago

it4need commented 2 years ago

Problem

As I already described in https://github.com/teamtnt/laravel-scout-tntsearch-driver/issues/338 we have now the same problem with a matched result.

The problem is used to be here: https://github.com/teamtnt/laravel-scout-tntsearch-driver/blob/b98729b0c7179218c9a5e1445922a9313d45c487/src/Engines/TNTSearchEngine.php#L221-L225

When $model->newCollection($results['ids']) we have a correct collection with IDs in it (based on models newCollection() method). Our custom collection is extending EloquentCollection where you will call ->map() on. The map() function of the EloquentCollection is implemented as follows:

    public function map(callable $callback)
    {
        $result = parent::map($callback);

        return $result->contains(function ($item) {
            return ! $item instanceof Model;
        }) ? $result->toBase() : $result;
    }

So the problem is that EloquentCollection uses ->toBase() which converts it back to \Support\Illumate\Collection, because in the collection we only have numbers and these are not instanceof Model.

Potential solution

Use $model->newCollection() on the final result as follows:

return $model->newCollection(collect($results['ids'])->map(function ($hit) use ($models) {
            if (isset($models[$hit])) {
                return $models[$hit];
            }
        })->filter()->all());

The PR that fixes these issue will follow in the next minutes.