summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-09-30 11:11:04 +0000
committerRobert Speicher <robert@gitlab.com>2016-09-30 11:11:04 +0000
commit6591b594d4907be0c235a9c05e40ac5bbb995d94 (patch)
tree236c49cef2211e10cd49ea425ad5cdb45fe9ff4a /lib
parent4f92f29e703cba47b835285041cbd40063958716 (diff)
parent56259155d5a0ecfbbafb7319651495582504cfb8 (diff)
downloadgitlab-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.rb3
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