Add params validations and remove extra params support

Remove label_name and milestone_title params support
Add mutually_exclusive validation for author_id and author_username
Add mutually_exclusive validation for assignee_id and assignee_username
Add validation to allow single value for asignee_username on CE
Add separate issue_stats_params helper for statistics params and
reuse in issues_params.

app/finders/issuable_finder.rb

@@ -275,14 +275,6 @@ class IssuableFinder
     params[:assignee_username].present?
   end
 
-  def assignee_username
-    if params[:assignee_username].is_a?(Array)
-      params[:assignee_username].first
-    else
-      params[:assignee_username]
-    end
-  end
-
   # rubocop: disable CodeReuse/ActiveRecord
   def assignee
     return @assignee if defined?(@assignee)
@@ -291,7 +283,7 @@ class IssuableFinder
       if assignee_id?
         User.find_by(id: params[:assignee_id])
       elsif assignee_username?
-        User.find_by_username(assignee_username)
+        User.find_by_username(params[:assignee_username])
       else
         nil
       end

changelogs/unreleased/ce-57402-add-issues-statistics-api-endpoints.yml

+---
+title: Add issues_statistics api endpoints and extend issues search api
+merge_request: 27366
+author:
+type: added

lib/api/helpers/issues_helpers.rb

@@ -24,7 +24,6 @@ module API
 
         args.delete(:id)
         args[:milestone_title] ||= args.delete(:milestone)
-        args[:milestone_title] ||= args.delete(:milestone_title)
         args[:label_name] ||= args.delete(:labels)
         args[:scope] = args[:scope].underscore if args[:scope]
 
@@ -35,10 +34,8 @@ module API
         # rubocop: disable CodeReuse/ActiveRecord
         finder = issue_finder(args)
         issues = finder.execute.with_api_entity_associations
-        order_by = declared_params[:sort].present? && %w(asc desc).include?(declared_params[:sort].downcase)
-        issues = issues.reorder(order_options_with_tie_breaker) if order_by
 
-        issues
+        issues.reorder(order_options_with_tie_breaker)
         # rubocop: enable CodeReuse/ActiveRecord
       end
 

lib/api/issues.rb

@@ -19,15 +19,10 @@ module API
         end
       end
 
-      params :issues_params do
-        optional :labels, :label_name, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
-        optional :with_labels_data, type: Boolean, desc: 'Return more label data than just lable title', default: false
+      params :issues_stats_params do
+        optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
         optional :milestone, type: String, desc: 'Milestone title'
-        optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at',
-                            desc: 'Return issues ordered by `created_at` or `updated_at` fields.'
-        optional :sort, type: String, default: 'desc',
-                        desc: 'Return issues sorted in `asc` or `desc` order.'
-        optional :milestone, :milestone_title, type: String, desc: 'Return issues for a specific milestone'
+        optional :milestone, type: String, desc: 'Return issues for a specific milestone'
         optional :iids, type: Array[Integer], desc: 'The IID array of issues'
         optional :search, type: String, desc: 'Search issues for text present in the title, description, or any combination of these'
         optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma'
@@ -35,23 +30,39 @@ module API
         optional :created_before, type: DateTime, desc: 'Return issues created before the specified time'
         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 :author_username, type: String, desc: 'Return issues which are authored by the user with the given username'
+        mutually_exclusive :author_id, :author_username
+
         optional :assignee_id, types: [Integer, String], integer_none_any: true,
                                desc: 'Return issues which are assigned to the user with the given ID'
-        optional :assignee_username, type: Array[String],
+        optional :assignee_username, type: Array[String], check_assignees_count: true,
+                                     coerce_with: Validations::CheckAssigneesCount.coerce,
                                      desc: 'Return issues which are assigned to the user with the given username'
+        mutually_exclusive :assignee_id, :assignee_username
+
         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'
         optional :confidential, type: Boolean, desc: 'Filter confidential or public issues'
-        optional :state, type: String, values: %w[opened closed all], default: 'all',
-                         desc: 'Return opened, closed, or all issues'
-        use :pagination
 
         use :issues_params_ee if Gitlab.ee?
       end
 
+      params :issues_params do
+        optional :with_labels_data, type: Boolean, desc: 'Return more label data than just lable title', default: false
+        optional :state, type: String, values: %w[opened closed all], default: 'all',
+                 desc: 'Return opened, closed, or all issues'
+        optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at',
+                 desc: 'Return issues ordered by `created_at` or `updated_at` fields.'
+        optional :sort, type: String, values: %w[asc desc], default: 'desc',
+                 desc: 'Return issues sorted in `asc` or `desc` order.'
+
+        use :issues_stats_params
+        use :pagination
+      end
+
       params :issue_params do
         optional :description, type: String, desc: 'The description of an issue'
         optional :assignee_ids, type: Array[Integer], desc: 'The array of user IDs to assign issue'
@@ -68,7 +79,7 @@ module API
 
     desc "Get currently authenticated user's issues statistics"
     params do
-      use :issues_params
+      use :issues_stats_params
       optional :scope, type: String, values: %w[created-by-me assigned-to-me created_by_me assigned_to_me all], default: 'created_by_me',
                        desc: 'Return issues for the given scope: `created_by_me`, `assigned_to_me` or `all`'
     end
@@ -131,7 +142,7 @@ module API
 
       desc 'Get statistics for the list of group issues'
       params do
-        use :issues_params
+        use :issues_stats_params
       end
       get ":id/issues_statistics" do
         group = find_group!(params[:id])
@@ -172,7 +183,7 @@ module API
 
       desc 'Get statistics for the list of project issues'
       params do
-        use :issues_params
+        use :issues_stats_params
       end
       get ":id/issues_statistics" do
         project = find_project!(params[:id])

lib/api/validations/check_assignees_count.rb

+# frozen_string_literal: true
+
+module API
+  module Validations
+    class CheckAssigneesCount < Grape::Validations::Base
+      def self.coerce
+        lambda do |value|
+          case value
+          when String
+            [value]
+          when Array
+            value
+          when CheckAssigneesCount
+            value
+          else
+            []
+          end
+        end
+      end
+
+      def validate_param!(attr_name, params)
+        unless param_allowed?(attr_name, params)
+          raise Grape::Exceptions::Validation,
+                params: [@scope.full_name(attr_name)],
+                message: "allows one value, but found #{params[attr_name].size}: #{params[attr_name].join(", ")}"
+        end
+      end
+
+      private
+
+      def param_allowed?(attr_name, params)
+        params[attr_name].size <= 1
+      end
+    end
+  end
+end

spec/requests/api/issues/get_group_issues_spec.rb

@@ -619,11 +619,33 @@ describe API::Issues do
         let!(:issue2) { create(:issue, author: user2, project: group_project, created_at: 2.days.ago) }
         let!(:issue3) { create(:issue, author: user2, assignees: [assignee, another_assignee], project: group_project, created_at: 1.day.ago) }
 
-        it 'returns issues with multiple assignees' do
-          get api("/groups/#{group.id}/issues", user), params: { assignee_username: [assignee.username, another_assignee.username] }
+        it 'returns issues with by assignee_username' do
+          get api(base_url, user), params: { assignee_username: [assignee.username], scope: 'all' }
 
+          expect(issue3.reload.assignees.pluck(:id)).to match_array([assignee.id, another_assignee.id])
           expect_paginated_array_response([issue3.id, group_confidential_issue.id])
         end
+
+        it 'returns issues by assignee_username as string' do
+          get api(base_url, user), params: { assignee_username: assignee.username, scope: 'all' }
+
+          expect(issue3.reload.assignees.pluck(:id)).to match_array([assignee.id, another_assignee.id])
+          expect_paginated_array_response([issue3.id, group_confidential_issue.id])
+        end
+
+        it 'returns error when multiple assignees are passed' do
+          get api(base_url, user), params: { assignee_username: [assignee.username, another_assignee.username], scope: 'all' }
+
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response["error"]).to include("allows one value, but found 2")
+        end
+
+        it 'returns error when assignee_username and assignee_id are passed together' do
+          get api(base_url, user), params: { assignee_username: [assignee.username], assignee_id: another_assignee.id, scope: 'all' }
+
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response["error"]).to include("mutually exclusive")
+        end
       end
     end
   end

spec/requests/api/issues/get_project_issues_spec.rb

@@ -490,11 +490,33 @@ describe API::Issues do
       let!(:issue2) { create(:issue, author: user2, project: project, created_at: 2.days.ago) }
       let!(:issue3) { create(:issue, author: user2, assignees: [assignee, another_assignee], project: project, created_at: 1.day.ago) }
 
-      it 'returns issues with multiple assignees' do
-        get api("#{base_url}/issues", user), params: { assignee_username: [assignee.username, another_assignee.username] }
+      it 'returns issues by assignee_username' do
+        get api("/issues", user), params: { assignee_username: [assignee.username], scope: 'all' }
 
+        expect(issue3.reload.assignees.pluck(:id)).to match_array([assignee.id, another_assignee.id])
         expect_paginated_array_response([confidential_issue.id, issue3.id])
       end
+
+      it 'returns issues by assignee_username as string' do
+        get api("/issues", user), params: { assignee_username: assignee.username, scope: 'all' }
+
+        expect(issue3.reload.assignees.pluck(:id)).to match_array([assignee.id, another_assignee.id])
+        expect_paginated_array_response([confidential_issue.id, issue3.id])
+      end
+
+      it 'returns error when multiple assignees are passed' do
+        get api("/issues", user), params: { assignee_username: [assignee.username, another_assignee.username], scope: 'all' }
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response["error"]).to include("allows one value, but found 2")
+      end
+
+      it 'returns error when assignee_username and assignee_id are passed together' do
+        get api("/issues", user), params: { assignee_username: [assignee.username], assignee_id: another_assignee.id, scope: 'all' }
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response["error"]).to include("mutually exclusive")
+      end
     end
   end
 

spec/requests/api/issues/issues_spec.rb

@@ -691,12 +691,33 @@ describe API::Issues do
         let!(:issue2) { create(:issue, author: user2, project: project, created_at: 2.days.ago) }
         let!(:issue3) { create(:issue, author: user2, assignees: [assignee, another_assignee], project: project, created_at: 1.day.ago) }
 
-        it 'returns issues with multiple assignees' do
-          get api("/issues", user), params: { assignee_username: [assignee.username, another_assignee.username], scope: 'all' }
+        it 'returns issues with by assignee_username' do
+          get api("/issues", user), params: { assignee_username: [assignee.username], scope: 'all' }
+
+          expect(issue3.reload.assignees.pluck(:id)).to match_array([assignee.id, another_assignee.id])
+          expect_paginated_array_response([confidential_issue.id, issue3.id])
+        end
+
+        it 'returns issues by assignee_username as string' do
+          get api("/issues", user), params: { assignee_username: assignee.username, scope: 'all' }
 
-          expect(issue3.reload.assignees).to eq([assignee, another_assignee])
+          expect(issue3.reload.assignees.pluck(:id)).to match_array([assignee.id, another_assignee.id])
           expect_paginated_array_response([confidential_issue.id, issue3.id])
         end
+
+        it 'returns error when multiple assignees are passed' do
+          get api("/issues", user), params: { assignee_username: [assignee.username, another_assignee.username], scope: 'all' }
+
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response["error"]).to include("allows one value, but found 2")
+        end
+
+        it 'returns error when assignee_username and assignee_id are passed together' do
+          get api("/issues", user), params: { assignee_username: [assignee.username], assignee_id: another_assignee.id, scope: 'all' }
+
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response["error"]).to include("mutually exclusive")
+        end
       end
     end
   end

spec/support/shared_examples/requests/api/issues_shared_example_spec.rb

@@ -28,19 +28,7 @@ shared_examples 'labeled issues with labels and label_name params' do
     it_behaves_like 'returns label names'
   end
 
-  context 'array of labeled issues when all labels match the label_name param' do
-    let(:params) { { label_name: "#{label.title},#{label_b.title},#{label_c.title}" } }
-
-    it_behaves_like 'returns label names'
-  end
-
-  context 'array of labeled issues when all labels match with label_name param as array' do
-    let(:params) { { label_name: [label.title, label_b.title, label_c.title] } }
-
-    it_behaves_like 'returns label names'
-  end
-
-  context 'with labels data' do
+  context 'when with_labels_data provided' do
     context 'array of labeled issues when all labels match' do
       let(:params) { { labels: "#{label.title},#{label_b.title},#{label_c.title}", with_labels_data: true } }
 
@@ -52,17 +40,5 @@ shared_examples 'labeled issues with labels and label_name params' do
 
       it_behaves_like 'returns basic label entity'
     end
-
-    context 'array of labeled issues when all labels match the label_name param' do
-      let(:params) { { label_name: "#{label.title},#{label_b.title},#{label_c.title}", with_labels_data: true } }
-
-      it_behaves_like 'returns basic label entity'
-    end
-
-    context 'array of labeled issues when all labels match with label_name param as array' do
-      let(:params) { { label_name: [label.title, label_b.title, label_c.title], with_labels_data: true } }
-
-      it_behaves_like 'returns basic label entity'
-    end
   end
 end