1 files changed, 62 insertions, 21 deletions

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 fb6024d0952..49794eceb5a 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
@@ -8,11 +8,48 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
   let_it_be(:merge_request) { create(:merge_request) }
 
   let(:project) { merge_request.project }
-  let(:client_double) { double(user: { id: 999, login: 'author', email: 'author@email.com' }) }
   let(:submitted_at) { Time.new(2017, 1, 1, 12, 00).utc }
+  let(:client_double) do
+    instance_double(
+      'Gitlab::GithubImport::Client',
+      user: { id: 999, login: 'author', email: 'author@email.com' }
+    )
+  end
 
   subject { described_class.new(review, project, client_double) }
 
+  shared_examples 'imports a reviewer for the Merge Request' do
+    it 'creates reviewer for the Merge Request' do
+      expect { subject.execute }.to change(MergeRequestReviewer, :count).by(1)
+
+      expect(merge_request.reviewers).to contain_exactly(author)
+    end
+
+    context 'when reviewer already exists' do
+      before do
+        create(
+          :merge_request_reviewer,
+          reviewer: author, merge_request: merge_request, state: 'unreviewed'
+        )
+      end
+
+      it 'does not change Merge Request reviewers' do
+        expect { subject.execute }.not_to change(MergeRequestReviewer, :count)
+
+        expect(merge_request.reviewers).to contain_exactly(author)
+      end
+    end
+  end
+
+  shared_examples 'imports an approval for the Merge Request' do
+    it 'creates an approval for the Merge Request' do
+      expect { subject.execute }.to change(Approval, :count).by(1)
+
+      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 author can be mapped to a gitlab user' do
     let_it_be(:author) { create(:user, email: 'author@email.com') }
 
@@ -20,34 +57,38 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
       context 'when the review is "APPROVED"' do
         let(:review) { create_review(type: 'APPROVED', note: '') }
 
-        it 'creates a note for the review and approves the Merge Request' do
-          expect { subject.execute }
-            .to change(Note, :count).by(1)
-            .and change(Approval, :count).by(1)
+        it_behaves_like 'imports an approval for the Merge Request'
+        it_behaves_like 'imports a reviewer for the Merge Request'
+
+        it 'creates a note for the review' do
+          expect { subject.execute }.to change(Note, :count).by(1)
 
           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
 
-        it 'does nothing if the user already approved the merge request' do
-          create(:approval, merge_request: merge_request, user: author)
+        context 'when the user already approved the merge request' do
+          before do
+            create(:approval, merge_request: merge_request, user: author)
+          end
 
-          expect { subject.execute }
-            .to change(Note, :count).by(0)
-            .and change(Approval, :count).by(0)
+          it 'does not import second approve and note' do
+            expect { subject.execute }
+              .to change(Note, :count).by(0)
+              .and change(Approval, :count).by(0)
+          end
         end
       end
 
       context 'when the review is "COMMENTED"' do
         let(:review) { create_review(type: 'COMMENTED', note: '') }
 
-        it 'creates a note for the review' do
+        it_behaves_like 'imports a reviewer for the Merge Request'
+
+        it 'does not create note for the review' do
           expect { subject.execute }.not_to change(Note, :count)
         end
       end
@@ -55,7 +96,9 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
       context 'when the review is "CHANGES_REQUESTED"' do
         let(:review) { create_review(type: 'CHANGES_REQUESTED', note: '') }
 
-        it 'creates a note for the review' do
+        it_behaves_like 'imports a reviewer for the Merge Request'
+
+        it 'does not create a note for the review' do
           expect { subject.execute }.not_to change(Note, :count)
         end
       end
@@ -65,10 +108,11 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
       context 'when the review is "APPROVED"' do
         let(:review) { create_review(type: 'APPROVED') }
 
+        it_behaves_like 'imports an approval for the Merge Request'
+        it_behaves_like 'imports a reviewer for the Merge Request'
+
         it 'creates a note for the review' do
-          expect { subject.execute }
-            .to change(Note, :count).by(2)
-            .and change(Approval, :count).by(1)
+          expect { subject.execute }.to change(Note, :count).by(2)
 
           note = merge_request.notes.where(system: false).last
           expect(note.note).to eq("**Review:** Approved\n\nnote")
@@ -80,9 +124,6 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
           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