diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb
index 349ca26ec03c931224659133acd3034b5d2d0ddc..3a087a3ef83062c44773a21a872bd2275a5bdc58 100644
--- a/lib/gitlab/bitbucket_import/importer.rb
+++ b/lib/gitlab/bitbucket_import/importer.rb
@@ -182,7 +182,6 @@ def import_pull_requests
target_branch_sha: target_branch_sha,
state: pull_request.state,
author_id: gitlab_user_id(project, pull_request.author),
- assignee_id: nil,
created_at: pull_request.created_at,
updated_at: pull_request.updated_at
)
diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb
index b7b2fe115c1fb0a9d5d7cd54d2c626fcbf719427..886fbaaff486e52dcfb6156366ea66db3013f5de 100644
--- a/lib/gitlab/bitbucket_server_import/importer.rb
+++ b/lib/gitlab/bitbucket_server_import/importer.rb
@@ -211,7 +211,6 @@ def import_bitbucket_pull_request(pull_request)
target_branch_sha: pull_request.target_branch_sha,
state_id: MergeRequest.available_states[pull_request.state],
author_id: author_id,
- assignee_id: nil,
created_at: pull_request.created_at,
updated_at: pull_request.updated_at
}
diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb
index 6d2aff63a4753b0be22cdd6ab7225c7554f88c9b..f09e0bd980633dde44fa8f88b3eed5267fddd20c 100644
--- a/lib/gitlab/github_import/importer/pull_request_importer.rb
+++ b/lib/gitlab/github_import/importer/pull_request_importer.rb
@@ -27,6 +27,7 @@ def execute
mr, already_exists = create_merge_request
if mr
+ set_merge_request_assignees(mr)
insert_git_data(mr, already_exists)
issuable_finder.cache_database_id(mr.id)
end
@@ -57,7 +58,6 @@ def create_merge_request
state_id: ::MergeRequest.available_states[pull_request.state],
milestone_id: milestone_finder.id_for(pull_request),
author_id: author_id,
- assignee_id: user_finder.assignee_id_for(pull_request),
created_at: pull_request.created_at,
updated_at: pull_request.updated_at
}
@@ -65,6 +65,10 @@ def create_merge_request
create_merge_request_without_hooks(project, attributes, pull_request.iid)
end
+ def set_merge_request_assignees(merge_request)
+ merge_request.assignee_ids = [user_finder.assignee_id_for(pull_request)]
+ end
+
def insert_git_data(merge_request, already_exists)
insert_or_replace_git_data(merge_request, pull_request.source_branch_sha, pull_request.target_branch_sha, already_exists)
# We need to create the branch after the merge request is
diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
index 877b4d4bbafbaffcebf7d9dde986a92e141c635f..bffae9e2ba0e0ef43aca2de4b370910ebdc202b5 100644
--- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
@@ -49,6 +49,10 @@
.to receive(:create_merge_request)
.and_return([mr, false])
+ expect(importer)
+ .to receive(:set_merge_request_assignees)
+ .with(mr)
+
expect(importer)
.to receive(:insert_git_data)
.with(mr, false)
@@ -75,11 +79,6 @@
.to receive(:author_id_for)
.with(pull_request)
.and_return([user.id, true])
-
- allow(importer.user_finder)
- .to receive(:assignee_id_for)
- .with(pull_request)
- .and_return(user.id)
end
it 'imports the pull request with the pull request author as the merge request author' do
@@ -97,7 +96,6 @@
state_id: 3,
milestone_id: milestone.id,
author_id: user.id,
- assignee_id: user.id,
created_at: created_at,
updated_at: updated_at
},
@@ -114,20 +112,72 @@
expect(mr).to be_instance_of(MergeRequest)
expect(exists).to eq(false)
end
+
+ context 'when the source and target branch are identical' do
+ before do
+ allow(pull_request).to receive_messages(
+ source_repository_id: pull_request.target_repository_id,
+ source_branch: 'master'
+ )
+ end
+
+ it 'uses a generated source branch name for the merge request' do
+ expect(importer)
+ .to receive(:insert_and_return_id)
+ .with(
+ {
+ iid: 42,
+ title: 'My Pull Request',
+ description: 'This is my pull request',
+ source_project_id: project.id,
+ target_project_id: project.id,
+ source_branch: 'master-42',
+ target_branch: 'master',
+ state_id: 3,
+ milestone_id: milestone.id,
+ author_id: user.id,
+ created_at: created_at,
+ updated_at: updated_at
+ },
+ project.merge_requests
+ )
+ .and_call_original
+
+ importer.create_merge_request
+ end
+ end
+
+ context 'when the import fails due to a foreign key error' do
+ it 'does not raise any errors' do
+ expect(importer)
+ .to receive(:insert_and_return_id)
+ .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key')
+
+ expect { importer.create_merge_request }.not_to raise_error
+ end
+ end
+
+ context 'when the merge request already exists' do
+ it 'returns the existing merge request' do
+ mr1, exists1 = importer.create_merge_request
+ mr2, exists2 = importer.create_merge_request
+
+ expect(mr2).to eq(mr1)
+ expect(exists1).to eq(false)
+ expect(exists2).to eq(true)
+ end
+ end
end
context 'when the author could not be found' do
- it 'imports the pull request with the project creator as the merge request author' do
+ before do
allow(importer.user_finder)
.to receive(:author_id_for)
.with(pull_request)
.and_return([project.creator_id, false])
+ end
- allow(importer.user_finder)
- .to receive(:assignee_id_for)
- .with(pull_request)
- .and_return(user.id)
-
+ it 'imports the pull request with the project creator as the merge request author' do
expect(importer)
.to receive(:insert_and_return_id)
.with(
@@ -142,7 +192,6 @@
state_id: 3,
milestone_id: milestone.id,
author_id: project.creator_id,
- assignee_id: user.id,
created_at: created_at,
updated_at: updated_at
},
@@ -153,93 +202,33 @@
importer.create_merge_request
end
end
+ end
- context 'when the source and target branch are identical' do
- it 'uses a generated source branch name for the merge request' do
- allow(importer.user_finder)
- .to receive(:author_id_for)
- .with(pull_request)
- .and_return([user.id, true])
-
- allow(importer.user_finder)
- .to receive(:assignee_id_for)
- .with(pull_request)
- .and_return(user.id)
-
- allow(pull_request)
- .to receive(:source_repository_id)
- .and_return(pull_request.target_repository_id)
-
- allow(pull_request)
- .to receive(:source_branch)
- .and_return('master')
+ describe '#set_merge_request_assignees' do
+ let_it_be(:merge_request) { create(:merge_request) }
- expect(importer)
- .to receive(:insert_and_return_id)
- .with(
- {
- iid: 42,
- title: 'My Pull Request',
- description: 'This is my pull request',
- source_project_id: project.id,
- target_project_id: project.id,
- source_branch: 'master-42',
- target_branch: 'master',
- state_id: 3,
- milestone_id: milestone.id,
- author_id: user.id,
- assignee_id: user.id,
- created_at: created_at,
- updated_at: updated_at
- },
- project.merge_requests
- )
- .and_call_original
+ before do
+ allow(importer.user_finder)
+ .to receive(:assignee_id_for)
+ .with(pull_request)
+ .and_return(user_id)
- importer.create_merge_request
- end
+ importer.set_merge_request_assignees(merge_request)
end
- context 'when the import fails due to a foreign key error' do
- it 'does not raise any errors' do
- allow(importer.user_finder)
- .to receive(:author_id_for)
- .with(pull_request)
- .and_return([user.id, true])
-
- allow(importer.user_finder)
- .to receive(:assignee_id_for)
- .with(pull_request)
- .and_return(user.id)
-
- expect(importer)
- .to receive(:insert_and_return_id)
- .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key')
+ context 'when pull request has an assignee' do
+ let(:user_id) { user.id }
- expect { importer.create_merge_request }.not_to raise_error
+ it 'sets merge request assignees' do
+ expect(merge_request.assignee_ids).to eq [user.id]
end
end
- context 'when the merge request already exists' do
- before do
- allow(importer.user_finder)
- .to receive(:author_id_for)
- .with(pull_request)
- .and_return([user.id, true])
-
- allow(importer.user_finder)
- .to receive(:assignee_id_for)
- .with(pull_request)
- .and_return(user.id)
- end
-
- it 'returns the existing merge request' do
- mr1, exists1 = importer.create_merge_request
- mr2, exists2 = importer.create_merge_request
+ context 'when pull request does not have any assignees' do
+ let(:user_id) { nil }
- expect(mr2).to eq(mr1)
- expect(exists1).to eq(false)
- expect(exists2).to eq(true)
+ it 'does not set merge request assignees' do
+ expect(merge_request.assignee_ids).to eq []
end
end
end
@@ -255,11 +244,6 @@
.to receive(:author_id_for)
.with(pull_request)
.and_return([user.id, true])
-
- allow(importer.user_finder)
- .to receive(:assignee_id_for)
- .with(pull_request)
- .and_return(user.id)
end
it 'does not create the source branch if merge request is merged' do
diff --git a/spec/lib/gitlab/import/merge_request_helpers_spec.rb b/spec/lib/gitlab/import/merge_request_helpers_spec.rb
index 42515888d4ffc6a977519e221f8e0bd06ca5e6d5..2b16599415279c18239f4d652bba59a156ee3dd8 100644
--- a/spec/lib/gitlab/import/merge_request_helpers_spec.rb
+++ b/spec/lib/gitlab/import/merge_request_helpers_spec.rb
@@ -19,8 +19,7 @@
source_branch: 'master-42',
target_branch: 'master',
state_id: 3,
- author_id: user.id,
- assignee_id: user.id
+ author_id: user.id
}
end