Update form in view

Fix project method

Fix project method

Add blocking to form

Add conditional params to projects controller

ee/app/assets/javascripts/approvals/mount_project_settings.js

@@ -16,6 +16,7 @@ export default function mountProjectSettingsApprovals(el) {
     ...el.dataset,
     prefix: 'project-settings',
     allowMultiRule: parseBoolean(el.dataset.allowMultiRule),
+    canEdit: parseBoolean(el.dataset.canEdit)
   });
 
   return new Vue({

ee/app/controllers/ee/projects_controller.rb

@@ -73,15 +73,12 @@ module EE
         approver_ids
         issues_template
         merge_requests_template
-        disable_overriding_approvers_per_merge_request
         repository_size_limit
         reset_approvals_on_push
         service_desk_enabled
         ci_cd_only
         use_custom_template
         packages_enabled
-        merge_requests_author_approval
-        merge_requests_disable_committers_approval
         require_password_to_approve
         group_with_project_templates_id
       ]
@@ -90,6 +87,8 @@ module EE
         attrs << %i[merge_pipelines_enabled]
       end
 
+      attrs = attrs + merge_request_rules_params
+
       if allow_mirror_params?
         attrs + mirror_params
       else
@@ -113,6 +112,24 @@ module EE
       end
     end
 
+    def merge_request_rules_params
+      attrs = []
+
+      if can?(current_user, :modify_merge_request_commiter_setting, @project)
+        attrs << :merge_requests_disable_committers_approval
+      end
+
+      if can?(current_user, :modify_approvers_rules, @project)
+        attrs << :disable_overriding_approvers_per_merge_request
+      end
+
+      if can?(current_user, :modify_merge_request_author_setting, @project)
+        attrs << :merge_requests_author_approval
+      end
+
+      attrs
+    end
+
     def allow_merge_pipelines_params?
       project&.feature_available?(:merge_pipelines)
     end

ee/app/models/ee/project.rb

@@ -259,7 +259,7 @@ module EE
     end
 
     def can_override_approvers?
-      !disable_overriding_approvers_per_merge_request?
+      !disable_overriding_approvers_per_merge_request
     end
 
     def shared_runners_available?

ee/app/policies/ee/project_policy.rb

@@ -202,6 +202,7 @@ module EE
         enable :destroy_package
         enable :admin_feature_flags_client
         enable :modify_approvers_rules
+        enable :modify_approvers_list
         enable :modify_merge_request_author_setting
         enable :modify_merge_request_commiter_setting
       end
@@ -331,6 +332,10 @@ module EE
         prevent :modify_merge_request_commiter_setting
       end
 
+      rule { owner_cannot_modify_approvers_rules & ~admin }.policy do
+        prevent :modify_approvers_list
+      end
+
       rule { web_ide_terminal_available & can?(:create_pipeline) & can?(:maintainer_access) }.enable :create_web_ide_terminal
 
       # Design abilities could also be prevented in the issue policy.

ee/app/views/projects/_merge_request_approvals_settings_form.html.haml

@@ -4,6 +4,7 @@
   = form.label :approver_ids, class: 'label-bold' do
     = _("Approval rules")
   #js-mr-approvals-settings{ data: { 'project_id': @project.id,
+    'can_edit': can?(current_user, :modify_approvers_list, @project).to_s,
     'project_path': expose_path(api_v4_projects_path(id: @project.id)),
     'settings_path': expose_path(api_v4_projects_approval_settings_path(id: @project.id)),
     'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: @project.id)),
@@ -26,7 +27,7 @@
 
 .form-group
   .form-check
-    = form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers, class: 'form-check-input', disabled: can?(current_user, :modify_approvers_rules, @project) }, false, true)
+    = form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers, class: 'form-check-input', disabled: !can?(current_user, :modify_approvers_rules, @project) }, false, true)
     = form.label :disable_overriding_approvers_per_merge_request, class: 'form-check-label' do
       %span= _('Can override approvers and approvals required per merge request')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'prevent-overriding-default-approvals'), target: '_blank'
@@ -39,7 +40,7 @@
 
 .form-group.self-approval
   .form-check
-    = form.check_box :merge_requests_author_approval, { class: 'form-check-input', checked: !project.merge_requests_author_approval?, disabled: can?(current_user, :modify_merge_request_author_setting, @project) }, false, true
+    = form.check_box :merge_requests_author_approval, { class: 'form-check-input', checked: !project.merge_requests_author_approval?, disabled: !can?(current_user, :modify_merge_request_author_setting, @project) }, false, true
     = form.label :merge_requests_author_approval, class: 'form-check-label' do
       %span= _('Prevent approval of merge requests by merge request author')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
@@ -47,7 +48,7 @@
 
 .form-group.committers-approval
   .form-check
-    = form.check_box :merge_requests_disable_committers_approval, { disabled: can?(current_user, :modify_merge_request_commiter_setting, @project),  class: 'form-check-input' }
+    = form.check_box :merge_requests_disable_committers_approval, { disabled: !can?(current_user, :modify_merge_request_commiter_setting, @project),  class: 'form-check-input' }
     = form.label :merge_requests_disable_committers_approval, class: 'form-check-label' do
       %span= _('Prevent approval of merge requests by merge request committers')
       = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',

ee/spec/controllers/projects_controller_spec.rb

@@ -317,6 +317,63 @@ describe ProjectsController do
         end
       end
     end
+
+    context 'merge request approvers settings' do
+      shared_examples 'merge request approvers rules' do
+        using RSpec::Parameterized::TableSyntax
+
+        where(:license_value, :setting_value, :param_value, :final_value) do
+          false | false | false | false
+          false | true  | false | false
+          false | false | true  | true
+          false | true  | true  | true
+          true  | false | false | false
+          true  | true  | false | nil
+          true  | false | true  | true
+          true  | true  | true  | nil
+        end
+
+        with_them do
+          before do
+            stub_licensed_features(merge_request_approvers_rules: license_value)
+            stub_application_setting(app_setting => setting_value)
+          end
+
+          it 'updates project if needed' do
+            put :update,
+              params: {
+                namespace_id: project.namespace,
+                id: project,
+                project: { setting => param_value }
+              }
+            project.reload
+
+            expect(project[setting]).to eq(final_value)
+          end
+        end
+      end
+
+      describe ':disable_overriding_approvers_per_merge_request' do
+        it_behaves_like 'merge request approvers rules' do
+          let(:app_setting) { :disable_overriding_approvers_per_merge_request }
+          let(:setting) { :disable_overriding_approvers_per_merge_request}
+        end
+      end
+
+      describe ':merge_requests_author_approval' do
+        it_behaves_like 'merge request approvers rules' do
+          let(:app_setting) { :prevent_merge_requests_author_approval }
+          let(:setting) { :merge_requests_author_approval }
+        end
+      end
+
+      describe ':merge_requests_disable_committers_approval' do
+        it_behaves_like 'merge request approvers rules' do
+          let(:app_setting) { :prevent_merge_requests_committers_approval }
+          let(:setting) { :merge_requests_disable_committers_approval }
+        end
+      end
+    end
   end
 
   describe '#download_export' do

ee/spec/policies/project_policy_spec.rb

@@ -1293,4 +1293,65 @@ describe ProjectPolicy do
       let(:policy) { :modify_merge_request_commiter_setting }
     end
   end
+
+  describe ':modify_approvers_list' do
+    let(:setting_name) { :disable_overriding_approvers_per_merge_request }
+    let(:policy) { :modify_approvers_list }
+    let(:project) { create(:project, namespace: owner.namespace) }
+
+    using RSpec::Parameterized::TableSyntax
+    context 'with merge request approvers rules available in license' do
+      where(:role, :setting, :allowed) do
+        :guest | true | false
+        :reporter | true | false
+        :developer | true | false
+        :maintainer | false | true
+        :maintainer | true | false
+        :owner | false | true
+        :owner | true | false
+        :admin | false | true
+        :admin | true | true
+      end
+
+      with_them do
+        let(:current_user) { public_send(role) }
+
+        before do
+          stub_licensed_features(merge_request_approvers_rules: true)
+          stub_application_setting(setting_name => setting)
+        end
+
+        it do
+          is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy))
+        end
+      end
+    end
+
+    context 'with merge request approvers not available in license' do
+      where(:role, :setting, :allowed) do
+        :guest | true | false
+        :reporter | true | false
+        :developer | true | false
+        :maintainer | false | true
+        :maintainer | true | true
+        :owner | false | true
+        :owner | true | true
+        :admin | true | true
+        :admin | false | true
+      end
+
+      with_them do
+        let(:current_user) { public_send(role) }
+
+        before do
+          stub_licensed_features(merge_request_approvers_rules: false)
+          stub_application_setting(setting_name => setting)
+        end
+
+        it do
+          is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy))
+        end
+      end
+    end
+  end
 end