diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb index 639423ed4bf37d6235aa6a74ddd7904e06b0b771..b2f9c083b5b9972771fde447ade4edfe2ae07fdc 100644 --- a/app/services/issuable/clone/base_service.rb +++ b/app/services/issuable/clone/base_service.rb @@ -44,7 +44,7 @@ def update_new_entity_description current_user, original_entity.description, original_entity.project, - new_entity.project + new_parent ).execute new_entity.update!(description: rewritten_description) @@ -69,7 +69,7 @@ def close_issue end def new_parent - new_entity.project || new_entity.group + new_entity.resource_parent end def group diff --git a/app/services/markdown_content_rewriter_service.rb b/app/services/markdown_content_rewriter_service.rb index f945990a1b471a38931c27678cb50ba421dec382..bc6fd592eaa392a23287c1fd666d133632ea4e78 100644 --- a/app/services/markdown_content_rewriter_service.rb +++ b/app/services/markdown_content_rewriter_service.rb @@ -7,6 +7,10 @@ class MarkdownContentRewriterService REWRITERS = [Gitlab::Gfm::ReferenceRewriter, Gitlab::Gfm::UploadsRewriter].freeze def initialize(current_user, content, source_parent, target_parent) + # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39654#note_399095117 + raise ArgumentError, 'The rewriter classes require that `source_parent` is a `Project`' \ + unless source_parent.is_a?(Project) + @current_user = current_user @content = content.presence @source_parent = source_parent diff --git a/app/services/notes/copy_service.rb b/app/services/notes/copy_service.rb index 31aaa219922d3be4ccc29b0392aa77ac8a3f5501..6e5b4596602c52d49b14e397370b27adb303e9b7 100644 --- a/app/services/notes/copy_service.rb +++ b/app/services/notes/copy_service.rb @@ -13,7 +13,6 @@ def initialize(current_user, from_noteable, to_noteable) @from_noteable = from_noteable @to_noteable = to_noteable @from_project = from_noteable.project - @to_project = to_noteable.project @new_discussion_ids = {} end @@ -27,8 +26,7 @@ def execute private - attr_reader :from_noteable, :to_noteable, :from_project, :to_project, - :current_user, :new_discussion_ids + attr_reader :from_noteable, :to_noteable, :from_project, :current_user, :new_discussion_ids def copy_note(note) new_note = note.dup @@ -40,7 +38,7 @@ def copy_note(note) def params_from_note(note, new_note) new_discussion_ids[note.discussion_id] ||= Discussion.discussion_id(new_note) - rewritten_note = MarkdownContentRewriterService.new(current_user, note.note, from_project, to_project).execute + rewritten_note = MarkdownContentRewriterService.new(current_user, note.note, from_project, to_noteable.resource_parent).execute new_params = { project: to_noteable.project, diff --git a/changelogs/unreleased/236190-bugfix-issue-promote-with-attachments.yml b/changelogs/unreleased/236190-bugfix-issue-promote-with-attachments.yml new file mode 100644 index 0000000000000000000000000000000000000000..4102c0649d3ea0df15d44848bc5026b0965ff401 --- /dev/null +++ b/changelogs/unreleased/236190-bugfix-issue-promote-with-attachments.yml @@ -0,0 +1,5 @@ +--- +title: Fix bug when promoting an Issue with attachments to an Epic +merge_request: 39654 +author: +type: fixed diff --git a/ee/spec/services/epics/issue_promote_service_spec.rb b/ee/spec/services/epics/issue_promote_service_spec.rb index 7f3c2f3e4c51cf18cf57638ac15df88a07d587b5..76e5c57d080d97b874f6ef01c47f5bd9a2c86c51 100644 --- a/ee/spec/services/epics/issue_promote_service_spec.rb +++ b/ee/spec/services/epics/issue_promote_service_spec.rb @@ -103,6 +103,18 @@ expect(epic.user_mentions.count).to eq 2 end end + + context 'when issue description has an attachment' do + let(:image_uploader) { build(:file_uploader, project: project) } + let(:description) { "A description and image: #{image_uploader.markdown_link}" } + + it 'copies the description, rewriting the attachment' do + new_image_uploader = Upload.last.retrieve_uploader + + expect(new_image_uploader.markdown_link).not_to eq(image_uploader.markdown_link) + expect(epic.description).to eq("A description and image: #{new_image_uploader.markdown_link}") + end + end end context 'when an issue belongs to an epic' do @@ -130,20 +142,28 @@ end end - context 'when promoted issue has notes' do - let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } - + context 'when issue has notes' do before do allow(Gitlab::Tracking).to receive(:event).with('epics', 'promote', an_instance_of(Hash)) issue.reload end - it 'creates a new epic with all notes' do + it 'copies all notes' do + discussion = create(:discussion_note_on_issue, noteable: issue, project: issue.project) + epic = subject.execute(issue) expect(epic.notes.count).to eq(issue.notes.count) expect(epic.notes.where(discussion_id: discussion.discussion_id).count).to eq(0) expect(issue.notes.where(discussion_id: discussion.discussion_id).count).to eq(1) end + + it 'copies note attachments' do + create(:discussion_note_on_issue, :with_attachment, noteable: issue, project: issue.project) + + epic = subject.execute(issue) + + expect(epic.notes.user.first.attachment).to be_kind_of(AttachmentUploader) + end end end end diff --git a/spec/services/markdown_content_rewriter_service_spec.rb b/spec/services/markdown_content_rewriter_service_spec.rb index a5cd2a25a37f468ad7e2a70b792f5b92aec1f450..47332bec31989a7cf1d87f942a6f588acc00496b 100644 --- a/spec/services/markdown_content_rewriter_service_spec.rb +++ b/spec/services/markdown_content_rewriter_service_spec.rb @@ -3,12 +3,20 @@ require 'spec_helper' RSpec.describe MarkdownContentRewriterService do - describe '#execute' do - let_it_be(:user) { create(:user) } - let_it_be(:source_parent) { create(:project, :public) } - let_it_be(:target_parent) { create(:project, :public) } - let(:content) { 'My content' } + let_it_be(:user) { create(:user) } + let_it_be(:source_parent) { create(:project, :public) } + let_it_be(:target_parent) { create(:project, :public) } + let(:content) { 'My content' } + + describe '#initialize' do + it 'raises an error if source_parent is not a Project' do + expect do + described_class.new(user, content, create(:group), target_parent) + end.to raise_error(ArgumentError, 'The rewriter classes require that `source_parent` is a `Project`') + end + end + describe '#execute' do subject { described_class.new(user, content, source_parent, target_parent).execute } it 'calls the rewriter classes successfully', :aggregate_failures do