From efb28aa46fe012ef400433924fdd69eb88bfc597 Mon Sep 17 00:00:00 2001 From: vten Date: Tue, 14 Jul 2020 09:35:00 -0600 Subject: [PATCH] Add user mapping by username to Bitbucket Server importer - Add optional user mapping by username to Bitbucket Server importer behind bitbucket_server_user_mapping_by_username feature flag --- ...8609-bitbucket-server-importer-by-slug.yml | 5 + ...bucket_server_user_mapping_by_username.yml | 7 + doc/user/project/import/bitbucket_server.md | 19 + .../representation/comment.rb | 4 +- .../representation/pull_request.rb | 6 + .../bitbucket_server_import/importer.rb | 43 ++- .../bitbucket_server/activities.json | 75 ++-- .../bitbucket_server/pull_request.json | 5 +- .../representation/comment_spec.rb | 25 +- .../representation/pull_request_spec.rb | 27 ++ .../bitbucket_server_import/importer_spec.rb | 356 +++++++++++++----- 11 files changed, 438 insertions(+), 134 deletions(-) create mode 100644 changelogs/unreleased/218609-bitbucket-server-importer-by-slug.yml create mode 100644 config/feature_flags/development/bitbucket_server_user_mapping_by_username.yml 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 00000000000000..8a633dcacf625a --- /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 00000000000000..3875cce4d281bc --- /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 90d33b72ca0fb7..8ecc0e1f9c4b0e 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 99b97a3b181b7a..7e55c446e76a22 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 c3e927d8de7b2f..2f377bdced2af9 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 18a1b64729e2b6..aca5a63a4240c9 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 09adfca9f31858..ddcb94b8f58906 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 6c7fcf3b04c80b..77f7eb330f5354 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 b568789bd97083..41cdaab7a001c6 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 a05d98f0d4aff2..d7b893e8081f6a 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 5eb27c51f9ee08..80ec5ec1fc7657 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) -- GitLab