Commit fcc6f275 authored by Gary Holtz's avatar Gary Holtz

Removes code that hides a source rule

This includes some of the tests as a result
parent 35032389
...@@ -205,7 +205,7 @@ class ApprovalState ...@@ -205,7 +205,7 @@ class ApprovalState
def wrapped_rules def wrapped_rules
strong_memoize(:wrapped_rules) do strong_memoize(:wrapped_rules) do
merge_request_rules = merge_request.approval_rules.applicable_to_branch(target_branch) merge_request_rules = merge_request.approval_rules.preload_users
merge_request_rules.map! do |rule| merge_request_rules.map! do |rule|
ApprovalWrappedRule.wrap(merge_request, rule) ApprovalWrappedRule.wrap(merge_request, rule)
......
...@@ -20,17 +20,16 @@ module EE ...@@ -20,17 +20,16 @@ module EE
has_many :approver_users, through: :approvers, source: :user has_many :approver_users, through: :approvers, source: :user
has_many :approver_groups, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :approver_groups, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :approval_rules, class_name: 'ApprovalMergeRequestRule', inverse_of: :merge_request do has_many :approval_rules, class_name: 'ApprovalMergeRequestRule', inverse_of: :merge_request do
def applicable_to_branch(branch) def preload_users
ActiveRecord::Associations::Preloader.new.preload( ActiveRecord::Associations::Preloader.new.preload(
self, self,
[:users, :groups, approval_project_rule: [:users, :groups, :protected_branches]] [:users, :groups, approval_project_rule: [:users, :groups, :protected_branches]]
) )
self.select do |rule| self.select do |rule|
next true unless rule.approval_project_rule.present?
next true if rule.overridden? next true if rule.overridden?
rule.approval_project_rule.applies_to_branch?(branch) rule
end end
end end
end end
......
---
title: Fixes a bug with merge request approval rules being changed after creation
merge_request: 43209
author:
type: fixed
...@@ -1482,9 +1482,9 @@ RSpec.describe ApprovalState do ...@@ -1482,9 +1482,9 @@ RSpec.describe ApprovalState do
) )
end end
it 'returns the rules that are applicable to the merge request target branch' do it 'returns the rules that are applicable to the merge request' do
expect(subject.user_defined_rules.map(&:approval_rule)).to eq([ expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
another_mr_rule mr_rule, another_mr_rule
]) ])
end end
...@@ -1493,7 +1493,7 @@ RSpec.describe ApprovalState do ...@@ -1493,7 +1493,7 @@ RSpec.describe ApprovalState do
it 'returns the rules that are applicable to the specified target_branch' do it 'returns the rules that are applicable to the specified target_branch' do
expect(subject.user_defined_rules.map(&:approval_rule)).to eq([ expect(subject.user_defined_rules.map(&:approval_rule)).to eq([
mr_rule mr_rule, another_mr_rule
]) ])
end end
end end
......
...@@ -27,72 +27,6 @@ RSpec.describe MergeRequest do ...@@ -27,72 +27,6 @@ RSpec.describe MergeRequest do
it { is_expected.to have_many(:approval_rules) } it { is_expected.to have_many(:approval_rules) }
it { is_expected.to have_many(:approval_merge_request_rule_sources).through(:approval_rules) } it { is_expected.to have_many(:approval_merge_request_rule_sources).through(:approval_rules) }
it { is_expected.to have_many(:approval_project_rules).through(:approval_merge_request_rule_sources) } it { is_expected.to have_many(:approval_project_rules).through(:approval_merge_request_rule_sources) }
describe 'approval_rules association' do
describe '#applicable_to_branch' do
let!(:rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
let(:branch) { 'stable' }
subject { merge_request.approval_rules.applicable_to_branch(branch) }
shared_examples_for 'with applicable rules to specified branch' do
it { is_expected.to eq([rule]) }
end
context 'when there are no associated source rules' do
it_behaves_like 'with applicable rules to specified branch'
end
context 'when there are associated source rules' do
let(:source_rule) { create(:approval_project_rule, project: merge_request.target_project) }
before do
rule.update!(approval_project_rule: source_rule)
end
context 'and rule is not overridden' do
before do
rule.update!(
name: source_rule.name,
approvals_required: source_rule.approvals_required,
users: source_rule.users,
groups: source_rule.groups
)
end
context 'and there are no associated protected branches to source rule' do
it_behaves_like 'with applicable rules to specified branch'
end
context 'and there are associated protected branches to source rule' do
before do
source_rule.update!(protected_branches: protected_branches)
end
context 'and branch matches' do
let(:protected_branches) { [create(:protected_branch, name: branch)] }
it_behaves_like 'with applicable rules to specified branch'
end
context 'and branch does not match anything' do
let(:protected_branches) { [create(:protected_branch, name: branch.reverse)] }
it { is_expected.to be_empty }
end
end
end
context 'and rule is overridden' do
before do
rule.update!(name: 'Overridden Rule')
end
it_behaves_like 'with applicable rules to specified branch'
end
end
end
end
end end
it_behaves_like 'an editable mentionable with EE-specific mentions' do it_behaves_like 'an editable mentionable with EE-specific mentions' 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