diff --git a/changelogs/unreleased/218609-bitbucket-server-importer-by-slug.yml b/changelogs/unreleased/218609-bitbucket-server-importer-by-slug.yml new file mode 100644 index 0000000000000000000000000000000000000000..8a633dcacf625add53f26a68b2e03b224d6d5b68 --- /dev/null +++ b/changelogs/unreleased/218609-bitbucket-server-importer-by-slug.yml @@ -0,0 +1,5 @@ +--- +title: Add user mapping by username when importing projects for Bitbucket Server importer +merge_request: 36885 +author: +type: added diff --git a/config/feature_flags/development/bitbucket_server_user_mapping_by_username.yml b/config/feature_flags/development/bitbucket_server_user_mapping_by_username.yml new file mode 100644 index 0000000000000000000000000000000000000000..3875cce4d281bce81ba702cf7713f5f95416343a --- /dev/null +++ b/config/feature_flags/development/bitbucket_server_user_mapping_by_username.yml @@ -0,0 +1,7 @@ +--- +name: bitbucket_server_user_mapping_by_username +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36885 +rollout_issue_url: +group: group::import +type: development +default_enabled: false diff --git a/doc/user/project/import/bitbucket_server.md b/doc/user/project/import/bitbucket_server.md index 90d33b72ca0fb7913d3c3f2be3c3107198ebc579..8ecc0e1f9c4b0e46bc56130cf6cb23ec732005f2 100644 --- a/doc/user/project/import/bitbucket_server.md +++ b/doc/user/project/import/bitbucket_server.md @@ -62,6 +62,25 @@ The importer will create any new namespaces (groups) if they don't exist or in the case the namespace is taken, the repository will be imported under the user's namespace that started the import process. +#### User assignment by username + +Alternatively, user assignment by username is available behind a `bitbucket_server_user_mapping_by_username` feature flag. +The importer will try to find a user in the GitLab user database using author's `username` or `slug` or `displayName`. +Falls back to author's `email` if user is not found by username. +Similarly to user assignment by email, if no such user is available, the project creator is set as the author. + +To enable or disable user assignment by username: + +Start a [Rails console](../../../administration/troubleshooting/debug.md#starting-a-rails-console-session). + +```ruby +# Enable +Feature.enable(:bitbucket_server_user_mapping_by_username) + +# Disable +Feature.disable(:bitbucket_server_user_mapping_by_username) +``` + ## Importing your Bitbucket repositories 1. Sign in to GitLab and go to your dashboard. diff --git a/lib/bitbucket_server/representation/comment.rb b/lib/bitbucket_server/representation/comment.rb index 99b97a3b181b7a3ca9a92c5a9b8f60ae8594526c..7e55c446e76a2226f94fc0aea6cf43ca1952ec03 100644 --- a/lib/bitbucket_server/representation/comment.rb +++ b/lib/bitbucket_server/representation/comment.rb @@ -38,7 +38,9 @@ def id end def author_username - author['displayName'] + author['username'] || + author['slug'] || + author['displayName'] end def author_email diff --git a/lib/bitbucket_server/representation/pull_request.rb b/lib/bitbucket_server/representation/pull_request.rb index c3e927d8de7b2ff8b2d52802fcb14b208410b1a6..2f377bdced2af9c36a739ebbfc80e4deb6a919bc 100644 --- a/lib/bitbucket_server/representation/pull_request.rb +++ b/lib/bitbucket_server/representation/pull_request.rb @@ -11,6 +11,12 @@ def author_email raw.dig('author', 'user', 'emailAddress') end + def author_username + raw.dig('author', 'user', 'username') || + raw.dig('author', 'user', 'slug') || + raw.dig('author', 'user', 'displayName') + end + def description raw['description'] end diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 18a1b64729e2b6d67aa83196c16387485a95df64..aca5a63a4240c9e910e092e9e1161376baca461b 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -61,17 +61,18 @@ def handle_errors }.to_json) end - def gitlab_user_id(email) - find_user_id(email) || project.creator_id - end + def find_user_id(by:, value:) + return unless value - def find_user_id(email) - return unless email + return users[value] if users.key?(value) - return users[email] if users.key?(email) + user = if by == :email + User.find_by_any_email(value, confirmed: true) + else + User.find_by_username(value) + end - user = User.find_by_any_email(email, confirmed: true) - users[email] = user&.id + users[value] = user&.id user&.id end @@ -197,9 +198,8 @@ def import_bitbucket_pull_request(pull_request) log_info(stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid) description = '' - description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author_email) + description += author_line(pull_request) description += pull_request.description if pull_request.description - author_id = gitlab_user_id(pull_request.author_email) attributes = { iid: pull_request.iid, @@ -212,7 +212,7 @@ def import_bitbucket_pull_request(pull_request) target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name), target_branch_sha: pull_request.target_branch_sha, state_id: MergeRequest.available_states[pull_request.state], - author_id: author_id, + author_id: author_id(pull_request), created_at: pull_request.created_at, updated_at: pull_request.updated_at } @@ -254,7 +254,7 @@ def import_merge_event(merge_request, merge_event) committer = merge_event.committer_email - user_id = gitlab_user_id(committer) + user_id = find_user_id(by: :email, value: committer) || project.creator_id timestamp = merge_event.merge_timestamp merge_request.update({ merge_commit_sha: merge_event.merge_commit }) metric = MergeRequest::Metrics.find_or_initialize_by(merge_request: merge_request) @@ -353,7 +353,7 @@ def import_standalone_pr_comments(pr_comments, merge_request) end def pull_request_comment_attributes(comment) - author = find_user_id(comment.author_email) + author = uid(comment) note = '' unless author @@ -397,6 +397,23 @@ def log_base_data def metrics @metrics ||= Gitlab::Import::Metrics.new(:bitbucket_server_importer, @project) end + + def author_line(rep_object) + return '' if uid(rep_object) + + @formatter.author_line(rep_object.author) + end + + def author_id(rep_object) + uid(rep_object) || project.creator_id + end + + def uid(rep_object) + find_user_id(by: :email, value: rep_object.author_email) unless Feature.enabled?(:bitbucket_server_user_mapping_by_username) + + find_user_id(by: :username, value: rep_object.author_username) || + find_user_id(by: :email, value: rep_object.author_email) + end end end end diff --git a/spec/fixtures/importers/bitbucket_server/activities.json b/spec/fixtures/importers/bitbucket_server/activities.json index 09adfca9f318585ba0d1f41c6bb3f588556ed2da..ddcb94b8f5890690e95fab150b4eee323b01a47a 100644 --- a/spec/fixtures/importers/bitbucket_server/activities.json +++ b/spec/fixtures/importers/bitbucket_server/activities.json @@ -20,7 +20,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [ @@ -38,7 +39,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [ @@ -56,7 +58,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -85,7 +88,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "createdDate": 1530164016725, @@ -115,7 +119,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "createdDate": 1530164026000, @@ -147,7 +152,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -194,7 +200,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -363,7 +370,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -383,7 +391,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -543,7 +552,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -563,7 +573,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [ @@ -581,7 +592,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [ @@ -599,7 +611,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -789,7 +802,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -809,7 +823,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -843,7 +858,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -863,7 +879,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "authorTimestamp": 1529727872000, @@ -880,7 +897,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "committerTimestamp": 1529727872000, @@ -951,7 +969,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -971,7 +990,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [ @@ -989,7 +1009,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -1038,7 +1059,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -1058,7 +1080,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" }, "comments": [], @@ -1092,7 +1115,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } }, @@ -1113,7 +1137,8 @@ ] }, "name": "root", - "slug": "root", + "slug": "slug", + "username": "username", "type": "NORMAL" } } diff --git a/spec/fixtures/importers/bitbucket_server/pull_request.json b/spec/fixtures/importers/bitbucket_server/pull_request.json index 6c7fcf3b04c80b15e23648bdac97ca6f090ee350..77f7eb330f5354a12b35c43ef7c435a2d25d62dd 100644 --- a/spec/fixtures/importers/bitbucket_server/pull_request.json +++ b/spec/fixtures/importers/bitbucket_server/pull_request.json @@ -5,8 +5,9 @@ "status":"UNAPPROVED", "user":{ "active":true, - "displayName":"root", + "displayName":"displayName", "emailAddress":"joe.montana@49ers.com", + "username": "username", "id":1, "links":{ "self":[ @@ -16,7 +17,7 @@ ] }, "name":"root", - "slug":"root", + "slug":"slug", "type":"NORMAL" } }, diff --git a/spec/lib/bitbucket_server/representation/comment_spec.rb b/spec/lib/bitbucket_server/representation/comment_spec.rb index b568789bd97083497f83eaf09098ab5eb0765f10..41cdaab7a001c664e7a450a7a8490380b12530e0 100644 --- a/spec/lib/bitbucket_server/representation/comment_spec.rb +++ b/spec/lib/bitbucket_server/representation/comment_spec.rb @@ -13,7 +13,30 @@ end describe '#author_username' do - it { expect(subject.author_username).to eq('root' ) } + it 'returns username' do + expect(subject.author_username).to eq('username') + end + + context 'when username is absent' do + before do + comment['comment']['author'].delete('username') + end + + it 'returns slug' do + expect(subject.author_username).to eq('slug') + end + end + + context 'when slug and username are absent' do + before do + comment['comment']['author'].delete('username') + comment['comment']['author'].delete('slug') + end + + it 'returns displayName' do + expect(subject.author_username).to eq('root') + end + end end describe '#author_email' do diff --git a/spec/lib/bitbucket_server/representation/pull_request_spec.rb b/spec/lib/bitbucket_server/representation/pull_request_spec.rb index a05d98f0d4aff252fcfd844bc1bacf4c2f52eb2b..d7b893e8081f6a23fb834a96ff8caa26e0480476 100644 --- a/spec/lib/bitbucket_server/representation/pull_request_spec.rb +++ b/spec/lib/bitbucket_server/representation/pull_request_spec.rb @@ -15,6 +15,33 @@ it { expect(subject.author_email).to eq('joe.montana@49ers.com') } end + describe '#author_username' do + it 'returns username' do + expect(subject.author_username).to eq('username') + end + + context 'when username is absent' do + before do + sample_data['author']['user'].delete('username') + end + + it 'returns slug' do + expect(subject.author_username).to eq('slug') + end + end + + context 'when slug and username are absent' do + before do + sample_data['author']['user'].delete('username') + sample_data['author']['user'].delete('slug') + end + + it 'returns displayName' do + expect(subject.author_username).to eq('displayName') + end + end + end + describe '#description' do it { expect(subject.description).to eq('Test') } end diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb index 5eb27c51f9ee085e5a8c5da8fb4958f9604cffdf..80ec5ec1fc765769241b8bb1af3a97a9a94a00d7 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb @@ -6,9 +6,10 @@ include ImportSpecHelper let(:import_url) { 'http://my-bitbucket' } - let(:user) { 'bitbucket' } + let(:bitbucket_user) { 'bitbucket' } + let(:project_creator) { create(:user, username: 'project_creator', email: 'project_creator@example.org') } let(:password) { 'test' } - let(:project) { create(:project, :repository, import_url: import_url) } + let(:project) { create(:project, :repository, import_url: import_url, creator: project_creator) } let(:now) { Time.now.utc.change(usec: 0) } let(:project_key) { 'TEST' } let(:repo_slug) { 'rouge' } @@ -19,7 +20,7 @@ before do data = project.create_or_update_import_data( data: { project_key: project_key, repo_slug: repo_slug }, - credentials: { base_uri: import_url, user: user, password: password } + credentials: { base_uri: import_url, user: bitbucket_user, password: password } ) data.save project.save @@ -51,12 +52,11 @@ end describe '#import_pull_requests' do - before do - allow(subject).to receive(:import_repository) - allow(subject).to receive(:delete_temp_branches) - allow(subject).to receive(:restore_branches) + let(:pull_request_author) { create(:user, username: 'pull_request_author', email: 'pull_request_author@example.org') } + let(:note_author) { create(:user, username: 'note_author', email: 'note_author@example.org') } - pull_request = instance_double( + let(:pull_request) do + instance_double( BitbucketServer::Representation::PullRequest, iid: 10, source_branch_sha: sample.commits.last, @@ -67,65 +67,172 @@ description: 'This is a test pull request', state: 'merged', author: 'Test Author', - author_email: project.owner.email, + author_email: pull_request_author.email, + author_username: pull_request_author.username, created_at: Time.now, updated_at: Time.now, raw: {}, merged?: true) + end - allow(subject.client).to receive(:pull_requests).and_return([pull_request]) - - @merge_event = instance_double( + let(:merge_event) do + instance_double( BitbucketServer::Representation::Activity, comment?: false, merge_event?: true, - committer_email: project.owner.email, + committer_email: pull_request_author.email, merge_timestamp: now, merge_commit: '12345678' ) + end - @pr_note = instance_double( + let(:pr_note) do + instance_double( BitbucketServer::Representation::Comment, note: 'Hello world', - author_email: 'unknown@gmail.com', - author_username: 'The Flash', + author_email: note_author.email, + author_username: note_author.username, comments: [], created_at: now, updated_at: now, parent_comment: nil) + end - @pr_comment = instance_double( + let(:pr_comment) do + instance_double( BitbucketServer::Representation::Activity, comment?: true, inline_comment?: false, merge_event?: false, - comment: @pr_note) + comment: pr_note) + end + + before do + allow(subject).to receive(:import_repository) + allow(subject).to receive(:delete_temp_branches) + allow(subject).to receive(:restore_branches) + + allow(subject.client).to receive(:pull_requests).and_return([pull_request]) end it 'imports merge event' do - expect(subject.client).to receive(:activities).and_return([@merge_event]) + expect(subject.client).to receive(:activities).and_return([merge_event]) expect { subject.execute }.to change { MergeRequest.count }.by(1) merge_request = MergeRequest.first - expect(merge_request.metrics.merged_by).to eq(project.owner) - expect(merge_request.metrics.merged_at).to eq(@merge_event.merge_timestamp) + expect(merge_request.metrics.merged_by).to eq(pull_request_author) + expect(merge_request.metrics.merged_at).to eq(merge_event.merge_timestamp) expect(merge_request.merge_commit_sha).to eq('12345678') expect(merge_request.state_id).to eq(3) end - it 'imports comments' do - expect(subject.client).to receive(:activities).and_return([@pr_comment]) + describe 'pull request author user mapping' do + before do + allow(subject.client).to receive(:activities).and_return([merge_event]) + end - expect { subject.execute }.to change { MergeRequest.count }.by(1) + shared_examples 'imports pull requests' do + it 'maps user' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) - merge_request = MergeRequest.first - expect(merge_request.notes.count).to eq(1) - note = merge_request.notes.first - expect(note.note).to end_with(@pr_note.note) - expect(note.author).to eq(project.owner) - expect(note.created_at).to eq(@pr_note.created_at) - expect(note.updated_at).to eq(@pr_note.created_at) + merge_request = MergeRequest.first + expect(merge_request.author).to eq(pull_request_author) + end + end + + context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: false) + end + + include_examples 'imports pull requests' + end + + context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: true) + end + + include_examples 'imports pull requests' do + context 'when username is not present' do + before do + allow(pull_request).to receive(:author_username).and_return(nil) + end + + it 'maps by email' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request = MergeRequest.first + expect(merge_request.author).to eq(pull_request_author) + end + end + end + end + + context 'when user is not found' do + before do + allow(pull_request).to receive(:author_username).and_return(nil) + allow(pull_request).to receive(:author_email).and_return(nil) + end + + it 'maps importer user' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request = MergeRequest.first + expect(merge_request.author).to eq(project_creator) + end + end + end + + describe 'comments' do + shared_examples 'imports comments' do + it 'imports comments' do + expect(subject.client).to receive(:activities).and_return([pr_comment]) + + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request = MergeRequest.first + expect(merge_request.notes.count).to eq(1) + note = merge_request.notes.first + expect(note.note).to end_with(pr_note.note) + expect(note.author).to eq(note_author) + expect(note.created_at).to eq(pr_note.created_at) + expect(note.updated_at).to eq(pr_note.created_at) + end + end + + context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: false) + end + + include_examples 'imports comments' + end + + context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: true) + end + + include_examples 'imports comments' + + context 'when username is not present' do + before do + allow(pr_note).to receive(:author_username).and_return(nil) + allow(subject.client).to receive(:activities).and_return([pr_comment]) + end + + it 'maps by email' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request = MergeRequest.first + expect(merge_request.notes.count).to eq(1) + note = merge_request.notes.first + expect(note.author).to eq(note_author) + end + end + end end context 'metrics' do @@ -135,7 +242,7 @@ before do allow(Gitlab::Metrics).to receive(:counter) { counter } allow(Gitlab::Metrics).to receive(:histogram) { histogram } - allow(subject.client).to receive(:activities).and_return([@merge_event]) + allow(subject.client).to receive(:activities).and_return([merge_event]) end it 'counts and measures duration of imported projects' do @@ -170,73 +277,137 @@ end end - it 'imports threaded discussions' do - reply = instance_double( - BitbucketServer::Representation::PullRequestComment, - author_email: 'someuser@gitlab.com', - author_username: 'Batman', - note: 'I agree', - created_at: now, - updated_at: now) + describe 'threaded discussions' do + let(:reply_author) { create(:user, username: 'reply_author', email: 'reply_author@example.org') } + let(:inline_note_author) { create(:user, username: 'inline_note_author', email: 'inline_note_author@example.org') } + + let(:reply) do + instance_double( + BitbucketServer::Representation::PullRequestComment, + author_email: reply_author.email, + author_username: reply_author.username, + note: 'I agree', + created_at: now, + updated_at: now) + end # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad - inline_note = instance_double( - BitbucketServer::Representation::PullRequestComment, - file_type: 'ADDED', - from_sha: sample.commits.first, - to_sha: sample.commits.last, - file_path: '.gitmodules', - old_pos: nil, - new_pos: 4, - note: 'Hello world', - author_email: 'unknown@gmail.com', - author_username: 'Superman', - comments: [reply], - created_at: now, - updated_at: now, - parent_comment: nil) + let(:inline_note) do + instance_double( + BitbucketServer::Representation::PullRequestComment, + file_type: 'ADDED', + from_sha: sample.commits.first, + to_sha: sample.commits.last, + file_path: '.gitmodules', + old_pos: nil, + new_pos: 4, + note: 'Hello world', + author_email: inline_note_author.email, + author_username: inline_note_author.username, + comments: [reply], + created_at: now, + updated_at: now, + parent_comment: nil) + end - allow(reply).to receive(:parent_comment).and_return(inline_note) + let(:inline_comment) do + instance_double( + BitbucketServer::Representation::Activity, + comment?: true, + inline_comment?: true, + merge_event?: false, + comment: inline_note) + end - inline_comment = instance_double( - BitbucketServer::Representation::Activity, - comment?: true, - inline_comment?: true, - merge_event?: false, - comment: inline_note) + before do + allow(reply).to receive(:parent_comment).and_return(inline_note) + allow(subject.client).to receive(:activities).and_return([inline_comment]) + end - expect(subject.client).to receive(:activities).and_return([inline_comment]) + shared_examples 'imports threaded discussions' do + 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.type).to eq('DiffNote') + expect(start_note.note).to end_with(inline_note.note) + expect(start_note.created_at).to eq(inline_note.created_at) + expect(start_note.updated_at).to eq(inline_note.updated_at) + expect(start_note.position.base_sha).to eq(inline_note.from_sha) + expect(start_note.position.start_sha).to eq(inline_note.from_sha) + expect(start_note.position.head_sha).to eq(inline_note.to_sha) + expect(start_note.position.old_line).to be_nil + expect(start_note.position.new_line).to eq(inline_note.new_pos) + expect(start_note.author).to eq(inline_note_author) + + reply_note = notes.last + # Make sure author and reply context is included + expect(reply_note.note).to start_with("> #{inline_note.note}\n\n#{reply.note}") + expect(reply_note.author).to eq(reply_author) + expect(reply_note.created_at).to eq(reply.created_at) + expect(reply_note.updated_at).to eq(reply.created_at) + expect(reply_note.position.base_sha).to eq(inline_note.from_sha) + expect(reply_note.position.start_sha).to eq(inline_note.from_sha) + expect(reply_note.position.head_sha).to eq(inline_note.to_sha) + expect(reply_note.position.old_line).to be_nil + expect(reply_note.position.new_line).to eq(inline_note.new_pos) + end + end - expect { subject.execute }.to change { MergeRequest.count }.by(1) + context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: false) + end - 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.type).to eq('DiffNote') - expect(start_note.note).to end_with(inline_note.note) - expect(start_note.created_at).to eq(inline_note.created_at) - expect(start_note.updated_at).to eq(inline_note.updated_at) - expect(start_note.position.base_sha).to eq(inline_note.from_sha) - expect(start_note.position.start_sha).to eq(inline_note.from_sha) - expect(start_note.position.head_sha).to eq(inline_note.to_sha) - expect(start_note.position.old_line).to be_nil - expect(start_note.position.new_line).to eq(inline_note.new_pos) - - reply_note = notes.last - # Make sure author and reply context is included - expect(reply_note.note).to start_with("*By #{reply.author_username} (#{reply.author_email})*\n\n") - expect(reply_note.note).to end_with("> #{inline_note.note}\n\n#{reply.note}") - expect(reply_note.author).to eq(project.owner) - expect(reply_note.created_at).to eq(reply.created_at) - expect(reply_note.updated_at).to eq(reply.created_at) - expect(reply_note.position.base_sha).to eq(inline_note.from_sha) - expect(reply_note.position.start_sha).to eq(inline_note.from_sha) - expect(reply_note.position.head_sha).to eq(inline_note.to_sha) - expect(reply_note.position.old_line).to be_nil - expect(reply_note.position.new_line).to eq(inline_note.new_pos) + include_examples 'imports threaded discussions' + end + + context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: true) + end + + include_examples 'imports threaded discussions' do + context 'when username is not present' do + before do + allow(reply).to receive(:author_username).and_return(nil) + allow(inline_note).to receive(:author_username).and_return(nil) + end + + it 'maps by email' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + notes = MergeRequest.first.notes.order(:id).to_a + + expect(notes.first.author).to eq(inline_note_author) + expect(notes.last.author).to eq(reply_author) + end + end + end + end + + context 'when user is not found' do + before do + allow(reply).to receive(:author_username).and_return(nil) + allow(reply).to receive(:author_email).and_return(nil) + allow(inline_note).to receive(:author_username).and_return(nil) + allow(inline_note).to receive(:author_email).and_return(nil) + end + + it 'maps importer user' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + notes = MergeRequest.first.notes.order(:id).to_a + + expect(notes.first.author).to eq(project_creator) + expect(notes.last.author).to eq(project_creator) + end + end end it 'falls back to comments if diff comments fail to validate' do @@ -312,6 +483,7 @@ state: 'merged', author: 'Test Author', author_email: project.owner.email, + author_username: 'author', created_at: Time.now, updated_at: Time.now, merged?: true)