diff options
author | Robert Speicher <robert@gitlab.com> | 2016-09-30 11:11:04 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2016-09-30 11:11:04 +0000 |
commit | 6591b594d4907be0c235a9c05e40ac5bbb995d94 (patch) | |
tree | 236c49cef2211e10cd49ea425ad5cdb45fe9ff4a /lib | |
parent | 4f92f29e703cba47b835285041cbd40063958716 (diff) | |
parent | 56259155d5a0ecfbbafb7319651495582504cfb8 (diff) | |
download | gitlab-ce-6591b594d4907be0c235a9c05e40ac5bbb995d94.tar.gz |
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
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/milestones.rb | 3 |
1 files changed, 1 insertions, 2 deletions
diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index 7a0cb7c99f3..9b73f6826cf 100644 --- a/lib/api/milestones.rb +++ b/lib/api/milestones.rb @@ -108,8 +108,7 @@ module API finder_params = { project_id: user_project.id, - milestone_title: @milestone.title, - state: 'all' + milestone_title: @milestone.title } issues = IssuesFinder.new(current_user, finder_params).execute |