Guarding against N+1 in search here.
What is this constant?
Isn’t this going to randomly fail if we add/remove any other queries from the request? We’ve seen this problem before with the WebHookSerializer test that checks for a specific amount of attributes and breaks all the time.
This guard here is meant to set a baseline for the number of ActiveRecord queries that search should execute. Search has suffered from multiple N+1 queries and we have not had a way to catch those. If a change results in more number of queries executed on search, the developer would then have to justify the increase.
We’ve seen this problem before with the WebHookSerializer test that checks for a specific amount of attributes and breaks all the time.
We need to intentionally keep the webhook payload small so every additional attribute added to the payload has to be justified. The test breaking is a good thing to me because it allows us to review the new attributes that are added.
I get the idea, but I’m not really convinced that these tests failing causes the developer to do anything other than change the number and resubmit. I’ve never seen anyone justify a webhook attribute and I’ve seen the test fail many times.
I agree catching N+1 queries is important, I just don’t feel this is a good solution since there will be way more false positives than actual issues caught.
The reindex here is intentional so that we update search data for changes like
Can we do this instead?