Add latest changes from gitlab-org/gitlab@13-7-stable-eev13.7.0-rc42

11 files changed, 573 insertions, 12 deletions

diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb
index bc734644d29..4000e0b2611 100644
--- a/spec/lib/gitlab/github_import/client_spec.rb
+++ b/spec/lib/gitlab/github_import/client_spec.rb
@@ -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')
@@ -39,6 +50,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')
@@ -478,7 +500,7 @@ RSpec.describe Gitlab::GithubImport::Client do
       it 'searches for repositories based on name' do
         expected_search_query = 'test in:name is:public,private user:user repo:repo1 repo:repo2 org:org1 org:org2'
 
-        expect(client).to receive(:each_page).with(:search_repositories, expected_search_query)
+        expect(client.octokit).to receive(:search_repositories).with(expected_search_query, {})
 
         client.search_repos_by_name('test')
       end
diff --git a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb
index 6188ba8ec3f..8ee534734f0 100644
--- a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb
@@ -49,6 +49,57 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
         importer.execute
       end
     end
+
+    context 'when LFS list download fails' do
+      it 'rescues and logs the known exceptions' do
+        exception = StandardError.new('Invalid Project URL')
+        importer = described_class.new(project, client, parallel: false)
+
+        expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service|
+          expect(service)
+            .to receive(:execute)
+            .and_raise(exception)
+        end
+
+        expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+          expect(logger)
+            .to receive(:error)
+            .with(
+              message: 'importer failed',
+              import_source: :github,
+              project_id: project.id,
+              parallel: false,
+              importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter',
+              'error.message': 'Invalid Project URL'
+            )
+        end
+
+        expect(Gitlab::ErrorTracking)
+          .to receive(:track_exception)
+          .with(
+            exception,
+            import_source: :github,
+            parallel: false,
+            project_id: project.id,
+            importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter'
+          ).and_call_original
+
+        importer.execute
+      end
+
+      it 'raises and logs the unknown exceptions' do
+        exception = Exception.new('Really bad news')
+        importer = described_class.new(project, client, parallel: false)
+
+        expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service|
+          expect(service)
+            .to receive(:execute)
+            .and_raise(exception)
+        end
+
+        expect { importer.execute }.to raise_error(exception)
+      end
+    end
   end
 
   describe '#sequential_import' do
@@ -56,18 +107,16 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
       importer = described_class.new(project, client, parallel: false)
       lfs_object_importer = double(:lfs_object_importer)
 
-      allow(importer)
-        .to receive(:each_object_to_import)
-        .and_yield(lfs_download_object)
+      expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service|
+        expect(service).to receive(:execute).and_return([lfs_download_object])
+      end
 
       expect(Gitlab::GithubImport::Importer::LfsObjectImporter)
-        .to receive(:new)
-        .with(
+        .to receive(:new).with(
           an_instance_of(Gitlab::GithubImport::Representation::LfsObject),
           project,
           client
-        )
-        .and_return(lfs_object_importer)
+        ).and_return(lfs_object_importer)
 
       expect(lfs_object_importer).to receive(:execute)
 
@@ -79,9 +128,9 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
     it 'imports each lfs object in parallel' do
       importer = described_class.new(project, client)
 
-      allow(importer)
-        .to receive(:each_object_to_import)
-        .and_yield(lfs_download_object)
+      expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service|
+        expect(service).to receive(:execute).and_return([lfs_download_object])
+      end
 
       expect(Gitlab::GithubImport::ImportLfsObjectWorker)
         .to receive(:perform_async)
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 46850618945..c7388314253 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
@@ -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)
diff --git a/spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb
new file mode 100644
index 00000000000..2999dc5bb41
--- /dev/null
+++ b/spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb
@@ -0,0 +1,41 @@
+# 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)
+      .and not_change(merge_request, :updated_at)
+
+    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
diff --git a/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb
new file mode 100644
index 00000000000..b2f993ac47c
--- /dev/null
+++ b/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
index 0835c6155b9..8a7867f3841 100644
--- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb
+++ b/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
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb
new file mode 100644
index 00000000000..b859cc727a6
--- /dev/null
+++ b/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb
@@ -0,0 +1,47 @@
+# 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
diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb
new file mode 100644
index 00000000000..5e2302f9662
--- /dev/null
+++ b/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb
index 578743be96b..1e31cd2f007 100644
--- a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb
+++ b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb
@@ -7,6 +7,10 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
     Class.new do
       include(Gitlab::GithubImport::ParallelScheduling)
 
+      def importer_class
+        Class
+      end
+
       def collection_method
         :issues
       end
@@ -63,6 +67,82 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
 
       importer.execute
     end
+
+    it 'logs the the process' do
+      importer = importer_class.new(project, client, parallel: false)
+
+      expect(importer)
+        .to receive(:sequential_import)
+        .and_return([])
+
+      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+        expect(logger)
+          .to receive(:info)
+          .with(
+            message: 'starting importer',
+            import_source: :github,
+            parallel: false,
+            project_id: project.id,
+            importer: 'Class'
+          )
+        expect(logger)
+          .to receive(:info)
+          .with(
+            message: 'importer finished',
+            import_source: :github,
+            parallel: false,
+            project_id: project.id,
+            importer: 'Class'
+          )
+      end
+
+      importer.execute
+    end
+
+    it 'logs the error when it fails' do
+      exception = StandardError.new('some error')
+
+      importer = importer_class.new(project, client, parallel: false)
+
+      expect(importer)
+        .to receive(:sequential_import)
+        .and_raise(exception)
+
+      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+        expect(logger)
+          .to receive(:info)
+          .with(
+            message: 'starting importer',
+            import_source: :github,
+            parallel: false,
+            project_id: project.id,
+            importer: 'Class'
+          )
+        expect(logger)
+          .to receive(:error)
+          .with(
+            message: 'importer failed',
+            import_source: :github,
+            project_id: project.id,
+            parallel: false,
+            importer: 'Class',
+            'error.message': 'some error'
+          )
+      end
+
+      expect(Gitlab::ErrorTracking)
+        .to receive(:track_exception)
+        .with(
+          exception,
+          import_source: :github,
+          parallel: false,
+          project_id: project.id,
+          importer: 'Class'
+        )
+        .and_call_original
+
+      expect { importer.execute }.to raise_error(exception)
+    end
   end
 
   describe '#sequential_import' do
diff --git a/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb
new file mode 100644
index 00000000000..f9763455468
--- /dev/null
+++ b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb
index 370eac1d993..27a82951b01 100644
--- a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb
+++ b/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