Commit 6f6f91d1 authored by Mark Chao's avatar Mark Chao

Sync code owner changes to approval rules

Update code owner rule when MR is updated.
parent 8cabf62b
...@@ -66,3 +66,5 @@ module MergeRequests ...@@ -66,3 +66,5 @@ module MergeRequests
end end
end end
end end
MergeRequests::CreateService.include(EE::MergeRequests::CreateService)
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
class ApprovalMergeRequestRule < ApplicationRecord class ApprovalMergeRequestRule < ApplicationRecord
include ApprovalRuleLike include ApprovalRuleLike
DEFAULT_NAME_FOR_CODE_OWNER = 'Code Owner'
scope :regular, -> { where(code_owner: false) } scope :regular, -> { where(code_owner: false) }
scope :code_owner, -> { where(code_owner: true) } # special code owner rules, updated internally when code changes scope :code_owner, -> { where(code_owner: true) } # special code owner rules, updated internally when code changes
......
...@@ -81,5 +81,14 @@ module EE ...@@ -81,5 +81,14 @@ module EE
compare_reports(::Ci::CompareLicenseManagementReportsService) compare_reports(::Ci::CompareLicenseManagementReportsService)
end end
def sync_code_owners_with_approvers
ActiveRecord::Base.transaction do
rule = approval_rules.code_owner.first
rule ||= approval_rules.code_owner.create!(name: ApprovalMergeRequestRule::DEFAULT_NAME_FOR_CODE_OWNER)
rule.users = code_owners
end
end
end end
end end
# frozen_string_literal: true
module EE
module MergeRequests
module CreateService
extend ::Gitlab::Utils::Override
override :after_create
def after_create(issuable)
super
issuable.sync_code_owners_with_approvers
end
end
end
end
...@@ -53,6 +53,8 @@ module EE ...@@ -53,6 +53,8 @@ module EE
new_code_owners = merge_request.code_owners - previous_code_owners new_code_owners = merge_request.code_owners - previous_code_owners
create_approvers(merge_request, new_code_owners) create_approvers(merge_request, new_code_owners)
merge_request.sync_code_owners_with_approvers
end end
results results
......
...@@ -70,6 +70,41 @@ describe MergeRequest do ...@@ -70,6 +70,41 @@ describe MergeRequest do
end end
end end
describe '#sync_code_owners_with_approvers' do
let(:owners) { [create(:user), create(:user)] }
before do
allow(subject).to receive(:code_owners).and_return(owners)
end
it 'sync code owner to the code owner rule' do
expect do
subject.sync_code_owners_with_approvers
end.to change { subject.approval_rules.count }.by(1)
expect(subject.approval_rules.code_owner.first.users).to contain_exactly(*owners)
end
context 'when code owner rule already exists' do
let!(:code_owner_rule) { subject.approval_rules.code_owner.create!(name: 'Code Owner') }
before do
code_owner_rule.users << create(:user)
end
it 'reuses existing rule' do
expect do
subject.sync_code_owners_with_approvers
end.not_to change { subject.approval_rules.count }
rule = subject.approval_rules.code_owner.first
expect(rule).to eq(code_owner_rule)
expect(rule.users).to contain_exactly(*owners)
end
end
end
describe '#base_pipeline' do describe '#base_pipeline' do
let!(:pipeline) { create(:ci_empty_pipeline, project: subject.project, sha: subject.diff_base_sha) } let!(:pipeline) { create(:ci_empty_pipeline, project: subject.project, sha: subject.diff_base_sha) }
......
# frozen_string_literal: true
require 'spec_helper'
describe MergeRequests::CreateService do
include ProjectForksHelper
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:service) { described_class.new(project, user, opts) }
let(:opts) do
{
title: 'Awesome merge_request',
description: 'please fix',
source_branch: 'feature',
target_branch: 'master',
force_remove_source_branch: '1'
}
end
before do
project.add_maintainer(user)
allow(service).to receive(:execute_hooks)
end
describe '#execute' do
context 'code owners' do
let!(:owners) { create_list(:user, 2) }
before do
allow(::Gitlab::CodeOwners).to receive(:for_merge_request).and_return(owners)
end
it 'syncs code owner to approval rule' do
merge_request = service.execute
rule = merge_request.approval_rules.code_owner.first
expect(rule.users).to contain_exactly(*owners)
end
end
end
end
...@@ -102,6 +102,9 @@ describe MergeRequests::RefreshService do ...@@ -102,6 +102,9 @@ describe MergeRequests::RefreshService do
it 'does not create Approver' do it 'does not create Approver' do
expect { subject }.not_to change { Approver.count } expect { subject }.not_to change { Approver.count }
rule = merge_request.approval_rules.code_owner.first
expect(rule.users).to eq(new_owners)
end end
end end
end end
...@@ -125,6 +128,9 @@ describe MergeRequests::RefreshService do ...@@ -125,6 +128,9 @@ describe MergeRequests::RefreshService do
expect(new_approver.user).to eq(owner) expect(new_approver.user).to eq(owner)
expect(new_approver.created_at).to be_present expect(new_approver.created_at).to be_present
expect(new_approver.updated_at).to be_present expect(new_approver.updated_at).to be_present
rule = merge_request.approval_rules.code_owner.first
expect(rule.users).to eq(new_owners)
end end
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