Commit 8c467b91 authored by Stan Hu's avatar Stan Hu

Fix Bitbucket Cloud importer omitting replies

Inline diff comments did not have the proper position, so even though
they had line codes the merge request validation would fail. Now
we cache the line position for each parent comment and use that.

Closes #50052
parent f3b36ac1
---
title: Fix Bitbucket Cloud importer omitting replies
merge_request: 21076
author:
type: fixed
......@@ -188,7 +188,8 @@ module Gitlab
end
def import_inline_comments(inline_comments, pull_request, merge_request)
line_code_map = {}
position_map = {}
discussion_map = {}
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
# the JSON.
parents.each do |comment|
line_code_map[comment.iid] = generate_line_code(comment)
position_map[comment.iid] = build_position(merge_request, comment)
end
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
inline_comments.each do |comment|
begin
attributes = pull_request_comment_attributes(comment)
attributes[:discussion_id] = discussion_map[comment.parent_id] if comment.has_parent?
attributes.merge!(
position: build_position(merge_request, comment),
line_code: line_code_map.fetch(comment.iid),
position: position_map[comment.iid],
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
errors << { type: :pull_request, iid: comment.iid, errors: e.message }
end
......@@ -240,10 +247,6 @@ module Gitlab
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)
{
project: project,
......
......@@ -69,6 +69,7 @@ describe Gitlab::BitbucketImport::Importer do
let(:project) do
create(
:project,
:repository,
import_source: project_identifier,
import_url: "https://bitbucket.org/#{project_identifier}.git",
import_data_attributes: { credentials: data }
......@@ -85,10 +86,84 @@ describe Gitlab::BitbucketImport::Importer do
}
end
let(:sample) { RepoHelpers.sample_compare }
before do
allow(importer).to receive(:gitlab_shell) { gitlab_shell }
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
before do
# HACK: Bitbucket::Representation.const_get('Issue') seems to return ::Issue without this
......
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