From f35c13675d1acdd3a19c9abb8c52a5a4ba15568f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Thu, 15 Dec 2022 00:28:53 +0800
Subject: [PATCH 1/8] Add backend support for group level MR settings

---
 .../settings/merge_requests_controller.rb     | 40 +++++++++++
 ee/app/models/ee/namespace_setting.rb         | 33 +++++++++
 ee/app/models/ee/project.rb                   | 22 ++++++
 .../models/gitlab_subscriptions/features.rb   |  1 +
 ee/config/routes/group.rb                     |  1 +
 .../merge_requests_controller_spec.rb         | 69 +++++++++++++++++++
 ee/spec/models/namespace_setting_spec.rb      | 45 ++++++++++++
 ee/spec/models/project_spec.rb                | 68 ++++++++++++++++++
 8 files changed, 279 insertions(+)
 create mode 100644 ee/app/controllers/groups/settings/merge_requests_controller.rb
 create mode 100644 ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb

diff --git a/ee/app/controllers/groups/settings/merge_requests_controller.rb b/ee/app/controllers/groups/settings/merge_requests_controller.rb
new file mode 100644
index 00000000000000..4b034e2448b415
--- /dev/null
+++ b/ee/app/controllers/groups/settings/merge_requests_controller.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Groups
+  module Settings
+    class MergeRequestsController < Groups::ApplicationController
+      layout 'group_settings'
+
+      before_action :authorize_admin_group!
+
+      feature_category :code_review
+
+      def update
+        return render_404 unless @group.licensed_feature_available?(:group_level_merge_checks_setting)
+
+        if Groups::UpdateService.new(@group, current_user, group_settings_params).execute
+          notice = "Group '#{@group.name}' was successfully updated."
+          redirect_to edit_group_path(@group, anchor: 'js-merge-requests-settings'), notice: notice
+        else
+          @group.reset
+          alert = "Group '#{@group.name}' could not be updated."
+          redirect_to edit_group_path(@group, anchor: 'js-merge-requests-settings'), alert: alert
+        end
+      end
+
+      private
+
+      def group_settings_params
+        params.require(:namespace_setting).permit(
+          %i[
+            only_allow_merge_if_pipeline_succeeds
+            allow_merge_on_skipped_pipeline
+            only_allow_merge_if_all_discussions_are_resolved
+          ]
+        )
+      end
+    end
+  end
+end
+
+Groups::Settings::MergeRequestsController.prepend_mod
diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb
index a56fd2a7dc1b77..791349434083ca 100644
--- a/ee/app/models/ee/namespace_setting.rb
+++ b/ee/app/models/ee/namespace_setting.rb
@@ -32,6 +32,36 @@ def prevent_forking_outside_group?
         saml_setting || root_ancestor.namespace_settings&.prevent_forking_outside_group
       end
 
+      # Defines instance methods:
+      #
+      # - only_allow_merge_if_pipeline_succeeds?(inherit_group_setting: false)
+      # - allow_merge_on_skipped_pipeline?(inherit_group_setting: false)
+      # - only_allow_merge_if_all_discussions_are_resolved?(inherit_group_setting: false)
+      # - only_allow_merge_if_pipeline_succeeds_locked?
+      # - allow_merge_on_skipped_pipeline_locked?
+      # - only_allow_merge_if_all_discussions_are_resolved_locked?
+      def self.cascading_with_parent_namespace(attribute)
+        define_method("#{attribute}_of_parent_group") do
+          namespace&.parent&.namespace_settings&.public_send("#{attribute}?", inherit_group_setting: true)
+        end
+
+        define_method("#{attribute}?") do |inherit_group_setting: false|
+          if !inherit_group_setting
+            public_send(attribute.to_s) # rubocop:disable GitlabSecurity/PublicSend
+          else
+            public_send(attribute.to_s) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+          end
+        end
+
+        define_method("#{attribute}_locked?") do
+          public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+        end
+      end
+
+      cascading_with_parent_namespace :only_allow_merge_if_pipeline_succeeds
+      cascading_with_parent_namespace :allow_merge_on_skipped_pipeline
+      cascading_with_parent_namespace :only_allow_merge_if_all_discussions_are_resolved
+
       private
 
       def enabling_user_cap?
@@ -68,6 +98,9 @@ def user_cap_enabled?
         unique_project_download_limit_allowlist
         auto_ban_user_on_excessive_projects_download
         default_compliance_framework_id
+        only_allow_merge_if_pipeline_succeeds
+        allow_merge_on_skipped_pipeline
+        only_allow_merge_if_all_discussions_are_resolved
       ].freeze
 
       override :allowed_namespace_settings_params
diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index 4ff6954d07329b..c166600277db3d 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -322,6 +322,28 @@ def custom_roles_enabled?
       end
     end
 
+    def self.cascading_with_parent_namespace(attribute)
+      define_method("#{attribute}_of_parent_group") do
+        self.group&.namespace_settings&.public_send("#{attribute}?", inherit_group_setting: true)
+      end
+
+      define_method("#{attribute}?") do |inherit_group_setting: false|
+        return super() unless licensed_feature_available?(:group_level_merge_checks_setting)
+
+        self.public_send(attribute) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+      end
+
+      define_method("#{attribute}_locked?") do
+        return super() unless licensed_feature_available?(:group_level_merge_checks_setting)
+
+        public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+      end
+    end
+
+    cascading_with_parent_namespace :only_allow_merge_if_pipeline_succeeds
+    cascading_with_parent_namespace :allow_merge_on_skipped_pipeline
+    cascading_with_parent_namespace :only_allow_merge_if_all_discussions_are_resolved
+
     def mirror_last_update_succeeded?
       !!import_state&.last_update_succeeded?
     end
diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb
index a19e25aecaf56d..0982104448b634 100644
--- a/ee/app/models/gitlab_subscriptions/features.rb
+++ b/ee/app/models/gitlab_subscriptions/features.rb
@@ -162,6 +162,7 @@ class Features
       coverage_check_approval_rule
       issuable_resource_links
       group_protected_branches
+      group_level_merge_checks_setting
     ].freeze
 
     ULTIMATE_FEATURES = %i[
diff --git a/ee/config/routes/group.rb b/ee/config/routes/group.rb
index 0c5cce66fbd57a..c022432775e81e 100644
--- a/ee/config/routes/group.rb
+++ b/ee/config/routes/group.rb
@@ -10,6 +10,7 @@
     namespace :settings do
       resource :reporting, only: [:show], controller: 'reporting'
       resource :domain_verification, only: [:show], controller: 'domain_verification'
+      resource :merge_requests, only: [:update]
     end
 
     resources :group_members, only: [], concerns: :access_requestable do
diff --git a/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb b/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
new file mode 100644
index 00000000000000..449999f2b00005
--- /dev/null
+++ b/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::Settings::MergeRequestsController do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'PATCH #update' do
+    subject do
+      patch :update, params: {
+        group_id: group,
+        namespace_setting: {
+          only_allow_merge_if_pipeline_succeeds: true,
+          allow_merge_on_skipped_pipeline: true,
+          only_allow_merge_if_all_discussions_are_resolved: true
+        }
+      }
+    end
+
+    context 'when user is not an admin' do
+      before do
+        group.add_owner(user)
+      end
+
+      it { is_expected.to have_gitlab_http_status(:not_found) }
+    end
+
+    context 'when user is an admin' do
+      let(:user) { create(:admin) }
+
+      before do
+        stub_licensed_features(group_level_merge_checks_setting: true)
+        group.add_owner(user)
+      end
+
+      it { is_expected.to redirect_to(edit_group_path(group, anchor: 'js-merge-requests-settings')) }
+
+      context 'when service execution went wrong' do
+        let(:update_service) { double }
+
+        before do
+          allow(Groups::UpdateService).to receive(:new).and_return(update_service)
+          allow(update_service).to receive(:execute).and_return(false)
+
+          subject
+        end
+
+        it 'returns a flash alert' do
+          expect(controller).to set_flash[:alert]
+            .to eq("Group '#{group.name}' could not be updated.")
+        end
+      end
+
+      context 'when service execution was successful' do
+        it 'returns a flash notice' do
+          subject
+
+          expect(controller).to set_flash[:notice]
+            .to eq("Group '#{group.name}' was successfully updated.")
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb
index e08f293e789128..b609d72020c52a 100644
--- a/ee/spec/models/namespace_setting_spec.rb
+++ b/ee/spec/models/namespace_setting_spec.rb
@@ -272,4 +272,49 @@
         ])
     end
   end
+
+  describe '.cascading_with_parent_namespace' do
+    let_it_be_with_reload(:group) { create(:group, :with_root_storage_statistics) }
+    let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
+
+    shared_examples '[configuration](inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
+      it 'return self value when no parent' do
+        group.namespace_settings.update!(attribute => true)
+        expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(group.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+
+        group.namespace_settings.update!(attribute => false)
+        expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
+        expect(group.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+      end
+
+      it 'return self value when unlocked' do
+        group.namespace_settings.update!(attribute => false)
+
+        subgroup.namespace_settings.update!(attribute => true)
+        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+
+        subgroup.namespace_settings.update!(attribute => false)
+        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
+        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+      end
+
+      it 'return parent value when locked' do
+        group.namespace_settings.update!(attribute => true)
+
+        subgroup.namespace_settings.update!(attribute => true)
+        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
+
+        subgroup.namespace_settings.update!(attribute => false)
+        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
+      end
+    end
+
+    it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
+    it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :allow_merge_on_skipped_pipeline
+    it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_all_discussions_are_resolved
+  end
 end
diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb
index 6fbde42aeee4ef..a30c3a5ceb1a3b 100644
--- a/ee/spec/models/project_spec.rb
+++ b/ee/spec/models/project_spec.rb
@@ -3731,6 +3731,74 @@ def stub_default_url_options(host)
     end
   end
 
+  describe '.cascading_with_parent_namespace' do
+    before do
+      stub_licensed_features(group_level_merge_checks_setting: true)
+    end
+
+    let_it_be_with_reload(:group) { create(:group, :with_root_storage_statistics) }
+    let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
+    let_it_be_with_reload(:project) { create(:project, group: subgroup) }
+    let_it_be_with_reload(:project_without_group) { create(:project) }
+
+    shared_examples '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
+      it 'return self value when no parent' do
+        expect(project_without_group.group).to be_nil
+
+        project_without_group.update!(attribute => true)
+        expect(project_without_group.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project_without_group.public_send("#{attribute}_locked?")).to be_falsey
+
+        project_without_group.update!(attribute => false)
+        expect(project_without_group.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
+        expect(project_without_group.public_send("#{attribute}_locked?")).to be_falsey
+      end
+
+      it 'return self value when unlocked' do
+        subgroup.namespace_settings.update!(attribute => false)
+        group.namespace_settings.update!(attribute => false)
+
+        project.update!(attribute => true)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project.public_send("#{attribute}_locked?")).to be_falsey
+
+        project.update!(attribute => false)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
+        expect(project.public_send("#{attribute}_locked?")).to be_falsey
+      end
+
+      it 'return locked value when locked subgroup' do
+        subgroup.namespace_settings.update!(attribute => true)
+        group.namespace_settings.update!(attribute => false)
+
+        project.update!(attribute => true)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+
+        project.update!(attribute => false)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+      end
+
+      it 'return locked value when locked group' do
+        subgroup.namespace_settings.update!(attribute => false)
+        group.namespace_settings.update!(attribute => true)
+
+        project.update!(attribute => true)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+
+        project.update!(attribute => false)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+      end
+    end
+
+    it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
+    it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :allow_merge_on_skipped_pipeline
+    it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_all_discussions_are_resolved
+  end
+
   describe '#okrs_mvc_feature_flag_enabled?' do
     let_it_be(:project) { create(:project) }
 
-- 
GitLab


From a542c92d79e8fae08370fada2e961f8d68e632ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Thu, 15 Dec 2022 01:09:07 +0800
Subject: [PATCH 2/8] Add feature flag about MR checks

---
 .../support_group_level_merge_checks_setting.yml          | 8 ++++++++
 ee/app/models/ee/project.rb                               | 4 ++--
 2 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 config/feature_flags/development/support_group_level_merge_checks_setting.yml

diff --git a/config/feature_flags/development/support_group_level_merge_checks_setting.yml b/config/feature_flags/development/support_group_level_merge_checks_setting.yml
new file mode 100644
index 00000000000000..ba738a7c09e81d
--- /dev/null
+++ b/config/feature_flags/development/support_group_level_merge_checks_setting.yml
@@ -0,0 +1,8 @@
+---
+name: support_group_level_merge_checks_setting
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102864
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/377723
+milestone: '15.8'
+type: development
+group: group::pipeline execution
+default_enabled: false
diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index c166600277db3d..e46f5b27a0795f 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -328,13 +328,13 @@ def self.cascading_with_parent_namespace(attribute)
       end
 
       define_method("#{attribute}?") do |inherit_group_setting: false|
-        return super() unless licensed_feature_available?(:group_level_merge_checks_setting)
+        return super() unless licensed_feature_available?(:group_level_merge_checks_setting) && ::Feature.enabled?(:support_group_level_merge_checks_setting, self)
 
         self.public_send(attribute) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
       end
 
       define_method("#{attribute}_locked?") do
-        return super() unless licensed_feature_available?(:group_level_merge_checks_setting)
+        return super() unless licensed_feature_available?(:group_level_merge_checks_setting) && ::Feature.enabled?(:support_group_level_merge_checks_setting, self)
 
         public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
       end
-- 
GitLab


From 85a460bf0f034453c3cae24be9166438dc374dae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Sun, 18 Dec 2022 20:46:56 +0800
Subject: [PATCH 3/8] Supplementary logic for `inherit_group_setting`

- Supplementary logic for `inherit_group_setting` param in project.rb
- Supplementary spec for `inherit_group_setting` param

Changelog: fixed
---
 ee/app/models/ee/project.rb                   |  6 +++-
 .../merge_requests_controller_spec.rb         | 13 ++++++---
 ee/spec/models/namespace_setting_spec.rb      | 29 +++++++++++++++++--
 ee/spec/models/project_spec.rb                | 14 +++++++--
 4 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index e46f5b27a0795f..4d44d03c217c57 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -330,7 +330,11 @@ def self.cascading_with_parent_namespace(attribute)
       define_method("#{attribute}?") do |inherit_group_setting: false|
         return super() unless licensed_feature_available?(:group_level_merge_checks_setting) && ::Feature.enabled?(:support_group_level_merge_checks_setting, self)
 
-        self.public_send(attribute) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+        if !inherit_group_setting
+          self.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend
+        else
+          self.public_send(attribute) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+        end
       end
 
       define_method("#{attribute}_locked?") do
diff --git a/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb b/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
index 449999f2b00005..9f8e49d1e17622 100644
--- a/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
+++ b/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Groups::Settings::MergeRequestsController do
+RSpec.describe Groups::Settings::MergeRequestsController, feature_category: :code_review do
   let_it_be(:group) { create(:group) }
   let_it_be(:user) { create(:user) }
 
@@ -44,9 +44,9 @@
         let(:update_service) { double }
 
         before do
-          allow(Groups::UpdateService).to receive(:new).and_return(update_service)
-          allow(update_service).to receive(:execute).and_return(false)
-
+          allow_next_instance_of(Groups::UpdateService) do |service|
+            allow(service).to receive(:execute).and_return(false)
+          end
           subject
         end
 
@@ -62,6 +62,11 @@
 
           expect(controller).to set_flash[:notice]
             .to eq("Group '#{group.name}' was successfully updated.")
+          expect(group.namespace_settings.reload).to have_attributes(
+            only_allow_merge_if_pipeline_succeeds: true,
+            allow_merge_on_skipped_pipeline: true,
+            only_allow_merge_if_all_discussions_are_resolved: true
+          )
         end
       end
     end
diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb
index b609d72020c52a..60f366fba01f65 100644
--- a/ee/spec/models/namespace_setting_spec.rb
+++ b/ee/spec/models/namespace_setting_spec.rb
@@ -274,8 +274,9 @@
   end
 
   describe '.cascading_with_parent_namespace' do
-    let_it_be_with_reload(:group) { create(:group, :with_root_storage_statistics) }
+    let_it_be_with_reload(:group) { create(:group) }
     let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
+    let_it_be_with_reload(:subsubgroup) { create(:group, parent: subgroup) }
 
     shared_examples '[configuration](inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
       it 'return self value when no parent' do
@@ -288,7 +289,7 @@
         expect(group.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
       end
 
-      it 'return self value when unlocked' do
+      it 'return self value when has parent but unlocked' do
         group.namespace_settings.update!(attribute => false)
 
         subgroup.namespace_settings.update!(attribute => true)
@@ -300,7 +301,7 @@
         expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
       end
 
-      it 'return parent value when locked' do
+      it 'return parent value when has parent and locked' do
         group.namespace_settings.update!(attribute => true)
 
         subgroup.namespace_settings.update!(attribute => true)
@@ -311,6 +312,28 @@
         expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
         expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
       end
+
+      it 'return grandparent value when locked by grandparent' do
+        group.namespace_settings.update!(attribute => true)
+        subgroup.namespace_settings.update!(attribute => false)
+        subsubgroup.namespace_settings.update!(attribute => false)
+
+        expect(subsubgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(subsubgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
+
+        group.namespace_settings.update!(attribute => false)
+        subsubgroup.reload
+        expect(subsubgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
+        expect(subsubgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+      end
+
+      it 'return self value when has parent and locked but send "inherit_group_setting=false"' do
+        group.namespace_settings.update!(attribute => true)
+
+        subgroup.namespace_settings.update!(attribute => false)
+        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to be_falsey
+      end
     end
 
     it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb
index a30c3a5ceb1a3b..377f84724b05c2 100644
--- a/ee/spec/models/project_spec.rb
+++ b/ee/spec/models/project_spec.rb
@@ -3736,7 +3736,7 @@ def stub_default_url_options(host)
       stub_licensed_features(group_level_merge_checks_setting: true)
     end
 
-    let_it_be_with_reload(:group) { create(:group, :with_root_storage_statistics) }
+    let_it_be_with_reload(:group) { create(:group) }
     let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
     let_it_be_with_reload(:project) { create(:project, group: subgroup) }
     let_it_be_with_reload(:project_without_group) { create(:project) }
@@ -3767,7 +3767,7 @@ def stub_default_url_options(host)
         expect(project.public_send("#{attribute}_locked?")).to be_falsey
       end
 
-      it 'return locked value when locked subgroup' do
+      it 'return parent value when locked by parent' do
         subgroup.namespace_settings.update!(attribute => true)
         group.namespace_settings.update!(attribute => false)
 
@@ -3780,7 +3780,7 @@ def stub_default_url_options(host)
         expect(project.public_send("#{attribute}_locked?")).to be_truthy
       end
 
-      it 'return locked value when locked group' do
+      it 'return grandparent value when locked by grandparent' do
         subgroup.namespace_settings.update!(attribute => false)
         group.namespace_settings.update!(attribute => true)
 
@@ -3792,6 +3792,14 @@ def stub_default_url_options(host)
         expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
         expect(project.public_send("#{attribute}_locked?")).to be_truthy
       end
+
+      it 'return self value when has parent and locked but send "inherit_group_setting=false"' do
+        subgroup.namespace_settings.update!(attribute => true)
+
+        project.update!(attribute => false)
+        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
+        expect(project.public_send("#{attribute}?", inherit_group_setting: false)).to be_falsey
+      end
     end
 
     it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
-- 
GitLab


From 9740cb3397ee81d6dfe8d34be4f9772aa47bb532 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Mon, 26 Dec 2022 13:29:32 +0800
Subject: [PATCH 4/8] Use TableSyntax to make testing more concise

---
 app/models/project.rb                    | 13 ++--
 ee/app/models/ee/namespace_setting.rb    | 21 +++--
 ee/app/models/ee/project.rb              |  8 +-
 ee/spec/models/namespace_setting_spec.rb | 99 ++++++++++--------------
 ee/spec/models/project_spec.rb           | 99 ++++++++++--------------
 5 files changed, 104 insertions(+), 136 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 85e9371191f759..b31ca1c71ca673 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -750,16 +750,13 @@ def self.public_or_visible_to_user(user = nil, min_access_level = nil)
     end
   end
 
-  # Defines instance methods:
+  # Define two instance methods:
   #
-  # - only_allow_merge_if_pipeline_succeeds?(inherit_group_setting: false)
-  # - allow_merge_on_skipped_pipeline?(inherit_group_setting: false)
-  # - only_allow_merge_if_all_discussions_are_resolved?(inherit_group_setting: false)
-  # - only_allow_merge_if_pipeline_succeeds_locked?
-  # - allow_merge_on_skipped_pipeline_locked?
-  # - only_allow_merge_if_all_discussions_are_resolved_locked?
+  # - [attribute]?(inherit_group_setting) Returns the final value after inheriting the parent group
+  # - [attribute]_locked?                 Returns true if the value is inherited from the parent group
+  #
+  # These functions will be overridden in EE to make sense afterwards
   def self.cascading_with_parent_namespace(attribute)
-    # method overriden in EE
     define_method("#{attribute}?") do |inherit_group_setting: false|
       self.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend
     end
diff --git a/ee/app/models/ee/namespace_setting.rb b/ee/app/models/ee/namespace_setting.rb
index 791349434083ca..e35f5931a6c3b8 100644
--- a/ee/app/models/ee/namespace_setting.rb
+++ b/ee/app/models/ee/namespace_setting.rb
@@ -32,29 +32,28 @@ def prevent_forking_outside_group?
         saml_setting || root_ancestor.namespace_settings&.prevent_forking_outside_group
       end
 
-      # Defines instance methods:
+      # Define three instance methods:
       #
-      # - only_allow_merge_if_pipeline_succeeds?(inherit_group_setting: false)
-      # - allow_merge_on_skipped_pipeline?(inherit_group_setting: false)
-      # - only_allow_merge_if_all_discussions_are_resolved?(inherit_group_setting: false)
-      # - only_allow_merge_if_pipeline_succeeds_locked?
-      # - allow_merge_on_skipped_pipeline_locked?
-      # - only_allow_merge_if_all_discussions_are_resolved_locked?
+      # - [attribute]_of_parent_group         Returns the configuration value of the parent group
+      # - [attribute]?(inherit_group_setting) Returns the final value after inheriting the parent group
+      # - [attribute]_locked?                 Returns true if the value is inherited from the parent group
       def self.cascading_with_parent_namespace(attribute)
         define_method("#{attribute}_of_parent_group") do
           namespace&.parent&.namespace_settings&.public_send("#{attribute}?", inherit_group_setting: true)
         end
 
         define_method("#{attribute}?") do |inherit_group_setting: false|
-          if !inherit_group_setting
-            public_send(attribute.to_s) # rubocop:disable GitlabSecurity/PublicSend
+          if inherit_group_setting
+            result = public_send(attribute.to_s) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
           else
-            public_send(attribute.to_s) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+            result = public_send(attribute.to_s) # rubocop:disable GitlabSecurity/PublicSend
           end
+
+          !!result
         end
 
         define_method("#{attribute}_locked?") do
-          public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+          !!public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
         end
       end
 
diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index 4d44d03c217c57..27fd0f00fa7f32 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -330,11 +330,13 @@ def self.cascading_with_parent_namespace(attribute)
       define_method("#{attribute}?") do |inherit_group_setting: false|
         return super() unless licensed_feature_available?(:group_level_merge_checks_setting) && ::Feature.enabled?(:support_group_level_merge_checks_setting, self)
 
-        if !inherit_group_setting
-          self.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend
+        if inherit_group_setting
+          result = self.public_send(attribute) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
         else
-          self.public_send(attribute) || public_send("#{attribute}_of_parent_group") # rubocop:disable GitlabSecurity/PublicSend
+          result = self.public_send(attribute) # rubocop:disable GitlabSecurity/PublicSend
         end
+
+        !!result
       end
 
       define_method("#{attribute}_locked?") do
diff --git a/ee/spec/models/namespace_setting_spec.rb b/ee/spec/models/namespace_setting_spec.rb
index 60f366fba01f65..5dfb3a4b221c68 100644
--- a/ee/spec/models/namespace_setting_spec.rb
+++ b/ee/spec/models/namespace_setting_spec.rb
@@ -274,70 +274,57 @@
   end
 
   describe '.cascading_with_parent_namespace' do
-    let_it_be_with_reload(:group) { create(:group) }
-    let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
-    let_it_be_with_reload(:subsubgroup) { create(:group, parent: subgroup) }
-
-    shared_examples '[configuration](inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
-      it 'return self value when no parent' do
-        group.namespace_settings.update!(attribute => true)
-        expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(group.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
-
-        group.namespace_settings.update!(attribute => false)
-        expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
-        expect(group.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+    context "when calling .cascading_with_parent_namespace" do
+      it 'create three instance methods for attribute' do
+        described_class.cascading_with_parent_namespace("any_configuration")
+        expect(described_class.instance_methods).to include(
+          :any_configuration_of_parent_group, :any_configuration_locked?, :any_configuration?)
       end
+    end
 
-      it 'return self value when has parent but unlocked' do
-        group.namespace_settings.update!(attribute => false)
-
-        subgroup.namespace_settings.update!(attribute => true)
-        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
-
-        subgroup.namespace_settings.update!(attribute => false)
-        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
-        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
-      end
-
-      it 'return parent value when has parent and locked' do
-        group.namespace_settings.update!(attribute => true)
-
-        subgroup.namespace_settings.update!(attribute => true)
-        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
+    context 'three configurations of MR checks' do
+      let_it_be_with_reload(:group) { create(:group) }
+      let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
+      let_it_be_with_reload(:subsubgroup) { create(:group, parent: subgroup) }
+
+      shared_examples '[configuration](inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
+        using RSpec::Parameterized::TableSyntax
+
+        where(:group_attr, :subgroup_attr, :subsubgroup_attr, :group_with_inherit_attr?, :group_without_inherit_attr?, :group_locked?, :subgroup_with_inherit_attr?, :subgroup_without_inherit_attr?, :subgroup_locked?, :subsubgroup_with_inherit_attr?, :subsubgroup_without_inherit_attr?, :subsubgroup_locked?) do
+          true  | true  | true      | true  | true  | false     | true  | true  | true      | true  | true  | true
+          true  | true  | false     | true  | true  | false     | true  | true  | true      | true  | false | true
+          true  | false | false     | true  | true  | false     | true  | false | true      | true  | false | true
+          false | true  | true      | false | false | false     | true  | true  | false     | true  | true  | true
+          false | true  | false     | false | false | false     | true  | true  | false     | true  | false | true
+          false | false | false     | false | false | false     | false | false | false     | false | false | false
+        end
 
-        subgroup.namespace_settings.update!(attribute => false)
-        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
-      end
+        with_them do
+          before do
+            group.namespace_settings.update!(attribute => group_attr)
+            subgroup.namespace_settings.update!(attribute => subgroup_attr)
+            subsubgroup.namespace_settings.update!(attribute => subsubgroup_attr)
+          end
 
-      it 'return grandparent value when locked by grandparent' do
-        group.namespace_settings.update!(attribute => true)
-        subgroup.namespace_settings.update!(attribute => false)
-        subsubgroup.namespace_settings.update!(attribute => false)
+          it 'returns correct value' do
+            expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to eq(group_with_inherit_attr?)
+            expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to eq(group_without_inherit_attr?)
+            expect(group.namespace_settings.public_send("#{attribute}_locked?")).to eq(group_locked?)
 
-        expect(subsubgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(subsubgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_truthy
+            expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to eq(subgroup_with_inherit_attr?)
+            expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to eq(subgroup_without_inherit_attr?)
+            expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to eq(subgroup_locked?)
 
-        group.namespace_settings.update!(attribute => false)
-        subsubgroup.reload
-        expect(subsubgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
-        expect(subsubgroup.namespace_settings.public_send("#{attribute}_locked?")).to be_falsey
+            expect(subsubgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to eq(subsubgroup_with_inherit_attr?)
+            expect(subsubgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to eq(subsubgroup_without_inherit_attr?)
+            expect(subsubgroup.namespace_settings.public_send("#{attribute}_locked?")).to eq(subsubgroup_locked?)
+          end
+        end
       end
 
-      it 'return self value when has parent and locked but send "inherit_group_setting=false"' do
-        group.namespace_settings.update!(attribute => true)
-
-        subgroup.namespace_settings.update!(attribute => false)
-        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to be_falsey
-      end
+      it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
+      it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :allow_merge_on_skipped_pipeline
+      it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_all_discussions_are_resolved
     end
-
-    it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
-    it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :allow_merge_on_skipped_pipeline
-    it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_all_discussions_are_resolved
   end
 end
diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb
index 377f84724b05c2..d5d5d9bb54bf20 100644
--- a/ee/spec/models/project_spec.rb
+++ b/ee/spec/models/project_spec.rb
@@ -3736,75 +3736,58 @@ def stub_default_url_options(host)
       stub_licensed_features(group_level_merge_checks_setting: true)
     end
 
-    let_it_be_with_reload(:group) { create(:group) }
-    let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
-    let_it_be_with_reload(:project) { create(:project, group: subgroup) }
-    let_it_be_with_reload(:project_without_group) { create(:project) }
-
-    shared_examples '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
-      it 'return self value when no parent' do
-        expect(project_without_group.group).to be_nil
-
-        project_without_group.update!(attribute => true)
-        expect(project_without_group.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project_without_group.public_send("#{attribute}_locked?")).to be_falsey
-
-        project_without_group.update!(attribute => false)
-        expect(project_without_group.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
-        expect(project_without_group.public_send("#{attribute}_locked?")).to be_falsey
+    context "when calling .cascading_with_parent_namespace" do
+      it 'create three instance methods for attribute' do
+        EE::Project.cascading_with_parent_namespace("any_configuration")
+        expect(EE::Project.instance_methods).to include(
+          :any_configuration_of_parent_group, :any_configuration_locked?, :any_configuration?)
       end
+    end
 
-      it 'return self value when unlocked' do
-        subgroup.namespace_settings.update!(attribute => false)
-        group.namespace_settings.update!(attribute => false)
-
-        project.update!(attribute => true)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project.public_send("#{attribute}_locked?")).to be_falsey
-
-        project.update!(attribute => false)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_falsey
-        expect(project.public_send("#{attribute}_locked?")).to be_falsey
-      end
+    context 'three configurations of MR checks' do
+      let_it_be_with_reload(:group) { create(:group) }
+      let_it_be_with_reload(:subgroup) { create(:group, parent: group) }
+      let_it_be_with_reload(:project) { create(:project, group: subgroup) }
 
-      it 'return parent value when locked by parent' do
-        subgroup.namespace_settings.update!(attribute => true)
-        group.namespace_settings.update!(attribute => false)
+      shared_examples '[configuration](inherit_group_setting: bool) and [configuration]_locked?' do |attribute|
+        using RSpec::Parameterized::TableSyntax
 
-        project.update!(attribute => true)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+        where(:group_attr, :subgroup_attr, :project_attr, :group_with_inherit_attr?, :group_without_inherit_attr?, :group_locked?, :subgroup_with_inherit_attr?, :subgroup_without_inherit_attr?, :subgroup_locked?, :project_with_inherit_attr?, :project_without_inherit_attr?, :project_locked?) do
+          true  | true  | true      | true  | true  | false     | true  | true  | true      | true  | true  | true
+          true  | true  | false     | true  | true  | false     | true  | true  | true      | true  | false | true
+          true  | false | false     | true  | true  | false     | true  | false | true      | true  | false | true
+          false | true  | true      | false | false | false     | true  | true  | false     | true  | true  | true
+          false | true  | false     | false | false | false     | true  | true  | false     | true  | false | true
+          false | false | false     | false | false | false     | false | false | false     | false | false | false
+        end
 
-        project.update!(attribute => false)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project.public_send("#{attribute}_locked?")).to be_truthy
-      end
+        with_them do
+          before do
+            group.namespace_settings.update!(attribute => group_attr)
+            subgroup.namespace_settings.update!(attribute => subgroup_attr)
+            project.update!(attribute => project_attr)
+          end
 
-      it 'return grandparent value when locked by grandparent' do
-        subgroup.namespace_settings.update!(attribute => false)
-        group.namespace_settings.update!(attribute => true)
+          it 'returns correct value' do
+            expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to eq(group_with_inherit_attr?)
+            expect(group.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to eq(group_without_inherit_attr?)
+            expect(group.namespace_settings.public_send("#{attribute}_locked?")).to eq(group_locked?)
 
-        project.update!(attribute => true)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+            expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: true)).to eq(subgroup_with_inherit_attr?)
+            expect(subgroup.namespace_settings.public_send("#{attribute}?", inherit_group_setting: false)).to eq(subgroup_without_inherit_attr?)
+            expect(subgroup.namespace_settings.public_send("#{attribute}_locked?")).to eq(subgroup_locked?)
 
-        project.update!(attribute => false)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project.public_send("#{attribute}_locked?")).to be_truthy
+            expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to eq(project_with_inherit_attr?)
+            expect(project.public_send("#{attribute}?", inherit_group_setting: false)).to eq(project_without_inherit_attr?)
+            expect(project.public_send("#{attribute}_locked?")).to eq(project_locked?)
+          end
+        end
       end
 
-      it 'return self value when has parent and locked but send "inherit_group_setting=false"' do
-        subgroup.namespace_settings.update!(attribute => true)
-
-        project.update!(attribute => false)
-        expect(project.public_send("#{attribute}?", inherit_group_setting: true)).to be_truthy
-        expect(project.public_send("#{attribute}?", inherit_group_setting: false)).to be_falsey
-      end
+      it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
+      it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :allow_merge_on_skipped_pipeline
+      it_behaves_like '[configuration](inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_all_discussions_are_resolved
     end
-
-    it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_pipeline_succeeds
-    it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :allow_merge_on_skipped_pipeline
-    it_behaves_like '[configuration]?(inherit_group_setting: bool) and [configuration]_locked?', :only_allow_merge_if_all_discussions_are_resolved
   end
 
   describe '#okrs_mvc_feature_flag_enabled?' do
-- 
GitLab


From 5bc509960b27ac1c79edf11d5b74e03288039ba0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Mon, 26 Dec 2022 15:02:16 +0800
Subject: [PATCH 5/8] Move controller spec to 'spec/requests'

---
 .../settings/merge_requests_controller_spec.rb      | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
 rename ee/spec/{controllers/ee => requests}/groups/settings/merge_requests_controller_spec.rb (82%)

diff --git a/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb b/ee/spec/requests/groups/settings/merge_requests_controller_spec.rb
similarity index 82%
rename from ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
rename to ee/spec/requests/groups/settings/merge_requests_controller_spec.rb
index 9f8e49d1e17622..c0fd8b1e80269d 100644
--- a/ee/spec/controllers/ee/groups/settings/merge_requests_controller_spec.rb
+++ b/ee/spec/requests/groups/settings/merge_requests_controller_spec.rb
@@ -12,7 +12,7 @@
 
   describe 'PATCH #update' do
     subject do
-      patch :update, params: {
+      patch group_settings_merge_requests_path(group), params: {
         group_id: group,
         namespace_setting: {
           only_allow_merge_if_pipeline_succeeds: true,
@@ -27,7 +27,10 @@
         group.add_owner(user)
       end
 
-      it { is_expected.to have_gitlab_http_status(:not_found) }
+      it 'respond status :not_found' do
+        subject
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
     end
 
     context 'when user is an admin' do
@@ -51,8 +54,7 @@
         end
 
         it 'returns a flash alert' do
-          expect(controller).to set_flash[:alert]
-            .to eq("Group '#{group.name}' could not be updated.")
+          expect(flash[:alert]).to eq("Group '#{group.name}' could not be updated.")
         end
       end
 
@@ -60,8 +62,7 @@
         it 'returns a flash notice' do
           subject
 
-          expect(controller).to set_flash[:notice]
-            .to eq("Group '#{group.name}' was successfully updated.")
+          expect(flash[:notice]).to eq("Group '#{group.name}' was successfully updated.")
           expect(group.namespace_settings.reload).to have_attributes(
             only_allow_merge_if_pipeline_succeeds: true,
             allow_merge_on_skipped_pipeline: true,
-- 
GitLab


From 054d3cd7e79c7e54b3cb266791cd1b5f8d48c7a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Wed, 28 Dec 2022 17:55:58 +0800
Subject: [PATCH 6/8] Update the return sentence of API

1. Support i18n
2. Show more detailed error messages

Changelog: changed
---
 .../groups/settings/merge_requests_controller.rb       | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/ee/app/controllers/groups/settings/merge_requests_controller.rb b/ee/app/controllers/groups/settings/merge_requests_controller.rb
index 4b034e2448b415..060a59a464cb2b 100644
--- a/ee/app/controllers/groups/settings/merge_requests_controller.rb
+++ b/ee/app/controllers/groups/settings/merge_requests_controller.rb
@@ -13,11 +13,17 @@ def update
         return render_404 unless @group.licensed_feature_available?(:group_level_merge_checks_setting)
 
         if Groups::UpdateService.new(@group, current_user, group_settings_params).execute
-          notice = "Group '#{@group.name}' was successfully updated."
+          notice = format(
+            _("Group '%{group_name}' was successfully updated."),
+            group_name: @group.name
+          )
           redirect_to edit_group_path(@group, anchor: 'js-merge-requests-settings'), notice: notice
         else
           @group.reset
-          alert = "Group '#{@group.name}' could not be updated."
+          alert = @group.errors.full_messages.to_sentence.presence || format(
+            _("Group '%{group_name}' could not be updated."),
+            group_name: @group.name
+          )
           redirect_to edit_group_path(@group, anchor: 'js-merge-requests-settings'), alert: alert
         end
       end
-- 
GitLab


From 38446784604da50d83023797bf7bb4734ffb32e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Wed, 28 Dec 2022 18:00:07 +0800
Subject: [PATCH 7/8] Update group of feature flag

Flag: 'support_group_level_merge_checks_setting'.
From 'group::pipeline execution' to 'group::compliance'

Changelog: changed
---
 .../development/support_group_level_merge_checks_setting.yml    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/feature_flags/development/support_group_level_merge_checks_setting.yml b/config/feature_flags/development/support_group_level_merge_checks_setting.yml
index ba738a7c09e81d..66cb98302610b4 100644
--- a/config/feature_flags/development/support_group_level_merge_checks_setting.yml
+++ b/config/feature_flags/development/support_group_level_merge_checks_setting.yml
@@ -4,5 +4,5 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102864
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/377723
 milestone: '15.8'
 type: development
-group: group::pipeline execution
+group: group::compliance
 default_enabled: false
-- 
GitLab


From 0d4ca70a71ac3f7ddff4f2e95cb8de6dba8d65eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E8=B7=AF=E5=B0=8F=E9=B9=BF?= <1551755561@qq.com>
Date: Wed, 28 Dec 2022 18:44:48 +0800
Subject: [PATCH 8/8] Update gitlab.pot with new sentences

---
 locale/gitlab.pot | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8bc96eb4945bb8..61dc71966d64e8 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -19240,6 +19240,12 @@ msgstr ""
 msgid "Group %{group_name} was successfully created."
 msgstr ""
 
+msgid "Group '%{group_name}' could not be updated."
+msgstr ""
+
+msgid "Group '%{group_name}' was successfully updated."
+msgstr ""
+
 msgid "Group Access Tokens"
 msgstr ""
 
-- 
GitLab