Improve method names and add more specs

4 files changed, 15 insertions, 13 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