Commit de8046fa authored by Luke Duncalfe's avatar Luke Duncalfe Committed by Dylan Griffith

Bugfix promoting an Issue with attachments to Epic

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38646 had
introduced a bug where `MarkdownContentRewriter` was passed the target's
`#project`. For Epics, this can be `nil`. Instead, we need to pass the
target's `#resource_parent`, which for an Epic, will be a Group.

https://gitlab.com/gitlab-org/gitlab/-/issues/236190
parent 4e5c5c78
......@@ -44,7 +44,7 @@ module Issuable
current_user,
original_entity.description,
original_entity.project,
new_entity.project
new_parent
).execute
new_entity.update!(description: rewritten_description)
......@@ -69,7 +69,7 @@ module Issuable
end
def new_parent
new_entity.project || new_entity.group
new_entity.resource_parent
end
def group
......
......@@ -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
......
......@@ -13,7 +13,6 @@ module Notes
@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 @@ module Notes
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 @@ module Notes
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,
......
---
title: Fix bug when promoting an Issue with attachments to an Epic
merge_request: 39654
author:
type: fixed
......@@ -103,6 +103,18 @@ RSpec.describe Epics::IssuePromoteService do
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 @@ RSpec.describe Epics::IssuePromoteService do
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
......
......@@ -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
......
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