First time experimenting with the Aggregator feature, and it's quite nice. However, I'm also a fan of the makeAllSearchableUsing() method in Scout which can implement a more specific query. (I'm aware that $relations can be used for eager loading and shouldBeSearchable() can be used to filter out results, but in my use case, the filtering logic relies on the result of a separate query, and this would create an n+1 query performance issue that would be resolved by putting the conditional logic in the main query to pull all searchable models.) The makeAllSearchable() method on the Aggregator class does not appear to honor the makeAllSearchableUsing() in the individual models the way the makeAllSearchable() method on the main Laravel\Scout\Searchable trait does.
I'm posing this as a question for discussion, because I realize that honoring any such makeAllSearchableUsing() methods in the individual models could be an undesirable behavior and/or breaking change for some folks (if they are using different logic in the individual index and the aggregator). Ideally there would be a way to optionally honor these methods, or perhaps to define them in the Aggregator class itself. I'm currently overriding the makeAllSearchable() method in the Aggregator class and chaining in that additional step along the lines of what the Laravel\Scout\Searchable trait does (note that this requires making the makeAllSearchableUsing() method public in the source model, otherwise it's not accessible to the Aggregator):
<?php
namespace App\Search;
use Algolia\ScoutExtended\Searchable\Aggregator;
use Illuminate\Database\Eloquent\SoftDeletes;
use Laravel\Scout\Events\ModelsImported;
class MyAggregator extends Aggregator
{
/**
* The names of the models that should be aggregated.
*
* @var string[]
*/
protected $models = [
\App\Models\FirstModel::class,
\App\Models\SecondModel::class,
];
/**
* Make all instances of the model searchable.
*
* @return void
*/
public static function makeAllSearchable()
{
foreach ((new static)->getModels() as $model) {
$instance = new $model;
$softDeletes =
in_array(SoftDeletes::class, class_uses_recursive($model)) && config('scout.soft_delete', false);
// Added these two lines to determine if there's a public 'makeAllSearchableUsing' method in the model instance)
$makeAllSearchableUsingReflection = new \ReflectionMethod($instance, 'makeAllSearchableUsing');
$applyMakeAllSearchableUsing = $makeAllSearchableUsingReflection->isPublic();
$instance->newQuery()->when($softDeletes, function ($query) {
$query->withTrashed();
})->when($applyMakeAllSearchableUsing, function ($query) use ($instance) { // Added these two lines
$instance->makeAllSearchableUsing($query);
})->orderBy($instance->getKeyName())->chunk(config('scout.chunk.searchable', 500), function ($models) {
$models = $models->map(function ($model) {
return static::create($model);
})->filter->shouldBeSearchable();
$models->searchable();
event(new ModelsImported($models));
});
}
}
}
I don't love doing it this way, since it's possible the underlying makeAllSearchable logic will change in future updates and this code will need to be kept in sync. Perhaps there's a more elegant way to do this? Could we add an optionally extendable makeAllSearchableUsing method on the Aggregator class itself?
Description
First time experimenting with the Aggregator feature, and it's quite nice. However, I'm also a fan of the
makeAllSearchableUsing()method in Scout which can implement a more specific query. (I'm aware that$relationscan be used for eager loading andshouldBeSearchable()can be used to filter out results, but in my use case, the filtering logic relies on the result of a separate query, and this would create an n+1 query performance issue that would be resolved by putting the conditional logic in the main query to pull all searchable models.) ThemakeAllSearchable()method on the Aggregator class does not appear to honor themakeAllSearchableUsing()in the individual models the way themakeAllSearchable()method on the mainLaravel\Scout\Searchabletrait does.I'm posing this as a question for discussion, because I realize that honoring any such
makeAllSearchableUsing()methods in the individual models could be an undesirable behavior and/or breaking change for some folks (if they are using different logic in the individual index and the aggregator). Ideally there would be a way to optionally honor these methods, or perhaps to define them in the Aggregator class itself. I'm currently overriding themakeAllSearchable()method in theAggregatorclass and chaining in that additional step along the lines of what theLaravel\Scout\Searchabletrait does (note that this requires making themakeAllSearchableUsing()method public in the source model, otherwise it's not accessible to the Aggregator):I don't love doing it this way, since it's possible the underlying
makeAllSearchablelogic will change in future updates and this code will need to be kept in sync. Perhaps there's a more elegant way to do this? Could we add an optionally extendablemakeAllSearchableUsingmethod on the Aggregator class itself?Steps To Reproduce
See above