Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-10-26

# Conflicts:
#	app/views/shared/boards/components/_board.html.haml

[ci skip]

app/assets/javascripts/boards/components/board.js

@@ -53,6 +53,9 @@ export default Vue.extend({
       const { issuesSize } = this.list;
       return `${n__('%d issue', '%d issues', issuesSize)}`;
     },
+    isNewIssueShown() {
+      return this.list.type === 'backlog' || (!this.disabled && this.list.type !== 'closed');
+    }
   },
   watch: {
     filter: {

app/assets/javascripts/boards/components/project_select.vue

@@ -46,7 +46,7 @@ export default {
       selectable: true,
       data: (term, callback) => {
         this.loading = true;
-        return Api.groupProjects(this.groupId, term, {}, projects => {
+        return Api.groupProjects(this.groupId, term, {with_issues_enabled: true}, projects => {
           this.loading = false;
           callback(projects);
         });

app/controllers/concerns/boards_responses.rb

@@ -50,7 +50,10 @@ module BoardsResponses
   end
 
   def authorize_create_issue
-    authorize_action_for!(project, :admin_issue)
+    list = List.find(issue_params[:list_id])
+    action = list.backlog? ? :create_issue : :admin_issue
+
+    authorize_action_for!(project, action)
   end
 
   def authorize_admin_list

app/finders/issuable_finder.rb

@@ -14,7 +14,7 @@
 #     project_id: integer
 #     milestone_title: string
 #     author_id: integer
-#     assignee_id: integer
+#     assignee_id: integer or 'None' or 'Any'
 #     search: string
 #     label_name: string
 #     sort: string
@@ -34,6 +34,11 @@ class IssuableFinder
 
   requires_cross_project_access unless: -> { project? }
 
+  # This is used as a common filter for None / Any
+  FILTER_NONE = 'none'.freeze
+  FILTER_ANY = 'any'.freeze
+
+  # This is accepted as a deprecated filter and is also used in unassigning users
   NONE = '0'.freeze
 
   attr_accessor :current_user, :params
@@ -236,16 +241,20 @@ class IssuableFinder
   # rubocop: enable CodeReuse/ActiveRecord
 
   def assignee_id?
-    params[:assignee_id].present? && params[:assignee_id].to_s != NONE
+    params[:assignee_id].present?
   end
 
   def assignee_username?
-    params[:assignee_username].present? && params[:assignee_username].to_s != NONE
+    params[:assignee_username].present?
   end
 
-  def no_assignee?
+  def filter_by_no_assignee?
     # Assignee_id takes precedence over assignee_username
-    params[:assignee_id].to_s == NONE || params[:assignee_username].to_s == NONE
+    [NONE, FILTER_NONE].include?(params[:assignee_id].to_s.downcase) || params[:assignee_username].to_s == NONE
+  end
+
+  def filter_by_any_assignee?
+    params[:assignee_id].to_s.downcase == FILTER_ANY
   end
 
   # rubocop: disable CodeReuse/ActiveRecord
@@ -399,16 +408,18 @@ class IssuableFinder
 
   # rubocop: disable CodeReuse/ActiveRecord
   def by_assignee(items)
-    if assignee
-      items = items.where(assignee_id: assignee.id)
-    elsif no_assignee?
-      items = items.where(assignee_id: nil)
+    if filter_by_no_assignee?
+      items.where(assignee_id: nil)
+    elsif filter_by_any_assignee?
+      items.where('assignee_id IS NOT NULL')
+    elsif assignee
+      items.where(assignee_id: assignee.id)
     elsif assignee_id? || assignee_username? # assignee not found
-      items = items.none
-    end
-
+      items.none
+    else
       items
     end
+  end
   # rubocop: enable CodeReuse/ActiveRecord
 
   # rubocop: disable CodeReuse/ActiveRecord

app/finders/issues_finder.rb

@@ -139,10 +139,12 @@ class IssuesFinder < IssuableFinder
 
   # rubocop: disable CodeReuse/ActiveRecord
   def by_assignee(items)
-    if assignee
-      items.assigned_to(assignee)
-    elsif no_assignee?
+    if filter_by_no_assignee?
       items.unassigned
+    elsif filter_by_any_assignee?
+      items.assigned
+    elsif assignee
+      items.assigned_to(assignee)
     elsif assignee_id? || assignee_username? # assignee not found
       items.none
     else

app/views/shared/boards/components/_board.html.haml

@@ -39,6 +39,7 @@
             {{ list.issuesSize }}
           = render_if_exists "shared/boards/components/list_weight"
 
+<<<<<<< HEAD
         - if can?(current_user, :admin_list, current_board_parent)
           %button.issue-count-badge-add-button.btn.btn-sm.btn-default.ml-1.has-tooltip.js-no-trigger-collapse{ type: "button",
             "@click" => "showNewIssueForm",
@@ -47,6 +48,16 @@
             "title" => _("New issue"),
             data: { placement: "top", container: "body" } }
             = icon("plus", class: "js-no-trigger-collapse")
+=======
+        %button.issue-count-badge-add-button.btn.btn-sm.btn-default.ml-1.has-tooltip.js-no-trigger-collapse{ type: "button",
+          "@click" => "showNewIssueForm",
+          "v-if" => "isNewIssueShown",
+          "aria-label" => _("New issue"),
+          "title" => _("New issue"),
+          data: { placement: "top", container: "body" } }
+          = icon("plus", class: "js-no-trigger-collapse")
+
+>>>>>>> upstream/master
     %board-list{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"',
       ":list" => "list",
       ":issues" => "list.issues",

changelogs/unreleased/52384-api-filter-assignee-none-any.yml

+---
+title: Add None/Any option for assignee_id in Issues and Merge Requests API
+merge_request: 22598
+author: Heinrich Lee Yu
+type: added

changelogs/unreleased/52545-guest-create-issue-in-group-board.yml

+---
+title: Always show new issue button in boards' Open list
+merge_request: 22557
+author: Heinrich Lee Yu
+type: fixed

doc/api/issues.md

@@ -40,7 +40,7 @@ GET /issues?my_reaction_emoji=star
 | `milestone`         | string           | no         | The milestone title. `No+Milestone` lists all issues with no milestone. `Any+Milestone` lists all issues that have an assigned milestone            |
 | `scope`             | string           | no         | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([Introduced][ce-13004] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ |
 | `author_id`         | integer          | no         | Return issues created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me`. _([Introduced][ce-13004] in GitLab 9.5)_          |
-| `assignee_id`       | integer          | no         | Return issues assigned to the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_                                                              |
+| `assignee_id`       | integer          | no         | Return issues assigned to the given user `id`. `None` returns unassigned issues. `Any` returns issues with an assignee. _([Introduced][ce-13004] in GitLab 9.5)_ |
 | `my_reaction_emoji` | string           | no         | Return issues reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_                                      |
 | `iids[]`            | Array[integer]   | no         | Return only the issues having the given `iid`                                                                                                       |
 | `order_by`          | string           | no         | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at`                                                               |
@@ -155,7 +155,7 @@ GET /groups/:id/issues?my_reaction_emoji=star
 | `milestone`         | string           | no         | The milestone title. `No+Milestone` lists all issues with no milestone                                                                                                           |
 | `scope`             | string           | no         | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([Introduced][ce-13004] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ |
 | `author_id`         | integer          | no         | Return issues created by the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_                                         |
-| `assignee_id`       | integer          | no         | Return issues assigned to the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_                                        |
+| `assignee_id`       | integer          | no         | Return issues assigned to the given user `id`. `None` returns unassigned issues. `Any` returns issues with an assignee. _([Introduced][ce-13004] in GitLab 9.5)_ |
 | `my_reaction_emoji` | string           | no         | Return issues reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_                |
 | `order_by`          | string           | no         | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at`                                         |
 | `sort`              | string           | no         | Return issues sorted in `asc` or `desc` order. Default is `desc`                                                              |
@@ -270,7 +270,7 @@ GET /projects/:id/issues?my_reaction_emoji=star
 | `milestone`         | string           | no         | The milestone title. `No+Milestone` lists all issues with no milestone                                                                                                           |
 | `scope`             | string           | no         | Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([Introduced][ce-13004] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ |
 | `author_id`         | integer          | no         | Return issues created by the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_                                         |
-| `assignee_id`       | integer          | no         | Return issues assigned to the given user `id` _([Introduced][ce-13004] in GitLab 9.5)_                                        |
+| `assignee_id`       | integer          | no         | Return issues assigned to the given user `id`. `None` returns unassigned issues. `Any` returns issues with an assignee. _([Introduced][ce-13004] in GitLab 9.5)_ |
 | `my_reaction_emoji` | string           | no         | Return issues reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_                |
 | `order_by`          | string           | no         | Return issues ordered by `created_at` or `updated_at` fields. Default is `created_at`                                         |
 | `sort`              | string           | no         | Return issues sorted in `asc` or `desc` order. Default is `desc`                                                              |

doc/api/merge_requests.md

@@ -42,7 +42,7 @@ Parameters:
 | `updated_before`    | datetime | no       | Return merge requests updated on or before the given time                                                              |
 | `scope`             | string   | no       | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead. |
 | `author_id`         | integer  | no       | Returns merge requests created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me`              |
-| `assignee_id`       | integer  | no       | Returns merge requests assigned to the given user `id`                                                                 |
+| `assignee_id`       | integer  | no       | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. |
 | `my_reaction_emoji` | string   | no       | Return merge requests reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_ |
 | `source_branch`     | string   | no       | Return merge requests with the given source branch                                                                     |
 | `target_branch`     | string   | no       | Return merge requests with the given target branch                                                                     |
@@ -167,7 +167,7 @@ Parameters:
 | `updated_before`    | datetime       | no       | Return merge requests updated on or before the given time                                                                      |
 | `scope`             | string         | no       | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([Introduced][ce-13060] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ |
 | `author_id`         | integer        | no       | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_                                 |
-| `assignee_id`       | integer        | no       | Returns merge requests assigned to the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_                                |
+| `assignee_id`       | integer        | no       | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ |
 | `my_reaction_emoji` | string         | no       | Return merge requests reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_         |
 | `source_branch`     | string   | no       | Return merge requests with the given source branch                                                                     |
 | `target_branch`     | string   | no       | Return merge requests with the given target branch                                                                     |
@@ -281,7 +281,7 @@ Parameters:
 | `updated_before`    | datetime       | no       | Return merge requests updated on or before the given time                                                                      |
 | `scope`             | string         | no       | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br>                                     |
 | `author_id`         | integer        | no       | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_                                 |
-| `assignee_id`       | integer        | no       | Returns merge requests assigned to the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_                                |
+| `assignee_id`       | integer        | no       | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ |
 | `my_reaction_emoji` | string         | no       | Return merge requests reacted by the authenticated user by the given `emoji` _([Introduced][ce-14016] in GitLab 10.0)_         |
 | `source_branch`     | string   | no       | Return merge requests with the given source branch                                                                     |
 | `target_branch`     | string   | no       | Return merge requests with the given target branch                                                                     |

lib/api/helpers/custom_validators.rb

@@ -10,8 +10,21 @@ module API
           raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: message(:absence)
         end
       end
+
+      class IntegerNoneAny < Grape::Validations::Base
+        def validate_param!(attr_name, params)
+          value = params[attr_name]
+
+          return if value.is_a?(Integer) ||
+              [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(value.to_s.downcase)
+
+          raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)],
+                                               message: "should be an integer, 'None' or 'Any'"
+        end
+      end
     end
   end
 end
 
 Grape::Validations.register_validator(:absence, ::API::Helpers::CustomValidators::Absence)
+Grape::Validations.register_validator(:integer_none_any, ::API::Helpers::CustomValidators::IntegerNoneAny)

lib/api/issues.rb

@@ -40,7 +40,8 @@ module API
         optional :updated_after, type: DateTime, desc: 'Return issues updated after the specified time'
         optional :updated_before, type: DateTime, desc: 'Return issues updated before the specified time'
         optional :author_id, type: Integer, desc: 'Return issues which are authored by the user with the given ID'
-        optional :assignee_id, type: Integer, desc: 'Return issues which are assigned to the user with the given ID'
+        optional :assignee_id, types: [Integer, String], integer_none_any: true,
+                               desc: 'Return issues which are assigned to the user with the given ID'
         optional :scope, type: String, values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all],
                          desc: 'Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`'
         optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji'

lib/api/merge_requests.rb

@@ -91,7 +91,8 @@ module API
         optional :updated_before, type: DateTime, desc: 'Return merge requests updated before the specified time'
         optional :view, type: String, values: %w[simple], desc: 'If simple, returns the `iid`, URL, title, description, and basic state of merge request'
         optional :author_id, type: Integer, desc: 'Return merge requests which are authored by the user with the given ID'
-        optional :assignee_id, type: Integer, desc: 'Return merge requests which are assigned to the user with the given ID'
+        optional :assignee_id, types: [Integer, String], integer_none_any: true,
+                               desc: 'Return merge requests which are assigned to the user with the given ID'
         optional :scope, type: String, values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all],
                          desc: 'Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`'
         optional :my_reaction_emoji, type: String, desc: 'Return issues reacted by the authenticated user by the given emoji'

spec/controllers/boards/issues_controller_spec.rb

@@ -208,13 +208,24 @@ describe Boards::IssuesController do
       end
     end
 
-    context 'with unauthorized user' do
+    context 'with guest user' do
+      context 'in open list' do
+        it 'returns a successful 200 response' do
+          open_list = board.lists.create(list_type: :backlog)
+          create_issue user: guest, board: board, list: open_list, title: 'New issue'
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'in label list' do
         it 'returns a forbidden 403 response' do
           create_issue user: guest, board: board, list: list1, title: 'New issue'
 
           expect(response).to have_gitlab_http_status(403)
         end
       end
+    end
 
     def create_issue(user:, board:, list:, title:)
       sign_in(user)

spec/features/boards/new_issue_spec.rb

@@ -94,8 +94,14 @@ describe 'Issue Boards new issue', :js do
       wait_for_requests
     end
 
-    it 'does not display new issue button' do
-      expect(page).to have_selector('.issue-count-badge-add-button', count: 0)
+    it 'displays new issue button in open list' do
+      expect(first('.board')).to have_selector('.issue-count-badge-add-button', count: 1)
+    end
+
+    it 'does not display new issue button in label list' do
+      page.within('.board:nth-child(2)') do
+        expect(page).not_to have_selector('.issue-count-badge-add-button')
+      end
     end
   end
 end

spec/finders/issues_finder_spec.rb

@@ -103,11 +103,37 @@ describe IssuesFinder do
       end
 
       context 'filtering by no assignee' do
-        let(:params) { { assignee_id: 0 } }
+        let(:params) { { assignee_id: 'None' } }
 
-        it 'returns issues not assign to any assignee' do
+        it 'returns issues not assigned to any assignee' do
           expect(issues).to contain_exactly(issue4)
         end
+
+        it 'returns issues not assigned to any assignee' do
+          params[:assignee_id] = 0
+
+          expect(issues).to contain_exactly(issue4)
+        end
+
+        it 'returns issues not assigned to any assignee' do
+          params[:assignee_id] = 'none'
+
+          expect(issues).to contain_exactly(issue4)
+        end
+      end
+
+      context 'filtering by any assignee' do
+        let(:params) { { assignee_id: 'Any' } }
+
+        it 'returns issues assigned to any assignee' do
+          expect(issues).to contain_exactly(issue1, issue2, issue3)
+        end
+
+        it 'returns issues assigned to any assignee' do
+          params[:assignee_id] = 'any'
+
+          expect(issues).to contain_exactly(issue1, issue2, issue3)
+        end
       end
 
       context 'filtering by group_id' do

spec/lib/api/helpers/custom_validators_spec.rb

+require 'spec_helper'
+
+describe API::Helpers::CustomValidators do
+  let(:scope) do
+    Struct.new(:opts) do
+      def full_name(attr_name)
+        attr_name
+      end
+    end
+  end
+
+  describe API::Helpers::CustomValidators::Absence do
+    subject do
+      described_class.new(['test'], {}, false, scope.new)
+    end
+
+    context 'empty param' do
+      it 'does not raise a validation error' do
+        expect_no_validation_error({})
+      end
+    end
+
+    context 'invalid parameters' do
+      it 'should raise a validation error' do
+        expect_validation_error({ 'test' => 'some_value' })
+      end
+    end
+  end
+
+  describe API::Helpers::CustomValidators::IntegerNoneAny do
+    subject do
+      described_class.new(['test'], {}, false, scope.new)
+    end
+
+    context 'valid parameters' do
+      it 'does not raise a validation error' do
+        expect_no_validation_error({ 'test' => 2 })
+        expect_no_validation_error({ 'test' => 100 })
+        expect_no_validation_error({ 'test' => 'None' })
+        expect_no_validation_error({ 'test' => 'Any' })
+        expect_no_validation_error({ 'test' => 'none' })
+        expect_no_validation_error({ 'test' => 'any' })
+      end
+    end
+
+    context 'invalid parameters' do
+      it 'should raise a validation error' do
+        expect_validation_error({ 'test' => 'some_other_string' })
+      end
+    end
+  end
+
+  def expect_no_validation_error(params)
+    expect { validate_test_param!(params) }.not_to raise_error
+  end
+
+  def expect_validation_error(params)
+    expect { validate_test_param!(params) }.to raise_error(Grape::Exceptions::Validation)
+  end
+
+  def validate_test_param!(params)
+    subject.validate_param!('test', params)
+  end
+end

spec/requests/api/issues_spec.rb

@@ -182,6 +182,24 @@ describe API::Issues do
         expect(first_issue['id']).to eq(issue2.id)
       end
 
+      it 'returns issues with no assignee' do
+        issue2 = create(:issue, author: user2, project: project)
+
+        get api('/issues', user), assignee_id: 'None', scope: 'all'
+
+        expect_paginated_array_response(size: 1)
+        expect(first_issue['id']).to eq(issue2.id)
+      end
+
+      it 'returns issues with any assignee' do
+        # This issue without assignee should not be returned
+        create(:issue, author: user2, project: project)
+
+        get api('/issues', user), assignee_id: 'Any', scope: 'all'
+
+        expect_paginated_array_response(size: 3)
+      end
+
       it 'returns issues reacted by the authenticated user by the given emoji' do
         issue2 = create(:issue, project: project, author: user, assignees: [user])
         award_emoji = create(:award_emoji, awardable: issue2, user: user2, name: 'star')

spec/requests/api/merge_requests_spec.rb

@@ -143,6 +143,23 @@ describe API::MergeRequests do
         expect_response_ordered_exactly(merge_request3)
       end
 
+      it 'returns an array of merge requests with no assignee' do
+        merge_request3 = create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, source_branch: 'other-branch')
+
+        get api('/merge_requests', user), assignee_id: 'None', scope: :all
+
+        expect_response_ordered_exactly(merge_request3)
+      end
+
+      it 'returns an array of merge requests with any assignee' do
+        # This MR with no assignee should not be returned
+        create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, source_branch: 'other-branch')
+
+        get api('/merge_requests', user), assignee_id: 'Any', scope: :all
+
+        expect_response_contain_exactly(merge_request, merge_request2, merge_request_closed, merge_request_merged, merge_request_locked)
+      end
+
       it 'returns an array of merge requests assigned to me' do
         merge_request3 = create(:merge_request, :simple, author: user, assignee: user2, source_project: project2, target_project: project2, source_branch: 'other-branch')