Remove an N+1 call rendering projects search results and fix false positive tests
What does this MR do?
Initially the intent of this MR was to remove an N+1 call rendering projects search results (additional context from #34457 (closed))
But when reviewing @lulalala noticed that the assertion in the test was doing nothing. As such this led to the following:
- The test was not running sidekiq and thus nothing was indexed and thus no results were being found in search and thus the N+1 query wasn't asserting anything meaningful
- Adding
sidekiq_inlint
globally in these tests meant I could remove a bunch of specific mentions ofsidekiq_might_not_need_inline
- A new assertion was added to prevent regressions so that we always verify we see results
- This new assertion showed other tests finding no results for different reasons
- The milestones search didn't match anything due to milestones not being named as expected so we just search
*
instead - The merge requests search didn't match anything due to an existing bug now described at #103458 (closed) but I didn't want to fix this now so I updated the test to keep the merge requests on the same project
- Adding a collection of merge requests to the same project using factories forced me expand the merge requests factory to avoid uniqueness constraint on
source_branch
- Debugging what the heck is the difference between
refresh_index
and the sidekiq led me to realise therefresh_index
is an Elasticsearch specific thing that has to do with ensuring that all the previous writes are refreshed and can be served up in subsequent searches.
Screenshots
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖