Commit 7938a5d6 authored by Mark Chao's avatar Mark Chao

Ensure approver references are all present

When assigning with _ids, if an id is no longer present,
Rails would raise error.
parent 32d449d1
...@@ -10,10 +10,12 @@ module Gitlab ...@@ -10,10 +10,12 @@ module Gitlab
class Approver < ActiveRecord::Base class Approver < ActiveRecord::Base
self.table_name = 'approvers' self.table_name = 'approvers'
belongs_to :user
end end
class ApproverGroup < ActiveRecord::Base class ApproverGroup < ActiveRecord::Base
self.table_name = 'approver_groups' self.table_name = 'approver_groups'
belongs_to :group
end end
class ApprovalMergeRequestRule < ActiveRecord::Base class ApprovalMergeRequestRule < ActiveRecord::Base
...@@ -56,11 +58,11 @@ module Gitlab ...@@ -56,11 +58,11 @@ module Gitlab
has_many :approval_rules, class_name: 'ApprovalMergeRequestRule' has_many :approval_rules, class_name: 'ApprovalMergeRequestRule'
def approver_ids def approver_ids
@approver_ids ||= Approver.where(target_type: 'MergeRequest', target_id: id).pluck('distinct user_id') @approver_ids ||= Approver.where(target_type: 'MergeRequest', target_id: id).joins(:user).pluck('distinct user_id')
end end
def approver_group_ids def approver_group_ids
@approver_group_ids ||= ApproverGroup.where(target_type: 'MergeRequest', target_id: id).pluck('distinct group_id') @approver_group_ids ||= ApproverGroup.where(target_type: 'MergeRequest', target_id: id).joins(:group).pluck('distinct group_id')
end end
def sync_code_owners_with_approvers def sync_code_owners_with_approvers
...@@ -78,11 +80,11 @@ module Gitlab ...@@ -78,11 +80,11 @@ module Gitlab
has_many :approval_rules, class_name: 'ApprovalProjectRule' has_many :approval_rules, class_name: 'ApprovalProjectRule'
def approver_ids def approver_ids
@approver_ids ||= Approver.where(target_type: 'Project', target_id: id).pluck('distinct user_id') @approver_ids ||= Approver.where(target_type: 'Project', target_id: id).joins(:user).pluck('distinct user_id')
end end
def approver_group_ids def approver_group_ids
@approver_group_ids ||= ApproverGroup.where(target_type: 'Project', target_id: id).pluck('distinct group_id') @approver_group_ids ||= ApproverGroup.where(target_type: 'Project', target_id: id).joins(:group).pluck('distinct group_id')
end end
end end
......
...@@ -74,6 +74,27 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do ...@@ -74,6 +74,27 @@ describe Gitlab::BackgroundMigration::MigrateApproverToApprovalRules do
expect(approval_rule.groups).to contain_exactly(group2) expect(approval_rule.groups).to contain_exactly(group2)
end end
end end
context 'when approver has user_id which no longer exists' do
before do
create_member_in(user1, :old_schema)
create_member_in(user2, :old_schema)
create_member_in(group1, :old_schema)
create_member_in(group2, :old_schema)
end
it 'ignores broken reference when assigning' do
User.where(id: user1).delete_all
Group.where(id: group1).delete_all
described_class.new.perform(target_type, target.id)
approval_rule = target.approval_rules.first
expect(approval_rule.users).to contain_exactly(user2)
expect(approval_rule.groups).to contain_exactly(group2)
end
end
end end
context 'merge request' do context 'merge request' 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