From 9728d2b27a7f09f6b40709863518a6411bdd7386 Mon Sep 17 00:00:00 2001 From: Eugenia Grieff <egrieff@gitlab.com> Date: Thu, 18 Jul 2019 18:51:42 +0000 Subject: [PATCH] Resolve "Milestones should not be set on epics (issue promotion)" --- app/models/concerns/issuable.rb | 7 ++++ .../issuable/clone/attributes_rewriter.rb | 2 + ee/app/models/concerns/ee/issuable.rb | 5 +++ ...ld-not-be-set-on-epics-issue-promotion.yml | 5 +++ ee/spec/models/concerns/ee/issuable_spec.rb | 13 +++++++ .../clone/attributes_rewriter_spec.rb | 38 ++++++++++--------- spec/models/concerns/issuable_spec.rb | 21 ++++++++++ 7 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 ee/changelogs/unreleased/10270-milestones-should-not-be-set-on-epics-issue-promotion.yml diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 69cc3150993..e26bb062718 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -427,6 +427,13 @@ module Issuable def wipless_title_changed(old_title) old_title != title end + + ## + # Overridden on EE module + # + def supports_milestone? + respond_to?(:milestone_id) + end end Issuable.prepend(EE::Issuable) # rubocop: disable Cop/InjectEnterpriseEditionModule diff --git a/app/services/issuable/clone/attributes_rewriter.rb b/app/services/issuable/clone/attributes_rewriter.rb index 0300cc0d8d3..3c061d35558 100644 --- a/app/services/issuable/clone/attributes_rewriter.rb +++ b/app/services/issuable/clone/attributes_rewriter.rb @@ -17,6 +17,8 @@ module Issuable private def cloneable_milestone + return unless new_entity.supports_milestone? + title = original_entity.milestone&.title return unless title diff --git a/ee/app/models/concerns/ee/issuable.rb b/ee/app/models/concerns/ee/issuable.rb index 91700939a9e..b5710559e17 100644 --- a/ee/app/models/concerns/ee/issuable.rb +++ b/ee/app/models/concerns/ee/issuable.rb @@ -25,6 +25,11 @@ module EE super end + override :supports_milestone? + def supports_milestone? + super && !is_a?(Epic) + end + def supports_epic? is_a?(Issue) && project.group end diff --git a/ee/changelogs/unreleased/10270-milestones-should-not-be-set-on-epics-issue-promotion.yml b/ee/changelogs/unreleased/10270-milestones-should-not-be-set-on-epics-issue-promotion.yml new file mode 100644 index 00000000000..3b86f718b46 --- /dev/null +++ b/ee/changelogs/unreleased/10270-milestones-should-not-be-set-on-epics-issue-promotion.yml @@ -0,0 +1,5 @@ +--- +title: Do not include milestone attribute when promoting issue to epic +merge_request: 14532 +author: +type: fixed diff --git a/ee/spec/models/concerns/ee/issuable_spec.rb b/ee/spec/models/concerns/ee/issuable_spec.rb index 45c55d928ab..b9638e6e98e 100644 --- a/ee/spec/models/concerns/ee/issuable_spec.rb +++ b/ee/spec/models/concerns/ee/issuable_spec.rb @@ -37,4 +37,17 @@ describe EE::Issuable do end end end + + describe '#supports_milestone?' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + context "for epics" do + let(:epic) { build(:epic) } + + it 'returns false' do + expect(epic.supports_milestone?).to be_falsy + end + end + end end diff --git a/ee/spec/services/ee/issuable/clone/attributes_rewriter_spec.rb b/ee/spec/services/ee/issuable/clone/attributes_rewriter_spec.rb index 1fafc4cbe38..4c58f26e85e 100644 --- a/ee/spec/services/ee/issuable/clone/attributes_rewriter_spec.rb +++ b/ee/spec/services/ee/issuable/clone/attributes_rewriter_spec.rb @@ -9,33 +9,35 @@ describe Issuable::Clone::AttributesRewriter do let(:original_issue) { create(:issue, project: project) } context 'when a new object is a group entity' do - let(:new_epic) { create(:epic, group: group) } + context 'when entity is an epic' do + let(:new_epic) { create(:epic, group: group) } - subject { described_class.new(user, original_issue, new_epic) } + subject { described_class.new(user, original_issue, new_epic) } - context 'setting labels' do - let!(:project_label1) { create(:label, title: 'label1', project: project) } - let!(:project_label2) { create(:label, title: 'label2', project: project) } - let!(:group_label1) { create(:group_label, title: 'group_label', group: group) } - let!(:group_label2) { create(:group_label, title: 'label2', group: group) } + context 'setting labels' do + let(:project_label1) { create(:label, title: 'label1', project: project) } + let!(:project_label2) { create(:label, title: 'label2', project: project) } + let(:group_label1) { create(:group_label, title: 'group_label', group: group) } + let!(:group_label2) { create(:group_label, title: 'label2', group: group) } - it 'keeps group labels and merges project labels where possible' do - original_issue.update(labels: [project_label1, project_label2, group_label1]) + it 'keeps group labels and merges project labels where possible' do + original_issue.update(labels: [project_label1, project_label2, group_label1]) - subject.execute + subject.execute - expect(new_epic.reload.labels).to match_array([group_label1, group_label2]) + expect(new_epic.reload.labels).to match_array([group_label1, group_label2]) + end end - end - context 'setting milestones' do - it 'sets milestone to nil when old issue milestone is not a group milestone' do - milestone = create(:milestone, title: 'milestone', project: project) - original_issue.update(milestone: milestone) + context 'setting milestones' do + it 'sets milestone attribute as nil' do + milestone = create(:milestone, title: 'milestone', group: group) + original_issue.update(milestone: milestone) - subject.execute + expect(new_epic).to receive(:update).with(labels: [], milestone: nil) - expect(new_epic.reload.milestone).to be_nil + subject.execute + end end end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 68224a56515..e19da41c3fe 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -774,4 +774,25 @@ describe Issuable do end end end + + describe '#supports_milestone?' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + context "for issues" do + let(:issue) { build(:issue, project: project) } + + it 'returns true' do + expect(issue.supports_milestone?).to be_truthy + end + end + + context "for merge requests" do + let(:merge_request) { build(:merge_request, target_project: project, source_project: project) } + + it 'returns true' do + expect(merge_request.supports_milestone?).to be_truthy + end + end + end end -- 2.30.9