Commit 4a7fc360 authored by Patrick Bajao's avatar Patrick Bajao

Support resetting approval rules to defaults

A new `reset_approval_rules_to_defaults` param that will be sent
by the frontend when approval rules are set to be reset to project
defaults.

When that param is set to `true`, the backend will delete all
existing any_approver/regular rules of the merge request. It will
also append inapplicable rules so it'll behave like creating rules
on MR creation.
parent dab25026
...@@ -29,7 +29,8 @@ module EE ...@@ -29,7 +29,8 @@ module EE
approval_rule_attributes, approval_rule_attributes,
:approvals_before_merge, :approvals_before_merge,
:approver_group_ids, :approver_group_ids,
:approver_ids :approver_ids,
:reset_approval_rules_to_defaults
) )
attrs attrs
......
...@@ -22,6 +22,7 @@ module ApprovalRules ...@@ -22,6 +22,7 @@ module ApprovalRules
def execute def execute
params.delete(:approval_rules_attributes) unless current_user.can?(:update_approvers, target) params.delete(:approval_rules_attributes) unless current_user.can?(:update_approvers, target)
params.delete(:reset_approval_rules_to_defaults) unless updating?
return params unless params.key?(:approval_rules_attributes) return params unless params.key?(:approval_rules_attributes)
...@@ -119,8 +120,8 @@ module ApprovalRules ...@@ -119,8 +120,8 @@ module ApprovalRules
end end
end end
# Append inapplicable rules on create so they're still associated to the MR # Append inapplicable rules on create or reset so they're still associated
# and will be available when the MR's target branch changes. # to the MR and will be available when the MR's target branch changes.
# #
# Inapplicable rules are approval rules scoped to protected branches that # Inapplicable rules are approval rules scoped to protected branches that
# does not match the specified `target_branch`. # does not match the specified `target_branch`.
...@@ -130,7 +131,7 @@ module ApprovalRules ...@@ -130,7 +131,7 @@ module ApprovalRules
# inapplicable. But in case the MR's `target_branch` changes to `master`, the # inapplicable. But in case the MR's `target_branch` changes to `master`, the
# `master` rule should be available. # `master` rule should be available.
def append_user_defined_inapplicable_rules(source_rule_ids) def append_user_defined_inapplicable_rules(source_rule_ids)
return if updating? return if updating? && !params[:reset_approval_rules_to_defaults]
return unless project.multiple_approval_rules_available? return unless project.multiple_approval_rules_available?
project project
......
...@@ -14,6 +14,8 @@ module EE ...@@ -14,6 +14,8 @@ module EE
old_approvers = merge_request.overall_approvers(exclude_code_owners: true) old_approvers = merge_request.overall_approvers(exclude_code_owners: true)
end end
reset_approval_rules(merge_request) if params.delete(:reset_approval_rules_to_defaults)
merge_request = super(merge_request) merge_request = super(merge_request)
if should_remove_old_approvers && merge_request.valid? if should_remove_old_approvers && merge_request.valid?
...@@ -55,6 +57,12 @@ module EE ...@@ -55,6 +57,12 @@ module EE
::Analytics::RefreshReassignData.new(merge_request).execute_async ::Analytics::RefreshReassignData.new(merge_request).execute_async
end end
def reset_approval_rules(merge_request)
return unless merge_request.project.can_override_approvers?
merge_request.approval_rules.regular_or_any_approver.delete_all
end
end end
end end
end end
...@@ -182,6 +182,7 @@ describe ApprovalRules::ParamsFilteringService do ...@@ -182,6 +182,7 @@ describe ApprovalRules::ParamsFilteringService do
context 'inapplicable user defined rules' do context 'inapplicable user defined rules' do
let!(:source_rule) { create(:approval_project_rule, project: project) } let!(:source_rule) { create(:approval_project_rule, project: project) }
let(:protected_branch) { create(:protected_branch, project: project, name: 'stable-*') } let(:protected_branch) { create(:protected_branch, project: project, name: 'stable-*') }
let(:approval_rules_attrs) { service.execute[:approval_rules_attributes] }
let(:params) do let(:params) do
{ {
...@@ -196,10 +197,63 @@ describe ApprovalRules::ParamsFilteringService do ...@@ -196,10 +197,63 @@ describe ApprovalRules::ParamsFilteringService do
end end
it 'does not add inapplicable user defined rules' do it 'does not add inapplicable user defined rules' do
params = service.execute aggregate_failures do
expect(approval_rules_attrs.size).to eq(1)
expect(approval_rules_attrs.first[:name]).to eq('foo')
end
end
context 'when reset_approval_rules_to_defaults is true' do
let(:params) do
{
approval_rules_attributes: [
{ id: rule1.id, name: 'foo', user_ids: [project_member.id, outsider.id] }
],
reset_approval_rules_to_defaults: true
}
end
expect(params[:approval_rules_attributes].size).to eq(1) context 'when multiple_approval_rules feature is available' do
expect(params[:approval_rules_attributes].first[:name]).to eq('foo') before do
stub_licensed_features(multiple_approval_rules: true)
end
it 'adds inapplicable user defined rules' do
aggregate_failures do
expect(approval_rules_attrs.size).to eq(2)
expect(approval_rules_attrs.first).to include(
id: rule1.id,
name: 'foo'
)
expect(approval_rules_attrs.last).to include(
name: source_rule.name,
approval_project_rule_id: source_rule.id,
user_ids: source_rule.user_ids,
group_ids: source_rule.group_ids,
approvals_required: source_rule.approvals_required,
rule_type: source_rule.rule_type
)
end
end
end
context 'when multiple_approval_rules feature is not available' do
before do
stub_licensed_features(multiple_approval_rules: false)
end
it 'does not add inapplicable user defined rules' do
aggregate_failures do
expect(approval_rules_attrs.size).to eq(1)
expect(approval_rules_attrs.first).to include(
id: rule1.id,
name: 'foo'
)
end
end
end
end end
end end
......
...@@ -241,5 +241,81 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -241,5 +241,81 @@ describe MergeRequests::UpdateService, :mailer do
end end
end end
end end
context 'reset_approval_rules_to_defaults param' do
let!(:existing_any_rule) { create(:any_approver_rule, merge_request: merge_request) }
let!(:existing_rule) { create(:approval_merge_request_rule, merge_request: merge_request) }
let(:rules) { merge_request.reload.approval_rules }
shared_examples_for 'undeletable existing approval rules' do
it 'does not delete existing approval rules' do
aggregate_failures do
expect(rules).not_to be_empty
expect(rules).to include(existing_any_rule)
expect(rules).to include(existing_rule)
end
end
end
context 'when approval rules can be overridden' do
before do
merge_request.project.update!(disable_overriding_approvers_per_merge_request: false)
end
context 'when not set' do
before do
update_merge_request({})
end
it_behaves_like 'undeletable existing approval rules'
end
context 'when set to false' do
before do
update_merge_request(reset_approval_rules_to_defaults: false)
end
it_behaves_like 'undeletable existing approval rules'
end
context 'when set to true' do
context 'and approval_rules_attributes param is not set' do
before do
update_merge_request(reset_approval_rules_to_defaults: true)
end
it 'deletes existing approval rules' do
expect(rules).to be_empty
end
end
context 'and approval_rules_attributes param is set' do
before do
update_merge_request(
reset_approval_rules_to_defaults: true,
approval_rules_attributes: [{ name: 'New Rule', approvals_required: 1 }]
)
end
it 'deletes existing approval rules and creates new one' do
aggregate_failures do
expect(rules.size).to eq(1)
expect(rules).not_to include(existing_any_rule)
expect(rules).not_to include(existing_rule)
end
end
end
end
end
context 'when approval rules cannot be overridden' do
before do
merge_request.project.update!(disable_overriding_approvers_per_merge_request: true)
update_merge_request(reset_approval_rules_to_defaults: true)
end
it_behaves_like 'undeletable existing approval rules'
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