Merge branch 'kassio/githubimporter-avoid-approval-exception' into 'master'

GithubImporter: Avoid validation exception

See merge request gitlab-org/gitlab!55376

changelogs/unreleased/kassio-githubimporter-avoid-approval-exception.yml

+---
+title: 'GithubImporter: Add Merge request approval only if it does not exists yet'
+merge_request: 55376
+author:
+type: fixed

lib/gitlab/github_import/importer/pull_request_review_importer.rb

@@ -77,12 +77,22 @@ module Gitlab
         def add_approval!(user_id)
           return unless review.review_type == 'APPROVED'
 
-          add_approval_system_note!(user_id)
-
-          merge_request.approvals.create!(
+          approval_attribues = {
+            merge_request_id: merge_request.id,
             user_id: user_id,
-            created_at: review.submitted_at
+            created_at: review.submitted_at,
+            updated_at: review.submitted_at
+          }
+
+          result = ::Approval.insert(
+            approval_attribues,
+            returning: [:id],
+            unique_by: [:user_id, :merge_request_id]
           )
+
+          if result.rows.present?
+            add_approval_system_note!(user_id)
+          end
         end
 
         def add_approval_system_note!(user_id)

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

@@ -19,8 +19,10 @@ 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' do
-          expect { subject.execute }.to change(Note, :count)
+        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)
 
           last_note = merge_request.notes.last
           expect(last_note.note).to eq('approved this merge request')
@@ -31,6 +33,14 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean
           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)
+
+          expect { subject.execute }
+            .to change(Note, :count).by(0)
+            .and change(Approval, :count).by(0)
+        end
       end
 
       context 'when the review is "COMMENTED"' do