Merge branch '247904-add-require-user-password-to-approve-to-group-level' into 'master'

Add Require user password to approve to group-level MR approval settings

See merge request gitlab-org/gitlab!57219

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

 <script>
-import { GlButton, GlForm, GlFormGroup, GlFormCheckbox, GlIcon, GlLink } from '@gitlab/ui';
+import { GlButton, GlForm, GlFormGroup } from '@gitlab/ui';
 import { mapActions, mapState } from 'vuex';
-import { helpPagePath } from '~/helpers/help_page_helper';
 import { __ } from '~/locale';
+import ApprovalSettingsCheckbox from './approval_settings_checkbox.vue';
 
 export default {
   components: {
+    ApprovalSettingsCheckbox,
     GlButton,
     GlForm,
     GlFormGroup,
-    GlFormCheckbox,
-    GlIcon,
-    GlLink,
   },
   props: {
     approvalSettingsPath: {
@@ -35,17 +33,14 @@ export default {
     },
   },
   links: {
-    preventAuthorApprovalDocsPath: helpPagePath(
-      'user/project/merge_requests/merge_request_approvals',
-      {
-        anchor: 'allowing-merge-request-authors-to-approve-their-own-merge-requests',
-      },
-    ),
+    preventAuthorApprovalDocsAnchor:
+      'allowing-merge-request-authors-to-approve-their-own-merge-requests',
+    requireUserPasswordDocsAnchor: 'require-authentication-when-approving-a-merge-request',
   },
   i18n: {
     authorApprovalLabel: __('Prevent MR approvals by the author.'),
+    requireUserPasswordLabel: __('Require user password for approvals.'),
     saveChanges: __('Save changes'),
-    helpLabel: __('Help'),
   },
 };
 </script>
@@ -53,15 +48,18 @@ export default {
 <template>
   <gl-form @submit.prevent="onSubmit">
     <gl-form-group>
-      <gl-form-checkbox
+      <approval-settings-checkbox
         v-model="settings.preventAuthorApproval"
+        :label="$options.i18n.authorApprovalLabel"
+        :anchor="$options.links.preventAuthorApprovalDocsAnchor"
         data-testid="prevent-author-approval"
-      >
-        {{ $options.i18n.authorApprovalLabel }}
-        <gl-link :href="$options.links.preventAuthorApprovalDocsPath" target="_blank">
-          <gl-icon name="question-o" :aria-label="$options.i18n.helpLabel" :size="16"
-        /></gl-link>
-      </gl-form-checkbox>
+      />
+      <approval-settings-checkbox
+        v-model="settings.requireUserPassword"
+        :label="$options.i18n.requireUserPasswordLabel"
+        :anchor="$options.links.requireUserPasswordDocsAnchor"
+        data-testid="require-user-password"
+      />
     </gl-form-group>
     <gl-button type="submit" variant="success" category="primary" :disabled="isLoading">
       {{ $options.i18n.saveChanges }}

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

+<script>
+import { GlFormCheckbox, GlIcon, GlLink } from '@gitlab/ui';
+import { helpPagePath } from '~/helpers/help_page_helper';
+import { __ } from '~/locale';
+import { APPROVALS_HELP_PATH } from '../constants';
+
+export default {
+  components: {
+    GlFormCheckbox,
+    GlIcon,
+    GlLink,
+  },
+  props: {
+    label: {
+      type: String,
+      required: true,
+    },
+    anchor: {
+      type: String,
+      required: true,
+    },
+    value: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    href() {
+      return helpPagePath(APPROVALS_HELP_PATH, { anchor: this.anchor });
+    },
+  },
+  methods: {
+    input(value) {
+      this.$emit('input', value);
+    },
+  },
+  i18n: {
+    helpLabel: __('Help'),
+  },
+};
+</script>
+
+<template>
+  <gl-form-checkbox :checked="value" @input="input">
+    {{ label }}
+    <gl-link :href="href" target="_blank">
+      <gl-icon name="question-o" :aria-label="$options.i18n.helpLabel" :size="16" />
+    </gl-link>
+  </gl-form-checkbox>
+</template>

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

@@ -45,3 +45,5 @@ export const APPROVAL_RULE_CONFIGS = {
     documentationText: __('Learn more about License-Check'),
   },
 };
+
+export const APPROVALS_HELP_PATH = 'user/project/merge_requests/merge_request_approvals';

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

@@ -26,6 +26,7 @@ export const fetchSettings = ({ commit }, endpoint) => {
 export const updateSettings = ({ commit, state }, endpoint) => {
   const payload = {
     allow_author_approval: !state.settings.preventAuthorApproval,
+    require_password_to_approve: state.settings.requireUserPassword,
   };
 
   commit(types.REQUEST_UPDATE_SETTINGS);

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

@@ -6,6 +6,7 @@ export default {
   },
   [types.RECEIVE_SETTINGS_SUCCESS](state, data) {
     state.settings.preventAuthorApproval = !data.allow_author_approval;
+    state.settings.requireUserPassword = data.require_password_to_approve;
     state.isLoading = false;
   },
   [types.RECEIVE_SETTINGS_ERROR](state) {
@@ -16,6 +17,7 @@ export default {
   },
   [types.UPDATE_SETTINGS_SUCCESS](state, data) {
     state.settings.preventAuthorApproval = !data.allow_author_approval;
+    state.settings.requireUserPassword = data.require_password_to_approve;
     state.isLoading = false;
   },
   [types.UPDATE_SETTINGS_ERROR](state) {

ee/spec/features/groups/group_settings_spec.rb

@@ -296,9 +296,11 @@ RSpec.describe 'Edit group settings' do
 
         within('[data-testid="merge-request-approval-settings"]') do
           click_button 'Expand'
-          expect(find('[data-testid="prevent-author-approval"]')).to be_checked
+          checkbox = find('[data-testid="prevent-author-approval"] > input')
 
-          find('[data-testid="prevent-author-approval"]').set(false)
+          expect(checkbox).to be_checked
+
+          checkbox.set(false)
           click_button 'Save changes'
           wait_for_all_requests
         end
@@ -308,7 +310,7 @@ RSpec.describe 'Edit group settings' do
 
         within('[data-testid="merge-request-approval-settings"]') do
           click_button 'Expand'
-          expect(find('[data-testid="prevent-author-approval"]')).not_to be_checked
+          expect(find('[data-testid="prevent-author-approval"] > input')).not_to be_checked
         end
       end
     end

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

+import { GlFormCheckbox, GlIcon, GlLink } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+
+import ApprovalSettingsCheckbox from 'ee/approvals/components/approval_settings_checkbox.vue';
+import { APPROVALS_HELP_PATH } from 'ee/approvals/constants';
+import { stubComponent } from 'helpers/stub_component';
+
+describe('ApprovalSettingsCheckbox', () => {
+  const label = 'Foo';
+  const anchor = 'bar-baz';
+
+  let wrapper;
+
+  const createWrapper = (props = {}) => {
+    wrapper = shallowMount(ApprovalSettingsCheckbox, {
+      propsData: { label, anchor, ...props },
+      stubs: {
+        GlFormCheckbox: stubComponent(GlFormCheckbox, {
+          props: ['checked'],
+        }),
+        GlIcon,
+        GlLink,
+      },
+    });
+  };
+
+  const findCheckbox = () => wrapper.findComponent(GlFormCheckbox);
+  const findIcon = () => wrapper.findComponent(GlIcon);
+  const findLink = () => wrapper.findComponent(GlLink);
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('rendering', () => {
+    beforeEach(() => {
+      createWrapper();
+    });
+
+    it('shows the label', () => {
+      expect(findCheckbox().text()).toBe(label);
+    });
+
+    it('sets the correct help link', () => {
+      expect(findLink().attributes('href')).toBe(`/help/${APPROVALS_HELP_PATH}#${anchor}`);
+    });
+
+    it('shows the icon', () => {
+      expect(findIcon().props('name')).toBe('question-o');
+    });
+  });
+
+  describe('value', () => {
+    it('defaults to false when no value is given', () => {
+      createWrapper();
+
+      expect(findCheckbox().props('checked')).toBe(false);
+    });
+
+    it('sets the checkbox to `true` when a `true` value is given', () => {
+      createWrapper({ value: true });
+
+      expect(findCheckbox().props('checked')).toBe(true);
+    });
+
+    it('emits an input event when the checkbox is changed', async () => {
+      createWrapper();
+
+      await findCheckbox().vm.$emit('input', true);
+
+      expect(wrapper.emitted('input')[0]).toStrictEqual([true]);
+    });
+  });
+});

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

@@ -5,6 +5,7 @@ import Vuex from 'vuex';
 import ApprovalSettings from 'ee/approvals/components/approval_settings.vue';
 import { createStoreOptions } from 'ee/approvals/stores';
 import groupSettingsModule from 'ee/approvals/stores/modules/group_settings';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -17,23 +18,24 @@ describe('ApprovalSettings', () => {
   const approvalSettingsPath = 'groups/22/merge_request_approval_settings';
 
   const createWrapper = () => {
-    wrapper = shallowMount(ApprovalSettings, {
-      localVue,
-      store: new Vuex.Store(store),
-      propsData: { approvalSettingsPath },
-    });
+    wrapper = extendedWrapper(
+      shallowMount(ApprovalSettings, {
+        localVue,
+        store: new Vuex.Store(store),
+        propsData: { approvalSettingsPath },
+      }),
+    );
   };
 
   const findForm = () => wrapper.findComponent(GlForm);
-  const findPreventAuthorApproval = () => wrapper.find('[data-testid="prevent-author-approval"]');
   const findSaveButton = () => wrapper.findComponent(GlButton);
 
   beforeEach(() => {
     store = createStoreOptions(groupSettingsModule());
 
-    jest.spyOn(store.modules.approvals.actions, 'fetchSettings').mockImplementation();
-    jest.spyOn(store.modules.approvals.actions, 'updateSettings').mockImplementation();
-    ({ actions } = store.modules.approvals);
+    actions = store.modules.approvals.actions;
+    jest.spyOn(actions, 'fetchSettings').mockImplementation();
+    jest.spyOn(actions, 'updateSettings').mockImplementation();
   });
 
   afterEach(() => {
@@ -47,14 +49,38 @@ describe('ApprovalSettings', () => {
     expect(actions.fetchSettings).toHaveBeenCalledWith(expect.any(Object), approvalSettingsPath);
   });
 
-  describe('interact with checkboxes', () => {
-    it('renders checkbox with correct value', async () => {
+  describe.each`
+    testid                       | setting                    | label                                     | anchor
+    ${'prevent-author-approval'} | ${'preventAuthorApproval'} | ${'Prevent MR approvals by the author.'}  | ${'allowing-merge-request-authors-to-approve-their-own-merge-requests'}
+    ${'require-user-password'}   | ${'requireUserPassword'}   | ${'Require user password for approvals.'} | ${'require-authentication-when-approving-a-merge-request'}
+  `('with $testid checkbox', ({ testid, setting, label, anchor }) => {
+    let checkbox = null;
+
+    beforeEach(() => {
+      store.modules.approvals.state.settings[setting] = false;
       createWrapper();
+      checkbox = wrapper.findByTestId(testid);
+    });
+
+    afterEach(() => {
+      checkbox = null;
+    });
+
+    it('renders', () => {
+      expect(checkbox.exists()).toBe(true);
+    });
+
+    it('has the anchor and label props', () => {
+      expect(checkbox.props()).toMatchObject({
+        anchor,
+        label,
+      });
+    });
 
-      const input = findPreventAuthorApproval();
-      await input.vm.$emit('input', false);
+    it('updates the store when the value is changed', async () => {
+      await checkbox.vm.$emit('input', true);
 
-      expect(store.modules.approvals.state.settings.preventAuthorApproval).toBe(false);
+      expect(store.modules.approvals.state.settings[setting]).toBe(true);
     });
   });
 

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

@@ -74,13 +74,14 @@ describe('EE approvals group settings module actions', () => {
       state = {
         settings: {
           preventAuthorApproval: false,
+          requireUserPassword: false,
         },
       };
     });
 
     describe('on success', () => {
       it('dispatches the request and updates payload', () => {
-        const data = { allow_author_approval: true };
+        const data = { allow_author_approval: true, require_password_to_approve: true };
         mock.onPut(approvalSettingsPath).replyOnce(httpStatus.OK, data);
 
         return testAction(

ee/spec/frontend/approvals/stores/modules/group_settings/mutations_spec.js

@@ -18,9 +18,13 @@ describe('Group settings store mutations', () => {
 
   describe('RECEIVE_SETTINGS_SUCCESS', () => {
     it('updates settings', () => {
-      mutations.RECEIVE_SETTINGS_SUCCESS(state, { allow_author_approval: true });
+      mutations.RECEIVE_SETTINGS_SUCCESS(state, {
+        allow_author_approval: true,
+        require_password_to_approve: true,
+      });
 
       expect(state.settings.preventAuthorApproval).toBe(false);
+      expect(state.settings.requireUserPassword).toBe(true);
       expect(state.isLoading).toBe(false);
     });
   });
@@ -43,9 +47,13 @@ describe('Group settings store mutations', () => {
 
   describe('UPDATE_SETTINGS_SUCCESS', () => {
     it('updates settings', () => {
-      mutations.UPDATE_SETTINGS_SUCCESS(state, { allow_author_approval: true });
+      mutations.UPDATE_SETTINGS_SUCCESS(state, {
+        allow_author_approval: true,
+        require_password_to_approve: true,
+      });
 
       expect(state.settings.preventAuthorApproval).toBe(false);
+      expect(state.settings.requireUserPassword).toBe(true);
       expect(state.isLoading).toBe(false);
     });
   });