Vulnerability finding: Update design of form

Updates form to use shared components like PageHeading,
SettingsSection and Crud.

Changelog: changed

ee/app/assets/javascripts/vulnerabilities/components/new_vulnerability/section_identifiers.vue → ee/app/assets/javascripts/vulnerabilities/components/new_vulnerability/identifiers_table.vue

 <script>
-import { GlFormGroup, GlFormInput, GlButton } from '@gitlab/ui';
-import { s__ } from '~/locale';
+import { GlFormGroup, GlFormInput, GlButton, GlTable } from '@gitlab/ui';
+import CrudComponent from '~/vue_shared/components/crud_component.vue';
+import { __, s__ } from '~/locale';
 import * as i18n from './i18n';
 
 export default {
@@ -8,6 +9,8 @@ export default {
     GlFormGroup,
     GlFormInput,
     GlButton,
+    GlTable,
+    CrudComponent,
   },
   props: {
     validationState: {
@@ -21,6 +24,11 @@ export default {
       identifiers: [{ identifierCode: '', identifierUrl: '' }],
     };
   },
+  computed: {
+    rowAttr() {
+      return { 'data-testid': 'identifier-row' };
+    },
+  },
   watch: {
     identifiers() {
       this.emitChanges();
@@ -64,78 +72,90 @@ export default {
     description: s__(
       'Vulnerability|Enter the associated CVE or CWE entries for this vulnerability.',
     ),
+    crudTitle: s__('Vulnerability|Vulnerability identifiers'),
     identifierCode: s__('Vulnerability|Identifier code'),
     identifierUrl: s__('Vulnerability|Identifier URL'),
     addIdentifier: s__('Vulnerability|Add another identifier'),
     removeIdentifierRow: s__('Vulnerability|Remove identifier row'),
   },
+  fields: [
+    {
+      key: 'code',
+      label: s__('Vulnerability|Identifier code'),
+      tdClass: 'gl-w-48',
+    },
+    {
+      key: 'url',
+      label: s__('Vulnerability|Identifier URL'),
+    },
+    {
+      key: 'actions',
+      label: __('Actions'),
+      thClass: 'gl-sr-only',
+      tdClass: 'gl-w-6 gl-text-right',
+    },
+  ],
 };
 </script>
 
 <template>
-  <section>
-    <header class="gl-my-6 gl-border-t-1 gl-border-t-default gl-pt-4 gl-border-t-solid">
-      <h3 class="gl-mb-3 gl-mt-0">
-        {{ $options.i18n.title }}
-      </h3>
-      <p>
-        {{ $options.i18n.description }}
-      </p>
-    </header>
-    <div
-      v-for="(identifier, index) in identifiers"
-      :key="index"
-      data-testid="identifier-row"
-      class="gl-mb-6 gl-flex"
-    >
-      <gl-form-group
-        :label="$options.i18n.identifierCode"
-        :label-for="`form-identifier-code-${index}`"
-        :state="validationStateIdentifierCode(index)"
-        :invalid-feedback="$options.i18n.errorIdentifierCode"
-        class="gl-mb-0 gl-mr-6"
-      >
-        <gl-form-input
-          :id="`form-identifier-code-${index}`"
-          v-model.trim="identifier.identifierCode"
+  <crud-component :title="$options.i18n.crudTitle" :description="$options.i18n.description">
+    <template #actions>
+      <gl-button size="small" @click="addIdentifier">{{ $options.i18n.addIdentifier }}</gl-button>
+    </template>
+
+    <gl-table :items="identifiers" :fields="$options.fields" :tbody-tr-attr="rowAttr" stacked="md">
+      <template #cell(code)="{ item, index }">
+        <gl-form-group
+          :label="$options.i18n.identifierCode"
+          :label-for="`form-identifier-code-${index}`"
+          label-class="gl-sr-only"
           :state="validationStateIdentifierCode(index)"
-          type="text"
-          @input="emitChanges"
-        />
-      </gl-form-group>
-      <gl-form-group
-        :label="$options.i18n.identifierUrl"
-        :state="validationStateIdentifierUrl(index)"
-        :invalid-feedback="$options.i18n.errorIdentifierUrl"
-        :label-for="`form-identifier-url-${index}`"
-        class="gl-mb-0 gl-grow"
-      >
-        <gl-form-input
-          :id="`form-identifier-url-${index}`"
-          v-model.trim="identifier.identifierUrl"
+          :invalid-feedback="$options.i18n.errorIdentifierCode"
+          class="gl-mb-0"
+        >
+          <gl-form-input
+            :id="`form-identifier-code-${index}`"
+            v-model.trim="item.identifierCode"
+            :placeholder="__('Code')"
+            :state="validationStateIdentifierCode(index)"
+            type="text"
+            @input="emitChanges"
+          />
+        </gl-form-group>
+      </template>
+
+      <template #cell(url)="{ item, index }">
+        <gl-form-group
+          :label="$options.i18n.identifierUrl"
+          :label-for="`form-identifier-url-${index}`"
+          label-class="gl-sr-only"
           :state="validationStateIdentifierUrl(index)"
-          type="text"
-          @input="emitChanges"
+          :invalid-feedback="$options.i18n.errorIdentifierUrl"
+          class="gl-mb-0"
+        >
+          <gl-form-input
+            :id="`form-identifier-url-${index}`"
+            v-model.trim="item.identifierUrl"
+            :state="validationStateIdentifierUrl(index)"
+            type="text"
+            @input="emitChanges"
+          />
+        </gl-form-group>
+      </template>
+
+      <template #cell(actions)="{ index }">
+        <gl-button
+          v-if="index > 0"
+          class="!gl-shadow-none"
+          :class="rowHasError(index) ? 'gl-self-center' : 'gl-self-end'"
+          icon="remove"
+          category="tertiary"
+          :aria-label="$options.i18n.removeIdentifierRow"
+          @click="removeIdentifier(index)"
         />
-      </gl-form-group>
-      <gl-button
-        v-if="index > 0"
-        class="gl-ml-4 !gl-shadow-none"
-        :class="rowHasError(index) ? 'gl-self-center' : 'gl-self-end'"
-        icon="remove"
-        :aria-label="$options.i18n.removeIdentifierRow"
-        @click="removeIdentifier(index)"
-      />
-      <!--
-        The first row does not contain a remove button and this creates
-        a misalignment. This button is here as a placeholder to align the rows,
-        it's not visible to the user but it occupies the space hence aligns the
-        rows properly.
-      -->
-      <gl-button v-else class="gl-invisible gl-ml-4 gl-self-end gl-shadow-none" icon="remove" />
-    </div>
-    <gl-button category="secondary" variant="confirm" @click="addIdentifier">{{
-      $options.i18n.addIdentifier
-    }}</gl-button>
-  </section>
+        <gl-button v-else class="gl-invisible gl-self-end gl-shadow-none" icon="remove" />
+      </template>
+    </gl-table>
+  </crud-component>
 </template>

ee/app/assets/javascripts/vulnerabilities/components/new_vulnerability/new_vulnerability.vue

@@ -6,8 +6,8 @@ import { convertToGraphQLId } from '~/graphql_shared/utils';
 import { TYPENAME_PROJECT } from '~/graphql_shared/constants';
 import { visitUrl } from '~/lib/utils/url_utility';
 import createVulnerabilityMutation from 'ee/security_dashboard/graphql/mutations/vulnerability_create.mutation.graphql';
+import PageHeading from '~/vue_shared/components/page_heading.vue';
 import SectionDetails from './section_details.vue';
-import SectionIdentifiers from './section_identifiers.vue';
 import SectionName from './section_name.vue';
 import SectionSolution from './section_solution.vue';
 
@@ -18,9 +18,9 @@ export default {
     GlButton,
     GlAlert,
     SectionDetails,
-    SectionIdentifiers,
     SectionName,
     SectionSolution,
+    PageHeading,
   },
   inject: ['projectId', 'vulnerabilityReportPath'],
   data() {
@@ -201,14 +201,12 @@ export default {
 
 <template>
   <div data-testid="new-vulnerability-form">
-    <header class="gl-my-4 gl-border-b-1 gl-border-b-default gl-border-b-solid">
-      <h2 class="gl-mb-3 gl-mt-0">
-        {{ $options.i18n.title }}
-      </h2>
-      <p data-testid="page-description">
+    <page-heading :heading="$options.i18n.title">
+      <template #description>
         {{ $options.i18n.description }}
-      </p>
-    </header>
+      </template>
+    </page-heading>
+
     <gl-form autocomplete="off" @submit.prevent="submitForm">
       <gl-alert v-if="shouldShowAlert" variant="danger" dismissible @dismiss="dismissAlert">
         <ul v-if="errors.length > 1" class="gl-mb-0 gl-pl-5">
@@ -216,17 +214,16 @@ export default {
         </ul>
         <span v-else>{{ errors[0] }}</span>
       </gl-alert>
-      <div class="gl-w-17/20 gl-p-4">
-        <section-name :validation-state="validation" @change="updateFormValues" />
-        <section-details :validation-state="validation" @change="updateFormValues" />
-        <section-identifiers :validation-state="validation" @change="updateFormValues" />
-        <section-solution @change="updateFormValues" />
-      </div>
-      <div class="gl-mt-5 gl-border-t-1 gl-border-t-default gl-pt-5 gl-border-t-solid">
+
+      <section-name :validation-state="validation" @change="updateFormValues" />
+      <section-details :validation-state="validation" @change="updateFormValues" />
+      <section-solution @change="updateFormValues" />
+
+      <div class="gl-mt-5 gl-flex gl-gap-3">
         <gl-button
           type="submit"
           variant="confirm"
-          class="js-no-auto-disable gl-mr-3"
+          class="js-no-auto-disable"
           :loading="submitting"
           >{{ $options.i18n.submitVulnerability }}</gl-button
         >

ee/app/assets/javascripts/vulnerabilities/components/new_vulnerability/section_details.vue

 <script>
 import {
+  GlButton,
+  GlIcon,
   GlFormGroup,
   GlFormInput,
   GlCollapsibleListbox,
@@ -7,6 +9,8 @@ import {
   GlFormRadioGroup,
 } from '@gitlab/ui';
 import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
+import IdentifiersTable from 'ee/vulnerabilities/components/new_vulnerability/identifiers_table.vue';
+import SettingsSection from '~/vue_shared/components/settings/settings_section.vue';
 import { VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
 import { SEVERITY_LEVELS } from 'ee/security_dashboard/constants';
 import { s__, __ } from '~/locale';
@@ -22,12 +26,16 @@ const DETECTION_METHODS = [
 
 export default {
   components: {
+    GlButton,
+    GlIcon,
     GlFormGroup,
     GlFormInput,
     GlCollapsibleListbox,
     GlFormRadio,
     GlFormRadioGroup,
     SeverityBadge,
+    SettingsSection,
+    IdentifiersTable,
   },
   props: {
     validationState: {
@@ -105,6 +113,9 @@ export default {
         detectionMethod: this.detectionMethod,
       });
     },
+    emitIdentifierChanges(payload) {
+      this.$emit('change', payload);
+    },
   },
   cvss: {
     none: [0, 0],
@@ -141,19 +152,11 @@ export default {
 </script>
 
 <template>
-  <section>
-    <header class="gl-my-6 gl-border-t-1 gl-border-t-default gl-pt-4 gl-border-t-solid">
-      <h3 class="gl-mb-3 gl-mt-0">
-        {{ $options.i18n.title }}
-      </h3>
-      <p>
-        {{ $options.i18n.description }}
-      </p>
-    </header>
+  <settings-section :heading="$options.i18n.title" :description="$options.i18n.description">
     <gl-form-group
       :label="$options.i18n.detectionMethod.label"
       label-for="form-detection-method"
-      class="gl-mb-6 gl-hidden"
+      class="gl-hidden"
     >
       <gl-collapsible-listbox
         id="form-detection-method"
@@ -163,21 +166,27 @@ export default {
         @select="selectDetectionMethod"
       />
     </gl-form-group>
-    <div class="gl-mb-6 gl-flex">
+    <div class="gl-mb-5 gl-flex">
       <gl-form-group
         :label="$options.i18n.severity.label"
         :state="validationState.severity"
         :invalid-feedback="$options.i18n.errorSeverity"
         label-for="form-severity"
-        class="gl-mb-0 gl-mr-6"
+        class="gl-mb-0 gl-mr-5"
       >
         <gl-collapsible-listbox
           id="form-severity"
-          :toggle-text="severityPlaceholder"
           :items="severityOptions"
           :selected="severity"
           @select="selectSeverity"
         >
+          <template #toggle>
+            <gl-button button-text-classes="gl-inline-flex gl-gap-2">
+              <template v-if="!severity">{{ severityPlaceholder }}</template>
+              <severity-badge v-else :severity="severity" />
+              <gl-icon aria-hidden="true" name="chevron-down" :size="16" variant="current" />
+            </gl-button>
+          </template>
           <template #list-item="{ item: { value } }">
             <severity-badge :severity="value" />
           </template>
@@ -197,9 +206,9 @@ export default {
     <gl-form-group
       :label="$options.i18n.status.label"
       :state="validationState.status"
-      :invalid-feedback="$options.i18n.errorStatus"
+      class="gl-mb-0"
     >
-      <p>{{ $options.i18n.status.description }}</p>
+      <p class="gl-text-subtle">{{ $options.i18n.status.description }}</p>
       <gl-form-radio-group :checked="statusId" @change="emitChanges">
         <label
           v-for="status in statusOptions"
@@ -214,6 +223,12 @@ export default {
           </gl-form-radio>
         </label>
       </gl-form-radio-group>
+
+      <template #invalid-feedback>
+        <p class="gl-mb-6">{{ $options.i18n.errorStatus }}</p>
+      </template>
     </gl-form-group>
-  </section>
+
+    <identifiers-table :validation-state="validationState" @change="emitIdentifierChanges" />
+  </settings-section>
 </template>

ee/app/assets/javascripts/vulnerabilities/components/new_vulnerability/section_name.vue

@@ -2,6 +2,7 @@
 import { GlFormGroup, GlFormInput } from '@gitlab/ui';
 import { s__, __ } from '~/locale';
 import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue';
+import SettingsSection from '~/vue_shared/components/settings/settings_section.vue';
 import * as i18n from './i18n';
 
 export default {
@@ -9,6 +10,7 @@ export default {
     GlFormGroup,
     GlFormInput,
     MarkdownEditor,
+    SettingsSection,
   },
   inject: ['markdownDocsPath', 'markdownPreviewPath'],
   props: {
@@ -57,7 +59,7 @@ export default {
 };
 </script>
 <template>
-  <section>
+  <settings-section>
     <gl-form-group
       :label="$options.i18n.vulnerabilityName.label"
       :description="$options.i18n.vulnerabilityName.description"
@@ -90,5 +92,5 @@ export default {
         />
       </div>
     </gl-form-group>
-  </section>
+  </settings-section>
 </template>

ee/app/assets/javascripts/vulnerabilities/components/new_vulnerability/section_solution.vue

 <script>
 import { s__, __ } from '~/locale';
 import MarkdownEditor from '~/vue_shared/components/markdown/markdown_editor.vue';
+import SettingsSection from '~/vue_shared/components/settings/settings_section.vue';
 
 export default {
   components: {
     MarkdownEditor,
+    SettingsSection,
   },
   inject: ['markdownDocsPath', 'markdownPreviewPath'],
   props: {
@@ -40,13 +42,8 @@ export default {
 };
 </script>
 <template>
-  <section>
-    <header class="gl-mb-3 gl-mt-6 gl-border-t-1 gl-border-t-default gl-pt-4 gl-border-t-solid">
-      <h3 class="gl-mt-0">
-        {{ $options.i18n.title }}
-      </h3>
-    </header>
-    <div class="gl-rounded-base gl-border-1 gl-border-solid gl-border-default">
+  <settings-section :heading="$options.i18n.title" heading-classes="!-gl-mb-2">
+    <div class="gl-border gl-rounded-base">
       <markdown-editor
         v-model="solution"
         :disable-attachments="true"
@@ -57,5 +54,5 @@ export default {
         @input="emitChanges"
       />
     </div>
-  </section>
+  </settings-section>
 </template>

ee/lib/ee/sidebars/projects/menus/security_compliance_menu.rb

@@ -108,7 +108,13 @@ def vulnerability_report_menu_item
               title: _('Vulnerability report'),
               link: project_security_vulnerability_report_index_path(context.project),
               super_sidebar_parent: ::Sidebars::Projects::SuperSidebarMenus::SecureMenu,
-              active_routes: { path: %w[projects/security/vulnerability_report#index projects/security/vulnerabilities#show] },
+              active_routes: {
+                path: %w[
+                  projects/security/vulnerability_report#index
+                  projects/security/vulnerabilities#show
+                  projects/security/vulnerabilities#new
+                ]
+              },
               item_id: :vulnerability_report
             )
           end

ee/spec/frontend/vulnerabilities/new_vulnerability/section_identifiers_spec.js → ee/spec/frontend/vulnerabilities/new_vulnerability/identifiers_table_spec.js

 import { nextTick } from 'vue';
 import { GlFormGroup } from '@gitlab/ui';
 import { mountExtended } from 'helpers/vue_test_utils_helper';
-import SectionIdentifiers from 'ee/vulnerabilities/components/new_vulnerability/section_identifiers.vue';
+import IdentifiersTable from 'ee/vulnerabilities/components/new_vulnerability/identifiers_table.vue';
 import {
   ERROR_IDENTIFIER_CODE,
   ERROR_IDENTIFIER_URL,
@@ -11,7 +11,7 @@ describe('New vulnerability - Section Identifiers', () => {
   let wrapper;
 
   const createWrapper = () => {
-    return mountExtended(SectionIdentifiers, {
+    return mountExtended(IdentifiersTable, {
       propsData: {
         validationState: { identifiers: [{ identifierCode: false }] },
       },

ee/spec/frontend/vulnerabilities/new_vulnerability/new_vulnerability_spec.js

@@ -9,9 +9,9 @@ import waitForPromises from 'helpers/wait_for_promises';
 import createVulnerabilityMutation from 'ee/security_dashboard/graphql/mutations/vulnerability_create.mutation.graphql';
 import NewVulnerability from 'ee/vulnerabilities/components/new_vulnerability/new_vulnerability.vue';
 import SectionName from 'ee/vulnerabilities/components/new_vulnerability/section_name.vue';
-import SectionIdentifiers from 'ee/vulnerabilities/components/new_vulnerability/section_identifiers.vue';
 import SectionDetails from 'ee/vulnerabilities/components/new_vulnerability/section_details.vue';
 import SectionSolution from 'ee/vulnerabilities/components/new_vulnerability/section_solution.vue';
+import PageHeading from '~/vue_shared/components/page_heading.vue';
 
 Vue.use(VueApollo);
 
@@ -37,8 +37,6 @@ describe('New vulnerability component', () => {
       severity: 'low',
       detectionMethod: 2,
       status: 'confirmed',
-    },
-    sectionIdentifiers: {
       identifiers: [
         {
           name: 'CWE-94',
@@ -56,7 +54,7 @@ describe('New vulnerability component', () => {
   const findSectionName = () => wrapper.findComponent(SectionName);
   const findSectionDetails = () => wrapper.findComponent(SectionDetails);
   const findSectionSolution = () => wrapper.findComponent(SectionSolution);
-  const findSectionIdentifiers = () => wrapper.findComponent(SectionIdentifiers);
+
   const findSubmitButton = () => wrapper.findAllComponents(GlButton).at(0);
   const findCancelButton = () => wrapper.findAllComponents(GlButton).at(1);
 
@@ -67,6 +65,9 @@ describe('New vulnerability component', () => {
         projectId,
         vulnerabilityReportPath,
       },
+      stubs: {
+        PageHeading,
+      },
     });
   };
 
@@ -79,7 +80,7 @@ describe('New vulnerability component', () => {
       expect(wrapper.findByRole('heading', { name: 'Add vulnerability finding' }).exists()).toBe(
         true,
       );
-      expect(wrapper.findByTestId('page-description').text()).toBe(
+      expect(wrapper.findByTestId('page-heading-description').text()).toBe(
         'Manually add a vulnerability entry into the vulnerability report.',
       );
     });
@@ -107,7 +108,6 @@ describe('New vulnerability component', () => {
   describe('form submission', () => {
     const updateFormValuesAndSubmitForm = async () => {
       findSectionName().vm.$emit('change', inputs.sectionName);
-      findSectionIdentifiers().vm.$emit('change', inputs.sectionIdentifiers);
       findSectionDetails().vm.$emit('change', inputs.sectionDetails);
       findSectionSolution().vm.$emit('change', inputs.sectionSolution);
       findForm().vm.$emit('submit', { preventDefault: jest.fn() });
@@ -136,7 +136,6 @@ describe('New vulnerability component', () => {
       expect(mutationHandlerMock).not.toHaveBeenCalled();
       expect(findSectionDetails().props('validationState')).toEqual(validationState);
       expect(findSectionName().props('validationState')).toEqual(validationState);
-      expect(findSectionIdentifiers().props('validationState')).toEqual(validationState);
     });
 
     it('submits the form successfully', async () => {

ee/spec/frontend/vulnerabilities/new_vulnerability/section_details_spec.js

@@ -53,7 +53,12 @@ describe('New vulnerability - Section Details', () => {
   `('displays and handles severity field: $value', async ({ value, index }) => {
     const listbox = findSeverity();
     await listbox.vm.$emit('select', value);
-    expect(listbox.findAllComponents(SeverityBadge).at(index).props('severity')).toBe(value);
+    expect(
+      listbox
+        .findAllComponents(SeverityBadge)
+        .at(index + 1)
+        .props('severity'),
+    ).toBe(value);
     expect(wrapper.emitted('change')[0][0]).toEqual({
       detectionMethod: -1,
       severity: value,

locale/gitlab.pot

@@ -65119,6 +65119,9 @@ msgstr ""
 msgid "Vulnerability|View training"
 msgstr ""
+msgid "Vulnerability|Vulnerability identifiers"
+msgstr ""
+
 msgid "Vulnerability|Vulnerable class:"
 msgstr ""