Merge branch 'add_security_scanners_for_vuln_check_rule' into 'master'

Add security scanners on Vulnerability-Check UI

See merge request gitlab-org/gitlab!66117

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

 <script>
-import { GlFormGroup, GlFormInput } from '@gitlab/ui';
+import {
+  GlFormGroup,
+  GlFormInput,
+  GlDropdown,
+  GlFormCheckbox,
+  GlFormCheckboxGroup,
+} from '@gitlab/ui';
 import { groupBy, isEqual, isNumber } from 'lodash';
 import { mapState, mapActions } from 'vuex';
+import { REPORT_TYPES } from 'ee/security_dashboard/store/constants';
 import ProtectedBranchesSelector from 'ee/vue_shared/components/branches_selector/protected_branches_selector.vue';
-import { sprintf, __, s__ } from '~/locale';
+import { sprintf } from '~/locale';
 import {
   ANY_BRANCH,
   TYPE_USER,
@@ -12,6 +19,7 @@ import {
   LICENSE_CHECK_NAME,
   VULNERABILITY_CHECK_NAME,
   COVERAGE_CHECK_NAME,
+  APPROVAL_DIALOG_I18N,
 } from '../constants';
 import ApproversList from './approvers_list.vue';
 import ApproversSelect from './approvers_select.vue';
@@ -31,6 +39,9 @@ export default {
     GlFormGroup,
     GlFormInput,
     ProtectedBranchesSelector,
+    GlDropdown,
+    GlFormCheckbox,
+    GlFormCheckboxGroup,
   },
   props: {
     initRule: {
@@ -62,6 +73,7 @@ export default {
       isFallback: false,
       containsHiddenGroups: false,
       serverValidationErrors: [],
+      scanners: [],
       ...this.getInitialData(),
     };
   },
@@ -89,11 +101,11 @@ export default {
     invalidName() {
       if (this.isMultiSubmission) {
         if (this.serverValidationErrors.includes('name has already been taken')) {
-          return this.$options.i18n.validations.ruleNameTaken;
+          return this.$options.APPROVAL_DIALOG_I18N.validations.ruleNameTaken;
         }
 
         if (!this.name) {
-          return this.$options.i18n.validations.ruleNameMissing;
+          return this.$options.APPROVAL_DIALOG_I18N.validations.ruleNameMissing;
         }
       }
 
@@ -101,15 +113,15 @@ export default {
     },
     invalidApprovalsRequired() {
       if (!isNumber(this.approvalsRequired)) {
-        return this.$options.i18n.validations.approvalsRequiredNotNumber;
+        return this.$options.APPROVAL_DIALOG_I18N.validations.approvalsRequiredNotNumber;
       }
 
       if (this.approvalsRequired < 0) {
-        return this.$options.i18n.validations.approvalsRequiredNegativeNumber;
+        return this.$options.APPROVAL_DIALOG_I18N.validations.approvalsRequiredNegativeNumber;
       }
 
       if (this.approvalsRequired < this.minApprovalsRequired) {
-        return sprintf(this.$options.i18n.validations.approvalsRequiredMinimum, {
+        return sprintf(this.$options.APPROVAL_DIALOG_I18N.validations.approvalsRequiredMinimum, {
           number: this.minApprovalsRequired,
         });
       }
@@ -118,7 +130,7 @@ export default {
     },
     invalidApprovers() {
       if (this.isMultiSubmission && this.approvers.length <= 0) {
-        return this.$options.i18n.validations.approversRequired;
+        return this.$options.APPROVAL_DIALOG_I18N.validations.approversRequired;
       }
 
       return '';
@@ -128,7 +140,14 @@ export default {
         !this.isMrEdit &&
         !this.branches.every((branch) => isEqual(branch, ANY_BRANCH) || isNumber(branch?.id))
       ) {
-        return this.$options.i18n.validations.branchesRequired;
+        return this.$options.APPROVAL_DIALOG_I18N.validations.branchesRequired;
+      }
+
+      return '';
+    },
+    invalidScanners() {
+      if (this.scanners.length <= 0) {
+        return this.$options.APPROVAL_DIALOG_I18N.validations.scannersRequired;
       }
 
       return '';
@@ -138,7 +157,8 @@ export default {
         this.isValidName &&
         this.isValidBranches &&
         this.isValidApprovalsRequired &&
-        this.isValidApprovers
+        this.isValidApprovers &&
+        this.areValidScanners
       );
     },
     isValidName() {
@@ -153,6 +173,9 @@ export default {
     isValidApprovers() {
       return !this.showValidation || !this.invalidApprovers;
     },
+    areValidScanners() {
+      return !this.showValidation || !this.isVulnerabilityCheck || !this.invalidScanners;
+    },
     isMultiSubmission() {
       return this.settings.allowMultiRule && !this.isFallbackSubmission;
     },
@@ -189,11 +212,33 @@ export default {
         groupRecords: this.groups,
         removeHiddenGroups: this.removeHiddenGroups,
         protectedBranchIds: this.branches.map((x) => x.id),
+        scanners: this.scanners,
       };
     },
     isEditing() {
       return Boolean(this.initRule);
     },
+    isVulnerabilityCheck() {
+      return VULNERABILITY_CHECK_NAME === this.name;
+    },
+    areAllScannersSelected() {
+      return this.scanners.length === Object.values(this.$options.REPORT_TYPES).length;
+    },
+    scannersText() {
+      switch (this.scanners.length) {
+        case Object.values(this.$options.REPORT_TYPES).length:
+          return this.$options.APPROVAL_DIALOG_I18N.form.allScannersSelectedLabel;
+        case 0:
+          return this.$options.APPROVAL_DIALOG_I18N.form.scannersSelectLabel;
+        case 1:
+          return this.$options.REPORT_TYPES[this.scanners[0]];
+        default:
+          return sprintf(this.$options.APPROVAL_DIALOG_I18N.form.multipleSelectedScannersLabel, {
+            scanner: this.$options.REPORT_TYPES[this.scanners[0]],
+            additionalScanners: this.scanners.length - 1,
+          });
+      }
+    },
   },
   watch: {
     approversToAdd(value) {
@@ -309,33 +354,15 @@ export default {
             containsHiddenGroups && !removeHiddenGroups ? [{ type: TYPE_HIDDEN_GROUPS }] : [],
           ),
         branches,
+        scanners: this.initRule.scanners || [],
       };
     },
-  },
-  i18n: {
-    form: {
-      approvalsRequiredLabel: s__('ApprovalRule|Approvals required'),
-      approvalTypeLabel: s__('ApprovalRule|Approver Type'),
-      approversLabel: s__('ApprovalRule|Add approvers'),
-      nameLabel: s__('ApprovalRule|Rule name'),
-      nameDescription: s__('ApprovalRule|Examples: QA, Security.'),
-      protectedBranchLabel: s__('ApprovalRule|Target branch'),
-      protectedBranchDescription: __(
-        'Apply this approval rule to any branch or a specific protected branch.',
-      ),
-    },
-    validations: {
-      approvalsRequiredNegativeNumber: __('Please enter a non-negative number'),
-      approvalsRequiredNotNumber: __('Please enter a valid number'),
-      approvalsRequiredMinimum: __(
-        'Please enter a number greater than %{number} (from the project settings)',
-      ),
-      approversRequired: __('Please select and add a member'),
-      branchesRequired: __('Please select a valid target branch'),
-      ruleNameTaken: __('Rule name is already taken.'),
-      ruleNameMissing: __('Please provide a name'),
+    setAllSelectedScanners() {
+      this.scanners = this.areAllScannersSelected ? [] : Object.keys(this.$options.REPORT_TYPES);
     },
   },
+  APPROVAL_DIALOG_I18N,
+  REPORT_TYPES,
 };
 </script>
 
@@ -343,8 +370,8 @@ export default {
   <form novalidate @submit.prevent.stop="submit">
     <gl-form-group
       v-if="showName"
-      :label="$options.i18n.form.nameLabel"
-      :description="$options.i18n.form.nameDescription"
+      :label="$options.APPROVAL_DIALOG_I18N.form.nameLabel"
+      :description="$options.APPROVAL_DIALOG_I18N.form.nameDescription"
       :state="isValidName"
       :invalid-feedback="invalidName"
       data-testid="name-group"
@@ -359,8 +386,8 @@ export default {
     </gl-form-group>
     <gl-form-group
       v-if="showProtectedBranch"
-      :label="$options.i18n.form.protectedBranchLabel"
-      :description="$options.i18n.form.protectedBranchDescription"
+      :label="$options.APPROVAL_DIALOG_I18N.form.protectedBranchLabel"
+      :description="$options.APPROVAL_DIALOG_I18N.form.protectedBranchDescription"
       :state="isValidBranches"
       :invalid-feedback="invalidBranches"
       data-testid="branches-group"
@@ -373,7 +400,30 @@ export default {
       />
     </gl-form-group>
     <gl-form-group
-      :label="$options.i18n.form.approvalsRequiredLabel"
+      v-if="isVulnerabilityCheck"
+      :label="$options.APPROVAL_DIALOG_I18N.form.scannersLabel"
+      :description="$options.APPROVAL_DIALOG_I18N.form.scannersDescription"
+      :state="areValidScanners"
+      :invalid-feedback="invalidScanners"
+      data-testid="scanners-group"
+    >
+      <gl-dropdown :text="scannersText">
+        <gl-form-checkbox
+          :checked="areAllScannersSelected"
+          class="gl-ml-2"
+          @change="setAllSelectedScanners"
+        >
+          {{ $options.APPROVAL_DIALOG_I18N.form.selectAllScannersLabel }}
+        </gl-form-checkbox>
+        <gl-form-checkbox-group
+          v-model="scanners"
+          :options="this.$options.REPORT_TYPES"
+          class="gl-ml-2"
+        />
+      </gl-dropdown>
+    </gl-form-group>
+    <gl-form-group
+      :label="$options.APPROVAL_DIALOG_I18N.form.approvalsRequiredLabel"
       :state="isValidApprovalsRequired"
       :invalid-feedback="invalidApprovalsRequired"
       data-testid="approvals-required-group"
@@ -389,7 +439,7 @@ export default {
       />
     </gl-form-group>
     <gl-form-group
-      :label="$options.i18n.form.approversLabel"
+      :label="$options.APPROVAL_DIALOG_I18N.form.approversLabel"
       :state="isValidApprovers"
       :invalid-feedback="invalidApprovers"
       data-testid="approvers-group"

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

@@ -70,3 +70,37 @@ export const APPROVAL_SETTINGS_I18N = {
   ),
   savingSuccessMessage: s__('ApprovalSettings|Merge request approval settings have been updated.'),
 };
+
+export const APPROVAL_DIALOG_I18N = {
+  form: {
+    approvalsRequiredLabel: s__('ApprovalRule|Approvals required'),
+    approvalTypeLabel: s__('ApprovalRule|Approver Type'),
+    approversLabel: s__('ApprovalRule|Add approvers'),
+    nameLabel: s__('ApprovalRule|Rule name'),
+    nameDescription: s__('ApprovalRule|Examples: QA, Security.'),
+    protectedBranchLabel: s__('ApprovalRule|Target branch'),
+    protectedBranchDescription: __(
+      'Apply this approval rule to any branch or a specific protected branch.',
+    ),
+    scannersLabel: s__('ApprovalRule|Security scanners'),
+    scannersSelectLabel: s__('ApprovalRule|Select scanners'),
+    scannersDescription: s__(
+      'ApprovalRule|Apply this approval rule to consider only the selected security scanners.',
+    ),
+    selectAllScannersLabel: s__('ApprovalRule|Select All'),
+    allScannersSelectedLabel: s__('ApprovalRule|All scanners'),
+    multipleSelectedScannersLabel: s__('ApprovalRule|%{scanner} +%{additionalScanners} more'),
+  },
+  validations: {
+    approvalsRequiredNegativeNumber: __('Please enter a non-negative number'),
+    approvalsRequiredNotNumber: __('Please enter a valid number'),
+    approvalsRequiredMinimum: __(
+      'Please enter a number greater than %{number} (from the project settings)',
+    ),
+    approversRequired: __('Please select and add a member'),
+    branchesRequired: __('Please select a valid target branch'),
+    ruleNameTaken: __('Rule name is already taken.'),
+    ruleNameMissing: __('Please provide a name'),
+    scannersRequired: s__('ApprovalRule|Please select at least one security scanner'),
+  },
+};

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

@@ -31,6 +31,7 @@ export const mapApprovalRuleRequest = (req) => ({
   groups: req.groups,
   remove_hidden_groups: req.removeHiddenGroups,
   protected_branch_ids: req.protectedBranchIds,
+  scanners: req.scanners,
 });
 
 export const mapApprovalFallbackRuleRequest = (req) => ({
@@ -50,6 +51,7 @@ export const mapApprovalRuleResponse = (res) => ({
   ruleType: res.rule_type,
   protectedBranches: res.protected_branches,
   overridden: res.overridden,
+  scanners: res.scanners,
 });
 
 export const mapApprovalSettingsResponse = (res) => ({

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

@@ -5,7 +5,12 @@ import Vuex from 'vuex';
 import ApproversList from 'ee/approvals/components/approvers_list.vue';
 import ApproversSelect from 'ee/approvals/components/approvers_select.vue';
 import RuleForm, { READONLY_NAMES } from 'ee/approvals/components/rule_form.vue';
-import { TYPE_USER, TYPE_GROUP, TYPE_HIDDEN_GROUPS } from 'ee/approvals/constants';
+import {
+  TYPE_USER,
+  TYPE_GROUP,
+  TYPE_HIDDEN_GROUPS,
+  VULNERABILITY_CHECK_NAME,
+} from 'ee/approvals/constants';
 import { createStoreOptions } from 'ee/approvals/stores';
 import projectSettingsModule from 'ee/approvals/stores/modules/project_settings';
 import ProtectedBranchesSelector from 'ee/vue_shared/components/branches_selector/protected_branches_selector.vue';
@@ -81,6 +86,7 @@ describe('EE Approvals RuleForm', () => {
   const findApproversList = () => wrapper.findComponent(ApproversList);
   const findProtectedBranchesSelector = () => wrapper.findComponent(ProtectedBranchesSelector);
   const findBranchesValidation = () => wrapper.findByTestId('branches-group');
+  const findScannersGroup = () => wrapper.findByTestId('scanners-group');
 
   const inputsAreValid = (inputs) => inputs.every((x) => x.props('state'));
 
@@ -180,6 +186,7 @@ describe('EE Approvals RuleForm', () => {
             userRecords,
             groupRecords,
             removeHiddenGroups: false,
+            scanners: [],
             protectedBranchIds: branches.map((x) => x.id),
           };
 
@@ -257,6 +264,7 @@ describe('EE Approvals RuleForm', () => {
           userRecords,
           groupRecords,
           removeHiddenGroups: false,
+          scanners: [],
           protectedBranchIds: branches.map((x) => x.id),
         };
 
@@ -335,6 +343,7 @@ describe('EE Approvals RuleForm', () => {
           userRecords,
           groupRecords,
           removeHiddenGroups: false,
+          scanners: [],
           protectedBranchIds: [],
         };
 
@@ -512,14 +521,14 @@ describe('EE Approvals RuleForm', () => {
 
     describe('with approval suggestions', () => {
       describe.each`
-        defaultRuleName          | expectedDisabledAttribute
-        ${'Vulnerability-Check'} | ${true}
-        ${'License-Check'}       | ${true}
-        ${'Coverage-Check'}      | ${true}
-        ${'Foo Bar Baz'}         | ${false}
+        defaultRuleName             | expectedDisabledAttribute | expectedDisplayedScanners
+        ${VULNERABILITY_CHECK_NAME} | ${true}                   | ${true}
+        ${'License-Check'}          | ${true}                   | ${false}
+        ${'Coverage-Check'}         | ${true}                   | ${false}
+        ${'Foo Bar Baz'}            | ${false}                  | ${false}
       `(
         'with defaultRuleName set to $defaultRuleName',
-        ({ defaultRuleName, expectedDisabledAttribute }) => {
+        ({ defaultRuleName, expectedDisabledAttribute, expectedDisplayedScanners }) => {
           beforeEach(() => {
             createComponent({
               initRule: null,
@@ -533,6 +542,12 @@ describe('EE Approvals RuleForm', () => {
           } the name text field`, () => {
             expect(findNameInput().props('disabled')).toBe(expectedDisabledAttribute);
           });
+
+          it(`it ${
+            expectedDisplayedScanners ? 'shows' : 'does not show'
+          } scanners dropdown`, () => {
+            expect(findScannersGroup().exists()).toBe(expectedDisplayedScanners);
+          });
         },
       );
     });
@@ -562,6 +577,48 @@ describe('EE Approvals RuleForm', () => {
         });
       });
     });
+
+    describe(`with ${VULNERABILITY_CHECK_NAME}`, () => {
+      describe('and without any scanners selected', () => {
+        beforeEach(() => {
+          createComponent({
+            initRule: {
+              ...TEST_RULE,
+              id: null,
+              name: VULNERABILITY_CHECK_NAME,
+              scanners: [],
+            },
+          });
+          findForm().trigger('submit');
+        });
+
+        it('does not dispatch the action on submit', () => {
+          expect(actions.postRule).not.toHaveBeenCalled();
+        });
+      });
+
+      describe('and with two scanners selected', () => {
+        const scanners = ['sast', 'dast'];
+        beforeEach(() => {
+          createComponent({
+            initRule: {
+              ...TEST_RULE,
+              id: null,
+              name: VULNERABILITY_CHECK_NAME,
+              scanners,
+            },
+          });
+          findForm().trigger('submit');
+        });
+
+        it('dispatches the action on submit', () => {
+          expect(actions.postRule).toHaveBeenCalledWith(
+            expect.anything(),
+            expect.objectContaining({ scanners }),
+          );
+        });
+      });
+    });
   });
 
   describe('when allow only single rule', () => {

locale/gitlab.pot

@@ -4130,9 +4130,18 @@ msgid_plural "ApprovalRuleSummary|%{count} approvals required from %{membersCoun
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "ApprovalRule|%{scanner} +%{additionalScanners} more"
+msgstr ""
+
 msgid "ApprovalRule|Add approvers"
 msgstr ""
 
+msgid "ApprovalRule|All scanners"
+msgstr ""
+
+msgid "ApprovalRule|Apply this approval rule to consider only the selected security scanners."
+msgstr ""
+
 msgid "ApprovalRule|Approval rules"
 msgstr ""
 
@@ -4151,9 +4160,21 @@ msgstr ""
 msgid "ApprovalRule|Name"
 msgstr ""
 
+msgid "ApprovalRule|Please select at least one security scanner"
+msgstr ""
+
 msgid "ApprovalRule|Rule name"
 msgstr ""
 
+msgid "ApprovalRule|Security scanners"
+msgstr ""
+
+msgid "ApprovalRule|Select All"
+msgstr ""
+
+msgid "ApprovalRule|Select scanners"
+msgstr ""
+
 msgid "ApprovalRule|Target branch"
 msgstr ""