Merge branch '60372-milestone-link-prevent-delete-issue-after-move-it-to-another-project-2' into 'master'

Add service to transfer Group Milestones when transferring a Project

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

9 files changed, 344 insertions, 1 deletions

diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index cb87b46a31d..915978d37b8 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -16,6 +16,7 @@ class Milestone < ApplicationRecord
   include Referable
   include StripAttribute
   include Milestoneish
+  include FromUnion
   include Gitlab::SQL::Pattern
 
   cache_markdown_field :title, pipeline: :single_line
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 2ab6e88599f..3555864f834 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -221,6 +221,7 @@ class IssuableBaseService < BaseService
       # We have to perform this check before saving the issuable as Rails resets
       # the changed fields upon calling #save.
       update_project_counters = issuable.project && update_project_counter_caches?(issuable)
+      ensure_milestone_available(issuable)
 
       if issuable.with_transaction_returning_status { issuable.save(touch: should_touch) }
         # We do not touch as it will affect a update on updated_at field
diff --git a/app/services/milestones/find_or_create_service.rb b/app/services/milestones/find_or_create_service.rb
new file mode 100644
index 00000000000..881011e5106
--- /dev/null
+++ b/app/services/milestones/find_or_create_service.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Milestones
+  class FindOrCreateService
+    attr_accessor :project, :current_user, :params
+
+    def initialize(project, user, params = {})
+      @project, @current_user, @params = project, user, params.dup
+    end
+
+    def execute
+      find_milestone || create_milestone
+    end
+
+    private
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def find_milestone
+      groups = project.group&.self_and_ancestors_ids
+      Milestone.for_projects_and_groups([project.id], groups).find_by(title: params["title"])
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    def create_milestone
+      return unless current_user.can?(:admin_milestone, project)
+
+      new_milestone if new_milestone.persisted?
+    end
+
+    def new_milestone
+      @new_milestone ||= CreateService.new(project, current_user, params).execute
+    end
+  end
+end
diff --git a/app/services/milestones/transfer_service.rb b/app/services/milestones/transfer_service.rb
new file mode 100644
index 00000000000..1efbfed4853
--- /dev/null
+++ b/app/services/milestones/transfer_service.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+# Milestones::TransferService class
+#
+# Used for recreating the missing group milestones at project level when
+# transferring a project to a new namespace
+#
+module Milestones
+  class TransferService
+    attr_reader :current_user, :old_group, :project
+
+    def initialize(current_user, old_group, project)
+      @current_user = current_user
+      @old_group = old_group
+      @project = project
+    end
+
+    def execute
+      return unless old_group.present?
+
+      Milestone.transaction do
+        milestones_to_transfer.find_each do |milestone|
+          new_milestone = find_or_create_milestone(milestone)
+
+          update_issues_milestone(milestone.id, new_milestone&.id)
+          update_merge_requests_milestone(milestone.id, new_milestone&.id)
+        end
+      end
+    end
+
+    private
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def milestones_to_transfer
+      Milestone.from_union([
+          group_milestones_applied_to_issues,
+          group_milestones_applied_to_merge_requests
+        ])
+        .reorder(nil)
+        .distinct
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def group_milestones_applied_to_issues
+      Milestone.joins(:issues)
+        .where(
+          issues: { project_id: project.id },
+          group_id: old_group.id
+        )
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def group_milestones_applied_to_merge_requests
+      Milestone.joins(:merge_requests)
+        .where(
+          merge_requests: { target_project_id: project.id },
+          group_id: old_group.id
+        )
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    def find_or_create_milestone(milestone)
+      params = milestone.attributes.slice('title', 'description', 'start_date', 'due_date')
+
+      FindOrCreateService.new(project, current_user, params).execute
+    end
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def update_issues_milestone(old_milestone_id, new_milestone_id)
+      Issue.where(project: project, milestone_id: old_milestone_id)
+        .update_all(milestone_id: new_milestone_id)
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def update_merge_requests_milestone(old_milestone_id, new_milestone_id)
+      MergeRequest.where(project: project, milestone_id: old_milestone_id)
+        .update_all(milestone_id: new_milestone_id)
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 233dcf37e35..078a751025f 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -72,6 +72,9 @@ module Projects
         # Move missing group labels to project
         Labels::TransferService.new(current_user, @old_group, project).execute
 
+        # Move missing group milestones
+        Milestones::TransferService.new(current_user, @old_group, project).execute
+
         # Move uploads
         move_project_uploads(project)
 
diff --git a/changelogs/unreleased/60372-milestone-link-prevent-delete-issue-after-move-it-to-another-projec.yml b/changelogs/unreleased/60372-milestone-link-prevent-delete-issue-after-move-it-to-another-projec.yml
new file mode 100644
index 00000000000..d9f4c17a668
--- /dev/null
+++ b/changelogs/unreleased/60372-milestone-link-prevent-delete-issue-after-move-it-to-another-projec.yml
@@ -0,0 +1,5 @@
+---
+title: Add service to transfer Group Milestones when transferring a Project
+merge_request: 31778
+author:
+type: added
diff --git a/spec/services/milestones/find_or_create_service_spec.rb b/spec/services/milestones/find_or_create_service_spec.rb
new file mode 100644
index 00000000000..ae3def30982
--- /dev/null
+++ b/spec/services/milestones/find_or_create_service_spec.rb
@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Milestones::FindOrCreateService do
+  describe '#execute' do
+    subject(:service) { described_class.new(project, user, params) }
+
+    let(:user)    { create(:user) }
+    let(:group)   { create(:group) }
+    let(:project) { create(:project, namespace: group) }
+    let(:params) do
+      {
+        title: '1.0',
+        description: 'First Release',
+        start_date: Date.today,
+        due_date: Date.today + 1.month
+      }.with_indifferent_access
+    end
+
+    context 'when finding milestone on project level' do
+      let!(:existing_project_milestone) { create(:milestone, project: project, title: '1.0') }
+
+      it 'returns existing milestone' do
+        expect(service.execute).to eq(existing_project_milestone)
+      end
+    end
+
+    context 'when finding milestone on group level' do
+      let!(:existing_group_milestone) { create(:milestone, group: group, title: '1.0') }
+
+      it 'returns existing milestone' do
+        expect(service.execute).to eq(existing_group_milestone)
+      end
+    end
+
+    context 'when not finding milestone' do
+      context 'when user has permissions' do
+        before do
+          project.add_developer(user)
+        end
+
+        context 'when params are valid' do
+          it 'creates a new milestone at project level using params' do
+            expect { service.execute }.to change(project.milestones, :count).by(1)
+
+            milestone = project.reload.milestones.last
+
+            expect(milestone.title).to eq(params[:title])
+            expect(milestone.description).to eq(params[:description])
+            expect(milestone.start_date).to eq(params[:start_date])
+            expect(milestone.due_date).to eq(params[:due_date])
+          end
+        end
+
+        context 'when params are not valid' do
+          before do
+            params[:start_date] = Date.today + 2.months
+          end
+
+          it 'returns nil' do
+            expect(service.execute).to be_nil
+          end
+        end
+      end
+
+      context 'when user does not have permissions' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'does not create a new milestone' do
+          expect { service.execute }.not_to change(project.milestones, :count)
+        end
+
+        it 'returns nil' do
+          expect(service.execute).to be_nil
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/milestones/transfer_service_spec.rb b/spec/services/milestones/transfer_service_spec.rb
new file mode 100644
index 00000000000..b3d41eb0763
--- /dev/null
+++ b/spec/services/milestones/transfer_service_spec.rb
@@ -0,0 +1,122 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Milestones::TransferService do
+  describe '#execute' do
+    subject(:service) { described_class.new(user, old_group, project) }
+
+    context 'when old_group is present' do
+      let(:user) { create(:admin) }
+      let(:new_group) { create(:group) }
+      let(:old_group) { create(:group) }
+      let(:project) { create(:project, namespace: old_group) }
+      let(:group_milestone) { create(:milestone, group: old_group)}
+      let(:group_milestone2) { create(:milestone, group: old_group)}
+      let(:project_milestone) { create(:milestone, project: project)}
+      let!(:issue_with_group_milestone) { create(:issue, project: project, milestone: group_milestone) }
+      let!(:issue_with_project_milestone) { create(:issue, project: project, milestone: project_milestone) }
+      let!(:mr_with_group_milestone) { create(:merge_request, source_project: project, source_branch: 'branch-1', milestone: group_milestone) }
+      let!(:mr_with_project_milestone) { create(:merge_request, source_project: project, source_branch: 'branch-2', milestone: project_milestone) }
+
+      before do
+        new_group.add_maintainer(user)
+        project.add_maintainer(user)
+        # simulate project transfer
+        project.update(group: new_group)
+      end
+
+      context 'without existing milestone at the new group level' do
+        it 'recreates the missing group milestones at project level' do
+          expect { service.execute }.to change(project.milestones, :count).by(1)
+        end
+
+        it 'applies new project milestone to issues with group milestone' do
+          service.execute
+          new_milestone = issue_with_group_milestone.reload.milestone
+
+          expect(new_milestone).not_to eq(group_milestone)
+          expect(new_milestone.title).to eq(group_milestone.title)
+          expect(new_milestone.project_milestone?).to be_truthy
+        end
+
+        it 'does not apply new project milestone to issues with project milestone' do
+          service.execute
+
+          expect(issue_with_project_milestone.reload.milestone).to eq(project_milestone)
+        end
+
+        it 'applies new project milestone to merge_requests with group milestone' do
+          service.execute
+          new_milestone = mr_with_group_milestone.reload.milestone
+
+          expect(new_milestone).not_to eq(group_milestone)
+          expect(new_milestone.title).to eq(group_milestone.title)
+          expect(new_milestone.project_milestone?).to be_truthy
+        end
+
+        it 'does not apply new project milestone to issuables with project milestone' do
+          service.execute
+
+          expect(mr_with_project_milestone.reload.milestone).to eq(project_milestone)
+        end
+
+        it 'does not recreate missing group milestones that are not applied to issues or merge requests' do
+          service.execute
+          new_milestone_title = project.reload.milestones.pluck(:title)
+
+          expect(new_milestone_title).to include(group_milestone.title)
+          expect(new_milestone_title).not_to include(group_milestone2.title)
+        end
+
+        context 'when find_or_create_milestone returns nil' do
+          before do
+            allow_any_instance_of(Milestones::FindOrCreateService).to receive(:execute).and_return(nil)
+          end
+
+          it 'removes issues group milestone' do
+            service.execute
+
+            expect(mr_with_group_milestone.reload.milestone).to be_nil
+          end
+
+          it 'removes merge requests group milestone' do
+            service.execute
+
+            expect(issue_with_group_milestone.reload.milestone).to be_nil
+          end
+        end
+      end
+
+      context 'with existing milestone at the new group level' do
+        let!(:existing_milestone) { create(:milestone, group: new_group, title: group_milestone.title) }
+
+        it 'does not create a new milestone' do
+          expect { service.execute }.not_to change(project.milestones, :count)
+        end
+
+        it 'applies existing milestone to issues with group milestone' do
+          service.execute
+
+          expect(issue_with_group_milestone.reload.milestone).to eq(existing_milestone)
+        end
+
+        it 'applies existing milestone to merge_requests with group milestone' do
+          service.execute
+
+          expect(mr_with_group_milestone.reload.milestone).to eq(existing_milestone)
+        end
+      end
+    end
+  end
+
+  context 'when old_group is not present' do
+    let(:user)        { create(:admin) }
+    let(:old_group)   { project.group }
+    let(:project)     { create(:project, namespace: user.namespace) }
+
+    it 'returns nil' do
+      expect(described_class.new(user, old_group, project).execute).to be_nil
+    end
+  end
+end
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index a47c10d991a..6b906f9372c 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -259,7 +259,7 @@ describe Projects::TransferService do
   end
 
   context 'missing group labels applied to issues or merge requests' do
-    it 'delegates tranfer to Labels::TransferService' do
+    it 'delegates transfer to Labels::TransferService' do
       group.add_owner(user)
 
       expect_any_instance_of(Labels::TransferService).to receive(:execute).once.and_call_original
@@ -268,6 +268,17 @@ describe Projects::TransferService do
     end
   end
 
+  context 'missing group milestones applied to issues or merge requests' do
+    it 'delegates transfer to Milestones::TransferService' do
+      group.add_owner(user)
+
+      expect(Milestones::TransferService).to receive(:new).with(user, project.group, project).and_call_original
+      expect_any_instance_of(Milestones::TransferService).to receive(:execute).once
+
+      transfer_project(project, user, group)
+    end
+  end
+
   context 'when hashed storage in use' do
     let(:hashed_project) { create(:project, :repository, namespace: user.namespace) }