Automatic merge of gitlab-org/gitlab master

GITALY_SERVER_VERSION

-ba8bc12dd0ff83fc726c6c09ee0cb0ebc311a748
+d99f9cbac9164647666f1778eb912568c261813d

ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/constants.js

+import { __, s__ } from '~/locale';
+
+export const FIELDS = {
+  CHECKBOX: {
+    key: 'checkbox',
+    class: 'checkbox',
+  },
+  DETECTED: {
+    key: 'detected',
+    label: s__('Vulnerability|Detected'),
+    class: 'detected',
+    sortable: true,
+  },
+  STATUS: {
+    key: 'state',
+    label: s__('Vulnerability|Status'),
+    class: 'status',
+    sortable: true,
+  },
+  SEVERITY: {
+    key: 'severity',
+    label: s__('Vulnerability|Severity'),
+    class: 'severity',
+    sortable: true,
+  },
+  DESCRIPTION: {
+    key: 'title',
+    label: __('Description'),
+    class: 'description gl-word-break-all',
+    sortable: true,
+  },
+  IDENTIFIER: {
+    key: 'identifier',
+    label: s__('Vulnerability|Identifier'),
+    class: 'identifier gl-word-break-all',
+  },
+  TOOL: {
+    key: 'reportType',
+    label: s__('Reports|Tool'),
+    class: 'scanner',
+    sortable: true,
+  },
+  ACTIVITY: {
+    key: 'activity',
+    label: s__('Vulnerability|Activity'),
+    thClass: 'gl-text-right ',
+    class: 'activity',
+  },
+};

ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list.vue

@@ -10,7 +10,7 @@ import {
 } from '@gitlab/ui';
 import DashboardHasNoVulnerabilities from 'ee/security_dashboard/components/shared/empty_states/dashboard_has_no_vulnerabilities.vue';
 import FiltersProducedNoResults from 'ee/security_dashboard/components/shared/empty_states/filters_produced_no_results.vue';
-import { VULNERABILITIES_PER_PAGE, DASHBOARD_TYPES } from 'ee/security_dashboard/store/constants';
+import { VULNERABILITIES_PER_PAGE } from 'ee/security_dashboard/store/constants';
 import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
 import convertReportType from 'ee/vue_shared/security_reports/store/utils/convert_report_type';
 import getPrimaryIdentifier from 'ee/vue_shared/security_reports/store/utils/get_primary_identifier';
@@ -19,7 +19,6 @@ import RemediatedBadge from 'ee/vulnerabilities/components/remediated_badge.vue'
 import { VULNERABILITY_STATES } from 'ee/vulnerabilities/constants';
 import { formatDate } from '~/lib/utils/datetime_utility';
 import { convertToSnakeCase } from '~/lib/utils/text_utility';
-import { s__, __ } from '~/locale';
 import AutoFixHelpText from '../auto_fix_help_text.vue';
 import IssuesBadge from '../issues_badge.vue';
 import SelectionSummary from '../selection_summary.vue';
@@ -53,16 +52,11 @@ export default {
     hasJiraVulnerabilitiesIntegrationEnabled: {
       default: false,
     },
-    canAdminVulnerability: {
-      default: false,
-    },
-    dashboardType: {},
   },
   props: {
-    filters: {
-      type: Object,
-      required: false,
-      default: () => ({}),
+    fields: {
+      type: Array,
+      required: true,
     },
     vulnerabilities: {
       type: Array,
@@ -87,12 +81,6 @@ export default {
     };
   },
   computed: {
-    isSortable() {
-      return Boolean(this.$listeners['sort-changed']);
-    },
-    isPipelineDashboard() {
-      return this.dashboardType === DASHBOARD_TYPES.PIPELINE;
-    },
     hasAnyScannersOtherThanGitLab() {
       return this.vulnerabilities.some(
         (v) => v.scanner?.vendor !== 'GitLab' && v.scanner?.vendor !== '',
@@ -108,75 +96,13 @@ export default {
       return Object.keys(this.selectedVulnerabilities).length;
     },
     shouldShowSelectionSummary() {
-      return this.canAdminVulnerability && this.numOfSelectedVulnerabilities > 0;
+      return this.numOfSelectedVulnerabilities > 0;
     },
     theadClass() {
       return this.shouldShowSelectionSummary ? 'below-selection-summary' : '';
     },
-    fields() {
-      const baseFields = [
-        {
-          key: 'checkbox',
-          class: 'checkbox',
-          skip: !this.canAdminVulnerability,
-        },
-        {
-          key: 'detected',
-          label: s__('Vulnerability|Detected'),
-          class: 'detected',
-          sortable: this.isSortable,
-          skip: this.isPipelineDashboard,
-        },
-        {
-          key: 'state',
-          label: s__('Vulnerability|Status'),
-          class: 'status',
-          sortable: this.isSortable,
-        },
-        {
-          key: 'severity',
-          label: s__('Vulnerability|Severity'),
-          class: 'severity',
-          sortable: this.isSortable,
-        },
-        {
-          key: 'title',
-          label: __('Description'),
-          class: 'description gl-word-break-all',
-          sortable: this.isSortable,
-        },
-        {
-          key: 'identifier',
-          label: s__('Vulnerability|Identifier'),
-          class: 'identifier gl-word-break-all',
-        },
-        {
-          key: 'reportType',
-          label: s__('Reports|Tool'),
-          class: 'scanner',
-          sortable: this.isSortable,
-        },
-        {
-          key: 'activity',
-          label: s__('Vulnerability|Activity'),
-          thClass: 'gl-text-right',
-          class: 'activity',
-          skip: this.isPipelineDashboard,
-        },
-      ].filter((f) => !f.skip);
-
-      // Apply gl-bg-white! to every header.
-      baseFields.forEach((field) => {
-        field.thClass = [field.thClass, 'gl-bg-white!']; // eslint-disable-line no-param-reassign
-      });
-
-      return baseFields;
-    },
   },
   watch: {
-    filters() {
-      this.selectedVulnerabilities = {};
-    },
     vulnerabilities() {
       const ids = new Set(this.vulnerabilities.map((v) => v.id));
 
@@ -282,10 +208,11 @@ export default {
     useConvertReportType(reportType) {
       return convertReportType(reportType);
     },
-    handleSortChange(args) {
-      if (args.sortBy) {
-        this.$emit('sort-changed', { ...args, sortBy: convertToSnakeCase(args.sortBy) });
-      }
+    handleSortChange(context) {
+      const fieldName = convertToSnakeCase(context.sortBy);
+      const direction = context.sortDesc ? 'desc' : 'asc';
+
+      this.$emit('sort-changed', `${fieldName}_${direction}`);
     },
     getVulnerabilityState(state = '') {
       const stateName = state.toLowerCase();
@@ -306,7 +233,6 @@ export default {
       @vulnerability-updated="deselectVulnerability"
     />
     <gl-table
-      v-if="filters"
       :busy="isLoading"
       :fields="fields"
       :items="vulnerabilities"
@@ -316,12 +242,12 @@ export default {
       sort-icon-left
       no-local-sorting
       stacked="sm"
-      class="vulnerability-list"
       show-empty
       responsive
       hover
       primary-key="id"
       :tbody-tr-class="{ 'gl-cursor-pointer': vulnerabilities.length }"
+      head-variant="white"
       @sort-changed="handleSortChange"
       @row-clicked="toggleVulnerability"
     >

ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_spec.js

-import { GlDeprecatedSkeletonLoading as GlSkeletonLoading, GlTable, GlTruncate } from '@gitlab/ui';
+import { GlSkeletonLoading, GlTable, GlTruncate } from '@gitlab/ui';
 import { capitalize } from 'lodash';
 import DashboardHasNoVulnerabilities from 'ee/security_dashboard/components/shared/empty_states/dashboard_has_no_vulnerabilities.vue';
 import FiltersProducedNoResults from 'ee/security_dashboard/components/shared/empty_states/filters_produced_no_results.vue';
 import IssuesBadge from 'ee/security_dashboard/components/shared/issues_badge.vue';
 import SelectionSummary from 'ee/security_dashboard/components/shared/selection_summary.vue';
 import VulnerabilityCommentIcon from 'ee/security_dashboard/components/shared/vulnerability_comment_icon.vue';
-import VulnerabilityList from 'ee/security_dashboard/components/shared/vulnerability_list.vue';
+import VulnerabilityList from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_list.vue';
 import { DASHBOARD_TYPES } from 'ee/security_dashboard/store/constants';
 import FalsePositiveBadge from 'ee/vulnerabilities/components/false_positive_badge.vue';
 import RemediatedBadge from 'ee/vulnerabilities/components/remediated_badge.vue';
 import { trimText } from 'helpers/text_helper';
 import { mountExtended } from 'helpers/vue_test_utils_helper';
+import { FIELDS } from 'ee/security_dashboard/components/shared/vulnerability_report/constants';
 import { generateVulnerabilities, vulnerabilities } from '../../mock_data';
 
+const { CHECKBOX, DETECTED, STATUS, SEVERITY, DESCRIPTION, IDENTIFIER, TOOL, ACTIVITY } = FIELDS;
+
 describe('Vulnerability list component', () => {
   let wrapper;
 
   const createWrapper = ({ props = {}, listeners, provide = {}, stubs } = {}) => {
-    return mountExtended(VulnerabilityList, {
+    wrapper = mountExtended(VulnerabilityList, {
       propsData: {
         vulnerabilities: [],
+        fields: [CHECKBOX, DETECTED, STATUS, SEVERITY, DESCRIPTION, IDENTIFIER, TOOL, ACTIVITY],
         ...props,
       },
       stubs: {
@@ -44,12 +48,9 @@ describe('Vulnerability list component', () => {
 
   const locationText = ({ file, startLine }) => `${file}:${startLine}`;
   const findTable = () => wrapper.findComponent(GlTable);
-  const findSortableColumn = () => wrapper.find('[aria-sort="descending"]');
   const findCell = (label) => wrapper.find(`.js-${label}`);
   const findRows = () => wrapper.findAll('tbody tr');
   const findRow = (index = 0) => findRows().at(index);
-  const findColumn = (className) => wrapper.find(`[role="columnheader"].${className}`);
-  const findRowById = (id) => wrapper.find(`tbody tr[data-pk="${id}"`);
   const findAutoFixBulbInRow = (row) => row.find('[data-testid="vulnerability-solutions-bulb"]');
   const findIssuesBadge = (index = 0) => wrapper.findAllComponents(IssuesBadge).at(index);
   const findRemediatedBadge = () => wrapper.findComponent(RemediatedBadge);
@@ -75,7 +76,7 @@ describe('Vulnerability list component', () => {
 
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
-      wrapper = createWrapper({ props: { vulnerabilities: newVulnerabilities } });
+      createWrapper({ props: { vulnerabilities: newVulnerabilities } });
     });
 
     it('should render a list of vulnerabilities', () => {
@@ -183,7 +184,7 @@ describe('Vulnerability list component', () => {
       'issues badge when "hasJiraVulnerabilitiesIntegrationEnabled" is set to "%s"',
       (hasJiraVulnerabilitiesIntegrationEnabled) => {
         beforeEach(() => {
-          wrapper = createWrapper({
+          createWrapper({
             props: { vulnerabilities: generateVulnerabilities() },
             provide: { hasJiraVulnerabilitiesIntegrationEnabled },
           });
@@ -204,28 +205,12 @@ describe('Vulnerability list component', () => {
     );
   });
 
-  describe('when user has no permission to admin vulnerabilities', () => {
-    beforeEach(() => {
-      wrapper = createWrapper({
-        props: { vulnerabilities },
-        provide: {
-          canAdminVulnerability: false,
-        },
-      });
-    });
-
-    it('should not show the checkboxes', () => {
-      expect(findDataCell('vulnerability-checkbox-all').exists()).toBe(false);
-      expect(findDataCell('vulnerability-checkbox').exists()).toBe(false);
-    });
-  });
-
   describe('when displayed on instance or group level dashboard', () => {
     let newVulnerabilities;
 
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
-      wrapper = createWrapper({
+      createWrapper({
         props: { vulnerabilities: newVulnerabilities, shouldShowProjectNamespace: true },
       });
     });
@@ -282,7 +267,7 @@ describe('Vulnerability list component', () => {
     let newVulnerabilities;
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
-      wrapper = createWrapper({
+      createWrapper({
         props: {
           vulnerabilities: newVulnerabilities,
           shouldShowIdentifier: true,
@@ -358,7 +343,7 @@ describe('Vulnerability list component', () => {
           },
         ],
       };
-      wrapper = createWrapper({ props: { vulnerabilities: newVulnerabilities } });
+      createWrapper({ props: { vulnerabilities: newVulnerabilities } });
     });
 
     it('should emit "vulnerability-clicked" with the vulnerability as a payload when a vulnerability-link is clicked', async () => {
@@ -382,7 +367,7 @@ describe('Vulnerability list component', () => {
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
       newVulnerabilities[0].userNotesCount = 1;
-      wrapper = createWrapper({ props: { vulnerabilities: newVulnerabilities } });
+      createWrapper({ props: { vulnerabilities: newVulnerabilities } });
     });
 
     it('should render the comments badge on the first vulnerability', () => {
@@ -403,7 +388,7 @@ describe('Vulnerability list component', () => {
         ...v,
         scanner: { vendor: 'GitLab' },
       }));
-      wrapper = createWrapper({
+      createWrapper({
         props: {
           vulnerabilities: newVulnerabilities,
           shouldShowReportType: true,
@@ -422,7 +407,7 @@ describe('Vulnerability list component', () => {
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
       newVulnerabilities = newVulnerabilities.map((v) => ({ ...v, scanner: { vendor: '' } }));
-      wrapper = createWrapper({
+      createWrapper({
         props: {
           vulnerabilities: newVulnerabilities,
           shouldShowReportType: true,
@@ -442,7 +427,7 @@ describe('Vulnerability list component', () => {
       newVulnerabilities = generateVulnerabilities();
       newVulnerabilities[0].scanner = { vendor: 'GitLab' };
       newVulnerabilities[1].scanner = { vendor: 'Third Party Scanner' };
-      wrapper = createWrapper({
+      createWrapper({
         props: {
           vulnerabilities: newVulnerabilities,
           shouldShowReportType: true,
@@ -461,7 +446,7 @@ describe('Vulnerability list component', () => {
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
       newVulnerabilities[0].falsePositive = true;
-      wrapper = createWrapper({
+      createWrapper({
         props: { vulnerabilities: newVulnerabilities },
         provide: {
           falsePositiveDocUrl: '/docs',
@@ -491,7 +476,7 @@ describe('Vulnerability list component', () => {
     beforeEach(() => {
       newVulnerabilities = generateVulnerabilities();
       newVulnerabilities[0].resolvedOnDefaultBranch = true;
-      wrapper = createWrapper({ props: { vulnerabilities: newVulnerabilities } });
+      createWrapper({ props: { vulnerabilities: newVulnerabilities } });
     });
 
     it('should render the remediated info badge on the first vulnerability', () => {
@@ -509,20 +494,22 @@ describe('Vulnerability list component', () => {
     });
   });
 
-  describe('when loading', () => {
-    beforeEach(() => {
-      wrapper = createWrapper({ props: { isLoading: true } });
-    });
+  describe('loading prop', () => {
+    it.each`
+      phrase        | isLoading
+      ${'show'}     | ${true}
+      ${'not show'} | ${false}
+    `('should $phrase the loading state', ({ isLoading }) => {
+      createWrapper({ props: { isLoading, vulnerabilities } });
 
-    it('should show the loading state', () => {
-      expect(findCell('status').exists()).toEqual(false);
-      expect(wrapper.find(GlSkeletonLoading).exists()).toEqual(true);
+      expect(findCell('status').exists()).toEqual(!isLoading);
+      expect(wrapper.find(GlSkeletonLoading).exists()).toEqual(isLoading);
     });
   });
 
   describe('with no vulnerabilities', () => {
     beforeEach(() => {
-      wrapper = createWrapper({ props: { filters: { someFilter: 'true' } } });
+      createWrapper();
     });
 
     it('should show the empty state', () => {
@@ -532,65 +519,13 @@ describe('Vulnerability list component', () => {
     });
   });
 
-  describe('with vulnerabilities when there are filters', () => {
-    it.each`
-      state
-      ${['DETECTED']}
-      ${['DISMISSED']}
-      ${[]}
-      ${['DETECTED', 'DISMISSED']}
-    `('should only show vulnerabilities that match filter $state', (state) => {
-      wrapper = createWrapper({ props: { vulnerabilities, filters: { state } } });
-      const filteredVulnerabilities = vulnerabilities.filter((x) =>
-        state.length ? state.includes(x.state) : true,
-      );
-      expect(findRows().length).toBe(filteredVulnerabilities.length);
+  describe('sort-changed listener', () => {
+    it('emits sort by data in expected format', () => {
+      createWrapper();
 
-      filteredVulnerabilities.forEach((vulnerability) => {
-        expect(findRowById(vulnerability.id).exists()).toBe(true);
-      });
-    });
-  });
-
-  describe('when has a sort-changed listener defined', () => {
-    let spy;
+      findTable().vm.$emit('sort-changed', { sortBy: 'state', sortDesc: true });
 
-    beforeEach(() => {
-      spy = jest.fn();
-      wrapper = createWrapper({
-        listeners: { 'sort-changed': spy },
-      });
-    });
-
-    it('is sortable', () => {
-      expect(findSortableColumn().attributes('class')).toContain('severity');
-    });
-
-    it('triggers the listener when sortBy is not an empty value', () => {
-      const args = { sortBy: 'severity', sortDesc: false };
-      findTable().vm.$emit('sort-changed', args);
-      expect(spy).toHaveBeenCalledWith(args);
-    });
-
-    it('triggers the listener when sortBy is camelCased and transforms it to snake_case', () => {
-      const args = { sortBy: 'reportType', sortDesc: false };
-      findTable().vm.$emit('sort-changed', args);
-      expect(spy).toHaveBeenCalledWith({ ...args, sortBy: 'report_type' });
-    });
-
-    it('does not trigger the listener when sortBy is an empty value', () => {
-      findTable().vm.$emit('sort-changed', {});
-      expect(spy).not.toHaveBeenCalled();
-    });
-  });
-
-  describe('when does not have a sort-changed listener defined', () => {
-    beforeEach(() => {
-      wrapper = createWrapper();
-    });
-
-    it('is not sortable', () => {
-      expect(findSortableColumn().exists()).toBe(false);
+      expect(wrapper.emitted('sort-changed')[0][0]).toBe('state_desc');
     });
   });
 
@@ -599,7 +534,7 @@ describe('Vulnerability list component', () => {
       findRow(index).find('[data-testid="vulnerability-checkbox"]');
 
     beforeEach(() => {
-      wrapper = createWrapper({ props: { vulnerabilities } });
+      createWrapper({ props: { vulnerabilities } });
     });
 
     it('will select and deselect vulnerabilities', async () => {
@@ -621,26 +556,15 @@ describe('Vulnerability list component', () => {
     });
   });
 
-  describe('when it is the pipeline dashboard', () => {
-    beforeEach(() => {
-      wrapper = createWrapper({
-        props: { vulnerabilities },
-        provide: { dashboardType: DASHBOARD_TYPES.PIPELINE },
-        stubs: {
-          GlTable,
-        },
-      });
-    });
+  describe('fields prop', () => {
+    it('shows the expected columns in the table', () => {
+      const fields = [STATUS, SEVERITY];
+      createWrapper({ props: { fields, vulnerabilities } });
 
-    it.each([['detected'], ['activity']])('does not render %s column', (className) => {
-      expect(findColumn(className).exists()).toBe(false);
+      // Check that there are only 2 columns.
+      expect(findRow().element.cells).toHaveLength(2);
+      expect(findCell(STATUS.class).exists()).toBe(true);
+      expect(findCell(SEVERITY.class).exists()).toBe(true);
     });
-
-    it.each([['status'], ['severity'], ['description'], ['identifier'], ['scanner']])(
-      'renders %s column',
-      (className) => {
-        expect(findColumn(className).exists()).toBe(true);
-      },
-    );
   });
 });