diff options
| -rw-r--r-- | app/models/milestone.rb | 1 | ||||
| -rw-r--r-- | app/services/issuable_base_service.rb | 1 | ||||
| -rw-r--r-- | app/services/milestones/find_or_create_service.rb | 34 | ||||
| -rw-r--r-- | app/services/milestones/transfer_service.rb | 84 | ||||
| -rw-r--r-- | app/services/projects/transfer_service.rb | 3 | ||||
| -rw-r--r-- | changelogs/unreleased/60372-milestone-link-prevent-delete-issue-after-move-it-to-another-projec.yml | 5 | ||||
| -rw-r--r-- | spec/services/milestones/find_or_create_service_spec.rb | 82 | ||||
| -rw-r--r-- | spec/services/milestones/transfer_service_spec.rb | 122 | ||||
| -rw-r--r-- | spec/services/projects/transfer_service_spec.rb | 13 |
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) } |