Add latest changes from gitlab-org/gitlab@13-12-stable-eev13.12.0-rc42

7 files changed, 84 insertions, 17 deletions

diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb
index 4000e0b2611..194dfb228ee 100644
--- a/spec/lib/gitlab/github_import/client_spec.rb
+++ b/spec/lib/gitlab/github_import/client_spec.rb
@@ -32,8 +32,9 @@ RSpec.describe Gitlab::GithubImport::Client 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
+      expect(client)
+        .to receive(:each_object)
+        .with(:pull_request_reviews, 'foo/bar', 999)
 
       client.pull_request_reviews('foo/bar', 999)
     end
diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
index 3bb57e152fe..ef0bb90db4a 100644
--- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
@@ -8,13 +8,14 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do
   let(:user) { create(:user) }
   let(:created_at) { Time.new(2017, 1, 1, 12, 00) }
   let(:updated_at) { Time.new(2017, 1, 1, 12, 15) }
+  let(:note_body) { 'This is my note' }
 
   let(:github_note) do
     Gitlab::GithubImport::Representation::Note.new(
       noteable_id: 1,
       noteable_type: 'Issue',
       author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'),
-      note: 'This is my note',
+      note: note_body,
       created_at: created_at,
       updated_at: updated_at,
       github_id: 1
@@ -92,6 +93,24 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do
           importer.execute
         end
       end
+
+      context 'when the note have invalid chars' do
+        let(:note_body) { %{There were an invalid char "\u0000" <= right here} }
+
+        it 'removes invalid chars' do
+          expect(importer.user_finder)
+            .to receive(:author_id_for)
+            .with(github_note)
+            .and_return([user.id, true])
+
+          expect { importer.execute }
+            .to change(project.notes, :count)
+            .by(1)
+
+          expect(project.notes.last.note)
+            .to eq('There were an invalid char "" <= right here')
+        end
+      end
     end
 
     context 'when the noteable does not exist' do
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
index 5002e0384f3..fa8b5e6ccf0 100644
--- 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
@@ -130,7 +130,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
             .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.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
@@ -153,6 +153,20 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
       end
     end
 
+    context 'when original author was deleted in github' do
+      let(:review) { create_review(type: 'APPROVED', note: '', author: nil) }
+
+      it 'creates a note for the review without the author information' do
+        expect { subject.execute }
+          .to change(Note, :count).by(1)
+
+        last_note = merge_request.notes.last
+        expect(last_note.note).to eq('**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 has a note text' do
       context 'when the review is "APPROVED"' do
         let(:review) { create_review(type: 'APPROVED') }
@@ -163,7 +177,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
 
           last_note = merge_request.notes.last
 
-          expect(last_note.note).to eq("*Created by author*\n\n**Review:** Approved\n\nnote")
+          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
@@ -178,7 +192,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
 
           last_note = merge_request.notes.last
 
-          expect(last_note.note).to eq("*Created by author*\n\n**Review:** Commented\n\nnote")
+          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
@@ -193,7 +207,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
 
           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.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
@@ -201,13 +215,13 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
     end
   end
 
-  def create_review(type:, note: 'note')
+  def create_review(type:, note: 'note', author: { id: 999, login: 'author' })
     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' }
+      author: author
     )
   end
 end
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
index b859cc727a6..4a47d103cde 100644
--- 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
@@ -23,12 +23,11 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do
   end
 
   describe '#id_for_already_imported_cache' do
-    it { expect(subject.id_for_already_imported_cache(double(number: 1))).to eq(1) }
+    it { expect(subject.id_for_already_imported_cache(double(id: 1))).to eq(1) }
   end
 
-  describe '#each_object_to_import' do
+  describe '#each_object_to_import', :clean_gitlab_redis_cache do
     it 'fetchs the merged pull requests data' do
-      pull_request = double
       create(
         :merged_merge_request,
         iid: 999,
@@ -36,12 +35,18 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do
         target_project: project
       )
 
+      pull_request = double
+
       allow(client)
         .to receive(:pull_request)
+        .exactly(:once) # ensure to be cached on the second call
         .with('http://somegithub.com', 999)
         .and_return(pull_request)
 
-      expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(pull_request)
+      expect { |b| subject.each_object_to_import(&b) }
+        .to yield_with_args(pull_request)
+
+      subject.each_object_to_import {}
     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
index 5e2302f9662..f18064f10aa 100644
--- 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
@@ -23,12 +23,18 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsReviewsImporter do
   end
 
   describe '#id_for_already_imported_cache' do
-    it { expect(subject.id_for_already_imported_cache(double(github_id: 1))).to eq(1) }
+    it { expect(subject.id_for_already_imported_cache(double(id: 1))).to eq(1) }
   end
 
-  describe '#each_object_to_import' do
+  describe '#each_object_to_import', :clean_gitlab_redis_cache do
     it 'fetchs the merged pull requests data' do
-      merge_request = create(:merge_request, source_project: project)
+      merge_request = create(
+        :merged_merge_request,
+        iid: 999,
+        source_project: project,
+        target_project: project
+      )
+
       review = double
 
       expect(review)
@@ -37,10 +43,14 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsReviewsImporter do
 
       allow(client)
         .to receive(:pull_request_reviews)
+        .exactly(:once) # ensure to be cached on the second call
         .with('github/repo', merge_request.iid)
         .and_return([review])
 
-      expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(review)
+      expect { |b| subject.each_object_to_import(&b) }
+        .to yield_with_args(review)
+
+      subject.each_object_to_import {}
     end
   end
 end
diff --git a/spec/lib/gitlab/github_import/markdown_text_spec.rb b/spec/lib/gitlab/github_import/markdown_text_spec.rb
index 4995caa0733..22bf10f36d8 100644
--- a/spec/lib/gitlab/github_import/markdown_text_spec.rb
+++ b/spec/lib/gitlab/github_import/markdown_text_spec.rb
@@ -20,11 +20,25 @@ RSpec.describe Gitlab::GithubImport::MarkdownText do
       expect(text.to_s).to eq('Hello')
     end
 
+    it 'returns the text when the author has no login' do
+      author = double(:author, login: nil)
+      text = described_class.new('Hello', author, true)
+
+      expect(text.to_s).to eq('Hello')
+    end
+
     it 'returns the text with an extra header when the author was not found' do
       author = double(:author, login: 'Alice')
       text = described_class.new('Hello', author)
 
       expect(text.to_s).to eq("*Created by: Alice*\n\nHello")
     end
+
+    it 'cleans invalid chars' do
+      author = double(:author, login: 'Alice')
+      text = described_class.format("\u0000Hello", author)
+
+      expect(text.to_s).to eq("*Created by: Alice*\n\nHello")
+    end
   end
 end
diff --git a/spec/lib/gitlab/github_import/user_finder_spec.rb b/spec/lib/gitlab/github_import/user_finder_spec.rb
index 0dd2bd4df45..20e67a784e1 100644
--- a/spec/lib/gitlab/github_import/user_finder_spec.rb
+++ b/spec/lib/gitlab/github_import/user_finder_spec.rb
@@ -61,6 +61,10 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do
       expect(finder).to receive(:find).with(user.id, user.login).and_return(42)
       expect(finder.user_id_for(user)).to eq(42)
     end
+
+    it 'does not fail with empty input' do
+      expect(finder.user_id_for(nil)).to eq(nil)
+    end
   end
 
   describe '#find' do