Skip to content

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 (closed).

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

There was an issue (#22414 (closed)) in the original implementation (!4960 (merged)) 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.

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

Why was this MR needed?

Because it's so annoying to have inaccurate counters!!

Screenshots

Before After
Screen_Shot_2016-09-22_at_10.34.30 Screen_Shot_2016-09-22_at_10.32.10

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #15356 (closed)

Merge request reports