Commit ed1e78d8 authored by Mark Chao's avatar Mark Chao

Fix MigrateApproverToApprovalRules migration

At this stage of migration, the rule_type column is not yet present,
therefore in migration the older column must be referenced referenced,
meaning older logic is copied into
Gitlab::BackgroundMigration::MigrateApproverToApprovalRules
parent 5a2b1e0c
---
title: Fix "rule_type does not exist" error during consume_remaining_migrate_approver_to_approval_rules_in_batch_jobs migration
merge_request: 13947
author:
type: fixed
...@@ -33,6 +33,32 @@ module Gitlab ...@@ -33,6 +33,32 @@ module Gitlab
def project def project
merge_request.target_project merge_request.target_project
end end
def self.find_or_create_code_owner_rule(merge_request, pattern)
merge_request.approval_rules.safe_find_or_create_by(
code_owner: true,
name: pattern
)
end
def self.safe_find_or_create_by(*args)
safe_ensure_unique(retries: 1) do
find_or_create_by(*args)
end
end
def self.safe_ensure_unique(retries: 0)
transaction(requires_new: true) do
yield
end
rescue ActiveRecord::RecordNotUnique
if retries > 0
retries -= 1
retry
end
false
end
end end
class ApprovalMergeRequestRuleSource < ActiveRecord::Base class ApprovalMergeRequestRuleSource < ActiveRecord::Base
...@@ -73,7 +99,21 @@ module Gitlab ...@@ -73,7 +99,21 @@ module Gitlab
return if state == 'merged' || state == 'closed' return if state == 'merged' || state == 'closed'
Gitlab::GitalyClient.allow_n_plus_1_calls do Gitlab::GitalyClient.allow_n_plus_1_calls do
::MergeRequest.find(id).sync_code_owners_with_approvers owners = ::MergeRequest.find(id).code_owners
if owners.present?
ApplicationRecord.transaction do
rule = approval_rules.code_owner.first
rule ||= ApprovalMergeRequestRule.find_or_create_code_owner_rule(
self,
'Code Owner'
)
rule.users = owners.uniq
end
else
approval_rules.code_owner.delete_all
end
end end
end end
end end
......
...@@ -203,9 +203,12 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do ...@@ -203,9 +203,12 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
it 'creates rule' do it 'creates rule' do
expect do expect do
described_class.new.perform(target_type, target.id) described_class.new.perform(target_type, target.id)
end.to change { target.approval_rules.code_owner.count }.by(1) end.to change { target.approval_rules.count }.by(1)
expect(target.approval_rules.code_owner.first.users).to contain_exactly(*owners) rule = target.approval_rules.first
expect(rule.read_attribute(:code_owner)).to eq(true)
expect(rule.users).to contain_exactly(*owners)
end end
end end
......
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