Merge branch 'kassio/add-github-merged-by-with-stage' into 'master'

Github Importer - import pull request merged by

See merge request gitlab-org/gitlab!48561

app/workers/all_queues.yml

@@ -707,6 +707,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: github_importer:github_import_import_pull_request_merged_by
+  :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:
@@ -763,6 +771,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: github_importer:github_import_stage_import_pull_requests_merged_by
+  :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:

app/workers/gitlab/github_import/advance_stage_worker.rb

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

app/workers/gitlab/github_import/import_pull_request_merged_by_worker.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GithubImport
+    class ImportPullRequestMergedByWorker # rubocop:disable Scalability/IdempotentWorker
+      include ObjectImporter
+
+      def representation_class
+        Gitlab::GithubImport::Representation::PullRequest
+      end
+
+      def importer_class
+        Importer::PullRequestMergedByImporter
+      end
+
+      def counter_name
+        :github_importer_imported_pull_requests_merged_by
+      end
+
+      def counter_description
+        'The number of imported GitHub pull requests merged by'
+      end
+    end
+  end
+end

app/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GithubImport
+    module Stage
+      class ImportPullRequestsMergedByWorker # 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 = Importer::PullRequestsMergedByImporter
+            .new(project, client)
+            .execute
+
+          project.import_state.refresh_jid_expiration
+
+          AdvanceStageWorker.perform_async(
+            project.id,
+            { waiter.key => waiter.jobs_remaining },
+            :issues_and_diff_notes
+          )
+        end
+      end
+    end
+  end
+end

app/workers/gitlab/github_import/stage/import_pull_requests_worker.rb

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

changelogs/unreleased/kassio-add-github-merged-by.yml

+---
+title: Github Importer - import the pull request `merged by` field
+merge_request: 48561
+author:
+type: changed

lib/gitlab/github_import/client.rb

@@ -76,6 +76,10 @@ module Gitlab
         with_rate_limit { octokit.repo(name) }
       end
 
+      def pull_request(repo_name, iid)
+        with_rate_limit { octokit.pull_request(repo_name, iid) }
+      end
+
       def labels(*args)
         each_object(:labels, *args)
       end

lib/gitlab/github_import/importer/pull_request_merged_by_importer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GithubImport
+    module Importer
+      class PullRequestMergedByImporter
+        def initialize(pull_request, project, client)
+          @project = project
+          @pull_request = pull_request
+          @client = client
+        end
+
+        def execute
+          merge_request = project.merge_requests.find_by_iid(pull_request.iid)
+          user_finder = GithubImport::UserFinder.new(project, client)
+          gitlab_user_id = user_finder.user_id_for(pull_request.merged_by)
+
+          if gitlab_user_id
+            timestamp = Time.new.utc
+            MergeRequest::Metrics.upsert({
+              target_project_id: project.id,
+              merge_request_id: merge_request.id,
+              merged_by_id: gitlab_user_id,
+              created_at: timestamp,
+              updated_at: timestamp
+            }, unique_by: :merge_request_id)
+          else
+            merge_request.notes.create!(
+              note: "*Merged by: #{pull_request.merged_by.login}*",
+              author_id: project.creator_id,
+              project: project,
+              created_at: pull_request.created_at
+            )
+          end
+        end
+
+        private
+
+        attr_reader :project, :pull_request, :client
+      end
+    end
+  end
+end

lib/gitlab/github_import/importer/pull_requests_merged_by_importer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GithubImport
+    module Importer
+      class PullRequestsMergedByImporter
+        include ParallelScheduling
+
+        def importer_class
+          PullRequestMergedByImporter
+        end
+
+        def representation_class
+          Gitlab::GithubImport::Representation::PullRequest
+        end
+
+        def sidekiq_worker_class
+          ImportPullRequestMergedByWorker
+        end
+
+        def collection_method
+          :pull_requests_merged_by
+        end
+
+        def id_for_already_imported_cache(pr)
+          pr.number
+        end
+
+        def each_object_to_import
+          project.merge_requests.with_state(:merged).find_each do |merge_request|
+            pull_request = client.pull_request(project.import_source, merge_request.iid)
+            yield(pull_request)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/github_import/representation/pull_request.rb

@@ -13,18 +13,16 @@ module Gitlab
                          :source_branch_sha, :target_branch, :target_branch_sha,
                          :milestone_number, :author, :assignee, :created_at,
                          :updated_at, :merged_at, :source_repository_id,
-                         :target_repository_id, :source_repository_owner
+                         :target_repository_id, :source_repository_owner, :merged_by
 
         # Builds a PR from a GitHub API response.
         #
         # issue - An instance of `Sawyer::Resource` containing the PR details.
         def self.from_api_response(pr)
-          assignee =
-            if pr.assignee
-              Representation::User.from_api_response(pr.assignee)
-            end
-
+          assignee = Representation::User.from_api_response(pr.assignee) if pr.assignee
           user = Representation::User.from_api_response(pr.user) if pr.user
+          merged_by = Representation::User.from_api_response(pr.merged_by) if pr.merged_by
+
           hash = {
             iid: pr.number,
             title: pr.title,
@@ -42,7 +40,8 @@ module Gitlab
             assignee: assignee,
             created_at: pr.created_at,
             updated_at: pr.updated_at,
-            merged_at: pr.merged_at
+            merged_at: pr.merged_at,
+            merged_by: merged_by
           }
 
           new(hash)
@@ -57,8 +56,8 @@ module Gitlab
 
           # Assignees are optional so we only convert it from a Hash if one was
           # set.
-          hash[:assignee] &&= Representation::User
-            .from_json_hash(hash[:assignee])
+          hash[:assignee] &&= Representation::User.from_json_hash(hash[:assignee])
+          hash[:merged_by] &&= Representation::User.from_json_hash(hash[:merged_by])
 
           new(hash)
         end

spec/lib/gitlab/github_import/client_spec.rb

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

spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb

@@ -43,7 +43,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitla
 
   describe '#execute' do
     it 'imports the pull request' do
-      mr = double(:merge_request, id: 10)
+      mr = double(:merge_request, id: 10, merged?: false)
 
       expect(importer)
         .to receive(:create_merge_request)

spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::Importer::PullRequestMergedByImporter, :clean_gitlab_redis_cache do
+  let_it_be(:merge_request) { create(:merged_merge_request) }
+  let(:project) { merge_request.project }
+  let(:created_at) { Time.new(2017, 1, 1, 12, 00).utc }
+  let(:client_double) { double(user: double(id: 999, login: 'merger', email: 'merger@email.com')) }
+
+  let(:pull_request) do
+    instance_double(
+      Gitlab::GithubImport::Representation::PullRequest,
+      iid: merge_request.iid,
+      created_at: created_at,
+      merged_by: double(id: 999, login: 'merger')
+    )
+  end
+
+  subject { described_class.new(pull_request, project, client_double) }
+
+  it 'assigns the merged by user when mapped' do
+    merge_user = create(:user, email: 'merger@email.com')
+
+    subject.execute
+
+    expect(merge_request.metrics.reload.merged_by).to eq(merge_user)
+  end
+
+  it 'adds a note referencing the merger user when the user cannot be mapped' do
+    expect { subject.execute }.to change(Note, :count).by(1)
+
+    last_note = merge_request.notes.last
+
+    expect(last_note.note).to eq("*Merged by: merger*")
+    expect(last_note.created_at).to eq(created_at)
+    expect(last_note.author).to eq(project.creator)
+  end
+end

spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb

@@ -29,6 +29,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do
       milestone: double(:milestone, number: 4),
       user: double(:user, id: 4, login: 'alice'),
       assignee: double(:user, id: 4, login: 'alice'),
+      merged_by: double(:user, id: 4, login: 'alice'),
       created_at: 1.second.ago,
       updated_at: 1.second.ago,
       merged_at: 1.second.ago

spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do
+  let(:client) { double }
+  let(:project) { create(:project, import_source: 'http://somegithub.com') }
+
+  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::PullRequest) }
+  end
+
+  describe '#importer_class' do
+    it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestMergedByImporter) }
+  end
+
+  describe '#collection_method' do
+    it { expect(subject.collection_method).to eq(:pull_requests_merged_by) }
+  end
+
+  describe '#id_for_already_imported_cache' do
+    it { expect(subject.id_for_already_imported_cache(double(number: 1))).to eq(1) }
+  end
+
+  describe '#each_object_to_import' do
+    it 'fetchs the merged pull requests data' do
+      pull_request = double
+      create(
+        :merged_merge_request,
+        iid: 999,
+        source_project: project,
+        target_project: project
+      )
+
+      allow(client)
+        .to receive(:pull_request)
+        .with('http://somegithub.com', 999)
+        .and_return(pull_request)
+
+      expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(pull_request)
+    end
+  end
+end

spec/lib/gitlab/github_import/representation/pull_request_spec.rb

@@ -115,6 +115,7 @@ RSpec.describe Gitlab::GithubImport::Representation::PullRequest do
         milestone: double(:milestone, number: 4),
         user: double(:user, id: 4, login: 'alice'),
         assignee: double(:user, id: 4, login: 'alice'),
+        merged_by: double(:user, id: 4, login: 'alice'),
         created_at: created_at,
         updated_at: updated_at,
         merged_at: merged_at

spec/workers/gitlab/github_import/import_pull_request_merged_by_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::ImportPullRequestMergedByWorker do
+  it { is_expected.to include_module(Gitlab::GithubImport::ObjectImporter) }
+
+  describe '#representation_class' do
+    it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequest) }
+  end
+
+  describe '#importer_class' do
+    it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestMergedByImporter) }
+  end
+
+  describe '#counter_name' do
+    it { expect(subject.counter_name).to eq(:github_importer_imported_pull_requests_merged_by) }
+  end
+
+  describe '#counter_description' do
+    it { expect(subject.counter_description).to eq('The number of imported GitHub pull requests merged by') }
+  end
+end

spec/workers/gitlab/github_import/stage/import_pull_requests_merged_by_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::Stage::ImportPullRequestsMergedByWorker do
+  let(:project) { create(:project) }
+  let(:import_state) { create(:import_state, project: project) }
+  let(:worker) { described_class.new }
+
+  describe '#import' do
+    it 'imports all the pull requests' do
+      importer = double(:importer)
+      client = double(:client)
+      waiter = Gitlab::JobWaiter.new(2, '123')
+
+      expect(Gitlab::GithubImport::Importer::PullRequestsMergedByImporter)
+        .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

spec/workers/gitlab/github_import/stage/import_pull_requests_worker_spec.rb

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