Merge branch 'issue_38777' into 'master'

Allow promoting project milestones to group milestones

Closes #38777

See merge request gitlab-org/gitlab-ce!14831

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)
 

app/services/milestones/promote_service.rb

+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

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

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
+

changelogs/unreleased/issue_38777.yml

+---
+title: Allow promoting project milestones to group milestones
+merge_request:
+author:
+type: added

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

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
 

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

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

spec/services/milestones/promote_service_spec.rb

+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