gitlab-org--gitlab-foss/spec
Robert Speicher 6591b594d4 Merge branch '15356-filters-should-change-issue-counts' into 'master'
Take filters in account in issuable counters

## What does this MR do?

This merge request ensure we display issuable counters that take in account all the selected filters, solving #15356.

## Are there points in the code the reviewer needs to double check?

There was an issue (#22414) in the original implementation (!4960) when more than one label was selected because calling `#count` when the ActiveRecordRelation contains a `.group` returns an OrderedHash. This merge request relies on [how Kaminari handle this case](https://github.com/amatsuda/kaminari/blob/master/lib/kaminari/models/active_record_relation_methods.rb#L24-L30).

A few things to note:
- The `COUNT` query issued by Kaminari for the pagination is now cached because it's already run by `ApplicationHelper#state_filters_text_for`, so in the end we issue one less SQL query than before;
- In the case when more than one label are selected, the `COUNT` queries return an OrderedHash in the form `{ ISSUABLE_ID => COUNT_OF_SELECTED_FILTERS }` on which `#count` is called: this drawback is already in place (for instance when loading https://gitlab.com/gitlab-org/gitlab-ce/issues?scope=all&state=all&utf8=%E2%9C%93&label_name%5B%5D=bug&label_name%5B%5D=regression) since that's how Kaminari solves this, **the difference is that now we do that two more times for the two states that are not currently selected**. I will let the ~Performance team decide if that's something acceptable or not, otherwise we will have to find another solution...
- The queries that count the # of issuable are a bit more complex than before, from:

    ```
   (0.6ms)  SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = $1 AND ("issues"."state" IN ('opened','reopened'))  [["project_id", 2]]
   (0.2ms)  SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = $1 AND ("issues"."state" IN ('closed'))  [["project_id", 2]]
   (0.2ms)  SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = $1  [["project_id", 2]]
    ```

    to

    ```
   (0.7ms)  SELECT COUNT(*) AS count_all, "issues"."id" AS issues_id FROM "issues" INNER JOIN "label_links" ON "label_links"."target_id" = "issues"."id" AND "label_links"."target_type" = $1 INNER JOIN "labels" ON "labels"."id" = "label_links"."label_id" WHERE "issues"."deleted_at" IS NULL AND ("issues"."state" IN ('opened','reopened')) AND "issues"."project_id" = 2 AND "labels"."title" IN ('bug', 'discussion') AND "labels"."project_id" = 2 GROUP BY "issues"."id" HAVING COUNT(DISTINCT labels.title) = 2  [["target_type", "Issue"]]
   (0.5ms)  SELECT COUNT(*) AS count_all, "issues"."id" AS issues_id FROM "issues" INNER JOIN "label_links" ON "label_links"."target_id" = "issues"."id" AND "label_links"."target_type" = $1 INNER JOIN "labels" ON "labels"."id" = "label_links"."label_id" WHERE "issues"."deleted_at" IS NULL AND ("issues"."state" IN ('closed')) AND "issues"."project_id" = 2 AND "labels"."title" IN ('bug', 'discussion') AND "labels"."project_id" = 2 GROUP BY "issues"."id" HAVING COUNT(DISTINCT labels.title) = 2  [["target_type", "Issue"]]
   (0.5ms)  SELECT COUNT(*) AS count_all, "issues"."id" AS issues_id FROM "issues" INNER JOIN "label_links" ON "label_links"."target_id" = "issues"."id" AND "label_links"."target_type" = $1 INNER JOIN "labels" ON "labels"."id" = "label_links"."label_id" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = 2 AND "labels"."title" IN ('bug', 'discussion') AND "labels"."project_id" = 2 GROUP BY "issues"."id" HAVING COUNT(DISTINCT labels.title) = 2  [["target_type", "Issue"]]
    ```
- We could cache the counters for a few minutes? The key could be `PROJECT_ID-ISSUABLE_TYPE-PARAMS`.

A few possible arguments in favor of "it's an acceptable solution":
- most of the time people filter with a single label => no performance problem here
- when filtering with more than one label, usually the result set is reduced, limiting the performance issues 

## What are the relevant issue numbers?

Closes #15356

See merge request !6496
2016-09-30 11:11:04 +00:00
..
config Small refactor and a few documentation fixes 2016-08-04 19:02:39 +02:00
controllers fix broken repo 500 errors in UI and added relevant specs 2016-09-29 16:58:14 +02:00
factories fix broken repo 500 errors in UI and added relevant specs 2016-09-29 16:58:14 +02:00
features Small improvements thanks to Robert's feedback 2016-09-30 12:02:54 +02:00
finders New AccessRequestsFinder 2016-09-28 08:46:59 +02:00
fixtures Returns the total number of issues in the JSON response 2016-08-31 09:30:37 +01:00
helpers Small improvements thanks to Robert's feedback 2016-09-30 12:02:54 +02:00
initializers Give priority to environment variables 2016-08-03 15:48:48 +01:00
javascripts Send ajax request for label update only if they are changed (#19472 !5071) 2016-09-27 10:23:15 -05:00
lib Merge branch 'lfs-ssh-authorization-fix' into 'master' 2016-09-28 18:13:34 +00:00
mailers Wrap List-Unsubscribe link in angle brackets 2016-09-26 16:01:17 +01:00
models Allowing ">" to be used for Milestone models's title and storing the value in db as unescaped. 2016-09-29 19:28:38 -07:00
policies Test if issue authors can access private projects 2016-09-20 14:57:23 -03:00
requests Merge branch 'koding-setting-api' into 'master' 2016-09-30 11:06:08 +00:00
routing Fix markdown help references 2016-08-26 09:38:21 -05:00
services Close todos when accepting a MR via the API. 2016-09-29 14:51:12 -04:00
support Small improvements thanks to Robert's feedback 2016-09-30 12:02:54 +02:00
tasks/gitlab Use File::exist? instead of File::exists? 2016-08-11 13:54:45 +03:00
uploaders
views Upgrade Devise from 4.1.1 to 4.2.0. 2016-09-27 20:08:49 -06:00
workers Added cron to prune events older than 12 months. 2016-09-07 19:41:25 +02:00
factories_spec.rb adds second batch of tests changed to active tense 2016-08-09 15:11:39 +01:00
rails_helper.rb
simplecov_env.rb Bump SimpleCov merge timeout to 365 days 2016-08-22 13:44:14 +02:00
spec_helper.rb Upgrade Devise from 4.1.1 to 4.2.0. 2016-09-27 20:08:49 -06:00
teaspoon_env.rb Add test coverage analysis for CoffeeScript (!5052) 2016-08-07 21:52:37 +02:00