Merge branch '335964-update-project-approval-section-to-vue' into 'master'

Update the project approval settings section to Vue

See merge request gitlab-org/gitlab!66536

app/assets/javascripts/pages/projects/edit/index.js

@@ -25,10 +25,6 @@ initProjectLoadingSpinner();
 initProjectPermissionsSettings();
 setupTransferEdit('.js-project-transfer-form', 'select.select2');
 
-dirtySubmitFactory(
-  document.querySelectorAll(
-    '.js-general-settings-form, .js-mr-settings-form, .js-mr-approvals-form',
-  ),
-);
+dirtySubmitFactory(document.querySelectorAll('.js-general-settings-form, .js-mr-settings-form'));
 
 initSearchSettings();

ee/app/assets/javascripts/approvals/components/approval_settings.vue

 <script>
 import { GlAlert, GlButton, GlForm, GlFormGroup, GlLoadingIcon } from '@gitlab/ui';
 import { isEmpty } from 'lodash';
-import { mapActions, mapState } from 'vuex';
+import { mapActions, mapGetters, mapState } from 'vuex';
 import { mapComputed } from '~/vuex_shared/bindings';
 import { APPROVAL_SETTINGS_I18N } from '../constants';
 import ApprovalSettingsCheckbox from './approval_settings_checkbox.vue';
@@ -20,6 +20,21 @@ export default {
       type: String,
       required: true,
     },
+    canPreventMrApprovalRuleEdit: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
+    canPreventAuthorApproval: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
+    canPreventCommittersApproval: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
   },
   computed: {
     ...mapState({
@@ -39,6 +54,7 @@ export default {
       undefined,
       (state) => state.approvalSettings.settings,
     ),
+    ...mapGetters(['settingChanged']),
     hasSettings() {
       return !isEmpty(this.settings);
     },
@@ -61,12 +77,11 @@ export default {
     },
   },
   links: {
-    preventAuthorApprovalDocsAnchor:
-      'allowing-merge-request-authors-to-approve-their-own-merge-requests',
-    preventMrApprovalRuleEditDocsAnchor: 'editing--overriding-approval-rules-per-merge-request',
-    requireUserPasswordDocsAnchor: 'require-authentication-when-approving-a-merge-request',
-    removeApprovalsOnPushDocsAnchor: 'resetting-approvals-on-push',
-    preventCommittersApprovalAnchor: 'prevent-approval-of-merge-requests-by-their-committers',
+    preventAuthorApprovalDocsAnchor: 'prevent-authors-from-approving-their-own-work',
+    preventMrApprovalRuleEditDocsAnchor: 'prevent-overrides-of-default-approvals',
+    requireUserPasswordDocsAnchor: 'require-authentication-for-approvals',
+    removeApprovalsOnPushDocsAnchor: 'reset-approvals-on-push',
+    preventCommittersApprovalDocsAnchor: 'prevent-committers-from-approving-their-own-work',
   },
   i18n: APPROVAL_SETTINGS_I18N,
 };
@@ -103,12 +118,24 @@ export default {
           v-model="preventAuthorApproval"
           :label="$options.i18n.authorApprovalLabel"
           :anchor="$options.links.preventAuthorApprovalDocsAnchor"
+          :locked="!canPreventAuthorApproval"
+          :locked-text="$options.i18n.lockedByAdmin"
           data-testid="prevent-author-approval"
         />
+        <approval-settings-checkbox
+          v-model="preventCommittersApproval"
+          :label="$options.i18n.preventCommittersApprovalLabel"
+          :anchor="$options.links.preventCommittersApprovalDocsAnchor"
+          :locked="!canPreventCommittersApproval"
+          :locked-text="$options.i18n.lockedByAdmin"
+          data-testid="prevent-committers-approval"
+        />
         <approval-settings-checkbox
           v-model="preventMrApprovalRuleEdit"
           :label="$options.i18n.preventMrApprovalRuleEditLabel"
           :anchor="$options.links.preventMrApprovalRuleEditDocsAnchor"
+          :locked="!canPreventMrApprovalRuleEdit"
+          :locked-text="$options.i18n.lockedByAdmin"
           data-testid="prevent-mr-approval-rule-edit"
         />
         <approval-settings-checkbox
@@ -123,14 +150,14 @@ export default {
           :anchor="$options.links.removeApprovalsOnPushDocsAnchor"
           data-testid="remove-approvals-on-push"
         />
-        <approval-settings-checkbox
-          v-model="preventCommittersApproval"
-          :label="$options.i18n.preventCommittersApprovalLabel"
-          :anchor="$options.links.preventCommittersApprovalAnchor"
-          data-testid="prevent-committers-approval"
-        />
       </gl-form-group>
-      <gl-button type="submit" variant="confirm" category="primary" :loading="isLoading">
+      <gl-button
+        type="submit"
+        variant="confirm"
+        category="primary"
+        :disabled="!settingChanged"
+        :loading="isLoading"
+      >
         {{ $options.i18n.saveChanges }}
       </gl-button>
     </gl-form>

ee/app/assets/javascripts/approvals/components/project_settings/app.vue

 <script>
 import App from '../app.vue';
+import ProjectApprovalSettings from './project_approval_settings.vue';
 import ProjectRules from './project_rules.vue';
 
 export default {
   components: {
     App,
+    ProjectApprovalSettings,
     ProjectRules,
   },
 };
 </script>
 
 <template>
-  <app :is-mr-edit="false">
-    <template #rules>
-      <project-rules />
-    </template>
-  </app>
+  <div>
+    <app :is-mr-edit="false">
+      <template #rules>
+        <project-rules />
+      </template>
+    </app>
+    <project-approval-settings class="gl-mt-5" />
+  </div>
 </template>

ee/app/assets/javascripts/approvals/components/project_settings/project_approval_settings.vue

+<script>
+import { mapState } from 'vuex';
+import { __ } from '~/locale';
+import ApprovalSettings from '../approval_settings.vue';
+
+export default {
+  components: {
+    ApprovalSettings,
+  },
+  computed: {
+    ...mapState({
+      approvalsPath: (state) => state.settings.approvalsPath,
+      canPreventMrApprovalRuleEdit: (state) => state.settings.canEdit,
+      canModifyAuthorSettings: (state) => state.settings.canModifyAuthorSettings,
+      canModifyCommiterSettings: (state) => state.settings.canModifyCommiterSettings,
+    }),
+  },
+  i18n: {
+    projectSettingsHeader: __('Approval settings'),
+  },
+};
+</script>
+
+<template>
+  <div data-testid="merge-request-approval-settings">
+    <label class="label-bold">
+      {{ $options.i18n.projectSettingsHeader }}
+    </label>
+    <approval-settings
+      :approval-settings-path="approvalsPath"
+      :can-prevent-author-approval="canModifyAuthorSettings"
+      :can-prevent-committers-approval="canModifyCommiterSettings"
+      :can-prevent-mr-approval-rule-edit="canPreventMrApprovalRuleEdit"
+    />
+  </div>
+</template>

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

@@ -47,7 +47,7 @@ export const APPROVAL_RULE_CONFIGS = {
   },
 };
 
-export const APPROVALS_HELP_PATH = 'user/project/merge_requests/merge_request_approvals';
+export const APPROVALS_HELP_PATH = 'user/project/merge_requests/approvals/settings';
 
 export const APPROVAL_SETTINGS_I18N = {
   authorApprovalLabel: s__('ApprovalSettings|Prevent MR approvals by the author.'),
@@ -69,6 +69,9 @@ export const APPROVAL_SETTINGS_I18N = {
     'ApprovalSettings|There was an error updating merge request approval settings.',
   ),
   savingSuccessMessage: s__('ApprovalSettings|Merge request approval settings have been updated.'),
+  lockedByAdmin: s__(
+    'ApprovalSettings|This setting is configured at the instance level and can only be changed by an administrator.',
+  ),
 };
 
 export const APPROVAL_DIALOG_I18N = {

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

@@ -112,3 +112,20 @@ export const groupApprovalsMappers = {
     allow_committer_approval: !state.settings.preventCommittersApproval,
   }),
 };
+
+export const projectApprovalsMappers = {
+  mapDataToState: (data) => ({
+    preventAuthorApproval: !data.merge_requests_author_approval,
+    preventMrApprovalRuleEdit: data.disable_overriding_approvers_per_merge_request,
+    requireUserPassword: data.require_password_to_approve,
+    removeApprovalsOnPush: data.reset_approvals_on_push,
+    preventCommittersApproval: data.merge_requests_disable_committers_approval,
+  }),
+  mapStateToPayload: (state) => ({
+    merge_requests_author_approval: !state.settings.preventAuthorApproval,
+    disable_overriding_approvers_per_merge_request: state.settings.preventMrApprovalRuleEdit,
+    require_password_to_approve: state.settings.requireUserPassword,
+    reset_approvals_on_push: state.settings.removeApprovalsOnPush,
+    merge_requests_disable_committers_approval: state.settings.preventCommittersApproval,
+  }),
+};

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

 import Vue from 'vue';
 import { parseBoolean } from '~/lib/utils/common_utils';
 import ProjectSettingsApp from './components/project_settings/app.vue';
+import { projectApprovalsMappers } from './mappers';
 import createStore from './stores';
+import approvalSettingsModule from './stores/modules/approval_settings';
 import projectSettingsModule from './stores/modules/project_settings';
 
 export default function mountProjectSettingsApprovals(el) {
@@ -15,15 +17,19 @@ export default function mountProjectSettingsApprovals(el) {
     coverageCheckHelpPagePath,
   } = el.dataset;
 
-  const store = createStore(
-    { approvals: projectSettingsModule() },
-    {
-      ...el.dataset,
-      prefix: 'project-settings',
-      allowMultiRule: parseBoolean(el.dataset.allowMultiRule),
-      canEdit: parseBoolean(el.dataset.canEdit),
-    },
-  );
+  const modules = {
+    approvalSettings: approvalSettingsModule({ updateMethod: 'post', ...projectApprovalsMappers }),
+    approvals: projectSettingsModule(),
+  };
+
+  const store = createStore(modules, {
+    ...el.dataset,
+    prefix: 'project-settings',
+    allowMultiRule: parseBoolean(el.dataset.allowMultiRule),
+    canEdit: parseBoolean(el.dataset.canEdit),
+    canModifyAuthorSettings: parseBoolean(el.dataset.canModifyAuthorSettings),
+    canModifyCommiterSettings: parseBoolean(el.dataset.canModifyCommiterSettings),
+  });
 
   return new Vue({
     el,

ee/app/assets/javascripts/approvals/stores/modules/approval_settings/actions.js

@@ -2,7 +2,7 @@ import * as Sentry from '@sentry/browser';
 import axios from '~/lib/utils/axios_utils';
 import * as types from './mutation_types';
 
-export default (mapStateToPayload) => ({
+export default (mapStateToPayload, updateMethod = 'put') => ({
   fetchSettings({ commit }, endpoint) {
     commit(types.REQUEST_SETTINGS);
 
@@ -22,8 +22,11 @@ export default (mapStateToPayload) => ({
   updateSettings({ commit, state }, endpoint) {
     commit(types.REQUEST_UPDATE_SETTINGS);
 
-    return axios
-      .put(endpoint, { ...mapStateToPayload(state) })
+    return axios({
+      method: updateMethod,
+      url: endpoint,
+      data: { ...mapStateToPayload(state) },
+    })
       .then(({ data }) => {
         commit(types.UPDATE_SETTINGS_SUCCESS, data);
       })

ee/app/assets/javascripts/approvals/stores/modules/approval_settings/getters.js

+export const settingChanged = ({ settings, initialSettings }) => {
+  return Object.entries(settings).findIndex(([key, value]) => initialSettings[key] !== value) > -1;
+};

ee/app/assets/javascripts/approvals/stores/modules/approval_settings/index.js

 import actionsFactory from './actions';
+import * as getters from './getters';
 import mutationsFactory from './mutations';
 import createState from './state';
 
-export default ({ mapStateToPayload, mapDataToState }) => ({
+export default ({ mapStateToPayload, mapDataToState, updateMethod = 'put' }) => ({
   state: createState(),
-  actions: actionsFactory(mapStateToPayload),
+  actions: actionsFactory(mapStateToPayload, updateMethod),
   mutations: mutationsFactory(mapDataToState),
+  getters,
 });

ee/app/assets/javascripts/approvals/stores/modules/approval_settings/mutations.js

@@ -8,6 +8,7 @@ export default (mapDataToState) => ({
   },
   [types.RECEIVE_SETTINGS_SUCCESS](state, data) {
     state.settings = { ...mapDataToState(data) };
+    state.initialSettings = { ...state.settings };
     state.isLoading = false;
   },
   [types.RECEIVE_SETTINGS_ERROR](state) {
@@ -21,6 +22,7 @@ export default (mapDataToState) => ({
   },
   [types.UPDATE_SETTINGS_SUCCESS](state, data) {
     state.settings = { ...mapDataToState(data) };
+    state.initialSettings = { ...state.settings };
     state.isLoading = false;
     state.isUpdated = true;
   },

ee/app/assets/javascripts/approvals/stores/modules/approval_settings/state.js

 export default () => ({
   settings: {},
+  initialSettings: {},
   isLoading: false,
   isUpdated: false,
   errorMessage: '',

ee/app/helpers/ee/projects_helper.rb

@@ -72,8 +72,11 @@ module EE
         data: {
           'project_id': project.id,
           'can_edit': can_modify_approvers.to_s,
+          'can_modify_author_settings': can_modify_author_settings.to_s,
+          'can_modify_commiter_settings': can_modify_commiter_settings.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)),
+          'approvals_path': expose_path(api_v4_projects_approvals_path(id: project.id)),
           'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: project.id)),
           'allow_multi_rule': project.multiple_approval_rules_available?.to_s,
           'eligible_approvers_docs_path': help_page_path('user/project/merge_requests/approvals/rules', anchor: 'eligible-approvers'),
@@ -99,6 +102,14 @@ module EE
       can?(current_user, :modify_approvers_rules, project)
     end
 
+    def can_modify_author_settings(project = @project)
+      can?(current_user, :modify_merge_request_author_setting, project)
+    end
+
+    def can_modify_commiter_settings(project = @project)
+      can?(current_user, :modify_merge_request_committer_setting, project)
+    end
+
     def permanent_delete_message(project)
       message = _('This action will %{strongOpen}permanently delete%{strongClose} %{codeOpen}%{project}%{codeClose} %{strongOpen}immediately%{strongClose}, including its repositories and all related resources, including issues and merge requests.')
       html_escape(message) % remove_message_data(project)

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

@@ -10,7 +10,4 @@
       = link_to _("Learn more."), help_page_path("user/project/merge_requests/approvals/index.md"), target: '_blank'
 
   .settings-content
-    = form_for @project, html: { class: "merge-request-approval-settings-form js-mr-approvals-form" }, authenticity_token: true do |f|
-      %input{ name: 'update_section', type: 'hidden', value: 'js-merge-request-approval-settings' }
-      = render 'projects/merge_request_approvals_settings_form', form: f, project: @project
-      = f.submit _("Save changes"), class: "btn gl-button btn-confirm gl-mt-4", data: { qa_selector: 'save_merge_request_approval_settings_button' }
+    = render 'projects/merge_request_approvals_settings_form'

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

-- 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.label :approver_ids, class: 'label-bold' do
+  = label :approver_ids, class: 'label-bold' do
     = _("Approval rules")
   #js-mr-approvals-settings{ approvals_app_data }
     .text-center.gl-mt-3
       = sprite_icon('spinner', size: 24, css_class: 'gl-spinner')
-
-%fieldset.form-group
-  %legend.h5.gl-border-none
-    = _('Approval settings')
-  .gl-form-checkbox-group
-    .gl-form-checkbox.custom-control.custom-checkbox
-      = form.check_box(:disable_overriding_approvers_per_merge_request, { class: 'custom-control-input', disabled: !can_modify_approvers })
-      = form.label :disable_overriding_approvers_per_merge_request, class: 'custom-control-label' do
-        %span= _('Prevent users from modifying MR approval rules in merge requests.')
-        = link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings', anchor: 'prevent-overrides-of-default-approvals'), target: '_blank'
-
-    .gl-form-checkbox.custom-control.custom-checkbox
-      = form.check_box :reset_approvals_on_push, class: 'custom-control-input'
-      = form.label :reset_approvals_on_push, class: 'custom-control-label' do
-        %span= _('Require new approvals when new commits are added to an MR.')
-
-    .gl-form-checkbox.custom-control.custom-checkbox
-      = form.check_box :merge_requests_author_approval, { class: 'custom-control-input', disabled: !can_modify_merge_request_author_settings }, false, true
-      = form.label :merge_requests_author_approval, class: 'custom-control-label' do
-
-        %span= _('Prevent MR approvals by the author.')
-        = link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings',
-          anchor: 'prevent-authors-from-approving-their-own-work'), target: '_blank'
-
-    .gl-form-checkbox.custom-control.custom-checkbox
-      = form.check_box :merge_requests_disable_committers_approval, { disabled: !can_modify_merge_request_committer_settings, class: 'custom-control-input' }
-      = form.label :merge_requests_disable_committers_approval, class: 'custom-control-label' do
-        %span= _('Prevent MR approvals from users who make commits to the MR.')
-        = link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings',
-          anchor: 'prevent-committers-from-approving-their-own-work'), target: '_blank'
-
-    - if password_authentication_enabled_for_web?
-      .gl-form-checkbox.custom-control.custom-checkbox
-        = form.check_box :require_password_to_approve, class: 'custom-control-input'
-        = form.label :require_password_to_approve, class: 'custom-control-label' do
-          %span= _('Require user password for approvals.')
-          = link_to sprite_icon('question-o'), help_page_path('user/project/merge_requests/approvals/settings',
-            anchor: 'require-authentication-for-approvals'), target: '_blank'

ee/spec/features/admin/admin_merge_requests_approvals_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe 'Admin interacts with merge requests approvals settings' do
     visit(admin_push_rule_path)
   end
 
-  it 'updates instance-level merge request approval settings and enforces project-level ones' do
+  it 'updates instance-level merge request approval settings and enforces project-level ones', :js do
     page.within('.merge-request-approval-settings') do
       check 'Prevent MR approvals by author.'
       check 'Prevent MR approvals from users who make commits to the MR.'
@@ -34,10 +34,10 @@ RSpec.describe 'Admin interacts with merge requests approvals settings' do
 
     visit edit_project_path(project)
 
-    page.within('#js-merge-request-approval-settings') do
-      expect(find('#project_merge_requests_author_approval')).to be_disabled.and be_checked
-      expect(find('#project_merge_requests_disable_committers_approval')).to be_disabled.and be_checked
-      expect(find('#project_disable_overriding_approvers_per_merge_request')).to be_disabled.and be_checked
+    page.within('[data-testid="merge-request-approval-settings"]') do
+      expect(find('[data-testid="prevent-author-approval"] > input')).to be_disabled.and be_checked
+      expect(find('[data-testid="prevent-committers-approval"] > input')).to be_disabled.and be_checked
+      expect(find('[data-testid="prevent-mr-approval-rule-edit"] > input')).to be_disabled.and be_checked
     end
   end
 end

ee/spec/features/projects/audit_events_spec.rb

@@ -138,12 +138,12 @@ RSpec.describe 'Projects > Audit Events', :js do
       project.add_developer(pete)
     end
 
-    it "appears in the project's audit events" do
+    it "appears in the project's audit events", :js do
       visit edit_project_path(project)
 
-      page.within('#js-merge-request-approval-settings') do
-        uncheck 'project_merge_requests_author_approval'
-        check 'project_merge_requests_disable_committers_approval'
+      page.within('[data-testid="merge-request-approval-settings"]') do
+        find('[data-testid="prevent-author-approval"] > input').set(false)
+        find('[data-testid="prevent-committers-approval"] > input').set(true)
         click_button 'Save changes'
       end
 

ee/spec/frontend/approvals/components/approval_settings_spec.js

@@ -20,10 +20,11 @@ describe('ApprovalSettings', () => {
 
   const approvalSettingsPath = 'groups/22/merge_request_approval_settings';
 
-  const setupStore = (data = {}) => {
+  const setupStore = (data = {}, initialData) => {
     const module = approvalSettingsModule(groupApprovalsMappers);
 
     module.state.settings = data;
+    module.state.initialSettings = initialData || data;
     actions = module.actions;
     jest.spyOn(actions, 'fetchSettings').mockImplementation();
     jest.spyOn(actions, 'updateSettings').mockImplementation();
@@ -33,12 +34,16 @@ describe('ApprovalSettings', () => {
     store = createStore({ approvalSettings: module });
   };
 
-  const createWrapper = () => {
+  const createWrapper = (props = {}) => {
     wrapper = extendedWrapper(
       shallowMount(ApprovalSettings, {
         localVue,
         store,
-        propsData: { approvalSettingsPath },
+        propsData: {
+          approvalSettingsPath,
+          ...props,
+        },
+        stubs: { GlButton },
       }),
     );
   };
@@ -89,14 +94,16 @@ describe('ApprovalSettings', () => {
   });
 
   describe('with settings', () => {
+    const settings = {
+      allow_author_approval: false,
+      allow_committer_approval: false,
+      allow_overrides_to_approver_list_per_merge_request: false,
+      require_password_to_approve: false,
+      retain_approvals_on_push: false,
+    };
+
     beforeEach(() => {
-      setupStore({
-        allow_author_approval: false,
-        allow_committer_approval: false,
-        allow_overrides_to_approver_list_per_merge_request: false,
-        require_password_to_approve: false,
-        retain_approvals_on_push: false,
-      });
+      setupStore(settings);
     });
 
     it('renders the form once successfully loaded', async () => {
@@ -109,14 +116,14 @@ describe('ApprovalSettings', () => {
       expect(findForm().exists()).toBe(true);
     });
 
-    it('renders enabled button when not loading', async () => {
+    it('renders the button as not loading when loaded', async () => {
       createWrapper();
       await waitForPromises();
 
       expect(findSaveButton().props('loading')).toBe(false);
     });
 
-    it('renders loading button when loading', async () => {
+    it('renders the button as loading when updating', async () => {
       createWrapper();
       await waitForPromises();
       await store.commit('REQUEST_UPDATE_SETTINGS');
@@ -124,13 +131,29 @@ describe('ApprovalSettings', () => {
       expect(findSaveButton().props('loading')).toBe(true);
     });
 
+    it('renders the button as disabled when setting are unchanged', async () => {
+      createWrapper();
+      await waitForPromises();
+
+      expect(findSaveButton().attributes('disabled')).toBe('true');
+    });
+
+    it('renders the button as enabled when a setting was changed', async () => {
+      setupStore({ ...settings, allow_author_approval: true }, settings);
+
+      createWrapper();
+      await waitForPromises();
+
+      expect(findSaveButton().attributes('disabled')).toBeUndefined();
+    });
+
     describe.each`
       testid                             | action                            | setting                        | labelKey                            | anchor
-      ${'prevent-author-approval'}       | ${'setPreventAuthorApproval'}     | ${'preventAuthorApproval'}     | ${'authorApprovalLabel'}            | ${'allowing-merge-request-authors-to-approve-their-own-merge-requests'}
-      ${'prevent-committers-approval'}   | ${'setPreventCommittersApproval'} | ${'preventCommittersApproval'} | ${'preventCommittersApprovalLabel'} | ${'prevent-approval-of-merge-requests-by-their-committers'}
-      ${'prevent-mr-approval-rule-edit'} | ${'setPreventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEditLabel'} | ${'editing--overriding-approval-rules-per-merge-request'}
-      ${'require-user-password'}         | ${'setRequireUserPassword'}       | ${'requireUserPassword'}       | ${'requireUserPasswordLabel'}       | ${'require-authentication-when-approving-a-merge-request'}
-      ${'remove-approvals-on-push'}      | ${'setRemoveApprovalsOnPush'}     | ${'removeApprovalsOnPush'}     | ${'removeApprovalsOnPushLabel'}     | ${'resetting-approvals-on-push'}
+      ${'prevent-author-approval'}       | ${'setPreventAuthorApproval'}     | ${'preventAuthorApproval'}     | ${'authorApprovalLabel'}            | ${'prevent-authors-from-approving-their-own-work'}
+      ${'prevent-committers-approval'}   | ${'setPreventCommittersApproval'} | ${'preventCommittersApproval'} | ${'preventCommittersApprovalLabel'} | ${'prevent-committers-from-approving-their-own-work'}
+      ${'prevent-mr-approval-rule-edit'} | ${'setPreventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEdit'} | ${'preventMrApprovalRuleEditLabel'} | ${'prevent-overrides-of-default-approvals'}
+      ${'require-user-password'}         | ${'setRequireUserPassword'}       | ${'requireUserPassword'}       | ${'requireUserPasswordLabel'}       | ${'require-authentication-for-approvals'}
+      ${'remove-approvals-on-push'}      | ${'setRemoveApprovalsOnPush'}     | ${'removeApprovalsOnPush'}     | ${'removeApprovalsOnPushLabel'}     | ${'reset-approvals-on-push'}
     `('with the $testid checkbox', ({ testid, action, setting, labelKey, anchor }) => {
       let checkbox = null;
 
@@ -214,5 +237,27 @@ describe('ApprovalSettings', () => {
         });
       });
     });
+
+    describe('locked settings', () => {
+      it.each`
+        property                          | value    | locked   | testid
+        ${'canPreventAuthorApproval'}     | ${true}  | ${false} | ${'prevent-author-approval'}
+        ${'canPreventMrApprovalRuleEdit'} | ${true}  | ${false} | ${'prevent-mr-approval-rule-edit'}
+        ${'canPreventCommittersApproval'} | ${true}  | ${false} | ${'prevent-committers-approval'}
+        ${'canPreventAuthorApproval'}     | ${false} | ${true}  | ${'prevent-author-approval'}
+        ${'canPreventMrApprovalRuleEdit'} | ${false} | ${true}  | ${'prevent-mr-approval-rule-edit'}
+        ${'canPreventCommittersApproval'} | ${false} | ${true}  | ${'prevent-committers-approval'}
+      `(
+        `when $property is $value, then $testid has "locked" set to $locked`,
+        ({ property, value, locked, testid }) => {
+          createWrapper({ [property]: value });
+
+          expect(wrapper.findByTestId(testid).props()).toMatchObject({
+            locked,
+            lockedText: APPROVAL_SETTINGS_I18N.lockedByAdmin,
+          });
+        },
+      );
+    });
   });
 });

ee/spec/frontend/approvals/components/project_settings/project_approval_settings_spec.js

+import { shallowMount } from '@vue/test-utils';
+import Vue from 'vue';
+import Vuex from 'vuex';
+import ApprovalSettings from 'ee/approvals/components/approval_settings.vue';
+import ProjectApprovalSettings from 'ee/approvals/components/project_settings/project_approval_settings.vue';
+import { projectApprovalsMappers } from 'ee/approvals/mappers';
+import createStore from 'ee/approvals/stores';
+import approvalSettingsModule from 'ee/approvals/stores/modules/approval_settings';
+
+Vue.use(Vuex);
+
+describe('ProjectApprovalSettings', () => {
+  let wrapper;
+  let store;
+
+  const findApprovalSettings = () => wrapper.find(ApprovalSettings);
+
+  const setupStore = (data = {}) => {
+    store = createStore({
+      approvalSettings: approvalSettingsModule(projectApprovalsMappers),
+    });
+
+    store.state.settings = data;
+  };
+
+  const createWrapper = () => {
+    wrapper = shallowMount(ProjectApprovalSettings, { store });
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+    store = null;
+  });
+
+  it('configures the app with the store values', () => {
+    setupStore({
+      approvalsPath: 'foo',
+      canEdit: true,
+      canModifyAuthorSettings: false,
+      canModifyCommiterSettings: true,
+    });
+    createWrapper();
+
+    expect(findApprovalSettings().props()).toMatchObject({
+      approvalSettingsPath: 'foo',
+      canPreventMrApprovalRuleEdit: true,
+      canPreventAuthorApproval: false,
+      canPreventCommittersApproval: true,
+    });
+  });
+});

ee/spec/frontend/approvals/stores/modules/approval_settings/actions_spec.js

@@ -61,11 +61,18 @@ describe('EE approvals group settings module actions', () => {
     });
   });
 
-  describe('updateSettings', () => {
+  describe.each`
+    httpMethod | onMethod
+    ${'put'}   | ${'onPut'}
+    ${'post'}  | ${'onPost'}
+  `('updateSetting with $httpMethod', ({ httpMethod, onMethod }) => {
+    let actionsWithMethod;
+
     beforeEach(() => {
       state = {
         settings: {},
       };
+      actionsWithMethod = actionsFactory((data) => data, httpMethod);
     });
 
     describe('on success', () => {
@@ -77,10 +84,10 @@ describe('EE approvals group settings module actions', () => {
           require_password_to_approve: true,
           retain_approvals_on_push: true,
         };
-        mock.onPut(approvalSettingsPath).replyOnce(httpStatus.OK, data);
+        mock[onMethod](approvalSettingsPath).replyOnce(httpStatus.OK, data);
 
         return testAction(
-          actions.updateSettings,
+          actionsWithMethod.updateSettings,
           approvalSettingsPath,
           state,
           [
@@ -95,10 +102,10 @@ describe('EE approvals group settings module actions', () => {
     describe('on error', () => {
       it('dispatches the request, updates payload and sets error message', () => {
         const data = { message: 'Internal Server Error' };
-        mock.onPut(approvalSettingsPath).replyOnce(httpStatus.INTERNAL_SERVER_ERROR, data);
+        mock[onMethod](approvalSettingsPath).replyOnce(httpStatus.INTERNAL_SERVER_ERROR, data);
 
         return testAction(
-          actions.updateSettings,
+          actionsWithMethod.updateSettings,
           approvalSettingsPath,
           state,
           [{ type: types.REQUEST_UPDATE_SETTINGS }, { type: types.UPDATE_SETTINGS_ERROR }],

ee/spec/frontend/approvals/stores/modules/approval_settings/getters_spec.js

+import * as getters from 'ee/approvals/stores/modules/approval_settings/getters';
+
+describe('Group settings store getters', () => {
+  let settings;
+  const initialSettings = {
+    preventAuthorApproval: true,
+    preventMrApprovalRuleEdit: true,
+    requireUserPassword: true,
+    removeApprovalsOnPush: true,
+  };
+
+  beforeEach(() => {
+    settings = { ...initialSettings };
+  });
+
+  describe('settingChanged', () => {
+    it('returns true when a setting is changed', () => {
+      settings.preventAuthorApproval = false;
+
+      expect(getters.settingChanged({ settings, initialSettings })).toBe(true);
+    });
+
+    it('returns false when the setting remains unchanged', () => {
+      expect(getters.settingChanged({ settings, initialSettings })).toBe(false);
+    });
+  });
+});

ee/spec/helpers/projects_helper_spec.rb

@@ -389,6 +389,9 @@ RSpec.describe ProjectsHelper do
       expect(subject[:data]).to eq({
         project_id: project.id,
         can_edit: 'true',
+        can_modify_author_settings: 'true',
+        can_modify_commiter_settings: 'true',
+        approvals_path: expose_path(api_v4_projects_approvals_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)),
         rules_path: expose_path(api_v4_projects_approval_settings_rules_path(id: project.id)),

locale/gitlab.pot

@@ -4224,6 +4224,9 @@ msgstr ""
 msgid "ApprovalSettings|There was an error updating merge request approval settings."
 msgstr ""
 
+msgid "ApprovalSettings|This setting is configured at the instance level and can only be changed by an administrator."
+msgstr ""
+
 msgid "ApprovalStatusTooltip|Adheres to separation of duties"
 msgstr ""
 
@@ -25047,9 +25050,6 @@ msgstr ""
 msgid "Prevent MR approvals by author."
 msgstr ""
 
-msgid "Prevent MR approvals by the author."
-msgstr ""
-
 msgid "Prevent MR approvals from users who make commits to the MR."
 msgstr ""
 
@@ -25065,9 +25065,6 @@ msgstr ""
 msgid "Prevent users from changing their profile name"
 msgstr ""
 
-msgid "Prevent users from modifying MR approval rules in merge requests."
-msgstr ""
-
 msgid "Prevent users from modifying MR approval rules in projects and merge requests."
 msgstr ""
 
@@ -28134,12 +28131,6 @@ msgstr ""
 msgid "Require all users to set up two-factor authentication"
 msgstr ""
 
-msgid "Require new approvals when new commits are added to an MR."
-msgstr ""
-
-msgid "Require user password for approvals."
-msgstr ""
-
 msgid "Required approvals (%{approvals_given} given)"
 msgstr ""