13 files changed, 127 insertions, 127 deletions

diff --git a/app/assets/stylesheets/framework/issue_box.scss b/app/assets/stylesheets/framework/issue_box.scss
index a66604e56ff..e51f230a680 100644
--- a/app/assets/stylesheets/framework/issue_box.scss
+++ b/app/assets/stylesheets/framework/issue_box.scss
@@ -45,9 +45,4 @@
   &.status-box-upcoming {
     background: $gl-text-color-secondary;
   }
-
-  &.status-box-milestone {
-    color: $gl-text-color;
-    background: $gray-darker;
-  }
 }
diff --git a/app/assets/stylesheets/pages/milestone.scss b/app/assets/stylesheets/pages/milestone.scss
index 1e92582d6d9..94bf32945fc 100644
--- a/app/assets/stylesheets/pages/milestone.scss
+++ b/app/assets/stylesheets/pages/milestone.scss
@@ -1,3 +1,5 @@
+$status-box-line-height: 26px;
+
 .issues-sortable-list .str-truncated {
   max-width: 90%;
 }
@@ -38,6 +40,7 @@
       font-size: $tooltip-font-size;
       margin-top: 0;
       margin-right: $gl-padding-4;
+      line-height: $status-box-line-height;
 
       @include media-breakpoint-down(xs) {
         line-height: unset;
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index b42116b0f36..868deea3f01 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -43,14 +43,7 @@ class Groups::MilestonesController < Groups::ApplicationController
   def update
     # Keep this compatible with legacy group milestones where we have to update
     # all projects milestones states at once.
-    if @milestone.legacy_group_milestone?
-      update_params = milestone_params.select { |key| key == "state_event" }
-      milestones = @milestone.milestones
-    else
-      update_params = milestone_params
-      milestones = [@milestone]
-    end
-
+    milestones, update_params = get_milestones_for_update
     milestones.each do |milestone|
       Milestones::UpdateService.new(milestone.parent, current_user, update_params).execute(milestone)
     end
@@ -71,6 +64,14 @@ class Groups::MilestonesController < Groups::ApplicationController
 
   private
 
+  def get_milestones_for_update
+    if @milestone.legacy_group_milestone?
+      [@milestone.milestones, legacy_milestone_params]
+    else
+      [[@milestone], milestone_params]
+    end
+  end
+
   def authorize_admin_milestones!
     return render_404 unless can?(current_user, :admin_milestone, group)
   end
@@ -79,6 +80,10 @@ class Groups::MilestonesController < Groups::ApplicationController
     params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
   end
 
+  def legacy_milestone_params
+    params.require(:milestone).permit(:state_event)
+  end
+
   def milestone_path
     if @milestone.legacy_group_milestone?
       group_milestone_path(group, @milestone.safe_title, title: @milestone.title)
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 9666080092b..c212e1804dc 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -237,12 +237,15 @@ module MilestonesHelper
     end
   end
 
-  def group_or_dashboard_milestone_path(milestone)
-    if milestone.group_milestone?
-      group_milestone_path(milestone.group, milestone.iid, milestone: { title: milestone.title })
-    else
-      dashboard_milestone_path(milestone.safe_title, title: milestone.title)
-    end
+  def group_or_project_milestone_path(milestone)
+    params =
+      if milestone.group_milestone?
+        { milestone: { title: milestone.title } }
+      else
+        { title: milestone.title }
+      end
+
+    milestone_path(milestone.milestone, params)
   end
 
   def can_admin_project_milestones?
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index e44a069b730..055ffe04646 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -42,7 +42,7 @@ module Milestoneish
   def issues_visible_to_user(user)
     memoize_per_user(user, :issues_visible_to_user) do
       IssuesFinder.new(user, issues_finder_params)
-        .execute.preload(:assignees).where(milestone_id: milestoneish_ids)
+        .execute.preload(:assignees).where(milestone_id: milestoneish_id)
     end
   end
 
diff --git a/app/models/dashboard_group_milestone.rb b/app/models/dashboard_group_milestone.rb
index 32e8104125c..c06823184c2 100644
--- a/app/models/dashboard_group_milestone.rb
+++ b/app/models/dashboard_group_milestone.rb
@@ -7,7 +7,7 @@ class DashboardGroupMilestone < GlobalMilestone
 
   override :initialize
   def initialize(milestone)
-    super(milestone.title, Array(milestone))
+    super
 
     @group_name = milestone.group.full_name
   end
@@ -19,22 +19,4 @@ class DashboardGroupMilestone < GlobalMilestone
              .active
              .map { |m| new(m) }
   end
-
-  override :group_milestone?
-  def group_milestone?
-    @first_milestone.group_milestone?
-  end
-
-  override :milestoneish_ids
-  def milestoneish_ids
-    milestones.map(&:id)
-  end
-
-  def group
-    @first_milestone.group
-  end
-
-  def iid
-    @first_milestone.iid
-  end
 end
diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb
index 96bc8090b81..9b377b70e5b 100644
--- a/app/models/dashboard_milestone.rb
+++ b/app/models/dashboard_milestone.rb
@@ -1,11 +1,15 @@
 # frozen_string_literal: true
 
 class DashboardMilestone < GlobalMilestone
-  def issues_finder_params
-    { authorized_only: true }
+  attr_reader :project_name
+
+  def initialize(milestone)
+    super
+
+    @project_name = milestone.project.full_name
   end
 
-  def dashboard_milestone?
+  def project_milestone?
     true
   end
 end
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 085ffd16c6a..4e82f3fed27 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -3,69 +3,78 @@
 class GlobalMilestone
   include Milestoneish
 
-  EPOCH = DateTime.parse('1970-01-01')
   STATE_COUNT_HASH = { opened: 0, closed: 0, all: 0 }.freeze
 
-  attr_accessor :title, :milestones
+  attr_reader :milestone
   alias_attribute :name, :title
 
+  delegate :title, :state, :due_date, :start_date, :participants, :project, :group, :expires_at, :closed?, :iid, :group_milestone?, :safe_title, :milestoneish_id, to: :milestone
+
+  def to_hash
+    {
+       name: title,
+       title: title,
+       group_name: group&.full_name,
+       project_name: project&.full_name
+    }
+  end
+
   def for_display
-    @first_milestone
+    @milestone
   end
 
   def self.build_collection(projects, params)
-    params =
-      { project_ids: projects.map(&:id), state: params[:state] }
-
-    child_milestones = MilestonesFinder.new(params).execute # rubocop: disable CodeReuse/Finder
-
-    milestones = child_milestones.select(:id, :title).group_by(&:title).map do |title, grouped|
-      milestones_relation = Milestone.where(id: grouped.map(&:id))
-      new(title, milestones_relation)
-    end
+    items = Milestone.of_projects(projects)
+                .reorder_by_due_date_asc
+                .order_by_name_asc
 
-    milestones.sort_by { |milestone| milestone.due_date || EPOCH }
+    Milestone.filter_by_state(items, params[:state]).map { |m| new(m) }
   end
 
+  # necessary for legacy milestones
   def self.build(projects, title)
-    child_milestones = Milestone.of_projects(projects).where(title: title)
-    return if child_milestones.blank?
+    milestones = Milestone.of_projects(projects).where(title: title)
+    return if milestones.blank?
 
-    new(title, child_milestones)
+    new(milestones.first)
   end
 
-  def self.count_by_state(milestones_by_state_and_title, state)
-    milestones_by_state_and_title.count do |(milestone_state, _), _|
-      milestone_state == state
+  def self.states_count(projects, group = nil)
+    legacy_group_milestones_count = legacy_group_milestone_states_count(projects)
+    group_milestones_count = group_milestones_states_count(group)
+
+    legacy_group_milestones_count.merge(group_milestones_count) do |k, legacy_group_milestones_count, group_milestones_count|
+      legacy_group_milestones_count + group_milestones_count
     end
   end
-  private_class_method :count_by_state
 
-  def initialize(title, milestones)
-    @title = title
-    @name = title
-    @milestones = milestones
-    @first_milestone = milestones.find {|m| m.description.present? } || milestones.first
-  end
+  def self.group_milestones_states_count(group)
+    return STATE_COUNT_HASH unless group
 
-  def milestoneish_ids
-    milestones.select(:id)
-  end
+    counts_by_state = Milestone.of_groups(group).count_by_state
 
-  def safe_title
-    @title.to_slug.normalize.to_s
+    {
+      opened: counts_by_state['active'] || 0,
+      closed: counts_by_state['closed'] || 0,
+      all: counts_by_state.values.sum
+    }
   end
 
-  def projects
-    @projects ||= Project.for_milestones(milestoneish_ids)
-  end
+  def self.legacy_group_milestone_states_count(projects)
+    return STATE_COUNT_HASH unless projects
 
-  def state
-    milestones.each do |milestone|
-      return 'active' if milestone.state != 'closed'
-    end
+    # We need to reorder(nil) on the projects, because the controller passes them in sorted.
+    relation = Milestone.of_projects(projects.reorder(nil)).count_by_state
 
-    'closed'
+    {
+      opened: relation['active'] || 0,
+      closed: relation['closed'] || 0,
+      all: relation.values.sum
+    }
+  end
+
+  def initialize(milestone)
+    @milestone = milestone
   end
 
   def active?
@@ -77,37 +86,14 @@ class GlobalMilestone
   end
 
   def issues
-    @issues ||= Issue.of_milestones(milestoneish_ids).includes(:project, :assignees, :labels)
+    @issues ||= Issue.of_milestones(milestone).includes(:project, :assignees, :labels)
   end
 
   def merge_requests
-    @merge_requests ||= MergeRequest.of_milestones(milestoneish_ids).includes(:target_project, :assignee, :labels)
-  end
-
-  def participants
-    @participants ||= milestones.map(&:participants).flatten.uniq
+    @merge_requests ||= MergeRequest.of_milestones(milestone).includes(:target_project, :assignee, :labels)
   end
 
   def labels
-    @labels ||= GlobalLabel.build_collection(milestones.includes(:labels).map(&:labels).flatten)
-                           .sort_by!(&:title)
-  end
-
-  def due_date
-    return @due_date if defined?(@due_date)
-
-    @due_date =
-      if @milestones.all? { |x| x.due_date == @milestones.first.due_date }
-        @milestones.first.due_date
-      end
-  end
-
-  def start_date
-    return @start_date if defined?(@start_date)
-
-    @start_date =
-      if @milestones.all? { |x| x.start_date == @milestones.first.start_date }
-        @milestones.first.start_date
-      end
+    @labels ||= GlobalLabel.build_collection(milestone.labels).sort_by!(&:title)
   end
 end
diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb
index 9dfaebacc83..a58537de319 100644
--- a/app/models/group_milestone.rb
+++ b/app/models/group_milestone.rb
@@ -1,18 +1,35 @@
 # frozen_string_literal: true
 # Group Milestones are milestones that can be shared among many projects within the same group
 class GroupMilestone < GlobalMilestone
-  attr_accessor :group
+  attr_reader :group, :milestones
 
   def self.build_collection(group, projects, params)
-    super(projects, params).each do |milestone|
-      milestone.group = group
+    params =
+      { state: params[:state] }
+
+    project_milestones = Milestone.of_projects(projects)
+    child_milestones = Milestone.filter_by_state(project_milestones, params[:state])
+    grouped_milestones = child_milestones.group_by(&:title)
+
+    grouped_milestones.map do |title, grouped|
+      new(title, grouped, group)
     end
   end
 
   def self.build(group, projects, title)
-    super(projects, title).tap do |milestone|
-      milestone&.group = group
-    end
+    child_milestones = Milestone.of_projects(projects).where(title: title)
+    return if child_milestones.blank?
+
+    new(title, child_milestones, group)
+  end
+
+  def initialize(title, milestones, group)
+    @milestones = milestones
+    @group = group
+  end
+
+  def milestone
+    @milestone ||= milestones.find { |m| m.description.present? } || milestones.first
   end
 
   def issues_finder_params
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 3cc8e2c44bb..db56533dea5 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -94,6 +94,10 @@ class Milestone < ActiveRecord::Base
       end
     end
 
+    def count_by_state
+      reorder(nil).group(:state).count
+    end
+
     def predefined?(milestone)
       milestone == Any ||
         milestone == None ||
@@ -215,7 +219,7 @@ class Milestone < ActiveRecord::Base
     self.title
   end
 
-  def milestoneish_ids
+  def milestoneish_id
     id
   end
 
diff --git a/app/views/dashboard/milestones/_milestone.html.haml b/app/views/dashboard/milestones/_milestone.html.haml
index b876d6fd1f3..89212eb6bf9 100644
--- a/app/views/dashboard/milestones/_milestone.html.haml
+++ b/app/views/dashboard/milestones/_milestone.html.haml
@@ -1,5 +1,5 @@
 = render 'shared/milestones/milestone',
-          milestone_path: group_or_dashboard_milestone_path(milestone),
+          milestone_path: group_or_project_milestone_path(milestone),
           issues_path: issues_dashboard_path(milestone_title: milestone.title),
           merge_requests_path: merge_requests_dashboard_path(milestone_title: milestone.title),
           milestone: milestone,
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index ed7fefba56d..40b8374848e 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -1,5 +1,5 @@
 - dashboard = local_assigns[:dashboard]
-- custom_dom_id = dom_id(milestone.try(:milestones) ? milestone.milestones.first : milestone)
+- custom_dom_id = dom_id(milestone.try(:milestone) ? milestone.milestone : milestone)
 - milestone_type = milestone.group_milestone? ? 'Group Milestone' : 'Project Milestone'
 
 %li{ class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: custom_dom_id }
@@ -21,10 +21,12 @@
               = milestone.group.full_name
           - if milestone.legacy_group_milestone?
             .projects
-              - milestone.milestones.each do |milestone|
-                = link_to milestone_path(milestone) do
-                  %span.label-badge.label-badge-blue.d-inline-block.append-bottom-5
-                    = dashboard ? milestone.project.full_name : milestone.project.name
+              - link_to milestone_path(milestone.milestone) do
+                %span.label-badge.label-badge-blue.d-inline-block.append-bottom-5
+                  = dashboard ? milestone.project.full_name : milestone.project.name
+          - if milestone.project
+            .label-badge.label-badge-gray.d-inline-block
+              = milestone.project.full_name
 
     .col-sm-4.milestone-progress
       = milestone_progress_bar(milestone)
@@ -58,5 +60,5 @@
             - else
               = link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-sm btn-grouped btn-close"
         - if dashboard
-          .status-box.status-box-milestone
+          .label-badge.label-badge-gray
             = milestone_type
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 0499b04a482..55b1c14022f 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -62,20 +62,19 @@
           %th Open issues
           %th State
           %th Due date
-      - milestone.milestones.each do |ms|
         %tr
           %td
-            - project_name = group ? ms.project.name : ms.project.full_name
-            = link_to project_name, project_milestone_path(ms.project, ms)
+            - project_name = group ? milestone.project.name : milestone.project.full_name
+            = link_to project_name, milestone_path(milestone.milestone)
           %td
-            = ms.issues_visible_to_user(current_user).opened.count
+            = milestone.milestone.issues_visible_to_user(current_user).opened.count
           %td
-            - if ms.closed?
+            - if milestone.closed?
               Closed
             - else
               Open
           %td
-            = ms.expires_at
+            = milestone.expires_at
 - elsif milestone.group_milestone?
   %br
   View