Add setting for reindexing every revision on entity update

Turns out that the fields reindexing was due to a separate unrelated bug with the entity type I was testing.

Hi. We have to test this. Im not 100% sure now but I think it’s due flags like is default revision and is last revision. Especially in case you have published revision which is not last (published article with new unpublished drafts for example). We have few millions of revisions with no performance problem (module needs to reindex in the queue).

@mkolar ok that does make sense.

I don't think we need to do this logic in a presave though? We can do it directly in the entity_update hook and do away with the magic search_api_revisions_update_ids property. In entity_update we can just select all revisions that aren't the current revision. We can also add a setting for this logic alongside the queue setting.

TBH not rly sure why we used search_api_revisions_update_ids and not doing it in entity_udpate hook (since we made that hook too) but i bet there was some good reason. What might be one of the reasons is that like this, its alterable by another module (but we are not doing this). But since its not improving anything why to change this, whats the reason.

Why you would like to add setting not to index all revisions since module itself doing those flags which needs to be updated? Why you need this module since you only care about default revision then? We can of course add this settings thats easy but not sure how to deactivate those flags if this is turned off. I dont have problem with the settings.

But since its not improving anything why to change this, whats the reason.

Because using magic properties is not best practice. There's no reason to keep this logic in a presave hook when we can combine it with the update hook and make it much cleaner.

Why you would like to add setting not to index all revisions since module itself doing those flags which needs to be updated

In our use case, we are not indexing any of these flags. We simply need to index basic information across many different entity types. This information does not change once a revision is saved, therefore reindexing every revision is redundant.

e can of course add this settings thats easy but not sure how to deactivate those flags if this is turned off, i dont want setting that causes wrong flags

It would just be another setting like the queue setting. We can have it enabled by default to retain the current behaviour, but allow users (like me) to disable it :)

If you want, you can proceed with the settings thing + moving it from magic property to entity save completely. It looks better to me now to move it, its already pretty old so cant remember why its in presave. When you have a patch, we will try the patch on our project if its ok. It might take a while. Thank you!

I've pushed a fair chunk of work, what I've done so far:

1. Moved all logic from .module files into a new PHP class for easier reuse + dependency injection
2. Heavily refactored the above functions to be more reusable
3. Moved presave hook logic into entity_update using an entity query
4. Added new index_previous_revisions setting which is used in hook_entity_update

You can see that the full test suite still passes that I committed in 📌 Write tests Fixed - and we can remove the line that emptied search_api_revisions_update_ids.

All that's left is additional test coverage for when the new setting is off.

2 weeks with no feedback, I'm going to merge this and cut a new alpha. Feel free to open a new issue for feedback.

Automatically closed - issue fixed for 2 weeks with no activity.