Merge branch '230383-add-severity-sorting' into 'master'

Add sorting functionality to severity column

See merge request gitlab-org/gitlab!42347

ee/app/assets/javascripts/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities.vue

@@ -28,6 +28,8 @@ export default {
       pageInfo: {},
       vulnerabilities: [],
       errorLoadingVulnerabilities: false,
+      sortBy: 'severity',
+      sortDirection: 'desc',
     };
   },
   apollo: {
@@ -37,6 +39,7 @@ export default {
         return {
           fullPath: this.groupFullPath,
           first: VULNERABILITIES_PER_PAGE,
+          sort: this.sort,
           ...this.filters,
         };
       },
@@ -56,6 +59,9 @@ export default {
     isLoadingFirstResult() {
       return this.isLoadingQuery && this.vulnerabilities.length === 0;
     },
+    sort() {
+      return `${this.sortBy}_${this.sortDirection}`;
+    },
   },
   methods: {
     onErrorDismiss() {
@@ -74,6 +80,10 @@ export default {
         });
       }
     },
+    handleSortChange({ sortBy, sortDesc }) {
+      this.sortDirection = sortDesc ? 'desc' : 'asc';
+      this.sortBy = sortBy;
+    },
   },
 };
 </script>
@@ -98,6 +108,7 @@ export default {
       :is-loading="isLoadingFirstResult"
       :vulnerabilities="vulnerabilities"
       should-show-project-namespace
+      @sort-changed="handleSortChange"
     />
     <gl-intersection-observer
       v-if="pageInfo.hasNextPage"

ee/app/assets/javascripts/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities.vue

@@ -27,12 +27,17 @@ export default {
       isFirstResultLoading: true,
       vulnerabilities: [],
       errorLoadingVulnerabilities: false,
+      sortBy: 'severity',
+      sortDirection: 'desc',
     };
   },
   computed: {
     isQueryLoading() {
       return this.$apollo.queries.vulnerabilities.loading;
     },
+    sort() {
+      return `${this.sortBy}_${this.sortDirection}`;
+    },
   },
   apollo: {
     vulnerabilities: {
@@ -41,6 +46,7 @@ export default {
       variables() {
         return {
           first: VULNERABILITIES_PER_PAGE,
+          sort: this.sort,
           ...this.filters,
         };
       },
@@ -69,6 +75,10 @@ export default {
         });
       }
     },
+    handleSortChange({ sortBy, sortDesc }) {
+      this.sortDirection = sortDesc ? 'desc' : 'asc';
+      this.sortBy = sortBy;
+    },
   },
 };
 </script>
@@ -93,6 +103,7 @@ export default {
       :is-loading="isFirstResultLoading"
       :vulnerabilities="vulnerabilities"
       should-show-project-namespace
+      @sort-changed="handleSortChange"
     />
     <gl-intersection-observer
       v-if="pageInfo.hasNextPage"

ee/app/assets/javascripts/security_dashboard/components/project_vulnerabilities.vue

@@ -34,6 +34,8 @@ export default {
       vulnerabilities: [],
       securityScanners: {},
       errorLoadingVulnerabilities: false,
+      sortBy: 'severity',
+      sortDirection: 'desc',
     };
   },
   apollo: {
@@ -43,6 +45,7 @@ export default {
         return {
           fullPath: this.projectFullPath,
           first: VULNERABILITIES_PER_PAGE,
+          sort: this.sort,
           ...this.filters,
         };
       },
@@ -86,6 +89,9 @@ export default {
     isLoadingFirstVulnerabilities() {
       return this.isLoadingVulnerabilities && this.vulnerabilities.length === 0;
     },
+    sort() {
+      return `${this.sortBy}_${this.sortDirection}`;
+    },
   },
   methods: {
     fetchNextPage() {
@@ -106,6 +112,10 @@ export default {
     refetchVulnerabilities() {
       this.$apollo.queries.vulnerabilities.refetch();
     },
+    handleSortChange({ sortBy, sortDesc }) {
+      this.sortDirection = sortDesc ? 'desc' : 'asc';
+      this.sortBy = sortBy;
+    },
   },
   i18n: {
     CONTAINER_SCANNING: __('Container Scanning'),
@@ -132,6 +142,7 @@ export default {
       :vulnerabilities="vulnerabilities"
       :security-scanners="securityScanners"
       @refetch-vulnerabilities="refetchVulnerabilities"
+      @sort-changed="handleSortChange"
     />
     <gl-intersection-observer
       v-if="pageInfo.hasNextPage"

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

@@ -27,7 +27,6 @@ export const SCANNER_ALERT_DISMISSED_LOCAL_STORAGE_KEY =
   'vulnerability_list_scanner_alert_dismissed';
 
 export default {
-  name: 'VulnerabilityList',
   components: {
     GlFormCheckbox,
     GlLink,
@@ -82,9 +81,14 @@ export default {
     return {
       selectedVulnerabilities: {},
       scannerAlertDismissed: 'false',
+      sortBy: 'severity',
+      sortDesc: true,
     };
   },
   computed: {
+    isSortable() {
+      return Boolean(this.$listeners['sort-changed']);
+    },
     hasAnyScannersOtherThanGitLab() {
       return this.vulnerabilities.some(v => v.scanner?.vendor !== 'GitLab');
     },
@@ -140,6 +144,7 @@ export default {
           label: s__('Vulnerability|Severity'),
           thClass: 'severity',
           tdClass: 'severity',
+          sortable: this.isSortable,
         },
         {
           key: 'title',
@@ -263,6 +268,11 @@ export default {
     useConvertReportType(reportType) {
       return convertReportType(reportType);
     },
+    handleSortChange(args) {
+      if (args.sortBy) {
+        this.$emit('sort-changed', args);
+      }
+    },
   },
   VULNERABILITIES_PER_PAGE,
   SCANNER_ALERT_DISMISSED_LOCAL_STORAGE_KEY,
@@ -287,10 +297,15 @@ export default {
       :fields="fields"
       :items="vulnerabilities"
       :thead-class="theadClass"
+      :sort-desc="sortDesc"
+      :sort-by="sortBy"
+      sort-icon-left
+      no-local-sorting
       stacked="sm"
       class="vulnerability-list"
       show-empty
       responsive
+      @sort-changed="handleSortChange"
     >
       <template #head(checkbox)>
         <gl-form-checkbox

ee/app/assets/javascripts/security_dashboard/graphql/group_vulnerabilities.graphql

@@ -10,6 +10,7 @@ query group(
   $reportType: [VulnerabilityReportType!]
   $scanner: [String!]
   $state: [VulnerabilityState!]
+  $sort: VulnerabilitySort
 ) {
   group(fullPath: $fullPath) {
     vulnerabilities(
@@ -20,6 +21,7 @@ query group(
       scanner: $scanner
       state: $state
       projectId: $projectId
+      sort: $sort
     ) {
       nodes {
         ...Vulnerability

ee/app/assets/javascripts/security_dashboard/graphql/instance_vulnerabilities.graphql

@@ -9,6 +9,7 @@ query instance(
   $reportType: [VulnerabilityReportType!]
   $scanner: [String!]
   $state: [VulnerabilityState!]
+  $sort: VulnerabilitySort
 ) {
   vulnerabilities(
     after: $after
@@ -18,6 +19,7 @@ query instance(
     state: $state
     projectId: $projectId
     scanner: $scanner
+    sort: $sort
   ) {
     nodes {
       ...Vulnerability

ee/app/assets/javascripts/security_dashboard/graphql/project_vulnerabilities.graphql

@@ -9,6 +9,7 @@ query project(
   $reportType: [VulnerabilityReportType!]
   $scanner: [String!]
   $state: [VulnerabilityState!]
+  $sort: VulnerabilitySort
 ) {
   project(fullPath: $fullPath) {
     vulnerabilities(
@@ -18,6 +19,7 @@ query project(
       reportType: $reportType
       scanner: $scanner
       state: $state
+      sort: $sort
     ) {
       nodes {
         ...Vulnerability

ee/changelogs/unreleased/230383-add-severity-sorting.yml

+---
+title: Add sorting functionality to vulnerability list
+merge_request: 42347
+author:
+type: added

ee/spec/frontend/security_dashboard/components/first_class_group_security_dashboard_vulnerabilities_spec.js

@@ -109,6 +109,23 @@ describe('First Class Group Dashboard Vulnerabilities Component', () => {
         vulnerabilities,
       });
     });
+
+    it('defaults to severity column for sorting', () => {
+      expect(wrapper.vm.sortBy).toBe('severity');
+    });
+
+    it('defaults to desc as sorting direction', () => {
+      expect(wrapper.vm.sortDirection).toBe('desc');
+    });
+
+    it('handles sorting', () => {
+      findVulnerabilities().vm.$listeners['sort-changed']({
+        sortBy: 'description',
+        sortDesc: false,
+      });
+      expect(wrapper.vm.sortBy).toBe('description');
+      expect(wrapper.vm.sortDirection).toBe('asc');
+    });
   });
 
   describe('when there is more than a page of vulnerabilities', () => {

ee/spec/frontend/security_dashboard/components/first_class_instance_security_dashboard_vulnerabilities_spec.js

@@ -125,6 +125,23 @@ describe('First Class Instance Dashboard Vulnerabilities Component', () => {
         vulnerabilities,
       });
     });
+
+    it('defaults to severity column for sorting', () => {
+      expect(wrapper.vm.sortBy).toBe('severity');
+    });
+
+    it('defaults to desc as sorting direction', () => {
+      expect(wrapper.vm.sortDirection).toBe('desc');
+    });
+
+    it('handles sorting', () => {
+      findVulnerabilities().vm.$listeners['sort-changed']({
+        sortBy: 'description',
+        sortDesc: false,
+      });
+      expect(wrapper.vm.sortBy).toBe('description');
+      expect(wrapper.vm.sortDirection).toBe('asc');
+    });
   });
 
   describe('when there is more than a page of vulnerabilities', () => {

ee/spec/frontend/security_dashboard/components/project_vulnerabilities_app_spec.js → ee/spec/frontend/security_dashboard/components/project_vulnerabilities_spec.js

@@ -76,6 +76,23 @@ describe('Vulnerabilities app component', () => {
     it('should not render the alert', () => {
       expect(findAlert().exists()).toBe(false);
     });
+
+    it('defaults to severity column for sorting', () => {
+      expect(wrapper.vm.sortBy).toBe('severity');
+    });
+
+    it('defaults to desc as sorting direction', () => {
+      expect(wrapper.vm.sortDirection).toBe('desc');
+    });
+
+    it('handles sorting', () => {
+      findVulnerabilityList().vm.$listeners['sort-changed']({
+        sortBy: 'description',
+        sortDesc: false,
+      });
+      expect(wrapper.vm.sortBy).toBe('description');
+      expect(wrapper.vm.sortDirection).toBe('asc');
+    });
   });
 
   describe('with more than a page of vulnerabilities', () => {

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

 import { mount } from '@vue/test-utils';
-import { GlDeprecatedSkeletonLoading as GlSkeletonLoading } from '@gitlab/ui';
+import { GlDeprecatedSkeletonLoading as GlSkeletonLoading, GlTable } from '@gitlab/ui';
 import { useLocalStorageSpy } from 'helpers/local_storage_helper';
 import RemediatedBadge from 'ee/vulnerabilities/components/remediated_badge.vue';
 import SecurityScannerAlert from 'ee/security_dashboard/components/security_scanner_alert.vue';
@@ -18,7 +18,7 @@ describe('Vulnerability list component', () => {
 
   let wrapper;
 
-  const createWrapper = ({ props = {} }) => {
+  const createWrapper = ({ props = {}, listeners }) => {
     return mount(VulnerabilityList, {
       propsData: {
         vulnerabilities: [],
@@ -27,6 +27,7 @@ describe('Vulnerability list component', () => {
       stubs: {
         GlPopover: true,
       },
+      listeners,
       provide: () => ({
         noVulnerabilitiesSvgPath: '#',
         dashboardDocumentation: '#',
@@ -37,6 +38,8 @@ describe('Vulnerability list component', () => {
     });
   };
 
+  const findTable = () => wrapper.find(GlTable);
+  const findSortableColumn = () => wrapper.find('[aria-sort="descending"]');
   const findCell = label => wrapper.find(`.js-${label}`);
   const findRow = (index = 0) => wrapper.findAll('tbody tr').at(index);
   const findSecurityScannerAlert = () => wrapper.find(SecurityScannerAlert);
@@ -444,4 +447,40 @@ describe('Vulnerability list component', () => {
       });
     });
   });
+
+  describe('when has a sort-changed listener defined', () => {
+    let spy;
+
+    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('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);
+    });
+  });
 });