Allow promoting project milestones to group milestones

10 files changed, 230 insertions, 13 deletions

diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index c94384d2a1a..980bbf699b6 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -2,13 +2,13 @@ class Projects::MilestonesController < Projects::ApplicationController
   include MilestoneActions
 
   before_action :check_issuables_available!
-  before_action :milestone, only: [:edit, :update, :destroy, :show, :merge_requests, :participants, :labels]
+  before_action :milestone, only: [:edit, :update, :destroy, :show, :merge_requests, :participants, :labels, :promote]
 
   # Allow read any milestone
   before_action :authorize_read_milestone!
 
   # Allow admin milestone
-  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels]
+  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels, :promote]
 
   respond_to :html
 
@@ -69,6 +69,14 @@ class Projects::MilestonesController < Projects::ApplicationController
     end
   end
 
+  def promote
+    promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
+    flash[:notice] = "Milestone has been promoted to group milestone."
+    redirect_to group_milestone_path(project.group, promoted_milestone.iid)
+  rescue Milestones::PromoteService::PromoteMilestoneError => error
+    redirect_to milestone, alert: error.message
+  end
+
   def destroy
     return access_denied! unless can?(current_user, :admin_milestone, @project)
 
diff --git a/app/services/milestones/promote_service.rb b/app/services/milestones/promote_service.rb
new file mode 100644
index 00000000000..bd9cfd4e0ea
--- /dev/null
+++ b/app/services/milestones/promote_service.rb
@@ -0,0 +1,80 @@
+module Milestones
+  class PromoteService < Milestones::BaseService
+    PromoteMilestoneError = Class.new(StandardError)
+
+    def execute(milestone)
+      check_project_milestone!(milestone)
+
+      Milestone.transaction do
+        # Destroy all milestones with same title across projects
+        destroy_old_milestones(milestone)
+
+        group_milestone = clone_project_milestone(milestone)
+
+        move_children_to_group_milestone(group_milestone)
+
+        # Just to be safe
+        unless group_milestone.valid?
+          raise_error(group_milestone.errors.full_messages.to_sentence)
+        end
+
+        group_milestone
+      end
+    end
+
+    private
+
+    def milestone_ids_for_merge(group_milestone)
+      # Pluck need to be used here instead of select so the array of ids
+      # is persistent after old milestones gets deleted.
+      @milestone_ids_for_merge ||= begin
+        search_params = { title: group_milestone.title, project_ids: group_project_ids, state: 'all' }
+        milestones = MilestonesFinder.new(search_params).execute
+        milestones.pluck(:id)
+      end
+    end
+
+    def move_children_to_group_milestone(group_milestone)
+      milestone_ids_for_merge(group_milestone).in_groups_of(100) do |milestone_ids|
+        update_children(group_milestone, milestone_ids)
+      end
+    end
+
+    def check_project_milestone!(milestone)
+      raise_error('Only project milestones can be promoted.') unless milestone.project_milestone?
+    end
+
+    def clone_project_milestone(milestone)
+      params = milestone.slice(:title, :description, :start_date, :due_date, :state_event)
+
+      create_service = CreateService.new(group, current_user, params)
+
+      create_service.execute
+    end
+
+    def update_children(group_milestone, milestone_ids)
+      issues = Issue.where(project_id: group_project_ids, milestone_id: milestone_ids)
+      merge_requests = MergeRequest.where(source_project_id: group_project_ids, milestone_id: milestone_ids)
+
+      [issues, merge_requests].each do |issuable_collection|
+        issuable_collection.update_all(milestone_id: group_milestone.id)
+      end
+    end
+
+    def group
+      @group ||= parent.group || raise_error('Project does not belong to a group.')
+    end
+
+    def destroy_old_milestones(group_milestone)
+      Milestone.where(id: milestone_ids_for_merge(group_milestone)).destroy_all
+    end
+
+    def group_project_ids
+      @group_project_ids ||= group.projects.map(&:id)
+    end
+
+    def raise_error(message)
+      raise PromoteMilestoneError, "Promotion failed - #{message}"
+    end
+  end
+end
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index a5153df1159..9fc297ab7f6 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -23,14 +23,18 @@
         = milestone_date_range(@milestone)
     .milestone-buttons
       - if can?(current_user, :admin_milestone, @project)
+        = link_to edit_project_milestone_path(@project, @milestone), class: "btn btn-grouped btn-nr" do
+          Edit
+
+        - if @project.group
+          = link_to promote_project_milestone_path(@milestone.project, @milestone), title: "Promote to Group Milestone", class: 'btn btn-grouped', data: { confirm: "Promoting this milestone will make it available for all projects inside the group. Existing project milestones with the same name will be merged. Are you sure?", toggle: "tooltip" }, method: :post do
+            Promote
+
         - if @milestone.active?
           = link_to 'Close milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-close btn-nr btn-grouped"
         - else
           = link_to 'Reopen milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :activate }), method: :put, class: "btn btn-reopen btn-nr btn-grouped"
 
-        = link_to edit_project_milestone_path(@project, @milestone), class: "btn btn-grouped btn-nr" do
-          Edit
-
         = link_to project_milestone_path(@project, @milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-grouped btn-danger" do
           Delete
 
@@ -40,6 +44,7 @@
   .detail-page-description.milestone-detail
     %h2.title
       = markdown_field(@milestone, :title)
+
     %div
       - if @milestone.description.present?
         .description
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index 305e2542281..7ba8f9d4313 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -49,6 +49,13 @@
           = link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-xs btn-grouped" do
             Edit
           \
+
+          - if @project.group
+            = link_to promote_project_milestone_path(milestone.project, milestone), title: "Promote to Group Milestone", class: 'btn btn-xs btn-grouped', data: { confirm: "Promoting this milestone will make it available for all projects inside the group. Existing project milestones with the same name will be merged. Are you sure?", toggle: "tooltip" }, method: :post do
+              Promote
+
           = link_to 'Close Milestone', project_milestone_path(@project, milestone, milestone: {state_event: :close }), method: :put, remote: true, class: "btn btn-xs btn-close btn-grouped"
+
           = link_to project_milestone_path(milestone.project, milestone), data: { confirm: 'Are you sure?' }, method: :delete, class: "btn btn-xs btn-remove btn-grouped" do
             Delete
+
diff --git a/changelogs/unreleased/issue_38777.yml b/changelogs/unreleased/issue_38777.yml
new file mode 100644
index 00000000000..5c49b2f7879
--- /dev/null
+++ b/changelogs/unreleased/issue_38777.yml
@@ -0,0 +1,5 @@
+---
+title: Allow promoting project milestones to group milestones
+merge_request:
+author:
+type: added
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 9f553085d50..746c0c46677 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -293,6 +293,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       resources :milestones, constraints: { id: /\d+/ } do
         member do
+          post :promote
           put :sort_issues
           put :sort_merge_requests
           get :merge_requests
diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md
index 876b98a4dc5..83adbd8cce2 100644
--- a/doc/user/project/milestones/index.md
+++ b/doc/user/project/milestones/index.md
@@ -29,7 +29,8 @@ In addition to that you will be able to filter issues or merge requests by group
 
 ## Milestone promotion
 
-You will be able to promote a project milestone to a group milestone [in the future](https://gitlab.com/gitlab-org/gitlab-ce/issues/35833).
+Project milestones can be promoted to group milestones if its project belongs to a group. When a milestone is promoted all other milestones across the group projects with the same title will be merged into it, which means all milestone's children like issues, merge requests and boards will be moved into the new promoted milestone.
+The promote button can be found in the milestone view or milestones list.
 
 ## Special milestone filters
 
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 573530d0db0..209979e642d 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -86,4 +86,32 @@ describe Projects::MilestonesController do
       expect(last_note).to eq('removed milestone')
     end
   end
+
+  describe '#promote' do
+    context 'promotion succeeds' do
+      before do
+        group = create(:group)
+        group.add_developer(user)
+        milestone.project.update(namespace: group)
+      end
+
+      it 'shows group milestone' do
+        post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
+
+        group_milestone = assigns(:milestone)
+
+        expect(response).to redirect_to(group_milestone_path(project.group, group_milestone.iid))
+        expect(flash[:notice]).to eq('Milestone has been promoted to group milestone.')
+      end
+    end
+
+    context 'promotion fails' do
+      it 'shows project milestone' do
+        post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
+
+        expect(response).to redirect_to(project_milestone_path(project, milestone))
+        expect(flash[:alert]).to eq('Promotion failed - Project does not belong to a group.')
+      end
+    end
+  end
 end
diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb
index 39d44245c3f..fb1281a6b42 100644
--- a/spec/routing/project_routing_spec.rb
+++ b/spec/routing/project_routing_spec.rb
@@ -426,18 +426,23 @@ describe 'project routing' do
     end
   end
 
-  #     project_milestones GET    /:project_id/milestones(.:format)          milestones#index
-  #                        POST   /:project_id/milestones(.:format)          milestones#create
-  #  new_project_milestone GET    /:project_id/milestones/new(.:format)      milestones#new
-  # edit_project_milestone GET    /:project_id/milestones/:id/edit(.:format) milestones#edit
-  #      project_milestone GET    /:project_id/milestones/:id(.:format)      milestones#show
-  #                        PUT    /:project_id/milestones/:id(.:format)      milestones#update
-  #                        DELETE /:project_id/milestones/:id(.:format)      milestones#destroy
+  #     project_milestones    GET    /:project_id/milestones(.:format)          milestones#index
+  #                           POST   /:project_id/milestones(.:format)          milestones#create
+  #  new_project_milestone    GET    /:project_id/milestones/new(.:format)      milestones#new
+  # edit_project_milestone    GET    /:project_id/milestones/:id/edit(.:format) milestones#edit
+  #      project_milestone    GET    /:project_id/milestones/:id(.:format)      milestones#show
+  #                           PUT    /:project_id/milestones/:id(.:format)      milestones#update
+  #                           DELETE /:project_id/milestones/:id(.:format)      milestones#destroy
+  # promote_project_milestone POST /:project_id/milestones/:id/promote          milestones#promote
   describe Projects::MilestonesController, 'routing' do
     it_behaves_like 'RESTful project resources' do
       let(:controller) { 'milestones' }
       let(:actions) { [:index, :create, :new, :edit, :show, :update] }
     end
+
+    it 'to #promote' do
+      expect(post('/gitlab/gitlabhq/milestones/1/promote')).to route_to('projects/milestones#promote', namespace_id: 'gitlab', project_id: 'gitlabhq', id: "1")
+    end
   end
 
   # project_labels GET    /:project_id/labels(.:format) labels#index
diff --git a/spec/services/milestones/promote_service_spec.rb b/spec/services/milestones/promote_service_spec.rb
new file mode 100644
index 00000000000..9f2df6d6d19
--- /dev/null
+++ b/spec/services/milestones/promote_service_spec.rb
@@ -0,0 +1,77 @@
+require 'spec_helper'
+
+describe Milestones::PromoteService do
+  let(:group) { create(:group) }
+  let(:project) { create(:project, namespace: group) }
+  let(:user) { create(:user) }
+  let(:milestone_title) { 'project milestone' }
+  let(:milestone) { create(:milestone, project: project, title: milestone_title) }
+  let(:service) { described_class.new(project, user) }
+
+  describe '#execute' do
+    before do
+      group.add_master(user)
+    end
+
+    context 'validations' do
+      it 'raises error if milestone does not belong to a project' do
+        allow(milestone).to receive(:project_milestone?).and_return(false)
+
+        expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
+      end
+
+      it 'raises error if project does not belong to a group' do
+        project.update(namespace: user.namespace)
+
+        expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
+      end
+    end
+
+    context 'without duplicated milestone titles across projects' do
+      it 'promotes project milestone to group milestone' do
+        promoted_milestone = service.execute(milestone)
+
+        expect(promoted_milestone).to be_group_milestone
+      end
+
+      it 'sets issuables with new promoted milestone' do
+        issue = create(:issue, milestone: milestone, project: project)
+        merge_request = create(:merge_request, milestone: milestone, source_project: project)
+
+        promoted_milestone = service.execute(milestone)
+
+        expect(promoted_milestone).to be_group_milestone
+        expect(issue.reload.milestone).to eq(promoted_milestone)
+        expect(merge_request.reload.milestone).to eq(promoted_milestone)
+      end
+    end
+
+    context 'with duplicated milestone titles across projects' do
+      let(:project_2) { create(:project, namespace: group) }
+      let!(:milestone_2) { create(:milestone, project: project_2, title: milestone_title) }
+
+      it 'deletes project milestones with the same title' do
+        promoted_milestone = service.execute(milestone)
+
+        expect(promoted_milestone).to be_group_milestone
+        expect(promoted_milestone).to be_valid
+        expect(Milestone.exists?(milestone.id)).to be_falsy
+        expect(Milestone.exists?(milestone_2.id)).to be_falsy
+      end
+
+      it 'sets all issuables with new promoted milestone' do
+        issue = create(:issue, milestone: milestone, project: project)
+        issue_2 = create(:issue, milestone: milestone_2, project: project_2)
+        merge_request = create(:merge_request, milestone: milestone, source_project: project)
+        merge_request_2 = create(:merge_request, milestone: milestone_2, source_project: project_2)
+
+        promoted_milestone = service.execute(milestone)
+
+        expect(issue.reload.milestone).to eq(promoted_milestone)
+        expect(issue_2.reload.milestone).to eq(promoted_milestone)
+        expect(merge_request.reload.milestone).to eq(promoted_milestone)
+        expect(merge_request_2.reload.milestone).to eq(promoted_milestone)
+      end
+    end
+  end
+end