Skip to content

AggregatorObserver delete method shouldn't check individual model syncing status #285

@goldmerc

Description

@goldmerc
  • Laravel version: 8.49.2
  • Algolia Scout Extended version: 1.20.0
  • Algolia Client Version: 3.0.2

Description

AggregatorObserver's deleted + forcedDelete methods check if syncing is disabled for the model...

    public function deleted($model): void
    {
        if (static::syncingDisabledFor($model)) {
            return;
        }

So, if the model class has syncing disabled, the aggregated index will keep the record for the deleted model.

The recommended way to avoid indexing the individual models that make up an aggregated index is...

Laravel\Scout\ModelObserver::disableSyncingFor(Article::class);
Laravel\Scout\ModelObserver::disableSyncingFor(Event::class);

Furthermore the saved() method of AggregatorObserver doesn't make the same check.

Shouldn't the saved and deleted methods be consistent in the logic used to decide whether a model is synced to the index?

And in both cases, if a check is to be made to see if syncing is disabled, shouldn't that be on the aggregator class and not the model class?

Anyway, I can't see why disabling sync for an individual model should effect the behaviour of an aggregated index? But maybe there is a reason I'm missing?

Lastly, if you do call disableSearchSyncing() on an Aggregator class, it will add the class to the list for disabled sync but because the AggregatorObserver is using the model class to check, it will ignore the Aggregator and follow the model class.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions