Review updates and cleanupce-57402-add-issues-statistics-api-endpoints

* Cleaned issues and issues_statistics docs
* Renamed param with_labels_data to with_labels_details
* Added spec for N+1 check when retrieving labels from issue
* Refactoed CheckAssigneesCount validation class

4 files changed, 24 insertions, 40 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 1f10dc6c0fc..625fada4f08 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -542,10 +542,15 @@ module API
     class IssueBasic < ProjectEntity
       expose :closed_at
       expose :closed_by, using: Entities::UserBasic
-      expose :labels do |issue|
-        # Avoids an N+1 query since labels are preloaded
-        issue.labels.map(&:title).sort
+
+      expose :labels do |issue, options|
+        if options[:with_labels_details]
+          ::API::Entities::LabelBasic.represent(issue.labels.sort_by(&:title))
+        else
+          issue.labels.map(&:title).sort
+        end
       end
+
       expose :milestone, using: Entities::Milestone
       expose :assignees, :author, using: Entities::UserBasic
 
@@ -573,15 +578,6 @@ module API
     class Issue < IssueBasic
       include ::API::Helpers::RelatedResourcesHelpers
 
-      expose :labels do |issue, options|
-        # Avoids an N+1 query since labels are preloaded
-        if options[:with_labels_data]
-          ::API::Entities::LabelBasic.represent(issue.labels.sort_by(&:title))
-        else
-          issue.labels.map(&:title).sort
-        end
-      end
-
       expose(:has_tasks) do |issue, _|
         !issue.task_list_items.empty?
       end
diff --git a/lib/api/helpers/issues_helpers.rb b/lib/api/helpers/issues_helpers.rb
index fb791691069..fc66cec5341 100644
--- a/lib/api/helpers/issues_helpers.rb
+++ b/lib/api/helpers/issues_helpers.rb
@@ -31,12 +31,10 @@ module API
       end
 
       def find_issues(args = {})
-        # rubocop: disable CodeReuse/ActiveRecord
         finder = issue_finder(args)
         issues = finder.execute.with_api_entity_associations
 
-        issues.reorder(order_options_with_tie_breaker)
-        # rubocop: enable CodeReuse/ActiveRecord
+        issues.reorder(order_options_with_tie_breaker) # rubocop: disable CodeReuse/ActiveRecord
       end
 
       def issues_statistics(args = {})
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 9fd2d959c22..0b4da01f3c8 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -51,7 +51,7 @@ module API
       end
 
       params :issues_params do
-        optional :with_labels_data, type: Boolean, desc: 'Return more label data than just lable title', default: false
+        optional :with_labels_details, 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',
@@ -80,15 +80,13 @@ module API
     desc "Get currently authenticated user's issues statistics"
     params do
       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',
+      optional :scope, type: String, values: %w[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
     get '/issues_statistics' do
       authenticate! unless params[:scope] == 'all'
 
-      stats = issues_statistics
-
-      present stats, with: Grape::Presenters::Presenter
+      present issues_statistics, with: Grape::Presenters::Presenter
     end
 
     resource :issues do
@@ -106,7 +104,7 @@ module API
 
         options = {
           with: Entities::Issue,
-          with_labels_data: declared_params[:with_labels_data],
+          with_labels_details: declared_params[:with_labels_details],
           current_user: current_user,
           issuable_metadata: issuable_meta_data(issues, 'Issue')
         }
@@ -132,7 +130,7 @@ module API
 
         options = {
           with: Entities::Issue,
-          with_labels_data: declared_params[:with_labels_data],
+          with_labels_details: declared_params[:with_labels_details],
           current_user: current_user,
           issuable_metadata: issuable_meta_data(issues, 'Issue')
         }
@@ -147,9 +145,7 @@ module API
       get ":id/issues_statistics" do
         group = find_group!(params[:id])
 
-        stats = issues_statistics(group_id: group.id, include_subgroups: true)
-
-        present stats, with: Grape::Presenters::Presenter
+        present issues_statistics(group_id: group.id, include_subgroups: true), with: Grape::Presenters::Presenter
       end
     end
 
@@ -172,7 +168,7 @@ module API
 
         options = {
           with: Entities::Issue,
-          with_labels_data: declared_params[:with_labels_data],
+          with_labels_details: declared_params[:with_labels_details],
           current_user: current_user,
           project: user_project,
           issuable_metadata: issuable_meta_data(issues, 'Issue')
@@ -188,9 +184,7 @@ module API
       get ":id/issues_statistics" do
         project = find_project!(params[:id])
 
-        stats = issues_statistics(project_id: project.id)
-
-        present stats, with: Grape::Presenters::Presenter
+        present issues_statistics(project_id: project.id), with: Grape::Presenters::Presenter
       end
 
       desc 'Get a single project issue' do
diff --git a/lib/api/validations/check_assignees_count.rb b/lib/api/validations/check_assignees_count.rb
index e19c88e97b1..836ec936b31 100644
--- a/lib/api/validations/check_assignees_count.rb
+++ b/lib/api/validations/check_assignees_count.rb
@@ -6,12 +6,8 @@ module API
       def self.coerce
         lambda do |value|
           case value
-          when String
-            [value]
-          when Array
-            value
-          when CheckAssigneesCount
-            value
+          when String, Array
+            Array.wrap(value)
           else
             []
           end
@@ -19,11 +15,11 @@ module API
       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
+        return if 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
 
       private