Merge branch '338790-use-vulnerability-list-on-group-report' into 'master'

Use new vulnerability list on new group report

See merge request gitlab-org/gitlab!72972

ee/app/assets/javascripts/security_dashboard/components/group/group_vulnerabilities.vue

@@ -2,7 +2,6 @@
 import { GlAlert, GlLoadingIcon, GlIntersectionObserver } from '@gitlab/ui';
 import produce from 'immer';
 import vulnerabilitiesQuery from 'ee/security_dashboard/graphql/queries/group_vulnerabilities.query.graphql';
-import { preparePageInfo } from 'ee/security_dashboard/helpers';
 import { VULNERABILITIES_PER_PAGE } from 'ee/security_dashboard/store/constants';
 import VulnerabilityList from '../shared/vulnerability_list.vue';
 
@@ -49,7 +48,7 @@ export default {
       },
       update: ({ group }) => group.vulnerabilities.nodes,
       result({ data }) {
-        this.pageInfo = preparePageInfo(data?.group?.vulnerabilities?.pageInfo);
+        this.pageInfo = data?.group?.vulnerabilities?.pageInfo;
       },
       error() {
         this.errorLoadingVulnerabilities = true;

ee/app/assets/javascripts/security_dashboard/components/group/vulnerability_report_development.vue

 <script>
+import { GlIntersectionObserver, GlLoadingIcon } from '@gitlab/ui';
+import produce from 'immer';
 import countsQuery from 'ee/security_dashboard/graphql/queries/vulnerability_severities_count.query.graphql';
+import vulnerabilitiesQuery from 'ee/security_dashboard/graphql/queries/group_vulnerabilities.query.graphql';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
+import DashboardNotConfiguredGroup from '../shared/empty_states/group_dashboard_not_configured.vue';
 import VulnerabilityCounts from '../shared/vulnerability_report/vulnerability_counts.vue';
+import VulnerabilityList from '../shared/vulnerability_report/vulnerability_list.vue';
+import { FIELDS } from '../shared/vulnerability_report/constants';
+
+const { CHECKBOX, DETECTED, STATUS, SEVERITY, DESCRIPTION, IDENTIFIER, TOOL, ACTIVITY } = FIELDS;
 
 export default {
-  components: { VulnerabilityCounts },
-  inject: ['groupFullPath'],
+  components: {
+    VulnerabilityCounts,
+    VulnerabilityList,
+    GlIntersectionObserver,
+    GlLoadingIcon,
+    DashboardNotConfiguredGroup,
+  },
+  inject: ['groupFullPath', 'canViewFalsePositive', 'canAdminVulnerability', 'hasProjects'],
   data() {
     return {
       counts: {},
+      vulnerabilities: [],
+      sort: undefined,
+      pageInfo: undefined,
     };
   },
   apollo: {
     counts: {
       query: countsQuery,
+      errorPolicy: 'none',
       variables() {
         return {
           fullPath: this.groupFullPath,
@@ -32,15 +50,98 @@ export default {
         });
       },
     },
+    vulnerabilities: {
+      query: vulnerabilitiesQuery,
+      errorPolicy: 'none',
+      variables() {
+        return {
+          fullPath: this.groupFullPath,
+          sort: this.sort,
+          vetEnabled: this.canViewFalsePositive,
+        };
+      },
+      update({ group }) {
+        this.pageInfo = group.vulnerabilities.pageInfo;
+        return group.vulnerabilities.nodes;
+      },
+      error() {
+        createFlash({
+          message: s__(
+            'SecurityReports|Error fetching the vulnerability list. Please check your network connection and try again.',
+          ),
+        });
+      },
+    },
   },
   computed: {
-    isCountsLoading() {
+    isLoadingCounts() {
       return this.$apollo.queries.counts.loading;
     },
+    // Used to show the loading icon at the bottom of the vulnerabilities list.
+    isLoadingVulnerabilities() {
+      return this.$apollo.queries.vulnerabilities.loading;
+    },
+    // Used to show the initial skeleton loader for the vulnerabilities list.
+    isLoadingInitialVulnerabilities() {
+      return this.isLoadingVulnerabilities && this.vulnerabilities.length <= 0;
+    },
+    hasNextPage() {
+      return this.pageInfo?.hasNextPage;
+    },
+    fields() {
+      return [
+        // Add the checkbox field if the user can use the bulk select feature.
+        ...[this.canAdminVulnerability ? CHECKBOX : []],
+        DETECTED,
+        STATUS,
+        SEVERITY,
+        DESCRIPTION,
+        IDENTIFIER,
+        TOOL,
+        ACTIVITY,
+      ];
+    },
+  },
+  methods: {
+    updateSort(sort) {
+      // Clear out the vulnerabilities so that the skeleton loader is shown.
+      this.vulnerabilities = [];
+      this.sort = sort;
+    },
+    fetchNextPage() {
+      this.$apollo.queries.vulnerabilities.fetchMore({
+        variables: { after: this.pageInfo.endCursor },
+        updateQuery: (previousResult, { fetchMoreResult }) => {
+          return produce(fetchMoreResult, (draftData) => {
+            draftData.group.vulnerabilities.nodes = [
+              ...previousResult.group.vulnerabilities.nodes,
+              ...draftData.group.vulnerabilities.nodes,
+            ];
+          });
+        },
+      });
+    },
   },
 };
 </script>
 
 <template>
-  <vulnerability-counts :counts="counts" :is-loading="isCountsLoading" />
+  <dashboard-not-configured-group v-if="!hasProjects" />
+
+  <div v-else class="gl-mt-5">
+    <vulnerability-counts :counts="counts" :is-loading="isLoadingCounts" />
+
+    <vulnerability-list
+      class="gl-mt-5"
+      :is-loading="isLoadingInitialVulnerabilities"
+      :vulnerabilities="vulnerabilities"
+      :fields="fields"
+      should-show-project-namespace
+      @sort-changed="updateSort"
+    />
+
+    <gl-intersection-observer v-if="hasNextPage" @appear="fetchNextPage">
+      <gl-loading-icon v-if="isLoadingVulnerabilities" size="md" />
+    </gl-intersection-observer>
+  </div>
 </template>

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

-#import "~/graphql_shared/fragments/pageInfoCursorsOnly.fragment.graphql"
 #import "../fragments/vulnerability.fragment.graphql"
 
 query groupVulnerabilities(
   $fullPath: ID!
   $after: String
-  $first: Int
+  $first: Int = 20
   $projectId: [ID!]
   $severity: [VulnerabilitySeverity!]
   $reportType: [VulnerabilityReportType!]
@@ -34,7 +33,8 @@ query groupVulnerabilities(
         ...VulnerabilityFragment
       }
       pageInfo {
-        ...PageInfo
+        endCursor
+        hasNextPage
       }
     }
   }

ee/app/assets/javascripts/security_dashboard/vulnerability_report_init.js

@@ -21,6 +21,7 @@ export default (el, dashboardType) => {
     notEnabledScannersHelpPath,
     noPipelineRunScannersHelpPath,
     hasVulnerabilities,
+    hasProjects,
     scanners,
     securityDashboardHelpPath,
     vulnerabilitiesExportEndpoint,
@@ -80,6 +81,7 @@ export default (el, dashboardType) => {
     autoFixMrsPath,
     canAdminVulnerability: parseBoolean(canAdminVulnerability),
     hasVulnerabilities: parseBoolean(hasVulnerabilities),
+    hasProjects: parseBoolean(hasProjects),
     scanners: scanners ? JSON.parse(scanners) : [],
     hasJiraVulnerabilitiesIntegrationEnabled: parseBoolean(
       hasJiraVulnerabilitiesIntegrationEnabled,

ee/app/helpers/groups/security_features_helper.rb

@@ -30,7 +30,8 @@ module Groups::SecurityFeaturesHelper
       scanners: VulnerabilityScanners::ListService.new(group).execute.to_json,
       can_admin_vulnerability: can?(current_user, :admin_vulnerability, group).to_s,
       false_positive_doc_url: help_page_path('user/application_security/vulnerabilities/index'),
-      can_view_false_positive: group.licensed_feature_available?(:sast_fp_reduction).to_s
+      can_view_false_positive: group.licensed_feature_available?(:sast_fp_reduction).to_s,
+      has_projects: group.projects.any?.to_s
     }
   end
 end

ee/spec/frontend/security_dashboard/components/group/group_vulnerabilities_spec.js

@@ -212,7 +212,7 @@ describe('Group Security Dashboard Vulnerabilities Component', () => {
         group: {
           vulnerabilities: {
             nodes: [],
-            pageInfo: { startCursor: '', endCursor: '' },
+            pageInfo: { endCursor: '', hasNextPage: '' },
           },
         },
       },

ee/spec/frontend/security_dashboard/components/group/vulnerability_report_development_spec.js

 import { createLocalVue } from '@vue/test-utils';
 import VueApollo from 'vue-apollo';
 import { nextTick } from 'vue';
+import { GlIntersectionObserver } from '@gitlab/ui';
+import VulnerabilityList from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_list.vue';
 import VulnerabilityReportDevelopment from 'ee/security_dashboard/components/group/vulnerability_report_development.vue';
 import VulnerabilityCounts from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_counts.vue';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import countsQuery from 'ee/security_dashboard/graphql/queries/vulnerability_severities_count.query.graphql';
+import vulnerabilitiesQuery from 'ee/security_dashboard/graphql/queries/group_vulnerabilities.query.graphql';
 import createMockApollo from 'helpers/mock_apollo_helper';
+import createFlash from '~/flash';
+
+jest.mock('~/flash');
 
 const localVue = createLocalVue();
 localVue.use(VueApollo);
@@ -21,27 +27,59 @@ const countsRequestHandler = jest.fn().mockResolvedValue({
   },
 });
 
+const createVulnerabilitiesRequestHandler = ({ hasNextPage }) =>
+  jest.fn().mockResolvedValue({
+    data: {
+      group: {
+        vulnerabilities: {
+          nodes: [],
+          pageInfo: { endCursor: 'abc', hasNextPage },
+        },
+      },
+    },
+  });
+
+const vulnerabilitiesRequestHandler = createVulnerabilitiesRequestHandler({ hasNextPage: true });
+
 describe('Vulnerability counts component', () => {
   let wrapper;
 
-  const createWrapper = ({ queries }) => {
+  const createWrapper = ({
+    countsHandler = countsRequestHandler,
+    vulnerabilitiesHandler = vulnerabilitiesRequestHandler,
+    canViewFalsePositive = true,
+  } = {}) => {
+    // Use the default request handlers if they weren't provided.
+    const queries = [
+      [countsQuery, countsHandler],
+      [vulnerabilitiesQuery, vulnerabilitiesHandler],
+    ];
+
     wrapper = shallowMountExtended(VulnerabilityReportDevelopment, {
       localVue,
       apolloProvider: createMockApollo(queries),
-      provide: { groupFullPath },
+      provide: {
+        groupFullPath,
+        canViewFalsePositive,
+        canAdminVulnerability: true,
+        hasProjects: true,
+      },
     });
   };
 
   const findVulnerabilityCounts = () => wrapper.findComponent(VulnerabilityCounts);
+  const findVulnerabilityList = () => wrapper.findComponent(VulnerabilityList);
+  const findIntersectionObserver = () => wrapper.findComponent(GlIntersectionObserver);
 
   afterEach(() => {
     wrapper.destroy();
     countsRequestHandler.mockClear();
+    vulnerabilitiesRequestHandler.mockClear();
   });
 
   describe('vulnerability counts query', () => {
-    it('calls the counts query once with the expected data', () => {
-      createWrapper({ queries: [[countsQuery, countsRequestHandler]] });
+    it('calls the query once with the expected data', () => {
+      createWrapper();
 
       expect(countsRequestHandler).toHaveBeenCalledTimes(1);
       expect(countsRequestHandler).toHaveBeenCalledWith(
@@ -52,8 +90,22 @@ describe('Vulnerability counts component', () => {
       );
     });
 
-    it('passes the isLoading prop with the expected values', async () => {
-      createWrapper({ queries: [[countsQuery, countsRequestHandler]] });
+    it('shows an error message if the query fails', async () => {
+      const countsHandler = jest.fn().mockRejectedValue(new Error());
+      createWrapper({ countsHandler });
+      // Have to wait 2 ticks here, one for the query to finish loading, and one more for the
+      // GraphQL error handler to be called.
+      await nextTick();
+      await nextTick();
+
+      expect(createFlash).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('vulnerability counts component', () => {
+    it('gets the expected isLoading prop from the counts query loading state', async () => {
+      createWrapper();
+      // The query will be loading until we use nextTick() to give micro-tasks a chance to run.
       expect(findVulnerabilityCounts().props('isLoading')).toBe(true);
 
       await nextTick();
@@ -61,12 +113,98 @@ describe('Vulnerability counts component', () => {
       expect(findVulnerabilityCounts().props('isLoading')).toBe(false);
     });
 
-    it('passes the results of the counts query to the vulnerability counts component', async () => {
-      createWrapper({ queries: [[countsQuery, countsRequestHandler]] });
-
+    it('gets the expected counts prop', async () => {
+      createWrapper();
       await nextTick();
 
       expect(findVulnerabilityCounts().props('counts')).toMatchObject(counts);
     });
   });
+
+  describe('group vulnerabilities query', () => {
+    it('calls the query once with the expected fullPath variable', () => {
+      createWrapper();
+
+      expect(vulnerabilitiesRequestHandler).toHaveBeenCalledTimes(1);
+      expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
+        expect.objectContaining({ fullPath: groupFullPath }),
+      );
+    });
+
+    it.each([true, false])(
+      'calls the query with the expected vetEnabled property when canViewFalsePositive is %s',
+      (canViewFalsePositive) => {
+        createWrapper({ canViewFalsePositive });
+
+        expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
+          expect.objectContaining({ vetEnabled: canViewFalsePositive }),
+        );
+      },
+    );
+
+    it('shows an error message if the query fails', async () => {
+      const vulnerabilitiesHandler = jest.fn().mockRejectedValue(new Error());
+      createWrapper({ vulnerabilitiesHandler });
+      // Have to wait 2 ticks here, one for the query to finish loading, and one more for the
+      // GraphQL error handler to be called.
+      await nextTick();
+      await nextTick();
+
+      expect(createFlash).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('vulnerability list component', () => {
+    it('gets the expected vulnerabilities prop', async () => {
+      createWrapper();
+      await nextTick();
+
+      const vulnerabilities = [];
+      wrapper.setData({ vulnerabilities });
+      await nextTick();
+
+      expect(findVulnerabilityList().props('vulnerabilities')).toBe(vulnerabilities);
+    });
+
+    it('calls the vulnerabilities query with the data from the sort-changed event', async () => {
+      createWrapper();
+      // First call should be undefined, which uses the default sort.
+      expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
+        expect.objectContaining({ sort: undefined }),
+      );
+
+      const sort = 'sort';
+      findVulnerabilityList().vm.$emit('sort-changed', sort);
+      await nextTick();
+
+      expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(expect.objectContaining({ sort }));
+    });
+  });
+
+  describe('intersection observer', () => {
+    it('is not shown when the vulnerabilities query is loading for the first time', () => {
+      createWrapper();
+
+      expect(findIntersectionObserver().exists()).toBe(false);
+    });
+
+    it('will fetch more data when the appear event is fired', async () => {
+      createWrapper();
+      await nextTick();
+      const spy = jest.spyOn(wrapper.vm.$apollo.queries.vulnerabilities, 'fetchMore');
+
+      findIntersectionObserver().vm.$emit('appear');
+
+      expect(spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('is not shown if there is no next page', async () => {
+      createWrapper({
+        vulnerabilitiesHandler: createVulnerabilitiesRequestHandler({ hasNextPage: false }),
+      });
+      await nextTick();
+
+      expect(findIntersectionObserver().exists()).toBe(false);
+    });
+  });
 });

ee/spec/helpers/groups/security_features_helper_spec.rb

@@ -80,7 +80,8 @@ RSpec.describe Groups::SecurityFeaturesHelper do
         vulnerabilities_export_endpoint: "/api/v4/security/groups/#{group.id}/vulnerability_exports",
         scanners: '[]',
         can_admin_vulnerability: 'true',
-        can_view_false_positive: 'false'
+        can_view_false_positive: 'false',
+        has_projects: 'false'
       }
     end