Merge branch '212754-reset-approval-rules-be' into 'master'

Support resetting approval rules to defaults

See merge request gitlab-org/gitlab!32744

ee/app/controllers/ee/projects/merge_requests/application_controller.rb

@@ -29,7 +29,8 @@ module EE
             approval_rule_attributes,
             :approvals_before_merge,
             :approver_group_ids,
-            :approver_ids
+            :approver_ids,
+            :reset_approval_rules_to_defaults
           )
 
           attrs

ee/app/services/approval_rules/params_filtering_service.rb

@@ -22,6 +22,7 @@ module ApprovalRules
 
     def execute
       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)
 
@@ -119,8 +120,8 @@ module ApprovalRules
       end
     end
 
-    # Append inapplicable rules on create so they're still associated to the MR
-    # and will be available when the MR's target branch changes.
+    # Append inapplicable rules on create or reset so they're still associated
+    # to the MR and will be available when the MR's target branch changes.
     #
     # Inapplicable rules are approval rules scoped to protected branches that
     # does not match the specified `target_branch`.
@@ -130,7 +131,7 @@ module ApprovalRules
     # inapplicable. But in case the MR's `target_branch` changes to `master`, the
     # `master` rule should be available.
     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?
 
       project

ee/app/services/ee/merge_requests/update_service.rb

@@ -14,6 +14,8 @@ module EE
           old_approvers = merge_request.overall_approvers(exclude_code_owners: true)
         end
 
+        reset_approval_rules(merge_request) if params.delete(:reset_approval_rules_to_defaults)
+
         merge_request = super(merge_request)
 
         if should_remove_old_approvers && merge_request.valid?
@@ -55,6 +57,12 @@ module EE
 
         ::Analytics::RefreshReassignData.new(merge_request).execute_async
       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

ee/spec/services/approval_rules/params_filtering_service_spec.rb

@@ -182,6 +182,7 @@ describe ApprovalRules::ParamsFilteringService do
       context 'inapplicable user defined rules' do
         let!(:source_rule) { create(:approval_project_rule, project: project) }
         let(:protected_branch) { create(:protected_branch, project: project, name: 'stable-*') }
+        let(:approval_rules_attrs) { service.execute[:approval_rules_attributes] }
 
         let(:params) do
           {
@@ -196,10 +197,63 @@ describe ApprovalRules::ParamsFilteringService do
         end
 
         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
 
-          expect(params[:approval_rules_attributes].size).to eq(1)
-          expect(params[:approval_rules_attributes].first[:name]).to eq('foo')
+        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
+
+          context 'when multiple_approval_rules feature is available' do
+            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
 

ee/spec/services/ee/merge_requests/update_service_spec.rb

@@ -241,5 +241,81 @@ describe MergeRequests::UpdateService, :mailer do
         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