diff --git a/lib/bitbucket_server/representation/pull_request.rb b/lib/bitbucket_server/representation/pull_request.rb index acbc44f61198ed444fbf5d93c140a2b8a9e2786d..66dba5fefc7940f229cc75697a52b7a1e118f6c6 100644 --- a/lib/bitbucket_server/representation/pull_request.rb +++ b/lib/bitbucket_server/representation/pull_request.rb @@ -21,6 +21,10 @@ def description raw['description'] end + def reviewers + raw['reviewers'] + end + def iid raw['id'] end @@ -75,6 +79,7 @@ def to_hash author_email: author_email, author_username: author_username, description: description, + reviewers: reviewers, created_at: created_at, updated_at: updated_at, state: state, diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index f3253027d5750d4af6ff4d13ab95304338e8ba30..1871dd3a89d5a9d3c4449571d959d88e6f0451b3 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -253,6 +253,7 @@ def import_bitbucket_pull_request(pull_request) iid: pull_request.iid, title: pull_request.title, description: description, + reviewer_ids: reviewers(pull_request.reviewers), source_project_id: project.id, source_branch: Gitlab::Git.ref_name(pull_request.source_branch_name), source_branch_sha: pull_request.source_branch_sha, @@ -450,6 +451,18 @@ def uid(rep_object) find_user_id(by: :email, value: rep_object.author_email) end end + + def reviewers(reviewers) + return [] unless reviewers.present? + + reviewers.filter_map do |reviewer| + if Feature.enabled?(:bitbucket_server_user_mapping_by_username, type: :ops) + find_user_id(by: :username, value: reviewer.dig('user', 'slug')) + else + find_user_id(by: :email, value: reviewer.dig('user', 'emailAddress')) + end + end + end end end end diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb index 5d306f989804fa7d55c8a23494302f70ccc1cd6a..34963452192d7956d7795deab37287f94a6925c2 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_importer.rb @@ -27,6 +27,7 @@ def execute iid: object[:iid], title: object[:title], description: description, + reviewer_ids: reviewers, source_project_id: project.id, source_branch: Gitlab::Git.ref_name(object[:source_branch_name]), source_branch_sha: object[:source_branch_sha], @@ -55,6 +56,18 @@ def author_line formatter.author_line(object[:author]) end + + def reviewers + return [] unless object[:reviewers].present? + + object[:reviewers].filter_map do |reviewer| + if Feature.enabled?(:bitbucket_server_user_mapping_by_username, type: :ops) + user_finder.find_user_id(by: :username, value: reviewer.dig('user', 'slug')) + else + user_finder.find_user_id(by: :email, value: reviewer.dig('user', 'emailAddress')) + end + end + end end end end diff --git a/lib/gitlab/import/merge_request_creator.rb b/lib/gitlab/import/merge_request_creator.rb index 8291372bba94430407dfb99da1ac008f4d5b6ea9..c6aceaf272cb2882a87d521dabe7d0344eaf9a98 100644 --- a/lib/gitlab/import/merge_request_creator.rb +++ b/lib/gitlab/import/merge_request_creator.rb @@ -25,12 +25,14 @@ def initialize(project) def execute(attributes) source_branch_sha = attributes.delete(:source_branch_sha) target_branch_sha = attributes.delete(:target_branch_sha) + reviewer_ids = attributes.delete(:reviewer_ids) iid = attributes[:iid] merge_request, already_exists = create_merge_request_without_hooks(project, attributes, iid) if merge_request insert_or_replace_git_data(merge_request, source_branch_sha, target_branch_sha, already_exists) + insert_merge_request_reviewers(merge_request, reviewer_ids) end merge_request diff --git a/lib/gitlab/import/merge_request_helpers.rb b/lib/gitlab/import/merge_request_helpers.rb index c5694d95aa1d679b9ad5130875ab72d37f360ae7..9fd393c61a080d65aecfc2770516fa32acf7f0cc 100644 --- a/lib/gitlab/import/merge_request_helpers.rb +++ b/lib/gitlab/import/merge_request_helpers.rb @@ -62,6 +62,13 @@ def insert_or_replace_git_data(merge_request, source_branch_sha, target_branch_s diff.save_git_content diff.set_as_latest_diff end + + def insert_merge_request_reviewers(merge_request, reviewers) + return unless reviewers.present? + + rows = reviewers.map { |reviewer_id| { merge_request_id: merge_request.id, user_id: reviewer_id } } + MergeRequestReviewer.insert_all(rows) + end end end end diff --git a/spec/fixtures/importers/bitbucket_server/pull_request.json b/spec/fixtures/importers/bitbucket_server/pull_request.json index 77f7eb330f5354a12b35c43ef7c435a2d25d62dd..262ea603dfea9aad9e48e58db0c6d5ae596203a6 100644 --- a/spec/fixtures/importers/bitbucket_server/pull_request.json +++ b/spec/fixtures/importers/bitbucket_server/pull_request.json @@ -1,147 +1,189 @@ { - "author":{ - "approved":false, - "role":"AUTHOR", - "status":"UNAPPROVED", - "user":{ - "active":true, - "displayName":"displayName", - "emailAddress":"joe.montana@49ers.com", - "username": "username", - "id":1, - "links":{ - "self":[ - { - "href":"http://localhost:7990/users/root" - } - ] - }, - "name":"root", - "slug":"slug", - "type":"NORMAL" + "author": { + "approved": false, + "role": "AUTHOR", + "status": "UNAPPROVED", + "user": { + "active": true, + "displayName": "displayName", + "emailAddress": "joe.montana@49ers.com", + "username": "username", + "id": 1, + "links": { + "self": [ + { + "href": "http://localhost:7990/users/root" + } + ] + }, + "name": "root", + "slug": "slug", + "type": "NORMAL" + } + }, + "closed": true, + "closedDate": 1530600648850, + "createdDate": 1530600635690, + "description": "Test", + "fromRef": { + "displayId": "root/CODE_OF_CONDUCTmd-1530600625006", + "id": "refs/heads/root/CODE_OF_CONDUCTmd-1530600625006", + "latestCommit": "074e2b4dddc5b99df1bf9d4a3f66cfc15481fdc8", + "repository": { + "forkable": true, + "id": 1, + "links": { + "clone": [ + { + "href": "http://root@localhost:7990/scm/test/rouge.git", + "name": "http" + }, + { + "href": "ssh://git@localhost:7999/test/rouge.git", + "name": "ssh" + } + ], + "self": [ + { + "href": "http://localhost:7990/projects/TEST/repos/rouge/browse" + } + ] + }, + "name": "rouge", + "project": { + "description": "Test", + "id": 1, + "key": "TEST", + "links": { + "self": [ + { + "href": "http://localhost:7990/projects/TEST" + } + ] + }, + "name": "test", + "public": false, + "type": "NORMAL" + }, + "public": false, + "scmId": "git", + "slug": "rouge", + "state": "AVAILABLE", + "statusMessage": "Available" + } + }, + "id": 7, + "links": { + "self": [ + { + "href": "http://localhost:7990/projects/TEST/repos/rouge/pull-requests/7" } - }, - "closed":true, - "closedDate":1530600648850, - "createdDate":1530600635690, - "description":"Test", - "fromRef":{ - "displayId":"root/CODE_OF_CONDUCTmd-1530600625006", - "id":"refs/heads/root/CODE_OF_CONDUCTmd-1530600625006", - "latestCommit":"074e2b4dddc5b99df1bf9d4a3f66cfc15481fdc8", - "repository":{ - "forkable":true, - "id":1, - "links":{ - "clone":[ - { - "href":"http://root@localhost:7990/scm/test/rouge.git", - "name":"http" - }, - { - "href":"ssh://git@localhost:7999/test/rouge.git", - "name":"ssh" - } - ], - "self":[ - { - "href":"http://localhost:7990/projects/TEST/repos/rouge/browse" - } - ] - }, - "name":"rouge", - "project":{ - "description":"Test", - "id":1, - "key":"TEST", - "links":{ - "self":[ - { - "href":"http://localhost:7990/projects/TEST" - } - ] - }, - "name":"test", - "public":false, - "type":"NORMAL" - }, - "public":false, - "scmId":"git", - "slug":"rouge", - "state":"AVAILABLE", - "statusMessage":"Available" - } - }, - "id":7, - "links":{ - "self":[ - { - "href":"http://localhost:7990/projects/TEST/repos/rouge/pull-requests/7" - } - ] - }, - "locked":false, - "open":false, - "participants":[ - - ], - "properties":{ - "commentCount":1, - "openTaskCount":0, - "resolvedTaskCount":0 - }, - "reviewers":[ + ] + }, + "locked": false, + "open": false, + "participants": [ - ], - "state":"MERGED", - "title":"Added a new line", - "toRef":{ - "displayId":"master", - "id":"refs/heads/master", - "latestCommit":"839fa9a2d434eb697815b8fcafaecc51accfdbbc", - "repository":{ - "forkable":true, - "id":1, - "links":{ - "clone":[ - { - "href":"http://root@localhost:7990/scm/test/rouge.git", - "name":"http" - }, - { - "href":"ssh://git@localhost:7999/test/rouge.git", - "name":"ssh" - } - ], - "self":[ - { - "href":"http://localhost:7990/projects/TEST/repos/rouge/browse" - } - ] - }, - "name":"rouge", - "project":{ - "description":"Test", - "id":1, - "key":"TEST", - "links":{ - "self":[ - { - "href":"http://localhost:7990/projects/TEST" - } - ] - }, - "name":"test", - "public":false, - "type":"NORMAL" - }, - "public":false, - "scmId":"git", - "slug":"rouge", - "state":"AVAILABLE", - "statusMessage":"Available" - } - }, - "updatedDate":1530600648850, - "version":2 + ], + "properties": { + "commentCount": 1, + "openTaskCount": 0, + "resolvedTaskCount": 0 + }, + "reviewers": [ + { + "user": { + "name": "Jane", + "emailAddress": "jane@doe.com", + "active": true, + "displayName": "Jane Doe", + "id": 3, + "slug": "jane_doe", + "type": "NORMAL", + "links": { + "self": [ + { + "href": "https://api.bitbucket.org/2.0/users/jane_doe" + } + ] + } + }, + "role": "REVIEWER", + "approved": false, + "status": "UNAPPROVED" + }, + { + "user": { + "name": "John", + "emailAddress": "john@smith.com", + "active": true, + "displayName": "John Smith", + "id": 2, + "slug": "john_smith", + "type": "NORMAL", + "links": { + "self": [ + { + "href": "https://api.bitbucket.org/2.0/users/john_smith" + } + ] + } + }, + "lastReviewedCommit": "38f1424ea696f5316907ad70b1f179b356b07d1b", + "role": "REVIEWER", + "approved": true, + "status": "APPROVED" + } + ], + "state": "MERGED", + "title": "Added a new line", + "toRef": { + "displayId": "master", + "id": "refs/heads/master", + "latestCommit": "839fa9a2d434eb697815b8fcafaecc51accfdbbc", + "repository": { + "forkable": true, + "id": 1, + "links": { + "clone": [ + { + "href": "http://root@localhost:7990/scm/test/rouge.git", + "name": "http" + }, + { + "href": "ssh://git@localhost:7999/test/rouge.git", + "name": "ssh" + } + ], + "self": [ + { + "href": "http://localhost:7990/projects/TEST/repos/rouge/browse" + } + ] + }, + "name": "rouge", + "project": { + "description": "Test", + "id": 1, + "key": "TEST", + "links": { + "self": [ + { + "href": "http://localhost:7990/projects/TEST" + } + ] + }, + "name": "test", + "public": false, + "type": "NORMAL" + }, + "public": false, + "scmId": "git", + "slug": "rouge", + "state": "AVAILABLE", + "statusMessage": "Available" + } + }, + "updatedDate": 1530600648850, + "version": 2 } diff --git a/spec/lib/bitbucket_server/representation/pull_request_spec.rb b/spec/lib/bitbucket_server/representation/pull_request_spec.rb index c5d0ee8908d68b329e0b668069b5b54d44cc765b..4d8bb3a44077ba6a797283e5bbadd960f01b282b 100644 --- a/spec/lib/bitbucket_server/representation/pull_request_spec.rb +++ b/spec/lib/bitbucket_server/representation/pull_request_spec.rb @@ -46,6 +46,10 @@ it { expect(subject.description).to eq('Test') } end + describe '#reviewers' do + it { expect(subject.reviewers.count).to eq(2) } + end + describe '#iid' do it { expect(subject.iid).to eq(7) } end @@ -114,6 +118,10 @@ author_username: "username", author: "root", description: "Test", + reviewers: contain_exactly( + hash_including('user' => hash_including('emailAddress' => 'jane@doe.com', 'slug' => 'jane_doe')), + hash_including('user' => hash_including('emailAddress' => 'john@smith.com', 'slug' => 'john_smith')) + ), source_branch_name: "refs/heads/root/CODE_OF_CONDUCTmd-1530600625006", source_branch_sha: "074e2b4dddc5b99df1bf9d4a3f66cfc15481fdc8", target_branch_name: "refs/heads/master", diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb index 3cff241105426f64d06cf85386dab9434798b339..4ff61bf329c9c5067b148559c47425f74075c30c 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb @@ -70,6 +70,7 @@ target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', + reviewers: [], state: 'merged', author: 'Test Author', author_email: pull_request_author.email, @@ -530,6 +531,7 @@ target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', + reviewers: sample.reviewers, state: 'merged', author: 'Test Author', author_email: pull_request_author.email, @@ -570,6 +572,7 @@ target_branch_name: Gitlab::Git::BRANCH_REF_PREFIX + sample.target_branch, title: 'This is a title', description: 'This is a test pull request', + reviewers: [], state: 'merged', author: 'Test Author', author_email: project.owner.email, diff --git a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb index 012cdcdd2608fa161444a6037d617bd92501792a..3c84d888c92ab09e8a602227d7dea7ef6d9b266b 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importers/pull_request_importer_spec.rb @@ -6,6 +6,8 @@ include AfterNextHelpers let_it_be(:project) { create(:project, :repository) } + let_it_be(:reviewer_1) { create(:user, username: 'john_smith', email: 'john@smith.com') } + let_it_be(:reviewer_2) { create(:user, username: 'jane_doe', email: 'jane@doe.com') } let(:pull_request_data) { Gitlab::Json.parse(fixture_file('importers/bitbucket_server/pull_request.json')) } let(:pull_request) { BitbucketServer::Representation::PullRequest.new(pull_request_data) } @@ -25,12 +27,27 @@ title: pull_request.title, source_branch: 'root/CODE_OF_CONDUCTmd-1530600625006', target_branch: 'master', + reviewer_ids: match_array([reviewer_1.id, reviewer_2.id]), state: pull_request.state, author_id: project.creator_id, description: "*Created by: #{pull_request.author}*\n\n#{pull_request.description}" ) end + context 'when the `bitbucket_server_user_mapping_by_username` flag is disabled' do + before do + stub_feature_flags(bitbucket_server_user_mapping_by_username: false) + end + + it 'imports reviewers correctly' do + importer.execute + + merge_request = project.merge_requests.find_by_iid(pull_request.iid) + + expect(merge_request.reviewer_ids).to match_array([reviewer_1.id, reviewer_2.id]) + end + end + it 'logs its progress' do expect(Gitlab::BitbucketServerImport::Logger) .to receive(:info).with(include(message: 'starting', iid: pull_request.iid)).and_call_original diff --git a/spec/lib/gitlab/import/merge_request_creator_spec.rb b/spec/lib/gitlab/import/merge_request_creator_spec.rb index 8f502216294c6a6b4918542f2c37a492beabd0f3..95d7ba6d833914161da76749f9932da2b8c8301b 100644 --- a/spec/lib/gitlab/import/merge_request_creator_spec.rb +++ b/spec/lib/gitlab/import/merge_request_creator_spec.rb @@ -2,14 +2,17 @@ require 'spec_helper' -RSpec.describe Gitlab::Import::MergeRequestCreator do +RSpec.describe Gitlab::Import::MergeRequestCreator, feature_category: :importers do let(:project) { create(:project, :repository) } + let_it_be(:reviewer_user) { create(:user) } subject { described_class.new(project) } describe '#execute' do let(:attributes) do - HashWithIndifferentAccess.new(merge_request.attributes.except('merge_params', 'suggested_reviewers')) + HashWithIndifferentAccess.new( + merge_request.attributes.except('merge_params', 'suggested_reviewers').merge(reviewer_ids: [reviewer_user.id]) + ) end context 'merge request already exists' do @@ -26,6 +29,7 @@ merge_request.reload + expect(merge_request.reviewer_ids).to contain_exactly(reviewer_user.id) expect(merge_request.merge_request_diffs.count).to eq(1) expect(merge_request.merge_request_diffs.first.commits.count).to eq(commits_count) expect(merge_request.latest_merge_request_diff_id).to eq(merge_request.merge_request_diffs.first.id) diff --git a/spec/lib/gitlab/import/merge_request_helpers_spec.rb b/spec/lib/gitlab/import/merge_request_helpers_spec.rb index 9626b7b893f405eba30c4145fd80e63af7d1c437..35a88dacfd7e1cd2f31e6b8e9e381666c97ae513 100644 --- a/spec/lib/gitlab/import/merge_request_helpers_spec.rb +++ b/spec/lib/gitlab/import/merge_request_helpers_spec.rb @@ -66,4 +66,26 @@ end end end + + describe '.insert_merge_request_reviewers' do + let_it_be(:merge_request) { create(:merge_request) } + + subject { helper.insert_merge_request_reviewers(merge_request, reviewers) } + + context 'when reviewers are not present' do + let(:reviewers) { nil } + + it 'does not insert reviewers' do + expect { subject }.not_to change { MergeRequestReviewer.count } + end + end + + context 'when reviewers are present' do + let(:reviewers) { create_list(:user, 3).pluck(:id) } + + it 'inserts reviewers' do + expect { subject }.to change { MergeRequestReviewer.count }.by(3) + end + end + end end diff --git a/spec/support/helpers/repo_helpers.rb b/spec/support/helpers/repo_helpers.rb index 45467fb70991ab744de8271a45b9606a6979ee59..d264356aa64bc1504da28ef50ee9a69e8368ba3b 100644 --- a/spec/support/helpers/repo_helpers.rb +++ b/spec/support/helpers/repo_helpers.rb @@ -120,11 +120,31 @@ def sample_compare(extra_changes = []) c1acaa58bbcbc3eafe538cb8274ba387047b69f8 ).reverse # last commit is recent one + reviewers = [ + { + "user" => { + "name" => "Jane", + "emailAddress" => "jane@doe.com", + "displayName" => "Jane Doe", + "slug" => "jane_doe" + } + }, + { + "user" => { + "name" => "John", + "emailAddress" => "john@smith.com", + "displayName" => "John Smith", + "slug" => "john_smith" + } + } + ] + OpenStruct.new( source_branch: 'master', target_branch: 'feature', changes: changes, - commits: commits + commits: commits, + reviewers: reviewers ) end