Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2020-11-29 18:09:41 +00:00
parent c1948ee4e5
commit 8e9cf65bd7
20 changed files with 696 additions and 4 deletions

View File

@ -715,6 +715,14 @@
:weight: 1
:idempotent:
:tags: []
- :name: github_importer:github_import_import_pull_request_review
:feature_category: :importers
:has_external_dependencies: true
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent:
:tags: []
- :name: github_importer:github_import_refresh_import_jid
:feature_category: :importers
:has_external_dependencies:
@ -779,6 +787,14 @@
:weight: 1
:idempotent:
:tags: []
- :name: github_importer:github_import_stage_import_pull_requests_reviews
:feature_category: :importers
:has_external_dependencies:
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent:
:tags: []
- :name: github_importer:github_import_stage_import_repository
:feature_category: :importers
:has_external_dependencies:

View File

@ -17,6 +17,7 @@ module Gitlab
# The known importer stages and their corresponding Sidekiq workers.
STAGES = {
pull_requests_merged_by: Stage::ImportPullRequestsMergedByWorker,
pull_request_reviews: Stage::ImportPullRequestsReviewsWorker,
issues_and_diff_notes: Stage::ImportIssuesAndDiffNotesWorker,
notes: Stage::ImportNotesWorker,
lfs_objects: Stage::ImportLfsObjectsWorker,

View File

@ -0,0 +1,25 @@
# frozen_string_literal: true
module Gitlab
module GithubImport
class ImportPullRequestReviewWorker # rubocop:disable Scalability/IdempotentWorker
include ObjectImporter
def representation_class
Gitlab::GithubImport::Representation::PullRequestReview
end
def importer_class
Importer::PullRequestReviewImporter
end
def counter_name
:github_importer_imported_pull_request_reviews
end
def counter_description
'The number of imported GitHub pull request reviews'
end
end
end
end

View File

@ -6,7 +6,7 @@ module Gitlab
include ObjectImporter
def representation_class
Representation::PullRequest
Gitlab::GithubImport::Representation::PullRequest
end
def importer_class

View File

@ -20,7 +20,7 @@ module Gitlab
AdvanceStageWorker.perform_async(
project.id,
{ waiter.key => waiter.jobs_remaining },
:issues_and_diff_notes
:pull_request_reviews
)
end
end

View File

@ -0,0 +1,36 @@
# frozen_string_literal: true
module Gitlab
module GithubImport
module Stage
class ImportPullRequestsReviewsWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
include GithubImport::Queue
include StageMethods
# client - An instance of Gitlab::GithubImport::Client.
# project - An instance of Project.
def import(client, project)
waiter =
if Feature.enabled?(:github_import_pull_request_reviews, project, default_enabled: true)
waiter = Importer::PullRequestsReviewsImporter
.new(project, client)
.execute
project.import_state.refresh_jid_expiration
waiter
else
JobWaiter.new
end
AdvanceStageWorker.perform_async(
project.id,
{ waiter.key => waiter.jobs_remaining },
:issues_and_diff_notes
)
end
end
end
end
end

View File

@ -0,0 +1,5 @@
---
title: Github Importer - import pull request reviews from Github
merge_request: 48632
author:
type: added

View File

@ -0,0 +1,8 @@
---
name: github_import_pull_request_reviews
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48632
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/289153
milestone: '13.7'
type: development
group: group::import
default_enabled: true

View File

@ -69,6 +69,10 @@ module Gitlab
with_rate_limit { octokit.user(username) }
end
def pull_request_reviews(repo_name, iid)
with_rate_limit { octokit.pull_request_reviews(repo_name, iid) }
end
# Returns the details of a GitHub repository.
#
# name - The path (in the form `owner/repository`) of the repository.

View File

@ -0,0 +1,101 @@
# frozen_string_literal: true
module Gitlab
module GithubImport
module Importer
class PullRequestReviewImporter
def initialize(review, project, client)
@review = review
@project = project
@client = client
@merge_request = project.merge_requests.find_by_id(review.merge_request_id)
end
def execute
user_finder = GithubImport::UserFinder.new(project, client)
gitlab_user_id = user_finder.user_id_for(review.author)
if gitlab_user_id
add_review_note!(gitlab_user_id)
add_approval!(gitlab_user_id)
else
add_complementary_review_note!(project.creator_id)
end
end
private
attr_reader :review, :merge_request, :project, :client
def add_review_note!(author_id)
return if review.note.empty?
add_note!(author_id, review_note_content)
end
def add_complementary_review_note!(author_id)
return if review.note.empty? && !review.approval?
note = "*Created by %{login}*\n\n%{note}" % {
note: review_note_content,
login: review.author.login
}
add_note!(author_id, note)
end
def review_note_content
header = "**Review:** #{review.review_type.humanize}"
if review.note.present?
"#{header}\n\n#{review.note}"
else
header
end
end
def add_note!(author_id, note)
note = Note.new(note_attributes(author_id, note))
note.save!
end
def note_attributes(author_id, note, extra = {})
{
importing: true,
noteable_id: merge_request.id,
noteable_type: 'MergeRequest',
project_id: project.id,
author_id: author_id,
note: note,
system: false,
created_at: review.submitted_at,
updated_at: review.submitted_at
}.merge(extra)
end
def add_approval!(user_id)
return unless review.review_type == 'APPROVED'
add_approval_system_note!(user_id)
merge_request.approvals.create!(
user_id: user_id,
created_at: review.submitted_at
)
end
def add_approval_system_note!(user_id)
attributes = note_attributes(
user_id,
'approved this merge request',
system: true,
system_note_metadata: SystemNoteMetadata.new(action: 'approved')
)
Note.create!(attributes)
end
end
end
end
end

View File

@ -11,7 +11,7 @@ module Gitlab
end
def representation_class
Representation::PullRequest
Gitlab::GithubImport::Representation::PullRequest
end
def sidekiq_worker_class

View File

@ -0,0 +1,41 @@
# frozen_string_literal: true
module Gitlab
module GithubImport
module Importer
class PullRequestsReviewsImporter
include ParallelScheduling
def importer_class
PullRequestReviewImporter
end
def representation_class
Gitlab::GithubImport::Representation::PullRequestReview
end
def sidekiq_worker_class
ImportPullRequestReviewWorker
end
def collection_method
:pull_request_reviews
end
def id_for_already_imported_cache(review)
review.github_id
end
def each_object_to_import
project.merge_requests.find_each do |merge_request|
reviews = client.pull_request_reviews(project.import_source, merge_request.iid)
reviews.each do |review|
review.merge_request_id = merge_request.id
yield(review)
end
end
end
end
end
end
end

View File

@ -0,0 +1,49 @@
# frozen_string_literal: true
module Gitlab
module GithubImport
module Representation
class PullRequestReview
include ToHash
include ExposeAttribute
attr_reader :attributes
expose_attribute :author, :note, :review_type, :submitted_at, :github_id, :merge_request_id
def self.from_api_response(review)
user = Representation::User.from_api_response(review.user) if review.user
new(
merge_request_id: review.merge_request_id,
author: user,
note: review.body,
review_type: review.state,
submitted_at: review.submitted_at,
github_id: review.id
)
end
# Builds a new note using a Hash that was built from a JSON payload.
def self.from_json_hash(raw_hash)
hash = Representation.symbolize_hash(raw_hash)
hash[:author] &&= Representation::User.from_json_hash(hash[:author])
hash[:submitted_at] = Time.parse(hash[:submitted_at]).in_time_zone
new(hash)
end
# attributes - A Hash containing the raw note details. The keys of this
# Hash must be Symbols.
def initialize(attributes)
@attributes = attributes
end
def approval?
review_type == 'APPROVED'
end
end
end
end
end

View File

@ -28,6 +28,17 @@ RSpec.describe Gitlab::GithubImport::Client do
end
end
describe '#pull_request_reviews' do
it 'returns the pull request reviews' do
client = described_class.new('foo')
expect(client.octokit).to receive(:pull_request_reviews).with('foo/bar', 999)
expect(client).to receive(:with_rate_limit).and_yield
client.pull_request_reviews('foo/bar', 999)
end
end
describe '#repository' do
it 'returns the details of a repository' do
client = described_class.new('foo')

View File

@ -0,0 +1,202 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean_gitlab_redis_cache do
using RSpec::Parameterized::TableSyntax
let_it_be(:merge_request) { create(:merge_request) }
let(:project) { merge_request.project }
let(:client_double) { double(user: double(id: 999, login: 'author', email: 'author@email.com')) }
let(:submitted_at) { Time.new(2017, 1, 1, 12, 00).utc }
subject { described_class.new(review, project, client_double) }
context 'when the review author can be mapped to a gitlab user' do
let_it_be(:author) { create(:user, email: 'author@email.com') }
context 'when the review has no note text' do
context 'when the review is "APPROVED"' do
let(:review) { create_review(type: 'APPROVED', note: '') }
it 'creates a note for the review' do
expect { subject.execute }.to change(Note, :count)
last_note = merge_request.notes.last
expect(last_note.note).to eq('approved this merge request')
expect(last_note.author).to eq(author)
expect(last_note.created_at).to eq(submitted_at)
expect(last_note.system_note_metadata.action).to eq('approved')
expect(merge_request.approved_by_users.reload).to include(author)
expect(merge_request.approvals.last.created_at).to eq(submitted_at)
end
end
context 'when the review is "COMMENTED"' do
let(:review) { create_review(type: 'COMMENTED', note: '') }
it 'creates a note for the review' do
expect { subject.execute }.not_to change(Note, :count)
end
end
context 'when the review is "CHANGES_REQUESTED"' do
let(:review) { create_review(type: 'CHANGES_REQUESTED', note: '') }
it 'creates a note for the review' do
expect { subject.execute }.not_to change(Note, :count)
end
end
end
context 'when the review has a note text' do
context 'when the review is "APPROVED"' do
let(:review) { create_review(type: 'APPROVED') }
it 'creates a note for the review' do
expect { subject.execute }
.to change(Note, :count).by(2)
.and change(Approval, :count).by(1)
note = merge_request.notes.where(system: false).last
expect(note.note).to eq("**Review:** Approved\n\nnote")
expect(note.author).to eq(author)
expect(note.created_at).to eq(submitted_at)
system_note = merge_request.notes.where(system: true).last
expect(system_note.note).to eq('approved this merge request')
expect(system_note.author).to eq(author)
expect(system_note.created_at).to eq(submitted_at)
expect(system_note.system_note_metadata.action).to eq('approved')
expect(merge_request.approved_by_users.reload).to include(author)
expect(merge_request.approvals.last.created_at).to eq(submitted_at)
end
end
context 'when the review is "COMMENTED"' do
let(:review) { create_review(type: 'COMMENTED') }
it 'creates a note for the review' do
expect { subject.execute }
.to change(Note, :count).by(1)
.and not_change(Approval, :count)
last_note = merge_request.notes.last
expect(last_note.note).to eq("**Review:** Commented\n\nnote")
expect(last_note.author).to eq(author)
expect(last_note.created_at).to eq(submitted_at)
end
end
context 'when the review is "CHANGES_REQUESTED"' do
let(:review) { create_review(type: 'CHANGES_REQUESTED') }
it 'creates a note for the review' do
expect { subject.execute }
.to change(Note, :count).by(1)
.and not_change(Approval, :count)
last_note = merge_request.notes.last
expect(last_note.note).to eq("**Review:** Changes requested\n\nnote")
expect(last_note.author).to eq(author)
expect(last_note.created_at).to eq(submitted_at)
end
end
end
end
context 'when the review author cannot be mapped to a gitlab user' do
context 'when the review has no note text' do
context 'when the review is "APPROVED"' do
let(:review) { create_review(type: 'APPROVED', note: '') }
it 'creates a note for the review with *Approved by by<author>*' do
expect { subject.execute }
.to change(Note, :count).by(1)
last_note = merge_request.notes.last
expect(last_note.note).to eq("*Created by author*\n\n**Review:** Approved")
expect(last_note.author).to eq(project.creator)
expect(last_note.created_at).to eq(submitted_at)
end
end
context 'when the review is "COMMENTED"' do
let(:review) { create_review(type: 'COMMENTED', note: '') }
it 'creates a note for the review with *Commented by<author>*' do
expect { subject.execute }.not_to change(Note, :count)
end
end
context 'when the review is "CHANGES_REQUESTED"' do
let(:review) { create_review(type: 'CHANGES_REQUESTED', note: '') }
it 'creates a note for the review with *Changes requested by <author>*' do
expect { subject.execute }.not_to change(Note, :count)
end
end
end
context 'when the review has a note text' do
context 'when the review is "APPROVED"' do
let(:review) { create_review(type: 'APPROVED') }
it 'creates a note for the review with *Approved by by<author>*' do
expect { subject.execute }
.to change(Note, :count).by(1)
last_note = merge_request.notes.last
expect(last_note.note).to eq("*Created by author*\n\n**Review:** Approved\n\nnote")
expect(last_note.author).to eq(project.creator)
expect(last_note.created_at).to eq(submitted_at)
end
end
context 'when the review is "COMMENTED"' do
let(:review) { create_review(type: 'COMMENTED') }
it 'creates a note for the review with *Commented by<author>*' do
expect { subject.execute }
.to change(Note, :count).by(1)
last_note = merge_request.notes.last
expect(last_note.note).to eq("*Created by author*\n\n**Review:** Commented\n\nnote")
expect(last_note.author).to eq(project.creator)
expect(last_note.created_at).to eq(submitted_at)
end
end
context 'when the review is "CHANGES_REQUESTED"' do
let(:review) { create_review(type: 'CHANGES_REQUESTED') }
it 'creates a note for the review with *Changes requested by <author>*' do
expect { subject.execute }
.to change(Note, :count).by(1)
last_note = merge_request.notes.last
expect(last_note.note).to eq("*Created by author*\n\n**Review:** Changes requested\n\nnote")
expect(last_note.author).to eq(project.creator)
expect(last_note.created_at).to eq(submitted_at)
end
end
end
end
def create_review(type:, note: 'note')
Gitlab::GithubImport::Representation::PullRequestReview.from_json_hash(
merge_request_id: merge_request.id,
review_type: type,
note: note,
submitted_at: submitted_at.to_s,
author: { id: 999, login: 'author' }
)
end
end

View File

@ -0,0 +1,46 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Importer::PullRequestsReviewsImporter do
let(:client) { double }
let(:project) { create(:project, import_source: 'github/repo') }
subject { described_class.new(project, client) }
it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) }
describe '#representation_class' do
it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequestReview) }
end
describe '#importer_class' do
it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestReviewImporter) }
end
describe '#collection_method' do
it { expect(subject.collection_method).to eq(:pull_request_reviews) }
end
describe '#id_for_already_imported_cache' do
it { expect(subject.id_for_already_imported_cache(double(github_id: 1))).to eq(1) }
end
describe '#each_object_to_import' do
it 'fetchs the merged pull requests data' do
merge_request = create(:merge_request, source_project: project)
review = double
expect(review)
.to receive(:merge_request_id=)
.with(merge_request.id)
allow(client)
.to receive(:pull_request_reviews)
.with('github/repo', merge_request.iid)
.and_return([review])
expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(review)
end
end
end

View File

@ -0,0 +1,72 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Representation::PullRequestReview do
let(:submitted_at) { Time.new(2017, 1, 1, 12, 00).utc }
shared_examples 'a PullRequest review' do
it 'returns an instance of PullRequest' do
expect(review).to be_an_instance_of(described_class)
expect(review.author).to be_an_instance_of(Gitlab::GithubImport::Representation::User)
expect(review.author.id).to eq(4)
expect(review.author.login).to eq('alice')
expect(review.note).to eq('note')
expect(review.review_type).to eq('APPROVED')
expect(review.submitted_at).to eq(submitted_at)
expect(review.github_id).to eq(999)
expect(review.merge_request_id).to eq(42)
end
end
describe '.from_api_response' do
let(:response) do
double(
:response,
id: 999,
merge_request_id: 42,
body: 'note',
state: 'APPROVED',
user: double(:user, id: 4, login: 'alice'),
submitted_at: submitted_at
)
end
it_behaves_like 'a PullRequest review' do
let(:review) { described_class.from_api_response(response) }
end
it 'does not set the user if the response did not include a user' do
allow(response)
.to receive(:user)
.and_return(nil)
review = described_class.from_api_response(response)
expect(review.author).to be_nil
end
end
describe '.from_json_hash' do
let(:hash) do
{
'github_id' => 999,
'merge_request_id' => 42,
'note' => 'note',
'review_type' => 'APPROVED',
'author' => { 'id' => 4, 'login' => 'alice' },
'submitted_at' => submitted_at.to_s
}
end
it_behaves_like 'a PullRequest review' do
let(:review) { described_class.from_json_hash(hash) }
end
it 'does not set the user if the response did not include a user' do
review = described_class.from_json_hash(hash.except('author'))
expect(review.author).to be_nil
end
end
end

View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::ImportPullRequestReviewWorker do
it { is_expected.to include_module(Gitlab::GithubImport::ObjectImporter) }
describe '#representation_class' do
it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequestReview) }
end
describe '#importer_class' do
it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestReviewImporter) }
end
describe '#counter_name' do
it { expect(subject.counter_name).to eq(:github_importer_imported_pull_request_reviews) }
end
describe '#counter_description' do
it { expect(subject.counter_description).to eq('The number of imported GitHub pull request reviews') }
end
end

View File

@ -27,7 +27,7 @@ RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsMergedByWorker do
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
.with(project.id, { '123' => 2 }, :issues_and_diff_notes)
.with(project.id, { '123' => 2 }, :pull_request_reviews)
worker.import(client, project)
end

View File

@ -0,0 +1,52 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsReviewsWorker do
let(:project) { create(:project) }
let(:import_state) { create(:import_state, project: project) }
let(:worker) { described_class.new }
let(:client) { double(:client) }
describe '#import' do
it 'does not import with the feature disabled' do
stub_feature_flags(github_import_pull_request_reviews: false)
expect(Gitlab::JobWaiter)
.to receive(:new)
.and_return(double(key: '123', jobs_remaining: 0))
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
.with(project.id, { '123' => 0 }, :issues_and_diff_notes)
worker.import(client, project)
end
it 'imports all the pull request reviews' do
stub_feature_flags(github_import_pull_request_reviews: true)
importer = double(:importer)
waiter = Gitlab::JobWaiter.new(2, '123')
expect(Gitlab::GithubImport::Importer::PullRequestsReviewsImporter)
.to receive(:new)
.with(project, client)
.and_return(importer)
expect(importer)
.to receive(:execute)
.and_return(waiter)
expect(import_state)
.to receive(:refresh_jid_expiration)
expect(Gitlab::GithubImport::AdvanceStageWorker)
.to receive(:perform_async)
.with(project.id, { '123' => 2 }, :issues_and_diff_notes)
worker.import(client, project)
end
end
end