Commit 203bfa65 authored by Małgorzata Ksionek's avatar Małgorzata Ksionek

Add cr remarks

Fix typos, remove comments, general cleanup

Update strings file

Add changelog entry

Fix grammar errors

Add strings file

Fix form call

Add code review remarks

Change feature name in licence

Update migration file

Add text file

Rename link and page

Back to old naming

Add docs file

Fix docs

Apply suggestion to doc/user/admin_area/merge_requests_approvals.md

Apply suggestion to doc/user/admin_area/merge_requests_approvals.md

Add cr remarks

Add admin condition to project policies

Move js file to ee

Fix schema file
parent 50506faa
# frozen_string_literal: true # frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddFieldsToApplicationSettingsForMergeRequestsApprovals < ActiveRecord::Migration[5.2] class AddFieldsToApplicationSettingsForMergeRequestsApprovals < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false DOWNTIME = false
disable_ddl_transaction!
def up def up
add_column_with_default(:application_settings, :disable_overriding_approvers_per_merge_request, add_column(:application_settings,
:disable_overriding_approvers_per_merge_request,
:boolean, :boolean,
default: false, default: false,
allow_null: false) null: false)
add_column_with_default(:application_settings, :prevent_merge_requests_author_approval, add_column(:application_settings,
:prevent_merge_requests_author_approval,
:boolean, :boolean,
default: false, default: false,
allow_null: false) null: false)
add_column_with_default(:application_settings, :prevent_merge_requests_committers_approval, add_column(:application_settings,
:prevent_merge_requests_committers_approval,
:boolean, :boolean,
default: false, default: false,
allow_null: false) null: false)
end end
def down def down
......
...@@ -1561,7 +1561,6 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do ...@@ -1561,7 +1561,6 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do
t.integer "start_date_sourcing_epic_id" t.integer "start_date_sourcing_epic_id"
t.integer "due_date_sourcing_epic_id" t.integer "due_date_sourcing_epic_id"
t.integer "health_status", limit: 2 t.integer "health_status", limit: 2
t.integer "state_id", limit: 2, default: 1, null: false
t.index ["assignee_id"], name: "index_epics_on_assignee_id" t.index ["assignee_id"], name: "index_epics_on_assignee_id"
t.index ["author_id"], name: "index_epics_on_author_id" t.index ["author_id"], name: "index_epics_on_author_id"
t.index ["closed_by_id"], name: "index_epics_on_closed_by_id" t.index ["closed_by_id"], name: "index_epics_on_closed_by_id"
...@@ -4712,7 +4711,6 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do ...@@ -4712,7 +4711,6 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do
add_foreign_key "epics", "epics", column: "start_date_sourcing_epic_id", name: "fk_9d480c64b2", on_delete: :nullify add_foreign_key "epics", "epics", column: "start_date_sourcing_epic_id", name: "fk_9d480c64b2", on_delete: :nullify
add_foreign_key "epics", "milestones", column: "due_date_sourcing_milestone_id", name: "fk_3c1fd1cccc", on_delete: :nullify add_foreign_key "epics", "milestones", column: "due_date_sourcing_milestone_id", name: "fk_3c1fd1cccc", on_delete: :nullify
add_foreign_key "epics", "milestones", column: "start_date_sourcing_milestone_id", name: "fk_1fbed67632", on_delete: :nullify add_foreign_key "epics", "milestones", column: "start_date_sourcing_milestone_id", name: "fk_1fbed67632", on_delete: :nullify
add_foreign_key "epics", "milestones", on_delete: :nullify
add_foreign_key "epics", "namespaces", column: "group_id", name: "fk_f081aa4489", on_delete: :cascade add_foreign_key "epics", "namespaces", column: "group_id", name: "fk_f081aa4489", on_delete: :cascade
add_foreign_key "epics", "users", column: "assignee_id", name: "fk_dccd3f98fc", on_delete: :nullify add_foreign_key "epics", "users", column: "assignee_id", name: "fk_dccd3f98fc", on_delete: :nullify
add_foreign_key "epics", "users", column: "author_id", name: "fk_3654b61b03", on_delete: :cascade add_foreign_key "epics", "users", column: "author_id", name: "fk_3654b61b03", on_delete: :cascade
......
...@@ -28,7 +28,7 @@ The Admin Area is made up of the following sections: ...@@ -28,7 +28,7 @@ The Admin Area is made up of the following sections:
| Abuse Reports | Manage [abuse reports](abuse_reports.md) submitted by your users. | | Abuse Reports | Manage [abuse reports](abuse_reports.md) submitted by your users. |
| License **(STARTER ONLY)** | Upload, display, and remove [licenses](license.md). | | License **(STARTER ONLY)** | Upload, display, and remove [licenses](license.md). |
| Kubernetes | Create and manage instance-level [Kubernetes clusters](../instance/clusters/index.md). | | Kubernetes | Create and manage instance-level [Kubernetes clusters](../instance/clusters/index.md). |
| Push Rules **(STARTER)** | Configure pre-defined Git [push rules](../../push_rules/push_rules.md) for projects. | Push Rules **(STARTER ONLY)** | Configure pre-defined Git [push rules](../../push_rules/push_rules.md) for projects. Also, configure [merge requests approvers rules](merge_requests_approvals.md). **(PREMIUM ONLY)** |
| Geo **(PREMIUM ONLY)** | Configure and maintain [Geo nodes](geo_nodes.md). | | Geo **(PREMIUM ONLY)** | Configure and maintain [Geo nodes](geo_nodes.md). |
| Deploy Keys | Create instance-wide [SSH deploy keys](../../ssh/README.md#deploy-keys). | | Deploy Keys | Create instance-wide [SSH deploy keys](../../ssh/README.md#deploy-keys). |
| Credentials **(ULTIMATE ONLY)** | View [credentials](credentials_inventory.md) that can be used to access your instance. | | Credentials **(ULTIMATE ONLY)** | View [credentials](credentials_inventory.md) that can be used to access your instance. |
......
---
type: reference, concepts
---
# Instance-level merge request approval rules **(PREMIUM ONLY)**
> Introduced in [GitLab Premium](https://gitlab.com/gitlab-org/gitlab/issues/39060) 12.8.
Merge request approvals rules prevent users overriding certain settings on a project
level. When configured, only administrators can change these settings on a project level
if they are enabled at an instance level.
To enable merge request approval rules for an instance:
1. Navigate to **{admin}** **Admin Area >** **{push-rules}** **Push Rules** and expand **Merge
requests approvals**.
1. Set the required rule.
1. Click **Save changes**.
GitLab administrators can later override these settings in a project’s settings.
## Available rules
Merge request approval rules that can be set at an instance level are:
- **Prevent approval of merge requests by merge request author**. Prevents non-admins
from allowing merge request authors to merge their own merge requests in individual
projects.
- **Prevent approval of merge requests by merge request committers**. Prevents
non-admins from allowing merge request committers to merge merge requests they were
committing to in individual projects.
- **Prevent users from modifying merge request approvers list**. Prevents non-admins
from modifying approvers list in project settings and in individual merge requests.
import initSettingsPanels from '~/settings_panels'; import initSettingsPanels from '~/settings_panels';
document.addEventListener('DOMContentLoaded', () => { document.addEventListener('DOMContentLoaded', () => {
// Initialize expandable settings panels
initSettingsPanels(); initSettingsPanels();
}); });
...@@ -50,7 +50,7 @@ module EE ...@@ -50,7 +50,7 @@ module EE
attrs << :updating_name_disabled_for_users attrs << :updating_name_disabled_for_users
end end
if License.feature_available?(:merge_request_approvers_rules) if License.feature_available?(:admin_merge_request_approvers_rules)
attrs += EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes attrs += EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes
end end
......
...@@ -115,7 +115,7 @@ module EE ...@@ -115,7 +115,7 @@ module EE
def merge_request_rules_params def merge_request_rules_params
attrs = [] attrs = []
if can?(current_user, :modify_merge_request_commiter_setting, project) if can?(current_user, :modify_merge_request_committer_setting, project)
attrs << :merge_requests_disable_committers_approval attrs << :merge_requests_disable_committers_approval
end end
......
...@@ -724,25 +724,25 @@ module EE ...@@ -724,25 +724,25 @@ module EE
end end
def disable_overriding_approvers_per_merge_request def disable_overriding_approvers_per_merge_request
return self[:disable_overriding_approvers_per_merge_request] unless License.feature_available?(:merge_request_approvers_rules) return super unless License.feature_available?(:admin_merge_request_approvers_rules)
::Gitlab::CurrentSettings.disable_overriding_approvers_per_merge_request? || ::Gitlab::CurrentSettings.disable_overriding_approvers_per_merge_request? ||
self[:disable_overriding_approvers_per_merge_request] super
end end
def merge_requests_author_approval def merge_requests_author_approval
return self[:merge_requests_author_approval] unless License.feature_available?(:merge_request_approvers_rules) return super unless License.feature_available?(:admin_merge_request_approvers_rules)
return false if ::Gitlab::CurrentSettings.prevent_merge_requests_author_approval? return false if ::Gitlab::CurrentSettings.prevent_merge_requests_author_approval?
self[:merge_requests_author_approval] super
end end
def merge_requests_disable_committers_approval def merge_requests_disable_committers_approval
return self[:merge_requests_disable_committers_approval] unless License.feature_available?(:merge_request_approvers_rules) return super unless License.feature_available?(:admin_merge_request_approvers_rules)
::Gitlab::CurrentSettings.prevent_merge_requests_committers_approval? || ::Gitlab::CurrentSettings.prevent_merge_requests_committers_approval? ||
self[:merge_requests_disable_committers_approval] super
end end
def license_compliance def license_compliance
......
...@@ -80,7 +80,7 @@ class License < ApplicationRecord ...@@ -80,7 +80,7 @@ class License < ApplicationRecord
ldap_group_sync_filter ldap_group_sync_filter
merge_pipelines merge_pipelines
merge_request_performance_metrics merge_request_performance_metrics
merge_request_approvers_rules admin_merge_request_approvers_rules
merge_trains merge_trains
metrics_reports metrics_reports
multiple_approval_rules multiple_approval_rules
......
...@@ -46,6 +46,27 @@ module EE ...@@ -46,6 +46,27 @@ module EE
!PushRule.global&.commit_committer_check !PushRule.global&.commit_committer_check
end end
with_scope :global
condition(:owner_cannot_modify_approvers_rules) do
License.feature_available?(:admin_merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.disable_overriding_approvers_per_merge_request
end
with_scope :global
condition(:owner_cannot_modify_merge_request_author_setting) do
License.feature_available?(:admin_merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.prevent_merge_requests_author_approval
end
with_scope :global
condition(:owner_cannot_modify_merge_request_committer_setting) do
License.feature_available?(:admin_merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.prevent_merge_requests_committers_approval
end
with_scope :subject with_scope :subject
condition(:commit_committer_check_available) do condition(:commit_committer_check_available) do
@subject.feature_available?(:commit_committer_check) @subject.feature_available?(:commit_committer_check)
...@@ -204,7 +225,7 @@ module EE ...@@ -204,7 +225,7 @@ module EE
enable :modify_approvers_rules enable :modify_approvers_rules
enable :modify_approvers_list enable :modify_approvers_list
enable :modify_merge_request_author_setting enable :modify_merge_request_author_setting
enable :modify_merge_request_commiter_setting enable :modify_merge_request_committer_setting
end end
rule { license_management_enabled & can?(:maintainer_access) }.enable :admin_software_license_policy rule { license_management_enabled & can?(:maintainer_access) }.enable :admin_software_license_policy
...@@ -290,24 +311,6 @@ module EE ...@@ -290,24 +311,6 @@ module EE
.default_project_deletion_protection .default_project_deletion_protection
end end
condition(:owner_cannot_modify_approvers_rules) do
@subject.feature_available?(:merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.disable_overriding_approvers_per_merge_request
end
condition(:owner_cannot_modify_merge_request_author_setting) do
@subject.feature_available?(:merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.prevent_merge_requests_author_approval
end
condition(:owner_cannot_modify_merge_request_commiter_setting) do
@subject.feature_available?(:merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.prevent_merge_requests_committers_approval
end
rule { needs_new_sso_session & ~admin }.policy do rule { needs_new_sso_session & ~admin }.policy do
prevent :guest_access prevent :guest_access
prevent :reporter_access prevent :reporter_access
...@@ -320,16 +323,16 @@ module EE ...@@ -320,16 +323,16 @@ module EE
prevent :read_project prevent :read_project
end end
rule { owner_cannot_modify_approvers_rules }.policy do rule { owner_cannot_modify_approvers_rules & ~admin }.policy do
prevent :modify_approvers_rules prevent :modify_approvers_rules
end end
rule { owner_cannot_modify_merge_request_author_setting }.policy do rule { owner_cannot_modify_merge_request_author_setting & ~admin }.policy do
prevent :modify_merge_request_author_setting prevent :modify_merge_request_author_setting
end end
rule { owner_cannot_modify_merge_request_commiter_setting }.policy do rule { owner_cannot_modify_merge_request_committer_setting & ~admin }.policy do
prevent :modify_merge_request_commiter_setting prevent :modify_merge_request_committer_setting
end end
rule { owner_cannot_modify_approvers_rules & ~admin }.policy do rule { owner_cannot_modify_approvers_rules & ~admin }.policy do
......
- return unless License.feature_available?(:merge_request_approvers_rules) - return unless License.feature_available?(:admin_merge_request_approvers_rules)
%section.settings.merge-request-approval-settings.no-animate#js-visibility-settings{ class: ('expanded' if expanded_by_default?) } %section.settings.merge-request-approval-settings.no-animate{ class: ('expanded' if expanded_by_default?) }
.settings-header .settings-header
%h4 %h4
= _('Merge requests approvals') = _('Merge requests approvals')
%button.btn.js-settings-toggle{ type: 'button' } %button.btn.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand') = expanded_by_default? ? _('Collapse') : _('Expand')
%p %p
= _('Settings to prevent self-approval across all projects in the instance. Only an administrator can modify those settings.') = _('Settings to prevent self-approval across all projects in the instance. Only an administrator can modify these settings.')
.settings-content .settings-content
%hr.clearfix %hr.clearfix.mt-0
= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'merge-request-approval-settings'), html: { class: 'fieldset-form' } do |f| = form_for @application_setting, url: general_admin_application_settings_path(anchor: 'merge-request-approval-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting) = form_errors(@application_setting)
...@@ -18,14 +18,14 @@ ...@@ -18,14 +18,14 @@
.form-check .form-check
= f.check_box :prevent_merge_requests_author_approval, class: 'form-check-input' = f.check_box :prevent_merge_requests_author_approval, class: 'form-check-input'
= f.label :prevent_merge_requests_author_approval, class: 'form-check-label' do = f.label :prevent_merge_requests_author_approval, class: 'form-check-label' do
= s_('Prevent approval of merge requests by merge request author') = _('Prevent approval of merge requests by merge request author')
.form-check .form-check
= f.check_box :prevent_merge_requests_committers_approval, class: 'form-check-input' = f.check_box :prevent_merge_requests_committers_approval, class: 'form-check-input'
= f.label :prevent_merge_requests_committers_approval, class: 'form-check-label' do = f.label :prevent_merge_requests_committers_approval, class: 'form-check-label' do
= s_('Prevent approval of merge requests by merge request commiters') = _('Prevent approval of merge requests by merge request committers')
.form-check .form-check
= f.check_box :disable_overriding_approvers_per_merge_request , class: 'form-check-input' = f.check_box :disable_overriding_approvers_per_merge_request , class: 'form-check-input'
= f.label :disable_overriding_approvers_per_merge_request , class: 'form-check-label' do = f.label :disable_overriding_approvers_per_merge_request , class: 'form-check-label' do
= s_('Users cannot modify merge request approvers list') = _('Prevent users from modifing merge request approvers list')
= f.submit _('Save changes'), class: "btn btn-success" = f.submit _('Save changes'), class: "btn btn-success"
...@@ -2,14 +2,14 @@ ...@@ -2,14 +2,14 @@
- page_title _("Push Rules") - page_title _("Push Rules")
- @content_class = "limit-container-width" unless fluid_layout - @content_class = "limit-container-width" unless fluid_layout
%section.settings.no-animate#js-visibility-settings{ class: ('expanded' if expanded_by_default?) } %section.settings.no-animate{ class: ('expanded' if expanded_by_default?) }
.settings-header .settings-header
%h4 %h4
= _('Push Rules') = _('Push Rules')
%button.btn.js-settings-toggle{ type: 'button' } %button.btn.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand') = expanded_by_default? ? _('Collapse') : _('Expand')
%p %p
= _('Rules that define what git pushes are accepted for a project. All newly created projects will use this settings.') = _('Rules that define what git pushes are accepted for a project. All newly created projects will use these settings.')
.settings-content .settings-content
= render 'push_rules' = render 'push_rules'
......
- can_override_approvers = project.can_override_approvers? - can_override_approvers = project.can_override_approvers?
- can_modify_approvers = can?(current_user, :modify_approvers_rules, @project)
- can_modify_merge_request_author_settings = can?(current_user, :modify_merge_request_author_setting, @project)
- can_modify_merge_request_committer_settings = can?(current_user, :modify_merge_request_committer_setting, @project)
.form-group .form-group
= form.label :approver_ids, class: 'label-bold' do = form.label :approver_ids, class: 'label-bold' do
= _("Approval rules") = _("Approval rules")
#js-mr-approvals-settings{ data: { 'project_id': @project.id, #js-mr-approvals-settings{ data: { 'project_id': @project.id,
'can_edit': can?(current_user, :modify_approvers_list, @project).to_s, 'can_edit': can_modify_approvers.to_s,
'project_path': expose_path(api_v4_projects_path(id: @project.id)), 'project_path': expose_path(api_v4_projects_path(id: @project.id)),
'settings_path': expose_path(api_v4_projects_approval_settings_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)), 'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: @project.id)),
...@@ -27,7 +30,7 @@ ...@@ -27,7 +30,7 @@
.form-group .form-group
.form-check .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_modify_approvers }, false, true)
= form.label :disable_overriding_approvers_per_merge_request, class: 'form-check-label' do = form.label :disable_overriding_approvers_per_merge_request, class: 'form-check-label' do
%span= _('Can override approvers and approvals required per merge request') %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' = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'prevent-overriding-default-approvals'), target: '_blank'
...@@ -40,7 +43,7 @@ ...@@ -40,7 +43,7 @@
.form-group.self-approval .form-group.self-approval
.form-check .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_modify_merge_request_author_settings }, false, true
= form.label :merge_requests_author_approval, class: 'form-check-label' do = form.label :merge_requests_author_approval, class: 'form-check-label' do
%span= _('Prevent approval of merge requests by merge request author') %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', = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
...@@ -48,7 +51,7 @@ ...@@ -48,7 +51,7 @@
.form-group.committers-approval .form-group.committers-approval
.form-check .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_modify_merge_request_committer_settings, class: 'form-check-input' }
= form.label :merge_requests_disable_committers_approval, class: 'form-check-label' do = form.label :merge_requests_disable_committers_approval, class: 'form-check-label' do
%span= _('Prevent approval of merge requests by merge request committers') %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', = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
......
---
title: Resolve Disable self-approval at the Instance level
merge_request: 23731
author:
type: added
...@@ -9,7 +9,7 @@ module API ...@@ -9,7 +9,7 @@ module API
helpers do helpers do
def filter_params(params) def filter_params(params)
unless can?(current_user, :modify_merge_request_commiter_setting, user_project) unless can?(current_user, :modify_merge_request_committer_setting, user_project)
params.delete(:merge_requests_disable_committers_approval) params.delete(:merge_requests_disable_committers_approval)
end end
...@@ -21,7 +21,6 @@ module API ...@@ -21,7 +21,6 @@ module API
params.delete(:merge_requests_author_approval) params.delete(:merge_requests_author_approval)
end end
# binding.pry
params params
end end
end end
......
...@@ -218,7 +218,7 @@ module EE ...@@ -218,7 +218,7 @@ module EE
::License.feature_available?(:repository_mirrors) ::License.feature_available?(:repository_mirrors)
end) end)
expose(*EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes, if: ->(_instance, _options) do expose(*EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes, if: ->(_instance, _options) do
::License.feature_available?(:merge_request_approvers_rules) ::License.feature_available?(:admin_merge_request_approvers_rules)
end) end)
expose :email_additional_text, if: ->(_instance, _opts) { ::License.feature_available?(:email_additional_text) } expose :email_additional_text, if: ->(_instance, _opts) { ::License.feature_available?(:email_additional_text) }
expose :file_template_project_id, if: ->(_instance, _opts) { ::License.feature_available?(:custom_file_templates) } expose :file_template_project_id, if: ->(_instance, _opts) { ::License.feature_available?(:custom_file_templates) }
......
...@@ -40,7 +40,7 @@ module EE ...@@ -40,7 +40,7 @@ module EE
optional :updating_name_disabled_for_users, type: Grape::API::Boolean, desc: 'Flag indicating if users are permitted to update their profile name' optional :updating_name_disabled_for_users, type: Grape::API::Boolean, desc: 'Flag indicating if users are permitted to update their profile name'
optional :disable_overriding_approvers_per_merge_request, type: Grape::API::Boolean, desc: 'Disable Users ability to overwrite approvers in merge requests.' optional :disable_overriding_approvers_per_merge_request, type: Grape::API::Boolean, desc: 'Disable Users ability to overwrite approvers in merge requests.'
optional :prevent_merge_requests_author_approval, type: Grape::API::Boolean, desc: 'Disable Merge request author ability to approve request.' optional :prevent_merge_requests_author_approval, type: Grape::API::Boolean, desc: 'Disable Merge request author ability to approve request.'
optional :prevent_merge_requests_committers_approval, type: Grape::API::Boolean, desc: 'Disable Merge request commiter ability to approve request.' optional :prevent_merge_requests_committers_approval, type: Grape::API::Boolean, desc: 'Disable Merge request committer ability to approve request.'
end end
end end
......
...@@ -35,7 +35,7 @@ module EE ...@@ -35,7 +35,7 @@ module EE
attrs = attrs.except(:updating_name_disabled_for_users) attrs = attrs.except(:updating_name_disabled_for_users)
end end
unless License.feature_available?(:merge_request_approvers_rules) unless License.feature_available?(:admin_merge_request_approvers_rules)
attrs = attrs.except(*EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes) attrs = attrs.except(*EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes)
end end
......
...@@ -141,7 +141,7 @@ describe Admin::ApplicationSettingsController do ...@@ -141,7 +141,7 @@ describe Admin::ApplicationSettingsController do
prevent_merge_requests_committers_approval: true prevent_merge_requests_committers_approval: true
} }
end end
let(:feature) { :merge_request_approvers_rules } let(:feature) { :admin_merge_request_approvers_rules }
it_behaves_like 'settings for licensed features' it_behaves_like 'settings for licensed features'
end end
......
...@@ -335,7 +335,7 @@ describe ProjectsController do ...@@ -335,7 +335,7 @@ describe ProjectsController do
with_them do with_them do
before do before do
stub_licensed_features(merge_request_approvers_rules: license_value) stub_licensed_features(admin_merge_request_approvers_rules: license_value)
stub_application_setting(app_setting => setting_value) stub_application_setting(app_setting => setting_value)
end end
......
...@@ -484,7 +484,6 @@ describe Project do ...@@ -484,7 +484,6 @@ describe Project do
context 'merge requests related settings' do context 'merge requests related settings' do
shared_examples 'setting modified by application setting' do shared_examples 'setting modified by application setting' do
context 'final setting' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:app_setting, :project_setting, :feature_enabled, :final_setting) do where(:app_setting, :project_setting, :feature_enabled, :final_setting) do
...@@ -512,11 +511,10 @@ describe Project do ...@@ -512,11 +511,10 @@ describe Project do
end end
end end
end end
end
describe '#disable_overriding_approvers_per_merge_request' do describe '#disable_overriding_approvers_per_merge_request' do
it_behaves_like 'setting modified by application setting' do it_behaves_like 'setting modified by application setting' do
let(:feature) { :merge_request_approvers_rules } let(:feature) { :admin_merge_request_approvers_rules }
let(:setting) { :disable_overriding_approvers_per_merge_request } let(:setting) { :disable_overriding_approvers_per_merge_request }
let(:application_setting) { :disable_overriding_approvers_per_merge_request } let(:application_setting) { :disable_overriding_approvers_per_merge_request }
end end
...@@ -524,14 +522,13 @@ describe Project do ...@@ -524,14 +522,13 @@ describe Project do
describe '#merge_requests_disable_committers_approval' do describe '#merge_requests_disable_committers_approval' do
it_behaves_like 'setting modified by application setting' do it_behaves_like 'setting modified by application setting' do
let(:feature) { :merge_request_approvers_rules } let(:feature) { :admin_merge_request_approvers_rules }
let(:setting) { :merge_requests_disable_committers_approval } let(:setting) { :merge_requests_disable_committers_approval }
let(:application_setting) { :prevent_merge_requests_committers_approval } let(:application_setting) { :prevent_merge_requests_committers_approval }
end end
end end
describe '#merge_requests_author_approval' do describe '#merge_requests_author_approval' do
context 'final setting' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:app_setting, :project_setting, :feature_enabled, :final_setting) do where(:app_setting, :project_setting, :feature_enabled, :final_setting) do
...@@ -547,7 +544,7 @@ describe Project do ...@@ -547,7 +544,7 @@ describe Project do
with_them do with_them do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:feature) { :merge_request_approvers_rules } let(:feature) { :admin_merge_request_approvers_rules }
let(:setting) { :merge_requests_author_approval } let(:setting) { :merge_requests_author_approval }
let(:application_setting) { :prevent_merge_requests_author_approval } let(:application_setting) { :prevent_merge_requests_author_approval }
...@@ -563,7 +560,6 @@ describe Project do ...@@ -563,7 +560,6 @@ describe Project do
end end
end end
end end
end
describe '#has_active_hooks?' do describe '#has_active_hooks?' do
context "with group hooks" do context "with group hooks" do
......
...@@ -1228,14 +1228,14 @@ describe ProjectPolicy do ...@@ -1228,14 +1228,14 @@ describe ProjectPolicy do
:owner | false | true :owner | false | true
:owner | true | false :owner | true | false
:admin | false | true :admin | false | true
:admin | true | false :admin | true | true
end end
with_them do with_them do
let(:current_user) { public_send(role) } let(:current_user) { public_send(role) }
before do before do
stub_licensed_features(merge_request_approvers_rules: true) stub_licensed_features(admin_merge_request_approvers_rules: true)
stub_application_setting(setting_name => setting) stub_application_setting(setting_name => setting)
end end
...@@ -1262,7 +1262,7 @@ describe ProjectPolicy do ...@@ -1262,7 +1262,7 @@ describe ProjectPolicy do
let(:current_user) { public_send(role) } let(:current_user) { public_send(role) }
before do before do
stub_licensed_features(merge_request_approvers_rules: false) stub_licensed_features(admin_merge_request_approvers_rules: false)
stub_application_setting(setting_name => setting) stub_application_setting(setting_name => setting)
end end
...@@ -1287,10 +1287,10 @@ describe ProjectPolicy do ...@@ -1287,10 +1287,10 @@ describe ProjectPolicy do
end end
end end
describe ':modify_merge_request_commiter_setting' do describe ':modify_merge_request_committer_setting' do
it_behaves_like 'merge request rules' do it_behaves_like 'merge request rules' do
let(:setting_name) { :prevent_merge_requests_committers_approval } let(:setting_name) { :prevent_merge_requests_committers_approval }
let(:policy) { :modify_merge_request_commiter_setting } let(:policy) { :modify_merge_request_committer_setting }
end end
end end
...@@ -1317,7 +1317,7 @@ describe ProjectPolicy do ...@@ -1317,7 +1317,7 @@ describe ProjectPolicy do
let(:current_user) { public_send(role) } let(:current_user) { public_send(role) }
before do before do
stub_licensed_features(merge_request_approvers_rules: true) stub_licensed_features(admin_merge_request_approvers_rules: true)
stub_application_setting(setting_name => setting) stub_application_setting(setting_name => setting)
end end
...@@ -1344,7 +1344,7 @@ describe ProjectPolicy do ...@@ -1344,7 +1344,7 @@ describe ProjectPolicy do
let(:current_user) { public_send(role) } let(:current_user) { public_send(role) }
before do before do
stub_licensed_features(merge_request_approvers_rules: false) stub_licensed_features(admin_merge_request_approvers_rules: false)
stub_application_setting(setting_name => setting) stub_application_setting(setting_name => setting)
end end
......
...@@ -107,14 +107,14 @@ describe API::ProjectApprovals do ...@@ -107,14 +107,14 @@ describe API::ProjectApprovals do
false | false | true | true false | false | true | true
false | true | true | true false | true | true | true
true | false | false | false true | false | false | false
true | true | false | nil true | true | false | false
true | false | true | true true | false | true | true
true | true | true | nil true | true | true | true
end end
with_them do with_them do
before do before do
stub_licensed_features(merge_request_approvers_rules: license_value) stub_licensed_features(admin_merge_request_approvers_rules: license_value)
stub_application_setting(app_setting => setting_value) stub_application_setting(app_setting => setting_value)
end end
...@@ -145,42 +145,6 @@ describe API::ProjectApprovals do ...@@ -145,42 +145,6 @@ describe API::ProjectApprovals do
end end
context 'updates merge requests settings' do context 'updates merge requests settings' do
# context 'updates merge requests author approval setting when possible' 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 | false
# true | false | true | true
# true | true | true | false
# end
# with_them do
# let(:current_user) { admin }
# let(:app_setting) { :prevent_merge_requests_committers_approval }
# let(:setting) { :merge_requests_disable_committers_approval }
# before do
# stub_licensed_features(merge_request_approvers_rules: license_value)
# stub_application_setting(app_setting => setting_value)
# end
# it 'changes settings properly' do
# settings = {
# setting => param_value
# }
# post api(url, current_user), params: settings
# expect(json_response.symbolize_keys).to include({ setting => final_value})
# end
# end
# end
it_behaves_like 'updates merge requests settings when possible' do it_behaves_like 'updates merge requests settings when possible' do
let(:current_user) { admin } let(:current_user) { admin }
let(:app_setting) { :disable_overriding_approvers_per_merge_request } let(:app_setting) { :disable_overriding_approvers_per_merge_request }
......
...@@ -172,7 +172,7 @@ describe API::Settings, 'EE Settings' do ...@@ -172,7 +172,7 @@ describe API::Settings, 'EE Settings' do
prevent_merge_requests_committers_approval: true prevent_merge_requests_committers_approval: true
} }
end end
let(:feature) { :merge_request_approvers_rules } let(:feature) { :admin_merge_request_approvers_rules }
it_behaves_like 'settings for licensed features' it_behaves_like 'settings for licensed features'
end end
......
...@@ -14107,9 +14107,6 @@ msgstr "" ...@@ -14107,9 +14107,6 @@ msgstr ""
msgid "Prevent approval of merge requests by merge request author" msgid "Prevent approval of merge requests by merge request author"
msgstr "" msgstr ""
msgid "Prevent approval of merge requests by merge request commiters"
msgstr ""
msgid "Prevent approval of merge requests by merge request committers" msgid "Prevent approval of merge requests by merge request committers"
msgstr "" msgstr ""
...@@ -14119,6 +14116,9 @@ msgstr "" ...@@ -14119,6 +14116,9 @@ msgstr ""
msgid "Prevent users from changing their profile name" msgid "Prevent users from changing their profile name"
msgstr "" msgstr ""
msgid "Prevent users from modifing merge request approvers list"
msgstr ""
msgid "Preview" msgid "Preview"
msgstr "" msgstr ""
...@@ -16340,7 +16340,7 @@ msgstr "" ...@@ -16340,7 +16340,7 @@ msgstr ""
msgid "Rook" msgid "Rook"
msgstr "" msgstr ""
msgid "Rules that define what git pushes are accepted for a project. All newly created projects will use this settings." msgid "Rules that define what git pushes are accepted for a project. All newly created projects will use these settings."
msgstr "" msgstr ""
msgid "Run CI/CD pipelines for external repositories" msgid "Run CI/CD pipelines for external repositories"
...@@ -17379,7 +17379,7 @@ msgstr "" ...@@ -17379,7 +17379,7 @@ msgstr ""
msgid "Settings" msgid "Settings"
msgstr "" msgstr ""
msgid "Settings to prevent self-approval across all projects in the instance. Only an administrator can modify those settings." msgid "Settings to prevent self-approval across all projects in the instance. Only an administrator can modify these settings."
msgstr "" msgstr ""
msgid "Severity: %{severity}" msgid "Severity: %{severity}"
...@@ -21121,9 +21121,6 @@ msgstr "" ...@@ -21121,9 +21121,6 @@ msgstr ""
msgid "Users" msgid "Users"
msgstr "" msgstr ""
msgid "Users cannot modify merge request approvers list"
msgstr ""
msgid "Users in License:" msgid "Users in License:"
msgstr "" msgstr ""
......
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