Merge branch 'djadmin-scanner-profile-validations' into 'master'

Add validation directive to DAST Scanner Profile

See merge request gitlab-org/gitlab!59243

app/assets/javascripts/vue_shared/directives/validation.js

@@ -33,6 +33,10 @@ const focusFirstInvalidInput = (e) => {
   }
 };
 
+const getInputElement = (el) => {
+  return el.querySelector('input') || el;
+};
+
 const isEveryFieldValid = (form) => Object.values(form.fields).every(({ state }) => state === true);
 
 const createValidator = (context, feedbackMap) => ({ el, reportInvalidInput = false }) => {
@@ -91,8 +95,9 @@ export default function initValidation(customFeedbackMap = {}) {
   const elDataMap = new WeakMap();
 
   return {
-    inserted(el, binding, { context }) {
+    inserted(element, binding, { context }) {
       const { arg: showGlobalValidation } = binding;
+      const el = getInputElement(element);
       const { form: formEl } = el;
 
       const validate = createValidator(context, feedbackMap);
@@ -121,7 +126,8 @@ export default function initValidation(customFeedbackMap = {}) {
 
       validate({ el, reportInvalidInput: showGlobalValidation });
     },
-    update(el, binding) {
+    update(element, binding) {
+      const el = getInputElement(element);
       const { arg: showGlobalValidation } = binding;
       const { validate, isTouched, isBlurred } = elDataMap.get(el);
       const showValidationFeedback = showGlobalValidation || (isTouched && isBlurred);

ee/app/assets/javascripts/security_configuration/dast_scanner_profiles/components/dast_scanner_profile_form.vue

@@ -14,8 +14,9 @@ import {
 import * as Sentry from '@sentry/browser';
 import { isEqual } from 'lodash';
 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 validation from '~/vue_shared/directives/validation';
 import { SCAN_TYPE, SCAN_TYPE_OPTIONS } from '../constants';
 import dastScannerProfileCreateMutation from '../graphql/dast_scanner_profile_create.mutation.graphql';
 import dastScannerProfileUpdateMutation from '../graphql/dast_scanner_profile_update.mutation.graphql';
@@ -41,6 +42,9 @@ export default {
     GlFormRadioGroup,
     tooltipIcon,
   },
+  directives: {
+    validation: validation(),
+  },
   props: {
     projectFullPath: {
       type: String,
@@ -68,17 +72,29 @@ export default {
     } = this.profile;
 
     const form = {
-      profileName: initFormField({ value: profileName }),
-      spiderTimeout: initFormField({ value: spiderTimeout }),
-      targetTimeout: initFormField({ value: targetTimeout }),
-      scanType: initFormField({ value: scanType }),
-      useAjaxSpider: initFormField({ value: useAjaxSpider }),
-      showDebugMessages: initFormField({ value: showDebugMessages }),
+      state: false,
+      showValidation: false,
+      fields: {
+        profileName: initFormField({ value: profileName }),
+        spiderTimeout: initFormField({ value: spiderTimeout }),
+        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 {
       form,
-      initialFormValues: serializeFormObject(form),
+      initialFormValues: serializeFormObject(form.fields),
       loading: false,
       showAlert: false,
     };
@@ -130,18 +146,10 @@ export default {
       };
     },
     formTouched() {
-      return !isEqual(serializeFormObject(this.form), this.initialFormValues);
-    },
-    formHasErrors() {
-      return Object.values(this.form).some(({ state }) => state === false);
-    },
-    requiredFieldEmpty() {
-      return Object.values(this.form).some(
-        ({ required, value }) => required && isEmptyValue(value),
-      );
+      return !isEqual(serializeFormObject(this.form.fields), this.initialFormValues);
     },
     isSubmitDisabled() {
-      return this.formHasErrors || this.requiredFieldEmpty || this.isPolicyProfile;
+      return this.isPolicyProfile;
     },
     isPolicyProfile() {
       return Boolean(this.profile?.referencedInSecurityPolicies?.length);
@@ -149,27 +157,13 @@ export default {
   },
 
   methods: {
-    validateTimeout(timeoutObject, range) {
-      const timeout = timeoutObject;
-
-      const hasValue = timeout.value !== '';
-      const isOutOfRange = timeout.value < range.min || timeout.value > range.max;
+    onSubmit() {
+      this.form.showValidation = true;
 
-      if (hasValue && isOutOfRange) {
-        timeout.state = false;
-        timeout.feedback = s__('DastProfiles|Please enter a valid timeout value');
+      if (!this.form.state) {
         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.hideErrors();
 
@@ -177,7 +171,7 @@ export default {
         input: {
           fullPath: this.projectFullPath,
           ...(this.isEdit ? { id: this.profile.id } : {}),
-          ...serializeFormObject(this.form),
+          ...serializeFormObject(this.form.fields),
         },
       };
 
@@ -237,7 +231,7 @@ export default {
 </script>
 
 <template>
-  <gl-form @submit.prevent="onSubmit">
+  <gl-form novalidate @submit.prevent="onSubmit">
     <h2 v-if="showHeader" class="gl-mb-6">{{ i18n.title }}</h2>
 
     <gl-alert
@@ -268,13 +262,19 @@ export default {
     </gl-alert>
 
     <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
-          v-model="form.profileName.value"
-          name="profile_name"
+          v-model="form.fields.profileName.value"
+          v-validation:[form.showValidation]
+          name="profileName"
           class="mw-460"
           data-testid="profile-name-input"
           type="text"
+          required
+          :state="form.fields.profileName.state"
         />
       </gl-form-group>
 
@@ -287,7 +287,7 @@ export default {
         </template>
 
         <gl-form-radio-group
-          v-model="form.scanType.value"
+          v-model="form.fields.scanType.value"
           :options="$options.SCAN_TYPE_OPTIONS"
           data-testid="scan-type-option"
         />
@@ -296,22 +296,24 @@ export default {
       <div class="row">
         <gl-form-group
           class="col-md-6 mb-0"
-          :state="form.spiderTimeout.state"
-          :invalid-feedback="form.spiderTimeout.feedback"
+          :invalid-feedback="form.fields.spiderTimeout.feedback"
+          :state="form.fields.spiderTimeout.state"
         >
           <template #label>
             {{ s__('DastProfiles|Spider timeout') }}
             <tooltip-icon :title="i18n.tooltips.spiderTimeout" />
           </template>
           <gl-form-input-group
-            v-model.number="form.spiderTimeout.value"
-            name="spider_timeout"
+            v-model.number="form.fields.spiderTimeout.value"
+            v-validation:[form.showValidation]
+            name="spiderTimeout"
             class="mw-460"
             data-testid="spider-timeout-input"
             type="number"
             :min="$options.spiderTimeoutRange.min"
             :max="$options.spiderTimeoutRange.max"
-            @input="validateSpiderTimeout"
+            :state="form.fields.spiderTimeout.state"
+            required
           >
             <template #append>
               <gl-input-group-text>{{ __('Minutes') }}</gl-input-group-text>
@@ -324,22 +326,24 @@ export default {
 
         <gl-form-group
           class="col-md-6 mb-0"
-          :state="form.targetTimeout.state"
-          :invalid-feedback="form.targetTimeout.feedback"
+          :invalid-feedback="form.fields.targetTimeout.feedback"
+          :state="form.fields.targetTimeout.state"
         >
           <template #label>
             {{ s__('DastProfiles|Target timeout') }}
             <tooltip-icon :title="i18n.tooltips.targetTimeout" />
           </template>
           <gl-form-input-group
-            v-model.number="form.targetTimeout.value"
-            name="target_timeout"
+            v-model.number="form.fields.targetTimeout.value"
+            v-validation:[form.showValidation]
+            name="targetTimeout"
             class="mw-460"
             data-testid="target-timeout-input"
             type="number"
             :min="$options.targetTimeoutRange.min"
             :max="$options.targetTimeoutRange.max"
-            @input="validateTargetTimeout"
+            :state="form.fields.targetTimeout.state"
+            required
           >
             <template #append>
               <gl-input-group-text>{{ __('Seconds') }}</gl-input-group-text>
@@ -359,7 +363,7 @@ export default {
             {{ s__('DastProfiles|AJAX spider') }}
             <tooltip-icon :title="i18n.tooltips.ajaxSpider" />
           </template>
-          <gl-form-checkbox v-model="form.useAjaxSpider.value">{{
+          <gl-form-checkbox v-model="form.fields.useAjaxSpider.value">{{
             s__('DastProfiles|Turn on AJAX spider')
           }}</gl-form-checkbox>
         </gl-form-group>
@@ -369,7 +373,7 @@ export default {
             {{ s__('DastProfiles|Debug messages') }}
             <tooltip-icon :title="i18n.tooltips.debugMessage" />
           </template>
-          <gl-form-checkbox v-model="form.showDebugMessages.value">{{
+          <gl-form-checkbox v-model="form.fields.showDebugMessages.value">{{
             s__('DastProfiles|Show debug messages')
           }}</gl-form-checkbox>
         </gl-form-group>

ee/changelogs/unreleased/djadmin-scanner-profile-validations.yml

+---
+title: Always display submit button for DAST Scanner Profile form
+merge_request: 59243
+author:
+type: changed

ee/spec/features/projects/security/dast_scanner_profiles_spec.rb

@@ -50,9 +50,9 @@ RSpec.describe 'User sees Scanner profile' do
   end
 
   def fill_in_profile_form
-    fill_in 'profile_name', with: "hello"
-    fill_in 'spider_timeout', with: "1"
-    fill_in 'target_timeout', with: "2"
+    fill_in 'profileName', with: "hello"
+    fill_in 'spiderTimeout', with: "1"
+    fill_in 'targetTimeout', with: "2"
     click_button 'Save profile'
     wait_for_requests
   end

ee/spec/frontend/security_configuration/dast_scanner_profiles_form/components/dast_scanner_profiles_form_spec.js

@@ -49,6 +49,18 @@ describe('DAST Scanner Profile', () => {
   const findPolicyProfileAlert = () => findByTestId('dast-policy-scanner-profile-alert');
   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) => {
     wrapper = mountFn(
       DastScannerProfileForm,
@@ -94,18 +106,18 @@ describe('DAST Scanner Profile', () => {
       createComponent();
     });
 
-    describe('is disabled if', () => {
+    describe('is enabled even if', () => {
       it('form contains errors', async () => {
         findProfileNameInput().vm.$emit('input', profileName);
         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 () => {
         findProfileNameInput().vm.$emit('input', '');
         await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
         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', () => {
     ${'Spider'} | ${findSpiderTimeoutInput} | ${[-1, 2881]} | ${spiderTimeout}
     ${'Target'} | ${findTargetTimeoutInput} | ${[0, 3601]}  | ${targetTimeout}
   `('$timeoutType Timeout', ({ finder, invalidValues, validValue }) => {
-    const errorMessage = 'Please enter a valid timeout value';
+    const errorMessage = 'Constraints not satisfied';
 
     beforeEach(() => {
       createFullComponent();
     });
 
     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);
     });
 
     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);
     });
 
@@ -175,14 +185,14 @@ describe('DAST Scanner Profile', () => {
       const createdProfileId = 30203;
 
       describe('on success', () => {
-        beforeEach(() => {
+        beforeEach(async () => {
           jest
             .spyOn(wrapper.vm.$apollo, 'mutate')
             .mockResolvedValue({ data: { [mutationKind]: { id: createdProfileId } } });
-          findProfileNameInput().vm.$emit('input', profileName);
-          findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
-          findTargetTimeoutInput().vm.$emit('input', targetTimeout);
-          submitForm();
+          await findProfileNameInput().vm.$emit('input', profileName);
+          await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
+          await findTargetTimeoutInput().vm.$emit('input', targetTimeout);
+          await submitForm();
         });
 
         it('sets loading state', () => {
@@ -219,19 +229,17 @@ describe('DAST Scanner Profile', () => {
       });
 
       describe('on top-level error', () => {
-        beforeEach(() => {
-          createComponent();
+        beforeEach(async () => {
+          createFullComponent();
           jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue();
-          const input = findTargetTimeoutInput();
-          input.vm.$emit('input', targetTimeout);
-          submitForm();
+          await fillAndSubmitForm();
         });
 
         it('resets loading state', () => {
           expect(findSubmitButton().props('loading')).toBe(false);
         });
 
-        it('shows an error alert', () => {
+        it('shows an error alert', async () => {
           expect(findAlert().exists()).toBe(true);
         });
       });
@@ -239,13 +247,14 @@ describe('DAST Scanner Profile', () => {
       describe('on errors as data', () => {
         const errors = ['Name is already taken', 'Value should be Int', 'error#3'];
 
-        beforeEach(() => {
+        beforeEach(async () => {
           jest
             .spyOn(wrapper.vm.$apollo, 'mutate')
             .mockResolvedValue({ data: { [mutationKind]: { errors } } });
-          const input = findSpiderTimeoutInput();
-          input.vm.$emit('input', spiderTimeout);
-          submitForm();
+          await findProfileNameInput().vm.$emit('input', profileName);
+          await findSpiderTimeoutInput().vm.$emit('input', spiderTimeout);
+          await findTargetTimeoutInput().vm.$emit('input', targetTimeout);
+          await submitForm();
         });
 
         it('resets loading state', () => {

locale/gitlab.pot

@@ -10199,9 +10199,6 @@ msgstr ""
 msgid "DastProfiles|Password form field"
 msgstr ""
 
-msgid "DastProfiles|Please enter a valid timeout value"
-msgstr ""
-
 msgid "DastProfiles|Profile name"
 msgstr ""
 

spec/frontend/vue_shared/directives/validation_spec.js

@@ -4,12 +4,18 @@ import validation from '~/vue_shared/directives/validation';
 describe('validation directive', () => {
   let wrapper;
 
-  const createComponent = ({ inputAttributes, showValidation } = {}) => {
+  const createComponent = ({ inputAttributes, showValidation, template } = {}) => {
     const defaultInputAttributes = {
       type: 'text',
       required: true,
     };
 
+    const defaultTemplate = `
+      <form>
+        <input v-validation:[showValidation] name="exampleField" v-bind="attributes" />
+      </form>
+    `;
+
     const component = {
       directives: {
         validation: validation(),
@@ -29,11 +35,7 @@ describe('validation directive', () => {
           },
         };
       },
-      template: `
-        <form>
-          <input v-validation:[showValidation] name="exampleField" v-bind="attributes" />
-        </form>
-      `,
+      template: template || defaultTemplate,
     };
 
     wrapper = shallowMount(component, { attachTo: document.body });
@@ -48,6 +50,12 @@ describe('validation directive', () => {
   const findForm = () => wrapper.find('form');
   const findInput = () => wrapper.find('input');
 
+  const setValueAndTriggerValidation = (value) => {
+    const input = findInput();
+    input.setValue(value);
+    input.trigger('blur');
+  };
+
   describe.each([true, false])(
     'with fields untouched and "showValidation" set to "%s"',
     (showValidation) => {
@@ -78,12 +86,6 @@ describe('validation directive', () => {
   `(
     'with input-attributes set to $inputAttributes',
     ({ inputAttributes, validValue, invalidValue }) => {
-      const setValueAndTriggerValidation = (value) => {
-        const input = findInput();
-        input.setValue(value);
-        input.trigger('blur');
-      };
-
       beforeEach(() => {
         createComponent({ inputAttributes });
       });
@@ -129,4 +131,52 @@ describe('validation directive', () => {
       });
     },
   );
+
+  describe('with group elements', () => {
+    const template = `
+      <form>
+        <div v-validation:[showValidation]>
+          <input name="exampleField" v-bind="attributes" />
+        </div>
+      </form>
+    `;
+    beforeEach(() => {
+      createComponent({
+        template,
+        inputAttributes: {
+          required: true,
+        },
+      });
+    });
+
+    describe('with invalid value', () => {
+      beforeEach(() => {
+        setValueAndTriggerValidation('');
+      });
+
+      it('should set correct field state', () => {
+        expect(getFormData().fields.exampleField).toEqual({
+          state: false,
+          feedback: expect.any(String),
+        });
+      });
+
+      it('should set correct feedback', () => {
+        expect(getFormData().fields.exampleField.feedback).toBe('Please fill out this field.');
+      });
+    });
+
+    describe('with valid value', () => {
+      beforeEach(() => {
+        setValueAndTriggerValidation('hello');
+      });
+
+      it('set the correct state', () => {
+        expect(getFormData().fields.exampleField).toEqual({
+          state: true,
+          feedback: '',
+        });
+      });
+    });
+  });
 });