Merge branch 'sh-fix-bitbucket-cloud-importer-replies' into 'master'
Fix Bitbucket Cloud importer omitting replies Closes #50052 See merge request gitlab-org/gitlab-ce!21076
This commit is contained in:
commit
be1ef711ed
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fix Bitbucket Cloud importer omitting replies
|
||||||
|
merge_request: 21076
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -188,7 +188,8 @@ module Gitlab
|
||||||
end
|
end
|
||||||
|
|
||||||
def import_inline_comments(inline_comments, pull_request, merge_request)
|
def import_inline_comments(inline_comments, pull_request, merge_request)
|
||||||
line_code_map = {}
|
position_map = {}
|
||||||
|
discussion_map = {}
|
||||||
|
|
||||||
children, parents = inline_comments.partition(&:has_parent?)
|
children, parents = inline_comments.partition(&:has_parent?)
|
||||||
|
|
||||||
|
@ -196,22 +197,28 @@ module Gitlab
|
||||||
# relationships. We assume that the child can appear in any order in
|
# relationships. We assume that the child can appear in any order in
|
||||||
# the JSON.
|
# the JSON.
|
||||||
parents.each do |comment|
|
parents.each do |comment|
|
||||||
line_code_map[comment.iid] = generate_line_code(comment)
|
position_map[comment.iid] = build_position(merge_request, comment)
|
||||||
end
|
end
|
||||||
|
|
||||||
children.each do |comment|
|
children.each do |comment|
|
||||||
line_code_map[comment.iid] = line_code_map.fetch(comment.parent_id, nil)
|
position_map[comment.iid] = position_map.fetch(comment.parent_id, nil)
|
||||||
end
|
end
|
||||||
|
|
||||||
inline_comments.each do |comment|
|
inline_comments.each do |comment|
|
||||||
begin
|
begin
|
||||||
attributes = pull_request_comment_attributes(comment)
|
attributes = pull_request_comment_attributes(comment)
|
||||||
|
attributes[:discussion_id] = discussion_map[comment.parent_id] if comment.has_parent?
|
||||||
|
|
||||||
attributes.merge!(
|
attributes.merge!(
|
||||||
position: build_position(merge_request, comment),
|
position: position_map[comment.iid],
|
||||||
line_code: line_code_map.fetch(comment.iid),
|
|
||||||
type: 'DiffNote')
|
type: 'DiffNote')
|
||||||
|
|
||||||
merge_request.notes.create!(attributes)
|
note = merge_request.notes.create!(attributes)
|
||||||
|
|
||||||
|
# We can't store a discussion ID until a note is created, so if
|
||||||
|
# replies are created before the parent the discussion ID won't be
|
||||||
|
# linked properly.
|
||||||
|
discussion_map[comment.iid] = note.discussion_id
|
||||||
rescue StandardError => e
|
rescue StandardError => e
|
||||||
errors << { type: :pull_request, iid: comment.iid, errors: e.message }
|
errors << { type: :pull_request, iid: comment.iid, errors: e.message }
|
||||||
end
|
end
|
||||||
|
@ -240,10 +247,6 @@ module Gitlab
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def generate_line_code(pr_comment)
|
|
||||||
Gitlab::Git.diff_line_code(pr_comment.file_path, pr_comment.new_pos, pr_comment.old_pos)
|
|
||||||
end
|
|
||||||
|
|
||||||
def pull_request_comment_attributes(comment)
|
def pull_request_comment_attributes(comment)
|
||||||
{
|
{
|
||||||
project: project,
|
project: project,
|
||||||
|
|
|
@ -69,6 +69,7 @@ describe Gitlab::BitbucketImport::Importer do
|
||||||
let(:project) do
|
let(:project) do
|
||||||
create(
|
create(
|
||||||
:project,
|
:project,
|
||||||
|
:repository,
|
||||||
import_source: project_identifier,
|
import_source: project_identifier,
|
||||||
import_url: "https://bitbucket.org/#{project_identifier}.git",
|
import_url: "https://bitbucket.org/#{project_identifier}.git",
|
||||||
import_data_attributes: { credentials: data }
|
import_data_attributes: { credentials: data }
|
||||||
|
@ -85,10 +86,84 @@ describe Gitlab::BitbucketImport::Importer do
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
|
let(:sample) { RepoHelpers.sample_compare }
|
||||||
|
|
||||||
before do
|
before do
|
||||||
allow(importer).to receive(:gitlab_shell) { gitlab_shell }
|
allow(importer).to receive(:gitlab_shell) { gitlab_shell }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
subject { described_class.new(project) }
|
||||||
|
|
||||||
|
describe '#import_pull_requests' do
|
||||||
|
before do
|
||||||
|
allow(subject).to receive(:import_wiki)
|
||||||
|
allow(subject).to receive(:import_issues)
|
||||||
|
|
||||||
|
pull_request = instance_double(
|
||||||
|
Bitbucket::Representation::PullRequest,
|
||||||
|
iid: 10,
|
||||||
|
source_branch_sha: sample.commits.last,
|
||||||
|
source_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.source_branch,
|
||||||
|
target_branch_sha: sample.commits.first,
|
||||||
|
target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch,
|
||||||
|
title: 'This is a title',
|
||||||
|
description: 'This is a test pull request',
|
||||||
|
state: 'merged',
|
||||||
|
author: 'other',
|
||||||
|
created_at: Time.now,
|
||||||
|
updated_at: Time.now)
|
||||||
|
|
||||||
|
# https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad
|
||||||
|
@inline_note = instance_double(
|
||||||
|
Bitbucket::Representation::PullRequestComment,
|
||||||
|
iid: 2,
|
||||||
|
file_path: '.gitmodules',
|
||||||
|
old_pos: nil,
|
||||||
|
new_pos: 4,
|
||||||
|
note: 'Hello world',
|
||||||
|
author: 'root',
|
||||||
|
created_at: Time.now,
|
||||||
|
updated_at: Time.now,
|
||||||
|
inline?: true,
|
||||||
|
has_parent?: false)
|
||||||
|
|
||||||
|
@reply = instance_double(
|
||||||
|
Bitbucket::Representation::PullRequestComment,
|
||||||
|
iid: 3,
|
||||||
|
file_path: '.gitmodules',
|
||||||
|
note: 'Hello world',
|
||||||
|
author: 'root',
|
||||||
|
created_at: Time.now,
|
||||||
|
updated_at: Time.now,
|
||||||
|
inline?: true,
|
||||||
|
has_parent?: true,
|
||||||
|
parent_id: 2)
|
||||||
|
|
||||||
|
comments = [@inline_note, @reply]
|
||||||
|
|
||||||
|
allow(subject.client).to receive(:repo)
|
||||||
|
allow(subject.client).to receive(:pull_requests).and_return([pull_request])
|
||||||
|
allow(subject.client).to receive(:pull_request_comments).with(anything, pull_request.iid).and_return(comments)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'imports threaded discussions' do
|
||||||
|
expect { subject.execute }.to change { MergeRequest.count }.by(1)
|
||||||
|
|
||||||
|
merge_request = MergeRequest.first
|
||||||
|
expect(merge_request.notes.count).to eq(2)
|
||||||
|
expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1)
|
||||||
|
|
||||||
|
notes = merge_request.notes.order(:id).to_a
|
||||||
|
start_note = notes.first
|
||||||
|
expect(start_note).to be_a(DiffNote)
|
||||||
|
expect(start_note.note).to eq(@inline_note.note)
|
||||||
|
|
||||||
|
reply_note = notes.last
|
||||||
|
expect(reply_note).to be_a(DiffNote)
|
||||||
|
expect(reply_note.note).to eq(@reply.note)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context 'issues statuses' do
|
context 'issues statuses' do
|
||||||
before do
|
before do
|
||||||
# HACK: Bitbucket::Representation.const_get('Issue') seems to return ::Issue without this
|
# HACK: Bitbucket::Representation.const_get('Issue') seems to return ::Issue without this
|
||||||
|
|
Loading…
Reference in New Issue