search

la-haute-societe / craft-elasticsearch

Bring the power of Elasticsearch to your Craft CMS projects
Other
18 stars 14 forks source link

Change entry or asset listener always push entry into queue #28

Open sfsmfc opened 2 years ago

sfsmfc commented 2 years ago

The event listener for changing/add entries doesn't respect the blacklist settings for entries or assets. If I change/add an entry/asset, which should not be indexed according the blacklist settings, the plugin the index queue job to the queue.

Testet with plugin version 1.5.0 and Craft 3.7.48.

nstCactus commented 2 years ago

Jobs are indeed added to the queue even for blacklisted entry types / asset volumes but the elements are NOT indexed.

The ElementIndexerService::indexElement() method is responsible for checking if elements should be indexed. This methods is called in several places of the plugin, including from the IndexElementJob class.

This is counter-intuitive and a little waste of resources, but we decided to do it that way to keep the code simpler.

sfsmfc commented 2 years ago

@nstCactus Sorry, if I have to contradict, but the indexer tries to index these items and if the indexing not failed, the items will be indexed.

The question is, why the indexer doesn't stop here (https://github.com/la-haute-societe/craft-elasticsearch/blob/1.5.1/src/services/ElementIndexerService.php#L156). I will have a look and will open a new issue, if I found something.

sfsmfc commented 1 year ago

@nstCactus Sorry for the late response. The problem is, that you test in src/services/ElementIndexerService.php#L155 only of type Entry for blacklist entries. The configuration for blacklisted asset volumes will be ignored. In PR https://github.com/la-haute-societe/craft-elasticsearch/pull/35, this "problem" will be fixed.

sfsmfc commented 1 year ago

@nstCactus Can you please apply my patch for this issue? I cannot reopen this issue.

nstCactus commented 1 year ago

Sorry @sfsmfc but we're currently quite busy and have no time to maintain this plugin. I'll try to schedule some time in September to review & merge your PR.

sfsmfc commented 1 month ago

@nstCactus Any news on this? The issue is still present.

nstCactus commented 1 month ago

Hi Simon,

I'm no longer a maintainer of this plugin since I quit La Yaute Société.

@lhs-dev is the one you need to ask.