Commit 5885a1c4 authored by Dheeraj Joshi's avatar Dheeraj Joshi

Improve validation for DAST Scanner Profile form

  - This makes use of validation directive for browser
    form validations
  - Keep submit button enabled always

Changelog: changed
parent c6059630
...@@ -14,8 +14,9 @@ import { ...@@ -14,8 +14,9 @@ import {
import * as Sentry from '@sentry/browser'; import * as Sentry from '@sentry/browser';
import { isEqual } from 'lodash'; import { isEqual } from 'lodash';
import { initFormField } from 'ee/security_configuration/utils'; import { initFormField } from 'ee/security_configuration/utils';
import { serializeFormObject, isEmptyValue } from '~/lib/utils/forms'; import { serializeFormObject } from '~/lib/utils/forms';
import { __, s__ } from '~/locale'; import { __, s__ } from '~/locale';
import validation from '~/vue_shared/directives/validation';
import { SCAN_TYPE, SCAN_TYPE_OPTIONS } from '../constants'; import { SCAN_TYPE, SCAN_TYPE_OPTIONS } from '../constants';
import dastScannerProfileCreateMutation from '../graphql/dast_scanner_profile_create.mutation.graphql'; import dastScannerProfileCreateMutation from '../graphql/dast_scanner_profile_create.mutation.graphql';
import dastScannerProfileUpdateMutation from '../graphql/dast_scanner_profile_update.mutation.graphql'; import dastScannerProfileUpdateMutation from '../graphql/dast_scanner_profile_update.mutation.graphql';
...@@ -41,6 +42,9 @@ export default { ...@@ -41,6 +42,9 @@ export default {
GlFormRadioGroup, GlFormRadioGroup,
tooltipIcon, tooltipIcon,
}, },
directives: {
validation: validation(),
},
props: { props: {
projectFullPath: { projectFullPath: {
type: String, type: String,
...@@ -68,17 +72,29 @@ export default { ...@@ -68,17 +72,29 @@ export default {
} = this.profile; } = this.profile;
const form = { const form = {
profileName: initFormField({ value: profileName }), state: false,
spiderTimeout: initFormField({ value: spiderTimeout }), showValidation: false,
targetTimeout: initFormField({ value: targetTimeout }), fields: {
scanType: initFormField({ value: scanType }), profileName: initFormField({ value: profileName }),
useAjaxSpider: initFormField({ value: useAjaxSpider }), spiderTimeout: initFormField({ value: spiderTimeout }),
showDebugMessages: initFormField({ value: showDebugMessages }), targetTimeout: initFormField({ value: targetTimeout }),
scanType: initFormField({ value: scanType, required: false, skipValidation: true }),
useAjaxSpider: initFormField({
value: useAjaxSpider,
required: false,
skipValidation: true,
}),
showDebugMessages: initFormField({
value: showDebugMessages,
required: false,
skipValidation: true,
}),
},
}; };
return { return {
form, form,
initialFormValues: serializeFormObject(form), initialFormValues: serializeFormObject(form.fields),
loading: false, loading: false,
showAlert: false, showAlert: false,
}; };
...@@ -130,18 +146,10 @@ export default { ...@@ -130,18 +146,10 @@ export default {
}; };
}, },
formTouched() { formTouched() {
return !isEqual(serializeFormObject(this.form), this.initialFormValues); return !isEqual(serializeFormObject(this.form.fields), this.initialFormValues);
},
formHasErrors() {
return Object.values(this.form).some(({ state }) => state === false);
},
requiredFieldEmpty() {
return Object.values(this.form).some(
({ required, value }) => required && isEmptyValue(value),
);
}, },
isSubmitDisabled() { isSubmitDisabled() {
return this.formHasErrors || this.requiredFieldEmpty || this.isPolicyProfile; return this.isPolicyProfile;
}, },
isPolicyProfile() { isPolicyProfile() {
return Boolean(this.profile?.referencedInSecurityPolicies?.length); return Boolean(this.profile?.referencedInSecurityPolicies?.length);
...@@ -149,27 +157,13 @@ export default { ...@@ -149,27 +157,13 @@ export default {
}, },
methods: { methods: {
validateTimeout(timeoutObject, range) { onSubmit() {
const timeout = timeoutObject; this.form.showValidation = true;
const hasValue = timeout.value !== '';
const isOutOfRange = timeout.value < range.min || timeout.value > range.max;
if (hasValue && isOutOfRange) { if (!this.form.state) {
timeout.state = false;
timeout.feedback = s__('DastProfiles|Please enter a valid timeout value');
return; return;
} }
timeout.state = true;
timeout.feedback = null;
},
validateSpiderTimeout() {
this.validateTimeout(this.form.spiderTimeout, this.$options.spiderTimeoutRange);
},
validateTargetTimeout() {
this.validateTimeout(this.form.targetTimeout, this.$options.targetTimeoutRange);
},
onSubmit() {
this.loading = true; this.loading = true;
this.hideErrors(); this.hideErrors();
...@@ -177,7 +171,7 @@ export default { ...@@ -177,7 +171,7 @@ export default {
input: { input: {
fullPath: this.projectFullPath, fullPath: this.projectFullPath,
...(this.isEdit ? { id: this.profile.id } : {}), ...(this.isEdit ? { id: this.profile.id } : {}),
...serializeFormObject(this.form), ...serializeFormObject(this.form.fields),
}, },
}; };
...@@ -237,7 +231,7 @@ export default { ...@@ -237,7 +231,7 @@ export default {
</script> </script>
<template> <template>
<gl-form @submit.prevent="onSubmit"> <gl-form novalidate @submit.prevent="onSubmit">
<h2 v-if="showHeader" class="gl-mb-6">{{ i18n.title }}</h2> <h2 v-if="showHeader" class="gl-mb-6">{{ i18n.title }}</h2>
<gl-alert <gl-alert
...@@ -268,13 +262,19 @@ export default { ...@@ -268,13 +262,19 @@ export default {
</gl-alert> </gl-alert>
<gl-form-group data-testid="dast-scanner-parent-group" :disabled="isPolicyProfile"> <gl-form-group data-testid="dast-scanner-parent-group" :disabled="isPolicyProfile">
<gl-form-group :label="s__('DastProfiles|Profile name')"> <gl-form-group
:label="s__('DastProfiles|Profile name')"
:invalid-feedback="form.fields.profileName.feedback"
>
<gl-form-input <gl-form-input
v-model="form.profileName.value" v-model="form.fields.profileName.value"
name="profile_name" v-validation:[form.showValidation]
name="profileName"
class="mw-460" class="mw-460"
data-testid="profile-name-input" data-testid="profile-name-input"
type="text" type="text"
required
:state="form.fields.profileName.state"
/> />
</gl-form-group> </gl-form-group>
...@@ -287,7 +287,7 @@ export default { ...@@ -287,7 +287,7 @@ export default {
</template> </template>
<gl-form-radio-group <gl-form-radio-group
v-model="form.scanType.value" v-model="form.fields.scanType.value"
:options="$options.SCAN_TYPE_OPTIONS" :options="$options.SCAN_TYPE_OPTIONS"
data-testid="scan-type-option" data-testid="scan-type-option"
/> />
...@@ -296,22 +296,24 @@ export default { ...@@ -296,22 +296,24 @@ export default {
<div class="row"> <div class="row">
<gl-form-group <gl-form-group
class="col-md-6 mb-0" class="col-md-6 mb-0"
:state="form.spiderTimeout.state" :invalid-feedback="form.fields.spiderTimeout.feedback"
:invalid-feedback="form.spiderTimeout.feedback" :state="form.fields.spiderTimeout.state"
> >
<template #label> <template #label>
{{ s__('DastProfiles|Spider timeout') }} {{ s__('DastProfiles|Spider timeout') }}
<tooltip-icon :title="i18n.tooltips.spiderTimeout" /> <tooltip-icon :title="i18n.tooltips.spiderTimeout" />
</template> </template>
<gl-form-input-group <gl-form-input-group
v-model.number="form.spiderTimeout.value" v-model.number="form.fields.spiderTimeout.value"
name="spider_timeout" v-validation:[form.showValidation]
name="spiderTimeout"
class="mw-460" class="mw-460"
data-testid="spider-timeout-input" data-testid="spider-timeout-input"
type="number" type="number"
:min="$options.spiderTimeoutRange.min" :min="$options.spiderTimeoutRange.min"
:max="$options.spiderTimeoutRange.max" :max="$options.spiderTimeoutRange.max"
@input="validateSpiderTimeout" :state="form.fields.spiderTimeout.state"
required
> >
<template #append> <template #append>
<gl-input-group-text>{{ __('Minutes') }}</gl-input-group-text> <gl-input-group-text>{{ __('Minutes') }}</gl-input-group-text>
...@@ -324,22 +326,24 @@ export default { ...@@ -324,22 +326,24 @@ export default {
<gl-form-group <gl-form-group
class="col-md-6 mb-0" class="col-md-6 mb-0"
:state="form.targetTimeout.state" :invalid-feedback="form.fields.targetTimeout.feedback"
:invalid-feedback="form.targetTimeout.feedback" :state="form.fields.targetTimeout.state"
> >
<template #label> <template #label>
{{ s__('DastProfiles|Target timeout') }} {{ s__('DastProfiles|Target timeout') }}
<tooltip-icon :title="i18n.tooltips.targetTimeout" /> <tooltip-icon :title="i18n.tooltips.targetTimeout" />
</template> </template>
<gl-form-input-group <gl-form-input-group
v-model.number="form.targetTimeout.value" v-model.number="form.fields.targetTimeout.value"
name="target_timeout" v-validation:[form.showValidation]
name="targetTimeout"
class="mw-460" class="mw-460"
data-testid="target-timeout-input" data-testid="target-timeout-input"
type="number" type="number"
:min="$options.targetTimeoutRange.min" :min="$options.targetTimeoutRange.min"
:max="$options.targetTimeoutRange.max" :max="$options.targetTimeoutRange.max"
@input="validateTargetTimeout" :state="form.fields.targetTimeout.state"
required
> >
<template #append> <template #append>
<gl-input-group-text>{{ __('Seconds') }}</gl-input-group-text> <gl-input-group-text>{{ __('Seconds') }}</gl-input-group-text>
...@@ -359,7 +363,7 @@ export default { ...@@ -359,7 +363,7 @@ export default {
{{ s__('DastProfiles|AJAX spider') }} {{ s__('DastProfiles|AJAX spider') }}
<tooltip-icon :title="i18n.tooltips.ajaxSpider" /> <tooltip-icon :title="i18n.tooltips.ajaxSpider" />
</template> </template>
<gl-form-checkbox v-model="form.useAjaxSpider.value">{{ <gl-form-checkbox v-model="form.fields.useAjaxSpider.value">{{
s__('DastProfiles|Turn on AJAX spider') s__('DastProfiles|Turn on AJAX spider')
}}</gl-form-checkbox> }}</gl-form-checkbox>
</gl-form-group> </gl-form-group>
...@@ -369,7 +373,7 @@ export default { ...@@ -369,7 +373,7 @@ export default {
{{ s__('DastProfiles|Debug messages') }} {{ s__('DastProfiles|Debug messages') }}
<tooltip-icon :title="i18n.tooltips.debugMessage" /> <tooltip-icon :title="i18n.tooltips.debugMessage" />
</template> </template>
<gl-form-checkbox v-model="form.showDebugMessages.value">{{ <gl-form-checkbox v-model="form.fields.showDebugMessages.value">{{
s__('DastProfiles|Show debug messages') s__('DastProfiles|Show debug messages')
}}</gl-form-checkbox> }}</gl-form-checkbox>
</gl-form-group> </gl-form-group>
......
---
title: Always display submit button for DAST Scanner Profile form
merge_request: 59243
author:
type: changed
...@@ -50,9 +50,9 @@ RSpec.describe 'User sees Scanner profile' do ...@@ -50,9 +50,9 @@ RSpec.describe 'User sees Scanner profile' do
end end
def fill_in_profile_form def fill_in_profile_form
fill_in 'profile_name', with: "hello" fill_in 'profileName', with: "hello"
fill_in 'spider_timeout', with: "1" fill_in 'spiderTimeout', with: "1"
fill_in 'target_timeout', with: "2" fill_in 'targetTimeout', with: "2"
click_button 'Save profile' click_button 'Save profile'
wait_for_requests wait_for_requests
end end
......
...@@ -49,6 +49,18 @@ describe('DAST Scanner Profile', () => { ...@@ -49,6 +49,18 @@ describe('DAST Scanner Profile', () => {
const findPolicyProfileAlert = () => findByTestId('dast-policy-scanner-profile-alert'); const findPolicyProfileAlert = () => findByTestId('dast-policy-scanner-profile-alert');
const submitForm = () => findForm().vm.$emit('submit', { preventDefault: () => {} }); const submitForm = () => findForm().vm.$emit('submit', { preventDefault: () => {} });
const setFieldValue = async (field, value) => {
await field.find('input').setValue(value);
field.trigger('blur');
};
const fillAndSubmitForm = async () => {
await setFieldValue(findProfileNameInput(), profileName);
await setFieldValue(findSpiderTimeoutInput(), spiderTimeout);
await setFieldValue(findTargetTimeoutInput(), targetTimeout);
await submitForm();
};
const componentFactory = (mountFn = shallowMount) => (options) => { const componentFactory = (mountFn = shallowMount) => (options) => {
wrapper = mountFn( wrapper = mountFn(
DastScannerProfileForm, DastScannerProfileForm,
...@@ -94,18 +106,18 @@ describe('DAST Scanner Profile', () => { ...@@ -94,18 +106,18 @@ describe('DAST Scanner Profile', () => {
createComponent(); createComponent();
}); });
describe('is disabled if', () => { describe('is enabled even if', () => {
it('form contains errors', async () => { it('form contains errors', async () => {
findProfileNameInput().vm.$emit('input', profileName); findProfileNameInput().vm.$emit('input', profileName);
await findSpiderTimeoutInput().vm.$emit('input', '12312'); await findSpiderTimeoutInput().vm.$emit('input', '12312');
expect(findSubmitButton().props('disabled')).toBe(true); expect(findSubmitButton().props('disabled')).toBe(false);
}); });
it('at least one field is empty', async () => { it('at least one field is empty', async () => {
findProfileNameInput().vm.$emit('input', ''); findProfileNameInput().vm.$emit('input', '');
await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout); await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
await findTargetTimeoutInput().vm.$emit('input', targetTimeout); await findTargetTimeoutInput().vm.$emit('input', targetTimeout);
expect(findSubmitButton().props('disabled')).toBe(true); expect(findSubmitButton().props('disabled')).toBe(false);
}); });
}); });
...@@ -124,21 +136,19 @@ describe('DAST Scanner Profile', () => { ...@@ -124,21 +136,19 @@ describe('DAST Scanner Profile', () => {
${'Spider'} | ${findSpiderTimeoutInput} | ${[-1, 2881]} | ${spiderTimeout} ${'Spider'} | ${findSpiderTimeoutInput} | ${[-1, 2881]} | ${spiderTimeout}
${'Target'} | ${findTargetTimeoutInput} | ${[0, 3601]} | ${targetTimeout} ${'Target'} | ${findTargetTimeoutInput} | ${[0, 3601]} | ${targetTimeout}
`('$timeoutType Timeout', ({ finder, invalidValues, validValue }) => { `('$timeoutType Timeout', ({ finder, invalidValues, validValue }) => {
const errorMessage = 'Please enter a valid timeout value'; const errorMessage = 'Constraints not satisfied';
beforeEach(() => { beforeEach(() => {
createFullComponent(); createFullComponent();
}); });
it.each(invalidValues)('is marked as invalid provided an invalid value', async (value) => { it.each(invalidValues)('is marked as invalid provided an invalid value', async (value) => {
await finder().find('input').setValue(value); await setFieldValue(finder().find('input'), value);
expect(wrapper.text()).toContain(errorMessage); expect(wrapper.text()).toContain(errorMessage);
}); });
it('is marked as valid provided a valid value', async () => { it('is marked as valid provided a valid value', async () => {
await finder().find('input').setValue(validValue); await setFieldValue(finder().find('input'), validValue);
expect(wrapper.text()).not.toContain(errorMessage); expect(wrapper.text()).not.toContain(errorMessage);
}); });
...@@ -175,14 +185,14 @@ describe('DAST Scanner Profile', () => { ...@@ -175,14 +185,14 @@ describe('DAST Scanner Profile', () => {
const createdProfileId = 30203; const createdProfileId = 30203;
describe('on success', () => { describe('on success', () => {
beforeEach(() => { beforeEach(async () => {
jest jest
.spyOn(wrapper.vm.$apollo, 'mutate') .spyOn(wrapper.vm.$apollo, 'mutate')
.mockResolvedValue({ data: { [mutationKind]: { id: createdProfileId } } }); .mockResolvedValue({ data: { [mutationKind]: { id: createdProfileId } } });
findProfileNameInput().vm.$emit('input', profileName); await findProfileNameInput().vm.$emit('input', profileName);
findSpiderTimeoutInput().vm.$emit('input', spiderTimeout); await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
findTargetTimeoutInput().vm.$emit('input', targetTimeout); await findTargetTimeoutInput().vm.$emit('input', targetTimeout);
submitForm(); await submitForm();
}); });
it('sets loading state', () => { it('sets loading state', () => {
...@@ -219,19 +229,17 @@ describe('DAST Scanner Profile', () => { ...@@ -219,19 +229,17 @@ describe('DAST Scanner Profile', () => {
}); });
describe('on top-level error', () => { describe('on top-level error', () => {
beforeEach(() => { beforeEach(async () => {
createComponent(); createFullComponent();
jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue(); jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue();
const input = findTargetTimeoutInput(); await fillAndSubmitForm();
input.vm.$emit('input', targetTimeout);
submitForm();
}); });
it('resets loading state', () => { it('resets loading state', () => {
expect(findSubmitButton().props('loading')).toBe(false); expect(findSubmitButton().props('loading')).toBe(false);
}); });
it('shows an error alert', () => { it('shows an error alert', async () => {
expect(findAlert().exists()).toBe(true); expect(findAlert().exists()).toBe(true);
}); });
}); });
...@@ -239,13 +247,14 @@ describe('DAST Scanner Profile', () => { ...@@ -239,13 +247,14 @@ describe('DAST Scanner Profile', () => {
describe('on errors as data', () => { describe('on errors as data', () => {
const errors = ['Name is already taken', 'Value should be Int', 'error#3']; const errors = ['Name is already taken', 'Value should be Int', 'error#3'];
beforeEach(() => { beforeEach(async () => {
jest jest
.spyOn(wrapper.vm.$apollo, 'mutate') .spyOn(wrapper.vm.$apollo, 'mutate')
.mockResolvedValue({ data: { [mutationKind]: { errors } } }); .mockResolvedValue({ data: { [mutationKind]: { errors } } });
const input = findSpiderTimeoutInput(); await findProfileNameInput().vm.$emit('input', profileName);
input.vm.$emit('input', spiderTimeout); await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
submitForm(); await findTargetTimeoutInput().vm.$emit('input', targetTimeout);
await submitForm();
}); });
it('resets loading state', () => { it('resets loading state', () => {
......
...@@ -10199,9 +10199,6 @@ msgstr "" ...@@ -10199,9 +10199,6 @@ msgstr ""
msgid "DastProfiles|Password form field" msgid "DastProfiles|Password form field"
msgstr "" msgstr ""
msgid "DastProfiles|Please enter a valid timeout value"
msgstr ""
msgid "DastProfiles|Profile name" msgid "DastProfiles|Profile name"
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