• Robert Speicher's avatar
    Merge branch '15356-filters-should-change-issue-counts' into 'master' · 6591b594
    Robert Speicher authored
    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
    6591b594
To find the state of this project's repository at the time of any of these versions, check out the tags.
CHANGELOG 126 KB