FEATURE: allow to extend topic_eager_loads in Search (PR #10625)

This additional interface is required by encrypt plugin

https://github.com/discourse/discourse-encrypt/pull/21

GitHub

I think we’re giving the plugins too much flexibility here by accepting a block? Instead, can the plugin register the exact table names that should be eager loaded?

Sorry I just saw the implementation in https://github.com/discourse/discourse-encrypt/pull/21. Instead of allowing the block to mutate topic_eager_loads, I think having the block explicitly return an array of table names to be pushed into the topic_eager_loads array will be clearer.

Also, should we also support plugins being able to pass in a table name(s) directly instead of having to do so through a block?

Let us include an example of how this method should be used as well.

Looks good to me. I think we should add a spec here as well.

Sorry this doesn’t look right. Shouldn’t we be pushing or concating into the array instead of overriding it?

#posts_eager_loads is a public method so we don’t have to use send here. Whether the method should be private or not though is another question to be answered another day.

Since Search.custom_topic_eager_load uses a class variable, we need to clean this up to prevent this test from leaking state.

Having to pass nil in here is quite odd. Maybe we can just provide a default value in the method definition?

yeah nah, there are to private keywords, I will remove the second one

O in this case, let us not test for a private method. Instead we can just execute a search and test whether the association has been eager loaded with post.association(:topic).loaded?