Revert to use merge requests count for group view
What does this MR do?
Related to #334039 (closed)
Revert the line in app/views/groups/merge_requests.html.haml that counts MRs and determine if we should render the empty_states template.
In an attempt to improve performance, in this previous MR we changed this template from using the helper method group_merge_requests_count (that performs an expensive COUNT query) to using @merge_requests&.size.
This was a mistake because while group_merge_requests_count(state: 'all') will return the count for all MRs (in any state and ignoring any filters), @merge_requests.size will only return the number of open MRs (affected by filters). As a result, we would be rendering the empty_states template (without the search bar) incorrectly.
This MR changes the template to use the method #issuables_count_for_state for calculating the total count of MRs.
Even though this method also uses an expensive COUNT query to calculate the result, it caches it in Gitlab::SafeRequestStore so it will be called once per request. For the empty_states template this method is already being called once so we know we are not introducing new queries.
Note: This proposed solution #334039 (comment 608268170) is a good idea, performance-wise, but #cached_issuables_count only considers open issuables at the moment.
Screenshots (strongly suggested)
| Before - with no Mrs | After - with no MRs |
|---|---|
 |
 |
| Before - with no open Mrs | After - with no open MRs |
|---|---|
 |
 |
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.