This additional interface is required by encrypt plugin
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.
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