Improve method names and add more specsissue_20900

8 files changed, 21 insertions, 20 deletions

diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 0f4d4d841ba..01599ce49c6 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -1,7 +1,7 @@
 module Milestoneish
   def closed_items_count(user)
     memoize_per_user(user, :closed_items_count) do
-      (count_issues_by_state(user)['closed']&.length || 0) + merge_requests.closed_and_merged.size
+      (count_issues_by_state(user)['closed'] || 0) + merge_requests.closed_and_merged.size
     end
   end
 
@@ -12,7 +12,7 @@ module Milestoneish
   end
 
   def total_issues_count(user)
-    issues_visible_to_user(user).length
+    count_issues_by_state(user).values.sum
   end
 
   def complete?(user)
@@ -44,6 +44,14 @@ module Milestoneish
     end
   end
 
+  def sorted_issues(user)
+    issues_visible_to_user(user).preload_associations.sort('label_priority')
+  end
+
+  def sorted_merge_requests
+    merge_requests.sort('label_priority')
+  end
+
   def upcoming?
     start_date && start_date.future?
   end
@@ -62,17 +70,11 @@ module Milestoneish
     due_date && due_date.past?
   end
 
-  def sorted_merge_requests
-    merge_requests.sort('label_priority')
-  end
-
   private
 
   def count_issues_by_state(user)
     memoize_per_user(user, :count_issues_by_state) do
-      # Need to group and count using ruby array to not break
-      # label ordering. Also it saves a SQL query.
-      issues_visible_to_user(user).to_a.group_by(&:state)
+      issues_visible_to_user(user).reorder(nil).group(:state).count
     end
   end
 
@@ -85,6 +87,6 @@ module Milestoneish
   # override in a class that includes this module to get a faster query
   # from IssuesFinder
   def issues_finder_params
-    { sort: 'label_priority' }
+    {}
   end
 end
diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb
index e2af4040870..646c1e5ce1a 100644
--- a/app/models/dashboard_milestone.rb
+++ b/app/models/dashboard_milestone.rb
@@ -1,5 +1,5 @@
 class DashboardMilestone < GlobalMilestone
   def issues_finder_params
-    super.merge({ authorized_only: true })
+    { authorized_only: true }
   end
 end
diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb
index 4db20470220..86d38e5468b 100644
--- a/app/models/group_milestone.rb
+++ b/app/models/group_milestone.rb
@@ -14,6 +14,6 @@ class GroupMilestone < GlobalMilestone
   end
 
   def issues_finder_params
-    super.merge({ group_id: group.id })
+    { group_id: group.id }
   end
 end
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 14647602c02..0a6fc064aec 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -187,6 +187,6 @@ class Milestone < ActiveRecord::Base
   end
 
   def issues_finder_params
-    super.merge({ project_id: project_id })
+    { project_id: project_id }
   end
 end
diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb
index 65b204d4dd2..bd5211b8e58 100644
--- a/app/serializers/issuable_entity.rb
+++ b/app/serializers/issuable_entity.rb
@@ -5,7 +5,6 @@ class IssuableEntity < Grape::Entity
   expose :description
   expose :lock_version
   expose :milestone_id
-  expose :position
   expose :state
   expose :title
   expose :updated_by_id
diff --git a/app/views/shared/milestones/_sidebar.html.haml b/app/views/shared/milestones/_sidebar.html.haml
index e75edd750ff..9bb87640319 100644
--- a/app/views/shared/milestones/_sidebar.html.haml
+++ b/app/views/shared/milestones/_sidebar.html.haml
@@ -68,10 +68,10 @@
         .sidebar-collapsed-icon
           %strong
             = icon('hashtag', 'aria-hidden': 'true')
-          %span= milestone.issues_visible_to_user(current_user).length
+          %span= milestone.issues_visible_to_user(current_user).count
         .title.hide-collapsed
           Issues
-          %span.badge= milestone.issues_visible_to_user(current_user).length
+          %span.badge= milestone.issues_visible_to_user(current_user).count
           - if project && can?(current_user, :create_issue, project)
             = link_to new_namespace_project_issue_path(project.namespace, project, issue: { milestone_id: milestone.id }), class: "pull-right", title: "New Issue" do
               New issue
@@ -79,11 +79,11 @@
           %span.milestone-stat
             = link_to milestones_browse_issuables_path(milestone, type: :issues) do
               Open:
-              = milestone.issues_visible_to_user(current_user).opened.length
+              = milestone.issues_visible_to_user(current_user).opened.count
           %span.milestone-stat
             = link_to milestones_browse_issuables_path(milestone, type: :issues, state: 'closed') do
               Closed:
-              = milestone.issues_visible_to_user(current_user).closed.length
+              = milestone.issues_visible_to_user(current_user).closed.count
 
     .block.merge-requests
       .sidebar-collapsed-icon
diff --git a/app/views/shared/milestones/_tabs.html.haml b/app/views/shared/milestones/_tabs.html.haml
index b1d24578daf..4de8a6cb15f 100644
--- a/app/views/shared/milestones/_tabs.html.haml
+++ b/app/views/shared/milestones/_tabs.html.haml
@@ -31,7 +31,7 @@
 .tab-content.milestone-content
   - if milestone.is_a?(GlobalMilestone) || can?(current_user, :read_issue, @project)
     .tab-pane.active#tab-issues{ data: { sort_endpoint: (sort_issues_namespace_project_milestone_path(@project.namespace, @project, @milestone) if @project && current_user) } }
-      = render 'shared/milestones/issues_tab', issues: milestone.issues_visible_to_user(current_user).preload_associations, show_project_name: show_project_name, show_full_project_name: show_full_project_name
+      = render 'shared/milestones/issues_tab', issues: milestone.sorted_issues(current_user), show_project_name: show_project_name, show_full_project_name: show_full_project_name
     .tab-pane#tab-merge-requests
       -# loaded async
       = render "shared/milestones/tab_loading"
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 36c2969a555..2562f085338 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -50,7 +50,7 @@
           - project_name = group ? ms.project.name : ms.project.name_with_namespace
           = link_to project_name, namespace_project_milestone_path(ms.project.namespace, ms.project, ms)
         %td
-          = ms.issues_visible_to_user(current_user).opened.length
+          = ms.issues_visible_to_user(current_user).opened.count
         %td
           - if ms.closed?
             Closed