Merge branch 'astoicescu-auto-approval-modal' into 'master'

Add confirmation modal for pending users auto-approval

See merge request gitlab-org/gitlab!55509

app/assets/javascripts/pages/admin/application_settings/general/components/signup_form.vue

@@ -7,9 +7,11 @@ import {
   GlFormRadioGroup,
   GlSprintf,
   GlLink,
+  GlModal,
 } from '@gitlab/ui';
+import { toSafeInteger } from 'lodash';
 import csrf from '~/lib/utils/csrf';
-import { s__, sprintf } from '~/locale';
+import { __, s__, sprintf } from '~/locale';
 import SignupCheckbox from './signup_checkbox.vue';
 
 const DENYLIST_TYPE_RAW = 'raw';
@@ -28,6 +30,7 @@ export default {
     GlSprintf,
     GlLink,
     SignupCheckbox,
+    GlModal,
   },
   inject: [
     'host',
@@ -51,6 +54,7 @@ export default {
   ],
   data() {
     return {
+      showModal: false,
       form: {
         signupEnabled: this.signupEnabled,
         requireAdminApproval: this.requireAdminApprovalAfterUserSignup,
@@ -74,6 +78,36 @@ export default {
     };
   },
   computed: {
+    isOldUserCapUnlimited() {
+      // User cap is set to unlimited if no value is provided in the field
+      return this.newUserSignupsCap === '';
+    },
+    isNewUserCapUnlimited() {
+      // User cap is set to unlimited if no value is provided in the field
+      return this.form.userCap === '';
+    },
+    hasUserCapChangedFromUnlimitedToLimited() {
+      return this.isOldUserCapUnlimited && !this.isNewUserCapUnlimited;
+    },
+    hasUserCapChangedFromLimitedToUnlimited() {
+      return !this.isOldUserCapUnlimited && this.isNewUserCapUnlimited;
+    },
+    hasUserCapBeenIncreased() {
+      if (this.hasUserCapChangedFromUnlimitedToLimited) {
+        return false;
+      }
+
+      const oldValueAsInteger = toSafeInteger(this.newUserSignupsCap);
+      const newValueAsInteger = toSafeInteger(this.form.userCap);
+
+      return this.hasUserCapChangedFromLimitedToUnlimited || newValueAsInteger > oldValueAsInteger;
+    },
+    canUsersBeAccidentallyApproved() {
+      const hasUserCapBeenToggledOff =
+        this.requireAdminApprovalAfterUserSignup && !this.form.requireAdminApproval;
+
+      return this.hasUserCapBeenIncreased || hasUserCapBeenToggledOff;
+    },
     signupEnabledHelpText() {
       const text = sprintf(
         s__(
@@ -99,10 +133,31 @@ export default {
       return text;
     },
   },
+  watch: {
+    showModal(value) {
+      if (value === true) {
+        this.$refs[this.$options.modal.id].show();
+      } else {
+        this.$refs[this.$options.modal.id].hide();
+      }
+    },
+  },
   methods: {
     submitButtonHandler() {
+      if (this.canUsersBeAccidentallyApproved) {
+        this.showModal = true;
+
+        return;
+      }
+
+      this.submitForm();
+    },
+    submitForm() {
       this.$refs.form.submit();
     },
+    modalHideHandler() {
+      this.showModal = false;
+    },
   },
   i18n: {
     buttonText: s__('ApplicationSettings|Save changes'),
@@ -141,6 +196,22 @@ export default {
     afterSignUpTextGroupLabel: s__('ApplicationSettings|After sign up text'),
     afterSignUpTextGroupDescription: s__('ApplicationSettings|Markdown enabled'),
   },
+  modal: {
+    id: 'signup-settings-modal',
+    actionPrimary: {
+      text: s__('ApplicationSettings|Approve users'),
+      attributes: {
+        variant: 'confirm',
+      },
+    },
+    actionCancel: {
+      text: __('Cancel'),
+    },
+    title: s__('ApplicationSettings|Approve all users in the pending approval status?'),
+    text: s__(
+      'ApplicationSettings|By making this change, you will automatically approve all users in pending approval status.',
+    ),
+  },
 };
 </script>
 
@@ -174,6 +245,7 @@ export default {
         :help-text="requireAdminApprovalHelpText"
         :label="$options.i18n.requireAdminApprovalLabel"
         data-qa-selector="require_admin_approval_after_user_signup_checkbox"
+        data-testid="require-admin-approval-checkbox"
       />
 
       <signup-checkbox
@@ -191,6 +263,7 @@ export default {
           v-model="form.userCap"
           type="text"
           name="application_setting[new_user_signups_cap]"
+          data-testid="user-cap-input"
         />
       </gl-form-group>
 
@@ -320,12 +393,25 @@ export default {
         ></textarea>
       </gl-form-group>
     </section>
+
     <gl-button
       data-qa-selector="save_changes_button"
       variant="confirm"
-      @click="submitButtonHandler"
+      @click.prevent="submitButtonHandler"
     >
       {{ $options.i18n.buttonText }}
     </gl-button>
+
+    <gl-modal
+      :ref="$options.modal.id"
+      :modal-id="$options.modal.id"
+      :action-cancel="$options.modal.actionCancel"
+      :action-primary="$options.modal.actionPrimary"
+      :title="$options.modal.title"
+      @primary="submitForm"
+      @hide="modalHideHandler"
+    >
+      {{ $options.modal.text }}
+    </gl-modal>
   </form>
 </template>

ee/changelogs/unreleased/astoicescu-auto-approval-modal.yml

+---
+title: Add confirmation modal for admin signup settings submit button, for cases where
+  pending users could be approved by mistake
+merge_request: 55509
+author:
+type: added

locale/gitlab.pot

@@ -3856,6 +3856,15 @@ msgstr ""
 msgid "ApplicationSettings|Allowed domains for sign-ups"
 msgstr ""
 
+msgid "ApplicationSettings|Approve all users in the pending approval status?"
+msgstr ""
+
+msgid "ApplicationSettings|Approve users"
+msgstr ""
+
+msgid "ApplicationSettings|By making this change, you will automatically approve all users in pending approval status."
+msgstr ""
+
 msgid "ApplicationSettings|Denied domains for sign-ups"
 msgstr ""
 

spec/frontend/admin/signup_restrictions/components/signup_form_spec.js

-import { GlButton } from '@gitlab/ui';
+import { GlButton, GlModal } from '@gitlab/ui';
 import { within, fireEvent } from '@testing-library/dom';
 import { shallowMount, mount } from '@vue/test-utils';
+import { stubComponent } from 'helpers/stub_component';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import SignupForm from '~/pages/admin/application_settings/general/components/signup_form.vue';
 import { mockData } from '../mock_data';
@@ -35,8 +36,15 @@ describe('Signup Form', () => {
   const findDenyListRawInputGroup = () => wrapper.findByTestId('domain-denylist-raw-input-group');
   const findDenyListFileInputGroup = () => wrapper.findByTestId('domain-denylist-file-input-group');
 
+  const findRequireAdminApprovalCheckbox = () =>
+    wrapper.findByTestId('require-admin-approval-checkbox');
+  const findUserCapInput = () => wrapper.findByTestId('user-cap-input');
+  const findModal = () => wrapper.find(GlModal);
+
   afterEach(() => {
     wrapper.destroy();
+
+    formSubmitSpy = null;
   });
 
   describe('form data', () => {
@@ -92,20 +100,6 @@ describe('Signup Form', () => {
     });
   });
 
-  describe('form submit', () => {
-    beforeEach(() => {
-      formSubmitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit').mockImplementation();
-
-      mountComponent({ stubs: { GlButton } });
-    });
-
-    it('submits the form when the primary action is clicked', () => {
-      findFormSubmitButton().trigger('click');
-
-      expect(formSubmitSpy).toHaveBeenCalled();
-    });
-  });
-
   describe('domain deny list', () => {
     describe('when it is set to raw from props', () => {
       beforeEach(() => {
@@ -195,4 +189,143 @@ describe('Signup Form', () => {
       });
     });
   });
+
+  describe('form submit button confirmation modal for side-effect of adding possibly unwanted new users', () => {
+    it.each`
+      requireAdminApprovalAction | userCapAction                          | buttonEffect
+      ${'unchanged from true'}   | ${'unchanged'}                         | ${'submits form'}
+      ${'unchanged from false'}  | ${'unchanged'}                         | ${'submits form'}
+      ${'toggled off'}           | ${'unchanged'}                         | ${'shows confirmation modal'}
+      ${'toggled on'}            | ${'unchanged'}                         | ${'submits form'}
+      ${'unchanged from false'}  | ${'increased'}                         | ${'shows confirmation modal'}
+      ${'unchanged from true'}   | ${'increased'}                         | ${'shows confirmation modal'}
+      ${'toggled off'}           | ${'increased'}                         | ${'shows confirmation modal'}
+      ${'toggled on'}            | ${'increased'}                         | ${'shows confirmation modal'}
+      ${'toggled on'}            | ${'decreased'}                         | ${'submits form'}
+      ${'unchanged from false'}  | ${'changed from limited to unlimited'} | ${'shows confirmation modal'}
+      ${'unchanged from false'}  | ${'changed from unlimited to limited'} | ${'submits form'}
+      ${'unchanged from false'}  | ${'unchanged from unlimited'}          | ${'submits form'}
+    `(
+      '$buttonEffect if require admin approval for new sign-ups is $requireAdminApprovalAction and the user cap is $userCapAction',
+      async ({ requireAdminApprovalAction, userCapAction, buttonEffect }) => {
+        let isModalDisplayed;
+
+        switch (buttonEffect) {
+          case 'shows confirmation modal':
+            isModalDisplayed = true;
+            break;
+          case 'submits form':
+            isModalDisplayed = false;
+            break;
+          default:
+            isModalDisplayed = false;
+            break;
+        }
+
+        const isFormSubmittedWhenClickingFormSubmitButton = !isModalDisplayed;
+
+        const injectedProps = {};
+
+        const USER_CAP_DEFAULT = 5;
+
+        switch (userCapAction) {
+          case 'changed from unlimited to limited':
+            injectedProps.newUserSignupsCap = '';
+            break;
+          case 'unchanged from unlimited':
+            injectedProps.newUserSignupsCap = '';
+            break;
+          default:
+            injectedProps.newUserSignupsCap = USER_CAP_DEFAULT;
+            break;
+        }
+
+        switch (requireAdminApprovalAction) {
+          case 'unchanged from true':
+            injectedProps.requireAdminApprovalAfterUserSignup = true;
+            break;
+          case 'unchanged from false':
+            injectedProps.requireAdminApprovalAfterUserSignup = false;
+            break;
+          case 'toggled off':
+            injectedProps.requireAdminApprovalAfterUserSignup = true;
+            break;
+          case 'toggled on':
+            injectedProps.requireAdminApprovalAfterUserSignup = false;
+            break;
+          default:
+            injectedProps.requireAdminApprovalAfterUserSignup = false;
+            break;
+        }
+
+        formSubmitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit').mockImplementation();
+
+        await mountComponent({
+          injectedProps,
+          stubs: { GlButton, GlModal: stubComponent(GlModal) },
+        });
+
+        findModal().vm.show = jest.fn();
+
+        if (
+          requireAdminApprovalAction === 'toggled off' ||
+          requireAdminApprovalAction === 'toggled on'
+        ) {
+          await findRequireAdminApprovalCheckbox().vm.$emit('input', false);
+        }
+
+        switch (userCapAction) {
+          case 'increased':
+            await findUserCapInput().vm.$emit('input', USER_CAP_DEFAULT + 1);
+            break;
+          case 'decreased':
+            await findUserCapInput().vm.$emit('input', USER_CAP_DEFAULT - 1);
+            break;
+          case 'changed from limited to unlimited':
+            await findUserCapInput().vm.$emit('input', '');
+            break;
+          case 'changed from unlimited to limited':
+            await findUserCapInput().vm.$emit('input', USER_CAP_DEFAULT);
+            break;
+          default:
+            break;
+        }
+
+        await findFormSubmitButton().trigger('click');
+
+        if (isFormSubmittedWhenClickingFormSubmitButton) {
+          expect(formSubmitSpy).toHaveBeenCalled();
+          expect(findModal().vm.show).not.toHaveBeenCalled();
+        } else {
+          expect(formSubmitSpy).not.toHaveBeenCalled();
+          expect(findModal().vm.show).toHaveBeenCalled();
+        }
+      },
+    );
+
+    describe('modal actions', () => {
+      beforeEach(async () => {
+        const INITIAL_USER_CAP = 5;
+
+        await mountComponent({
+          injectedProps: {
+            newUserSignupsCap: INITIAL_USER_CAP,
+          },
+          stubs: { GlButton, GlModal: stubComponent(GlModal) },
+        });
+
+        await findUserCapInput().vm.$emit('input', INITIAL_USER_CAP + 1);
+
+        await findFormSubmitButton().trigger('click');
+      });
+
+      it('submits the form after clicking approve users button', async () => {
+        formSubmitSpy = jest.spyOn(HTMLFormElement.prototype, 'submit').mockImplementation();
+
+        await findModal().vm.$emit('primary');
+
+        expect(formSubmitSpy).toHaveBeenCalled();
+      });
+    });
+  });
 });