Commit b9a7ee94 authored by Kassio Borges's avatar Kassio Borges

GithubImporter: Avoid validation exception

Avoid creating Merge Request approval if it already exists. This will
avoid the following exception: `ActiveRecord::RecordInvalid: Validation
failed: User has already been taken`

Related to:
- https://gitlab.com/gitlab-org/gitlab/-/issues/296107
parent 778045ee
---
title: 'GithubImporter: Add Merge request approval only if it does not exists yet'
merge_request: 55376
author:
type: fixed
......@@ -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)
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment