Commit 65545099 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'issue_40337' into 'master'

Fix promoting milestone updating all issuables without milestone

Closes #40337

See merge request gitlab-org/gitlab-ce!15487
parents 6dd89059 3f6e45ed
......@@ -6,14 +6,14 @@ module Milestones
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
# Destroy all milestones with same title across projects
destroy_old_milestones(milestone)
# Rollback if milestone is not valid
unless group_milestone.valid?
raise_error(group_milestone.errors.full_messages.to_sentence)
end
......@@ -35,7 +35,7 @@ module Milestones
end
def move_children_to_group_milestone(group_milestone)
milestone_ids_for_merge(group_milestone).in_groups_of(100) do |milestone_ids|
milestone_ids_for_merge(group_milestone).in_groups_of(100, false) do |milestone_ids|
update_children(group_milestone, milestone_ids)
end
end
......@@ -49,7 +49,12 @@ module Milestones
create_service = CreateService.new(group, current_user, params)
create_service.execute
milestone = create_service.execute
# milestone won't be valid here because of duplicated title
milestone.save(validate: false)
milestone
end
def update_children(group_milestone, milestone_ids)
......@@ -65,12 +70,12 @@ module Milestones
@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
def destroy_old_milestones(milestone)
Milestone.where(id: milestone_ids_for_merge(milestone)).destroy_all
end
def group_project_ids
@group_project_ids ||= group.projects.map(&:id)
@group_project_ids ||= group.projects.pluck(:id)
end
def raise_error(message)
......
---
title: Fix promoting milestone updating all issuables without milestone
merge_request:
author:
type: fixed
......@@ -25,6 +25,18 @@ describe Milestones::PromoteService do
expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
end
it 'does not promote milestone and update issuables if promoted milestone is not valid' do
issue = create(:issue, milestone: milestone, project: project)
merge_request = create(:merge_request, milestone: milestone, source_project: project)
allow_any_instance_of(Milestone).to receive(:valid?).and_return(false)
expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError)
expect(milestone.reload).to be_persisted
expect(issue.reload.milestone).to eq(milestone)
expect(merge_request.reload.milestone).to eq(milestone)
end
end
context 'without duplicated milestone titles across projects' do
......@@ -34,6 +46,16 @@ describe Milestones::PromoteService do
expect(promoted_milestone).to be_group_milestone
end
it 'does not update issuables without milestone with the new promoted milestone' do
issue_without_milestone = create(:issue, project: project, milestone: nil)
merge_request_without_milestone = create(:merge_request, milestone: nil, source_project: project)
service.execute(milestone)
expect(issue_without_milestone.reload.milestone).to be_nil
expect(merge_request_without_milestone.reload.milestone).to be_nil
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)
......@@ -59,6 +81,20 @@ describe Milestones::PromoteService do
expect(Milestone.exists?(milestone_2.id)).to be_falsy
end
it 'does not update issuables without milestone with the new promoted milestone' do
issue_without_milestone_1 = create(:issue, project: project, milestone: nil)
issue_without_milestone_2 = create(:issue, project: project_2, milestone: nil)
merge_request_without_milestone_1 = create(:merge_request, milestone: nil, source_project: project)
merge_request_without_milestone_2 = create(:merge_request, milestone: nil, source_project: project_2)
service.execute(milestone)
expect(issue_without_milestone_1.reload.milestone).to be_nil
expect(issue_without_milestone_2.reload.milestone).to be_nil
expect(merge_request_without_milestone_1.reload.milestone).to be_nil
expect(merge_request_without_milestone_2.reload.milestone).to be_nil
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)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment