diff options
-rw-r--r-- | changelogs/unreleased/sh-fix-bitbucket-cloud-importer-replies.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/bitbucket_import/importer.rb | 23 | ||||
-rw-r--r-- | spec/lib/gitlab/bitbucket_import/importer_spec.rb | 75 |
3 files changed, 93 insertions, 10 deletions
diff --git a/changelogs/unreleased/sh-fix-bitbucket-cloud-importer-replies.yml b/changelogs/unreleased/sh-fix-bitbucket-cloud-importer-replies.yml new file mode 100644 index 00000000000..3f7044833f1 --- /dev/null +++ b/changelogs/unreleased/sh-fix-bitbucket-cloud-importer-replies.yml @@ -0,0 +1,5 @@ +--- +title: Fix Bitbucket Cloud importer omitting replies +merge_request: 21076 +author: +type: fixed diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index f3999e690fa..fa0186c854c 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -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, diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index 05c232d22cf..7a681bc6610 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -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 |