kellerjmrtn
commented
1 month ago Having the same issue as #285, I'd like to bump this PR. We've patched it in our application with a very similar fix (simply removing the static::syncingDisabledFor($model) check and adding a static::syncingDisabledFor($aggregator) check in the loop), but an official fix would be helpful
Do you need someone to take it over? I could create my own PR if desired
Describe your change
This PR changes the AggregatorObserver delete and forceDelete methods to check syncingDisabledFor using the Aggregator class rather than the Model class. This brings these methods inline with the behaviour of the saved method.
It also updates the delete method to reflect changes made in the parent ModelObserver class.
What problem is this fixing?
If you want to use an aggregator but don't want to index individually the models being aggregated, the algolia/scout-extended docs recommend to simply disable syncing for those models, eg...
Laravel\Scout\ModelObserver::disableSyncingFor(Article::class); Laravel\Scout\ModelObserver::disableSyncingFor(Event::class);
However, the AggregatorObserver delete and forceDelete methods were both checking syncingDisabledFor using the Model class rather than the Aggregator class. This meant that if you deleted a model which had syncing disabled (because you only wanted an Aggregated Index and not individual indexes), it was not deleted from the aggregated index.
Conversely, the saved method does check syncingDisabledFor with the Aggregator class, which I believe should be the correct behaviour.