Commit 376dafb0 authored by Stan Hu's avatar Stan Hu

Merge branch '218609-bitbucket-server-importer-by-slug' into 'master'

Add user mapping by username to Bitbucket Server importer

Closes #218609

See merge request gitlab-org/gitlab!36885
parents d93fc08b efb28aa4
---
title: Add user mapping by username when importing projects for Bitbucket Server importer
merge_request: 36885
author:
type: added
---
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
...@@ -62,6 +62,25 @@ The importer will create any new namespaces (groups) if they don't exist or in ...@@ -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 the case the namespace is taken, the repository will be imported under the user's
namespace that started the import process. 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 ## Importing your Bitbucket repositories
1. Sign in to GitLab and go to your dashboard. 1. Sign in to GitLab and go to your dashboard.
......
...@@ -38,7 +38,9 @@ module BitbucketServer ...@@ -38,7 +38,9 @@ module BitbucketServer
end end
def author_username def author_username
author['displayName'] author['username'] ||
author['slug'] ||
author['displayName']
end end
def author_email def author_email
......
...@@ -11,6 +11,12 @@ module BitbucketServer ...@@ -11,6 +11,12 @@ module BitbucketServer
raw.dig('author', 'user', 'emailAddress') raw.dig('author', 'user', 'emailAddress')
end end
def author_username
raw.dig('author', 'user', 'username') ||
raw.dig('author', 'user', 'slug') ||
raw.dig('author', 'user', 'displayName')
end
def description def description
raw['description'] raw['description']
end end
......
...@@ -61,17 +61,18 @@ module Gitlab ...@@ -61,17 +61,18 @@ module Gitlab
}.to_json) }.to_json)
end end
def gitlab_user_id(email) def find_user_id(by:, value:)
find_user_id(email) || project.creator_id return unless value
end
def find_user_id(email) return users[value] if users.key?(value)
return unless email
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[value] = user&.id
users[email] = user&.id
user&.id user&.id
end end
...@@ -197,9 +198,8 @@ module Gitlab ...@@ -197,9 +198,8 @@ module Gitlab
log_info(stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid) log_info(stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid)
description = '' 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 description += pull_request.description if pull_request.description
author_id = gitlab_user_id(pull_request.author_email)
attributes = { attributes = {
iid: pull_request.iid, iid: pull_request.iid,
...@@ -212,7 +212,7 @@ module Gitlab ...@@ -212,7 +212,7 @@ module Gitlab
target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name), target_branch: Gitlab::Git.ref_name(pull_request.target_branch_name),
target_branch_sha: pull_request.target_branch_sha, target_branch_sha: pull_request.target_branch_sha,
state_id: MergeRequest.available_states[pull_request.state], state_id: MergeRequest.available_states[pull_request.state],
author_id: author_id, author_id: author_id(pull_request),
created_at: pull_request.created_at, created_at: pull_request.created_at,
updated_at: pull_request.updated_at updated_at: pull_request.updated_at
} }
...@@ -254,7 +254,7 @@ module Gitlab ...@@ -254,7 +254,7 @@ module Gitlab
committer = merge_event.committer_email 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 timestamp = merge_event.merge_timestamp
merge_request.update({ merge_commit_sha: merge_event.merge_commit }) merge_request.update({ merge_commit_sha: merge_event.merge_commit })
metric = MergeRequest::Metrics.find_or_initialize_by(merge_request: merge_request) metric = MergeRequest::Metrics.find_or_initialize_by(merge_request: merge_request)
...@@ -353,7 +353,7 @@ module Gitlab ...@@ -353,7 +353,7 @@ module Gitlab
end end
def pull_request_comment_attributes(comment) def pull_request_comment_attributes(comment)
author = find_user_id(comment.author_email) author = uid(comment)
note = '' note = ''
unless author unless author
...@@ -397,6 +397,23 @@ module Gitlab ...@@ -397,6 +397,23 @@ module Gitlab
def metrics def metrics
@metrics ||= Gitlab::Import::Metrics.new(:bitbucket_server_importer, @project) @metrics ||= Gitlab::Import::Metrics.new(:bitbucket_server_importer, @project)
end 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 end
end end
...@@ -20,7 +20,8 @@ ...@@ -20,7 +20,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [ "comments": [
...@@ -38,7 +39,8 @@ ...@@ -38,7 +39,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [ "comments": [
...@@ -56,7 +58,8 @@ ...@@ -56,7 +58,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -85,7 +88,8 @@ ...@@ -85,7 +88,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"createdDate": 1530164016725, "createdDate": 1530164016725,
...@@ -115,7 +119,8 @@ ...@@ -115,7 +119,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"createdDate": 1530164026000, "createdDate": 1530164026000,
...@@ -147,7 +152,8 @@ ...@@ -147,7 +152,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -194,7 +200,8 @@ ...@@ -194,7 +200,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -363,7 +370,8 @@ ...@@ -363,7 +370,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -383,7 +391,8 @@ ...@@ -383,7 +391,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -543,7 +552,8 @@ ...@@ -543,7 +552,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -563,7 +573,8 @@ ...@@ -563,7 +573,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [ "comments": [
...@@ -581,7 +592,8 @@ ...@@ -581,7 +592,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [ "comments": [
...@@ -599,7 +611,8 @@ ...@@ -599,7 +611,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -789,7 +802,8 @@ ...@@ -789,7 +802,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -809,7 +823,8 @@ ...@@ -809,7 +823,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -843,7 +858,8 @@ ...@@ -843,7 +858,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -863,7 +879,8 @@ ...@@ -863,7 +879,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"authorTimestamp": 1529727872000, "authorTimestamp": 1529727872000,
...@@ -880,7 +897,8 @@ ...@@ -880,7 +897,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"committerTimestamp": 1529727872000, "committerTimestamp": 1529727872000,
...@@ -951,7 +969,8 @@ ...@@ -951,7 +969,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -971,7 +990,8 @@ ...@@ -971,7 +990,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [ "comments": [
...@@ -989,7 +1009,8 @@ ...@@ -989,7 +1009,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -1038,7 +1059,8 @@ ...@@ -1038,7 +1059,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -1058,7 +1080,8 @@ ...@@ -1058,7 +1080,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
}, },
"comments": [], "comments": [],
...@@ -1092,7 +1115,8 @@ ...@@ -1092,7 +1115,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
}, },
...@@ -1113,7 +1137,8 @@ ...@@ -1113,7 +1137,8 @@
] ]
}, },
"name": "root", "name": "root",
"slug": "root", "slug": "slug",
"username": "username",
"type": "NORMAL" "type": "NORMAL"
} }
} }
......
...@@ -5,8 +5,9 @@ ...@@ -5,8 +5,9 @@
"status":"UNAPPROVED", "status":"UNAPPROVED",
"user":{ "user":{
"active":true, "active":true,
"displayName":"root", "displayName":"displayName",
"emailAddress":"joe.montana@49ers.com", "emailAddress":"joe.montana@49ers.com",
"username": "username",
"id":1, "id":1,
"links":{ "links":{
"self":[ "self":[
...@@ -16,7 +17,7 @@ ...@@ -16,7 +17,7 @@
] ]
}, },
"name":"root", "name":"root",
"slug":"root", "slug":"slug",
"type":"NORMAL" "type":"NORMAL"
} }
}, },
......
...@@ -13,7 +13,30 @@ RSpec.describe BitbucketServer::Representation::Comment do ...@@ -13,7 +13,30 @@ RSpec.describe BitbucketServer::Representation::Comment do
end end
describe '#author_username' do 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 end
describe '#author_email' do describe '#author_email' do
......
...@@ -15,6 +15,33 @@ RSpec.describe BitbucketServer::Representation::PullRequest do ...@@ -15,6 +15,33 @@ RSpec.describe BitbucketServer::Representation::PullRequest do
it { expect(subject.author_email).to eq('joe.montana@49ers.com') } it { expect(subject.author_email).to eq('joe.montana@49ers.com') }
end 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 describe '#description' do
it { expect(subject.description).to eq('Test') } it { expect(subject.description).to eq('Test') }
end end
......
...@@ -6,9 +6,10 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -6,9 +6,10 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
include ImportSpecHelper include ImportSpecHelper
let(:import_url) { 'http://my-bitbucket' } 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(: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(:now) { Time.now.utc.change(usec: 0) }
let(:project_key) { 'TEST' } let(:project_key) { 'TEST' }
let(:repo_slug) { 'rouge' } let(:repo_slug) { 'rouge' }
...@@ -19,7 +20,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -19,7 +20,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
before do before do
data = project.create_or_update_import_data( data = project.create_or_update_import_data(
data: { project_key: project_key, repo_slug: repo_slug }, 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 data.save
project.save project.save
...@@ -51,12 +52,11 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -51,12 +52,11 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
end end
describe '#import_pull_requests' do describe '#import_pull_requests' do
before do let(:pull_request_author) { create(:user, username: 'pull_request_author', email: 'pull_request_author@example.org') }
allow(subject).to receive(:import_repository) let(:note_author) { create(:user, username: 'note_author', email: 'note_author@example.org') }
allow(subject).to receive(:delete_temp_branches)
allow(subject).to receive(:restore_branches)
pull_request = instance_double( let(:pull_request) do
instance_double(
BitbucketServer::Representation::PullRequest, BitbucketServer::Representation::PullRequest,
iid: 10, iid: 10,
source_branch_sha: sample.commits.last, source_branch_sha: sample.commits.last,
...@@ -67,65 +67,172 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -67,65 +67,172 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
description: 'This is a test pull request', description: 'This is a test pull request',
state: 'merged', state: 'merged',
author: 'Test Author', author: 'Test Author',
author_email: project.owner.email, author_email: pull_request_author.email,
author_username: pull_request_author.username,
created_at: Time.now, created_at: Time.now,
updated_at: Time.now, updated_at: Time.now,
raw: {}, raw: {},
merged?: true) merged?: true)
end
allow(subject.client).to receive(:pull_requests).and_return([pull_request]) let(:merge_event) do
instance_double(
@merge_event = instance_double(
BitbucketServer::Representation::Activity, BitbucketServer::Representation::Activity,
comment?: false, comment?: false,
merge_event?: true, merge_event?: true,
committer_email: project.owner.email, committer_email: pull_request_author.email,
merge_timestamp: now, merge_timestamp: now,
merge_commit: '12345678' merge_commit: '12345678'
) )
end
@pr_note = instance_double( let(:pr_note) do
instance_double(
BitbucketServer::Representation::Comment, BitbucketServer::Representation::Comment,
note: 'Hello world', note: 'Hello world',
author_email: 'unknown@gmail.com', author_email: note_author.email,
author_username: 'The Flash', author_username: note_author.username,
comments: [], comments: [],
created_at: now, created_at: now,
updated_at: now, updated_at: now,
parent_comment: nil) parent_comment: nil)
end
@pr_comment = instance_double( let(:pr_comment) do
instance_double(
BitbucketServer::Representation::Activity, BitbucketServer::Representation::Activity,
comment?: true, comment?: true,
inline_comment?: false, inline_comment?: false,
merge_event?: 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 end
it 'imports merge event' do 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) expect { subject.execute }.to change { MergeRequest.count }.by(1)
merge_request = MergeRequest.first merge_request = MergeRequest.first
expect(merge_request.metrics.merged_by).to eq(project.owner) 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.metrics.merged_at).to eq(merge_event.merge_timestamp)
expect(merge_request.merge_commit_sha).to eq('12345678') expect(merge_request.merge_commit_sha).to eq('12345678')
expect(merge_request.state_id).to eq(3) expect(merge_request.state_id).to eq(3)
end end
it 'imports comments' do describe 'pull request author user mapping' do
expect(subject.client).to receive(:activities).and_return([@pr_comment]) 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 merge_request = MergeRequest.first
expect(merge_request.notes.count).to eq(1) expect(merge_request.author).to eq(pull_request_author)
note = merge_request.notes.first end
expect(note.note).to end_with(@pr_note.note) end
expect(note.author).to eq(project.owner)
expect(note.created_at).to eq(@pr_note.created_at) context 'when bitbucket_server_user_mapping_by_username feature flag is disabled' do
expect(note.updated_at).to eq(@pr_note.created_at) 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 end
context 'metrics' do context 'metrics' do
...@@ -135,7 +242,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -135,7 +242,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
before do before do
allow(Gitlab::Metrics).to receive(:counter) { counter } allow(Gitlab::Metrics).to receive(:counter) { counter }
allow(Gitlab::Metrics).to receive(:histogram) { histogram } 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 end
it 'counts and measures duration of imported projects' do it 'counts and measures duration of imported projects' do
...@@ -170,73 +277,137 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -170,73 +277,137 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
end end
end end
it 'imports threaded discussions' do describe 'threaded discussions' do
reply = instance_double( let(:reply_author) { create(:user, username: 'reply_author', email: 'reply_author@example.org') }
BitbucketServer::Representation::PullRequestComment, let(:inline_note_author) { create(:user, username: 'inline_note_author', email: 'inline_note_author@example.org') }
author_email: 'someuser@gitlab.com',
author_username: 'Batman', let(:reply) do
note: 'I agree', instance_double(
created_at: now, BitbucketServer::Representation::PullRequestComment,
updated_at: now) 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 # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad
inline_note = instance_double( let(:inline_note) do
BitbucketServer::Representation::PullRequestComment, instance_double(
file_type: 'ADDED', BitbucketServer::Representation::PullRequestComment,
from_sha: sample.commits.first, file_type: 'ADDED',
to_sha: sample.commits.last, from_sha: sample.commits.first,
file_path: '.gitmodules', to_sha: sample.commits.last,
old_pos: nil, file_path: '.gitmodules',
new_pos: 4, old_pos: nil,
note: 'Hello world', new_pos: 4,
author_email: 'unknown@gmail.com', note: 'Hello world',
author_username: 'Superman', author_email: inline_note_author.email,
comments: [reply], author_username: inline_note_author.username,
created_at: now, comments: [reply],
updated_at: now, created_at: now,
parent_comment: nil) 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( before do
BitbucketServer::Representation::Activity, allow(reply).to receive(:parent_comment).and_return(inline_note)
comment?: true, allow(subject.client).to receive(:activities).and_return([inline_comment])
inline_comment?: true, end
merge_event?: false,
comment: inline_note)
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 include_examples 'imports threaded discussions'
expect(merge_request.notes.count).to eq(2) end
expect(merge_request.notes.map(&:discussion_id).uniq.count).to eq(1)
context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do
notes = merge_request.notes.order(:id).to_a before do
start_note = notes.first stub_feature_flags(bitbucket_server_user_mapping_by_username: true)
expect(start_note.type).to eq('DiffNote') end
expect(start_note.note).to end_with(inline_note.note)
expect(start_note.created_at).to eq(inline_note.created_at) include_examples 'imports threaded discussions' do
expect(start_note.updated_at).to eq(inline_note.updated_at) context 'when username is not present' do
expect(start_note.position.base_sha).to eq(inline_note.from_sha) before do
expect(start_note.position.start_sha).to eq(inline_note.from_sha) allow(reply).to receive(:author_username).and_return(nil)
expect(start_note.position.head_sha).to eq(inline_note.to_sha) allow(inline_note).to receive(:author_username).and_return(nil)
expect(start_note.position.old_line).to be_nil end
expect(start_note.position.new_line).to eq(inline_note.new_pos)
it 'maps by email' do
reply_note = notes.last expect { subject.execute }.to change { MergeRequest.count }.by(1)
# Make sure author and reply context is included
expect(reply_note.note).to start_with("*By #{reply.author_username} (#{reply.author_email})*\n\n") notes = MergeRequest.first.notes.order(:id).to_a
expect(reply_note.note).to end_with("> #{inline_note.note}\n\n#{reply.note}")
expect(reply_note.author).to eq(project.owner) expect(notes.first.author).to eq(inline_note_author)
expect(reply_note.created_at).to eq(reply.created_at) expect(notes.last.author).to eq(reply_author)
expect(reply_note.updated_at).to eq(reply.created_at) end
expect(reply_note.position.base_sha).to eq(inline_note.from_sha) end
expect(reply_note.position.start_sha).to eq(inline_note.from_sha) end
expect(reply_note.position.head_sha).to eq(inline_note.to_sha) end
expect(reply_note.position.old_line).to be_nil
expect(reply_note.position.new_line).to eq(inline_note.new_pos) 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 end
it 'falls back to comments if diff comments fail to validate' do it 'falls back to comments if diff comments fail to validate' do
...@@ -312,6 +483,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do ...@@ -312,6 +483,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do
state: 'merged', state: 'merged',
author: 'Test Author', author: 'Test Author',
author_email: project.owner.email, author_email: project.owner.email,
author_username: 'author',
created_at: Time.now, created_at: Time.now,
updated_at: Time.now, updated_at: Time.now,
merged?: true) merged?: true)
......
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