Commit 91927a54 authored by Andrei Stoicescu's avatar Andrei Stoicescu Committed by Vitaly Slobodin

Add confirmation modal for pending users auto-approval

parent 760d6c5f
...@@ -7,9 +7,11 @@ import { ...@@ -7,9 +7,11 @@ import {
GlFormRadioGroup, GlFormRadioGroup,
GlSprintf, GlSprintf,
GlLink, GlLink,
GlModal,
} from '@gitlab/ui'; } from '@gitlab/ui';
import { toSafeInteger } from 'lodash';
import csrf from '~/lib/utils/csrf'; import csrf from '~/lib/utils/csrf';
import { s__, sprintf } from '~/locale'; import { __, s__, sprintf } from '~/locale';
import SignupCheckbox from './signup_checkbox.vue'; import SignupCheckbox from './signup_checkbox.vue';
const DENYLIST_TYPE_RAW = 'raw'; const DENYLIST_TYPE_RAW = 'raw';
...@@ -28,6 +30,7 @@ export default { ...@@ -28,6 +30,7 @@ export default {
GlSprintf, GlSprintf,
GlLink, GlLink,
SignupCheckbox, SignupCheckbox,
GlModal,
}, },
inject: [ inject: [
'host', 'host',
...@@ -51,6 +54,7 @@ export default { ...@@ -51,6 +54,7 @@ export default {
], ],
data() { data() {
return { return {
showModal: false,
form: { form: {
signupEnabled: this.signupEnabled, signupEnabled: this.signupEnabled,
requireAdminApproval: this.requireAdminApprovalAfterUserSignup, requireAdminApproval: this.requireAdminApprovalAfterUserSignup,
...@@ -74,6 +78,36 @@ export default { ...@@ -74,6 +78,36 @@ export default {
}; };
}, },
computed: { 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() { signupEnabledHelpText() {
const text = sprintf( const text = sprintf(
s__( s__(
...@@ -99,10 +133,31 @@ export default { ...@@ -99,10 +133,31 @@ export default {
return text; return text;
}, },
}, },
watch: {
showModal(value) {
if (value === true) {
this.$refs[this.$options.modal.id].show();
} else {
this.$refs[this.$options.modal.id].hide();
}
},
},
methods: { methods: {
submitButtonHandler() { submitButtonHandler() {
if (this.canUsersBeAccidentallyApproved) {
this.showModal = true;
return;
}
this.submitForm();
},
submitForm() {
this.$refs.form.submit(); this.$refs.form.submit();
}, },
modalHideHandler() {
this.showModal = false;
},
}, },
i18n: { i18n: {
buttonText: s__('ApplicationSettings|Save changes'), buttonText: s__('ApplicationSettings|Save changes'),
...@@ -141,6 +196,22 @@ export default { ...@@ -141,6 +196,22 @@ export default {
afterSignUpTextGroupLabel: s__('ApplicationSettings|After sign up text'), afterSignUpTextGroupLabel: s__('ApplicationSettings|After sign up text'),
afterSignUpTextGroupDescription: s__('ApplicationSettings|Markdown enabled'), 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> </script>
...@@ -174,6 +245,7 @@ export default { ...@@ -174,6 +245,7 @@ export default {
:help-text="requireAdminApprovalHelpText" :help-text="requireAdminApprovalHelpText"
:label="$options.i18n.requireAdminApprovalLabel" :label="$options.i18n.requireAdminApprovalLabel"
data-qa-selector="require_admin_approval_after_user_signup_checkbox" data-qa-selector="require_admin_approval_after_user_signup_checkbox"
data-testid="require-admin-approval-checkbox"
/> />
<signup-checkbox <signup-checkbox
...@@ -191,6 +263,7 @@ export default { ...@@ -191,6 +263,7 @@ export default {
v-model="form.userCap" v-model="form.userCap"
type="text" type="text"
name="application_setting[new_user_signups_cap]" name="application_setting[new_user_signups_cap]"
data-testid="user-cap-input"
/> />
</gl-form-group> </gl-form-group>
...@@ -320,12 +393,25 @@ export default { ...@@ -320,12 +393,25 @@ export default {
></textarea> ></textarea>
</gl-form-group> </gl-form-group>
</section> </section>
<gl-button <gl-button
data-qa-selector="save_changes_button" data-qa-selector="save_changes_button"
variant="confirm" variant="confirm"
@click="submitButtonHandler" @click.prevent="submitButtonHandler"
> >
{{ $options.i18n.buttonText }} {{ $options.i18n.buttonText }}
</gl-button> </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> </form>
</template> </template>
---
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
...@@ -3856,6 +3856,15 @@ msgstr "" ...@@ -3856,6 +3856,15 @@ msgstr ""
msgid "ApplicationSettings|Allowed domains for sign-ups" msgid "ApplicationSettings|Allowed domains for sign-ups"
msgstr "" 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" msgid "ApplicationSettings|Denied domains for sign-ups"
msgstr "" msgstr ""
......
import { GlButton } from '@gitlab/ui'; import { GlButton, GlModal } from '@gitlab/ui';
import { within, fireEvent } from '@testing-library/dom'; import { within, fireEvent } from '@testing-library/dom';
import { shallowMount, mount } from '@vue/test-utils'; import { shallowMount, mount } from '@vue/test-utils';
import { stubComponent } from 'helpers/stub_component';
import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import SignupForm from '~/pages/admin/application_settings/general/components/signup_form.vue'; import SignupForm from '~/pages/admin/application_settings/general/components/signup_form.vue';
import { mockData } from '../mock_data'; import { mockData } from '../mock_data';
...@@ -35,8 +36,15 @@ describe('Signup Form', () => { ...@@ -35,8 +36,15 @@ describe('Signup Form', () => {
const findDenyListRawInputGroup = () => wrapper.findByTestId('domain-denylist-raw-input-group'); const findDenyListRawInputGroup = () => wrapper.findByTestId('domain-denylist-raw-input-group');
const findDenyListFileInputGroup = () => wrapper.findByTestId('domain-denylist-file-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(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
formSubmitSpy = null;
}); });
describe('form data', () => { describe('form data', () => {
...@@ -92,20 +100,6 @@ describe('Signup Form', () => { ...@@ -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('domain deny list', () => {
describe('when it is set to raw from props', () => { describe('when it is set to raw from props', () => {
beforeEach(() => { beforeEach(() => {
...@@ -195,4 +189,143 @@ describe('Signup Form', () => { ...@@ -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();
});
});
});
}); });
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