1. 30 Sep, 2016 13 commits
    • 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
    • Robert Speicher's avatar
      Merge branch 'koding-setting-api' into 'master' · 4f92f29e
      Robert Speicher authored
      Expose the Koding application settings in the API
      
      ## Why was this MR needed?
      
      When saving the GitLab application secrets in Koding, and authorising your admin user to have access to the UI, we want to let Koding enable the integration, and populate the url in GitLab for the user.
      
      ## What are the relevant issue numbers?
      
      Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/22705
      
      See merge request !6555
      4f92f29e
    • Valery Sizov's avatar
      Merge branch '22768-revert-to-touch-file-system' into 'master' · f80e7683
      Valery Sizov authored
      Revert to use Mounter method to check existence
      
      ## What does this MR do?
      
      Revert a change on gitlab-ce that we never get into gitlab-ee due to spec failures and a lack of a proper solution. So we want to keep both repos in the same codebase about this.
      
      ## Are there points in the code the reviewer needs to double check?
      
      ## Why was this MR needed?
      
      ## Screenshots (if relevant)
      
      ## Does this MR meet the acceptance criteria?
      
      - [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
      - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
      - [ ] API support added
      - Tests
        - [ ] Added for this feature/bug
        - [ ] All builds are passing
      - [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
      - [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
      - [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
      - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
      
      ## What are the relevant issue numbers?
      
      Closes #22768
      
      See merge request !6590
      f80e7683
    • Rémy Coutable's avatar
      56259155
    • Rémy Coutable's avatar
      Cache the issuable counters for 2 minutes · 383dafdf
      Rémy Coutable authored
      Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
      383dafdf
    • Rémy Coutable's avatar
      9b361a3f
    • Rémy Coutable's avatar
      Merge branch '22452-milestone-title-unnecessary-escaping-fix' into 'master' · f9887a10
      Rémy Coutable authored
      This MR fixes a bug that unnecessary escapes reserved HTML characters for Milestone's title.  See #22452. 
      
      ## Are there points in the code the reviewer needs to double check?
      
      - Unescaping of sanitized milestone title before it is being stored in the database. See `Milestone#title` and a private method called `Milestone#sanitize_title`
      - Sufficient tests were added (Model and API tests were modified/added).
      
      ## Why was this MR needed?
      
      To allow reserved HTML characters in a milestone's title, such as "PHP migration 5.6 -> 7.0". The text appears in 'milestones' and in a dropdown during issue creation, issue list, and in another dropdown for issue filter. 
      
      Closes #22452
      
      See merge request !6533
      f9887a10
    • Rémy Coutable's avatar
      Merge branch 'mr_api_todo_close' into 'master' · dde96231
      Rémy Coutable authored
      Closes todos for a merge request when the MR is accepted via the API by the MR assignee.
      
      ## Are there points in the code the reviewer needs to double check?
      
      Please review refresh service test changes to see if they are correct - I think in those cases, the todos should actually be cleared instead of left pending.
      
      ## Why was this MR needed?
      
      To make the API behavior consistent with the UI behavior (accepting your own MRs closes the todo item and prevents them from piling up).
      
      Closes #22477
      
      See merge request !6486
      dde96231
    • Douwe Maan's avatar
      Merge branch 'rc-new-access-requests-finder' into 'master' · db00b495
      Douwe Maan authored
      New `AccessRequestsFinder`
      
      Part of #21979.
      
      ## Does this MR meet the acceptance criteria?
      
      - [x] API support added
      - Tests
        - [x] Added for this feature/bug
        - [x] All builds are passing
      - [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
      - [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
      - [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
      - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
      
      See merge request !6268
      db00b495
    • Paco Guzman's avatar
      Revert to use Mounter method to check existence · 32e32fa0
      Paco Guzman authored
      See: https://gitlab.com/gitlab-org/gitlab-ce/commit/6280fd3777920670ee42111fddf29576cbf85988#note_14766241
      
      We wanted to avoid accesses to the file system, because it seems reasonable that if the mounter column has some content it’s because there are an associate file. This is an speed boost when you access to a bunch of build instances to show their information.
      
      The problem is that solution doesn’t work if you uses that method to detect the thing and you’re changing the uploaded file on instances that had that column empty. Until you don’t persist the instance the database column will be empty so we can have false negatives
      32e32fa0
    • Makoto Scott-Hinkle's avatar
      c81c853a
    • Makoto Scott-Hinkle's avatar
      Merge remote-tracking branch... · 1f6f4a1f
      Makoto Scott-Hinkle authored
      Merge remote-tracking branch 'origin/22452-milestone-title-unnecessary-escaping-fix' into 22452-milestone-title-unnecessary-escaping-fix
      1f6f4a1f
    • Makoto Scott-Hinkle's avatar
      Allowing ">" to be used for Milestone models's title and storing the value in db as unescaped. · 82b13a21
      Makoto Scott-Hinkle authored
      Updating test value for milestone title
      
      Adding API test for title with reserved HTML characters.
      
      Updating changelog
      
      Adding the MR number for fixing bug #22452.
      
      removing duplicate line
      
      Updating MR number.
      82b13a21
  2. 29 Sep, 2016 27 commits