From a056dfa9a077def4c3ffb958d3f86f7c9d7c2096 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= <rdavila84@gmail.com>
Date: Mon, 22 Feb 2016 19:08:00 -0500
Subject: [PATCH] Refactor GlobalMilestone queries.

Make methods return ActiveRecord Relations instead of Arrays.
---
 app/models/concerns/issuable.rb               |  1 +
 app/models/global_milestone.rb                | 26 +++++++++----------
 app/models/merge_request.rb                   |  1 -
 app/models/project.rb                         |  1 +
 .../dashboard/milestones/_milestone.html.haml |  2 +-
 app/views/dashboard/milestones/show.html.haml |  2 +-
 .../groups/milestones/_milestone.html.haml    |  2 +-
 app/views/groups/milestones/show.html.haml    |  2 +-
 8 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 286d6655861..a3c4a3d2776 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -29,6 +29,7 @@ module Issuable
     scope :assigned, -> { where("assignee_id IS NOT NULL") }
     scope :unassigned, -> { where("assignee_id IS NULL") }
     scope :of_projects, ->(ids) { where(project_id: ids) }
+    scope :of_milestones, ->(ids) { where(milestone_id: ids) }
     scope :opened, -> { with_state(:opened, :reopened) }
     scope :only_opened, -> { with_state(:opened) }
     scope :only_reopened, -> { with_state(:reopened) }
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 7ee276255a0..e4dd90b631e 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -28,27 +28,27 @@ class GlobalMilestone
   end
 
   def projects
-    milestones.map { |milestone| milestone.project }
+    @projects ||= Project.for_milestones(milestones.map(&:id))
   end
 
-  def issue_count
-    milestones.map { |milestone| milestone.issues.count }.sum
+  def issues_count
+    issues.count
   end
 
   def merge_requests_count
-    milestones.map { |milestone| milestone.merge_requests.count }.sum
+    merge_requests.count
   end
 
   def open_items_count
-    milestones.map { |milestone| milestone.open_items_count }.sum
+    opened_issues.count + opened_merge_requests.count
   end
 
   def closed_items_count
-    milestones.map { |milestone| milestone.closed_items_count }.sum
+    closed_issues.count + closed_merge_requests.count
   end
 
   def total_items_count
-    milestones.map { |milestone| milestone.total_items_count }.sum
+    issues_count + merge_requests_count
   end
 
   def percent_complete
@@ -76,11 +76,11 @@ class GlobalMilestone
   end
 
   def issues
-    @issues ||= milestones.map(&:issues).flatten.group_by(&:state)
+    @issues ||= Issue.of_milestones(milestones.map(&:id))
   end
 
   def merge_requests
-    @merge_requests ||= milestones.map(&:merge_requests).flatten.group_by(&:state)
+    @merge_requests ||= MergeRequest.of_milestones(milestones.map(&:id))
   end
 
   def participants
@@ -88,19 +88,19 @@ class GlobalMilestone
   end
 
   def opened_issues
-    issues.values_at("opened", "reopened").compact.flatten
+    issues.opened
   end
 
   def closed_issues
-    issues['closed']
+    issues.closed
   end
 
   def opened_merge_requests
-    merge_requests.values_at("opened", "reopened").compact.flatten
+    merge_requests.opened
   end
 
   def closed_merge_requests
-    merge_requests.values_at("closed", "merged", "locked").compact.flatten
+    merge_requests.with_states(:closed, :merged, :locked)
   end
 
   def complete?
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 025b522cf66..f575494e2bf 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -139,7 +139,6 @@ class MergeRequest < ActiveRecord::Base
   scope :of_projects, ->(ids) { where(target_project_id: ids) }
   scope :opened, -> { with_states(:opened, :reopened) }
   scope :merged, -> { with_state(:merged) }
-  scope :closed, -> { with_state(:closed) }
   scope :closed_and_merged, -> { with_states(:closed, :merged) }
 
   scope :join_project, -> { joins(:target_project) }
diff --git a/app/models/project.rb b/app/models/project.rb
index 148eab692ff..3a28d5d7fee 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -215,6 +215,7 @@ class Project < ActiveRecord::Base
   scope :public_only, -> { where(visibility_level: Project::PUBLIC) }
   scope :public_and_internal_only, -> { where(visibility_level: Project.public_and_internal_levels) }
   scope :non_archived, -> { where(archived: false) }
+  scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids) }
 
   state_machine :import_status, initial: :none do
     event :import_start do
diff --git a/app/views/dashboard/milestones/_milestone.html.haml b/app/views/dashboard/milestones/_milestone.html.haml
index 7c882a32702..ea6c304d7de 100644
--- a/app/views/dashboard/milestones/_milestone.html.haml
+++ b/app/views/dashboard/milestones/_milestone.html.haml
@@ -8,7 +8,7 @@
   .row
     .col-sm-6
       = link_to issues_dashboard_path(milestone_title: milestone.title) do
-        = pluralize milestone.issue_count, 'Issue'
+        = pluralize milestone.issues_count, 'Issue'
       &middot;
       = link_to merge_requests_dashboard_path(milestone_title: milestone.title) do
         = pluralize milestone.merge_requests_count, 'Merge Request'
diff --git a/app/views/dashboard/milestones/show.html.haml b/app/views/dashboard/milestones/show.html.haml
index 3810267577c..bbe0b0905ce 100644
--- a/app/views/dashboard/milestones/show.html.haml
+++ b/app/views/dashboard/milestones/show.html.haml
@@ -52,7 +52,7 @@
   %li.active
     = link_to '#tab-issues', 'data-toggle' => 'tab' do
       Issues
-      %span.badge= @milestone.issue_count
+      %span.badge= @milestone.issues_count
   %li
     = link_to '#tab-merge-requests', 'data-toggle' => 'tab' do
       Merge Requests
diff --git a/app/views/groups/milestones/_milestone.html.haml b/app/views/groups/milestones/_milestone.html.haml
index a20bf75bc39..50558d7dce8 100644
--- a/app/views/groups/milestones/_milestone.html.haml
+++ b/app/views/groups/milestones/_milestone.html.haml
@@ -8,7 +8,7 @@
   .row
     .col-sm-6
       = link_to issues_group_path(@group, milestone_title: milestone.title) do
-        = pluralize milestone.issue_count, 'Issue'
+        = pluralize milestone.issues_count, 'Issue'
       &middot;
       = link_to merge_requests_group_path(@group, milestone_title: milestone.title) do
         = pluralize milestone.merge_requests_count, 'Merge Request'
diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml
index 1233da85524..405df1d3433 100644
--- a/app/views/groups/milestones/show.html.haml
+++ b/app/views/groups/milestones/show.html.haml
@@ -58,7 +58,7 @@
   %li.active
     = link_to '#tab-issues', 'data-toggle' => 'tab' do
       Issues
-      %span.badge= @milestone.issue_count
+      %span.badge= @milestone.issues_count
   %li
     = link_to '#tab-merge-requests', 'data-toggle' => 'tab' do
       Merge Requests
-- 
GitLab