Commit 2aa4d7f1 authored by Stan Hu's avatar Stan Hu

Fix bug where fallback diff notes would not have an associated position

parent 0b91a533
...@@ -253,15 +253,15 @@ module Gitlab ...@@ -253,15 +253,15 @@ module Gitlab
# Bitbucket Server supports the ability to comment on any line, not just the # Bitbucket Server supports the ability to comment on any line, not just the
# line in the diff. If we can't add the note as a DiffNote, fallback to creating # line in the diff. If we can't add the note as a DiffNote, fallback to creating
# a regular note. # a regular note.
create_fallback_diff_note(merge_request, comment) create_fallback_diff_note(merge_request, comment, position)
rescue StandardError => e rescue StandardError => e
errors << { type: :pull_request, id: comment.id, errors: e.message } errors << { type: :pull_request, id: comment.id, errors: e.message }
nil nil
end end
def create_fallback_diff_note(merge_request, comment) def create_fallback_diff_note(merge_request, comment, position)
attributes = pull_request_comment_attributes(comment) attributes = pull_request_comment_attributes(comment)
attributes[:note] = "*Comment on file: #{comment.file_path}, old line: #{comment.old_pos}, new line: #{comment.new_pos}*\n\n" + attributes[:note] attributes[:note] = "*Comment on #{position.old_path}:#{position.old_line} -> #{position.new_path}:#{position.new_line}*\n\n" + attributes[:note]
merge_request.notes.create!(attributes) merge_request.notes.create!(attributes)
end end
......
...@@ -178,6 +178,13 @@ describe Gitlab::BitbucketServerImport::Importer do ...@@ -178,6 +178,13 @@ describe Gitlab::BitbucketServerImport::Importer do
end end
it 'falls back to comments if diff comments fail to validate' do it 'falls back to comments if diff comments fail to validate' do
reply = instance_double(
BitbucketServer::Representation::Comment,
author_email: 'someuser@gitlab.com',
note: 'I agree',
created_at: now,
updated_at: now)
# https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad
inline_note = instance_double( inline_note = instance_double(
BitbucketServer::Representation::PullRequestComment, BitbucketServer::Representation::PullRequestComment,
...@@ -189,7 +196,7 @@ describe Gitlab::BitbucketServerImport::Importer do ...@@ -189,7 +196,7 @@ describe Gitlab::BitbucketServerImport::Importer do
new_pos: 9, new_pos: 9,
note: 'This is a note with an invalid line position.', note: 'This is a note with an invalid line position.',
author_email: project.owner.email, author_email: project.owner.email,
comments: [], comments: [reply],
created_at: now, created_at: now,
updated_at: now, updated_at: now,
parent_comment: nil) parent_comment: nil)
...@@ -201,15 +208,18 @@ describe Gitlab::BitbucketServerImport::Importer do ...@@ -201,15 +208,18 @@ describe Gitlab::BitbucketServerImport::Importer do
merge_event?: false, merge_event?: false,
comment: inline_note) comment: inline_note)
allow(reply).to receive(:parent_comment).and_return(inline_note)
expect(subject.client).to receive(:activities).and_return([inline_comment]) expect(subject.client).to receive(:activities).and_return([inline_comment])
expect { subject.execute }.to change { MergeRequest.count }.by(1) expect { subject.execute }.to change { MergeRequest.count }.by(1)
merge_request = MergeRequest.first merge_request = MergeRequest.first
expect(merge_request.notes.count).to eq(1) expect(merge_request.notes.count).to eq(2)
note = merge_request.notes.first notes = merge_request.notes
expect(note.note).to start_with('*Comment on file:') expect(notes.first.note).to start_with('*Comment on .gitmodules')
expect(notes.second.note).to start_with('*Comment on .gitmodules')
end end
it 'restores branches of inaccessible SHAs' do it 'restores branches of inaccessible SHAs' 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