Merge branch '350343-use-pagination-for-vulnerability-report' into 'master'

Use pagination for vulnerability report paging

See merge request gitlab-org/gitlab!79834

app/assets/javascripts/clusters/agents/index.js

 import Vue from 'vue';
 import AgentShowPage from 'ee_else_ce/clusters/agents/components/show.vue';
 import apolloProvider from './graphql/provider';
+import createRouter from './router';
 
 export default () => {
   const el = document.querySelector('#js-cluster-agent-details');
@@ -20,6 +21,7 @@ export default () => {
   return new Vue({
     el,
     apolloProvider,
+    router: createRouter(),
     provide: {
       activityEmptyStateImage,
       agentName,

app/assets/javascripts/clusters/agents/router.js

+import Vue from 'vue';
+import VueRouter from 'vue-router';
+
+Vue.use(VueRouter);
+
+// Vue Router requires a component to render if the route matches, but since we're only using it for
+// querystring handling, we'll create an empty component.
+const EmptyRouterComponent = {
+  render(createElement) {
+    return createElement('div');
+  },
+};
+
+export default () => {
+  // Name and path here don't really matter since we're not rendering anything if the route matches.
+  const routes = [{ path: '/', name: 'cluster_agents', component: EmptyRouterComponent }];
+  return new VueRouter({
+    mode: 'history',
+    base: window.location.pathname,
+    routes,
+  });
+};

config/feature_flags/development/vulnerability_report_pagination.yml

+---
+name: vulnerability_report_pagination
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79834
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351975
+milestone: '14.8'
+type: development
+group: group::threat insights
+default_enabled: false

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

 <script>
-import { GlLoadingIcon, GlIntersectionObserver } from '@gitlab/ui';
+import { GlLoadingIcon, GlIntersectionObserver, GlKeysetPagination } from '@gitlab/ui';
 import { produce } from 'immer';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import VulnerabilityList from './vulnerability_list.vue';
 
+const PAGE_SIZE = 20;
+
 // Deep searches an object for a key called 'vulnerabilities'. If it's not found, it will traverse
 // down the object's first property until either it's found, or there's nothing left to search. Note
 // that this will only check the first property of any object, not all of them.
@@ -19,7 +22,8 @@ const deepFindVulnerabilities = (data) => {
 };
 
 export default {
-  components: { GlLoadingIcon, GlIntersectionObserver, VulnerabilityList },
+  components: { GlLoadingIcon, GlIntersectionObserver, VulnerabilityList, GlKeysetPagination },
+  mixins: [glFeatureFlagsMixin()],
   inject: ['fullPath', 'canViewFalsePositive', 'hasJiraVulnerabilitiesIntegrationEnabled'],
   props: {
     query: {
@@ -49,7 +53,9 @@ export default {
     return {
       vulnerabilities: [],
       sort: undefined,
-      pageInfo: undefined,
+      pageInfo: {},
+      // The "before" querystring value on page load.
+      initialBefore: this.$route.query.before,
     };
   },
   apollo: {
@@ -64,6 +70,13 @@ export default {
           sort: this.sort,
           vetEnabled: this.canViewFalsePositive,
           includeExternalIssueLinks: this.hasJiraVulnerabilitiesIntegrationEnabled,
+          // If we're using "after" we need to use "first", and if we're using "before" we need to
+          // use "last". See this comment for more info:
+          // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79834#note_831878506
+          first: this.before ? null : PAGE_SIZE,
+          last: this.before ? PAGE_SIZE : null,
+          before: this.before,
+          after: this.after,
           ...this.filters,
         };
       },
@@ -85,28 +98,70 @@ export default {
     },
   },
   computed: {
+    before: {
+      get() {
+        // The GraphQL query can only have a "before" or an "after" but not both, so if both are in
+        // the querystring, we'll only use "after" and pretend the "before" doesn't exist.
+        return this.after ? undefined : this.$route.query.before;
+      },
+      set(before) {
+        // Check if before will change, otherwise Vue Router will throw a "you're navigating to the
+        // same route" error.
+        if (before !== this.before) {
+          this.pushQuerystring({ before, after: undefined });
+        }
+      },
+    },
+    after: {
+      get() {
+        return this.$route.query.after;
+      },
+      set(after) {
+        // Check if after will change, otherwise Vue Router will throw a "you're navigating to the
+        // same route" error.
+        if (after !== this.after) {
+          this.pushQuerystring({ before: undefined, after });
+        }
+      },
+    },
     // Used to show the infinite scrolling loading spinner.
     isLoadingVulnerabilities() {
       return this.$apollo.queries.vulnerabilities.loading;
     },
     // Used to show the initial skeleton loader.
-    isLoadingInitialVulnerabilities() {
-      return this.isLoadingVulnerabilities && this.vulnerabilities.length <= 0;
+    shouldShowLoadingSkeleton() {
+      return this.shouldUsePagination
+        ? this.isLoadingVulnerabilities
+        : this.isLoadingVulnerabilities && this.vulnerabilities.length <= 0;
     },
-    hasNextPage() {
-      return this.pageInfo?.hasNextPage;
+    shouldUsePagination() {
+      return this.glFeatures.vulnerabilityReportPagination;
     },
   },
   watch: {
-    filters() {
-      // Clear out the vulnerabilities so that the skeleton loader is shown.
-      this.vulnerabilities = [];
+    filters(newFilters, oldFilters) {
+      if (this.shouldUsePagination) {
+        // The first time the filters are set, it's done by the vulnerability-filters component, so
+        // we don't want to reset the paging. Every time after that will be from user interaction.
+        if (oldFilters !== null) {
+          this.resetPaging();
+        }
+      } else {
+        // Clear out the vulnerabilities so that the skeleton loader is shown.
+        this.vulnerabilities = [];
+      }
     },
   },
   methods: {
     updateSort(sort) {
-      // Clear out the vulnerabilities so that the skeleton loader is shown.
-      this.vulnerabilities = [];
+      if (this.shouldUsePagination) {
+        // Reset the paging whenever the sort is changed.
+        this.resetPaging();
+      } else {
+        // Clear out the vulnerabilities so that the skeleton loader is shown.
+        this.vulnerabilities = [];
+      }
+
       this.sort = sort;
     },
     fetchNextPage() {
@@ -119,6 +174,19 @@ export default {
         },
       });
     },
+    getNextPage() {
+      this.after = this.pageInfo.endCursor || this.before;
+    },
+    getPrevPage() {
+      this.before = this.pageInfo.startCursor || this.after;
+    },
+    resetPaging() {
+      this.before = undefined;
+      this.after = undefined;
+    },
+    pushQuerystring(data) {
+      this.$router.push({ query: { ...this.$route.query, ...data } });
+    },
   },
 };
 </script>
@@ -126,7 +194,7 @@ export default {
 <template>
   <div>
     <vulnerability-list
-      :is-loading="isLoadingInitialVulnerabilities"
+      :is-loading="shouldShowLoadingSkeleton"
       :vulnerabilities="vulnerabilities"
       :fields="fields"
       :should-show-project-namespace="showProjectNamespace"
@@ -134,7 +202,19 @@ export default {
       @sort-changed="updateSort"
     />
 
-    <gl-intersection-observer v-if="hasNextPage" @appear="fetchNextPage">
+    <div v-if="shouldUsePagination" class="gl-text-center gl-mt-6">
+      <gl-keyset-pagination
+        :has-previous-page="pageInfo.hasPreviousPage"
+        :has-next-page="pageInfo.hasNextPage"
+        :start-cursor="pageInfo.startCursor"
+        :end-cursor="pageInfo.endCursor"
+        :disabled="isLoadingVulnerabilities"
+        @next="getNextPage"
+        @prev="getPrevPage"
+      />
+    </div>
+
+    <gl-intersection-observer v-else-if="pageInfo.hasNextPage" @appear="fetchNextPage">
       <gl-loading-icon v-if="isLoadingVulnerabilities" size="md" />
     </gl-intersection-observer>
   </div>

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

@@ -29,6 +29,11 @@ export default {
       type: Object,
       required: true,
     },
+    isActive: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
     showProjectFilter: {
       type: Boolean,
       required: false,
@@ -91,6 +96,7 @@ export default {
     </div>
 
     <vulnerability-list-graphql
+      v-if="isActive"
       class="gl-mt-6"
       :query="query"
       :fields="fieldsToShow"

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

@@ -7,6 +7,7 @@ import VulnerabilityReportHeader from './vulnerability_report_header.vue';
 import VulnerabilityReport from './vulnerability_report.vue';
 import { REPORT_TAB } from './constants';
 
+const DEVELOPMENT_TAB_INDEX = 0;
 const OPERATIONAL_TAB_INDEX = 1;
 
 export default {
@@ -44,6 +45,9 @@ export default {
       const query = {
         ...this.$route.query,
         tab: index === OPERATIONAL_TAB_INDEX ? REPORT_TAB.OPERATIONAL : undefined,
+        // Reset pagination when the tab is changed.
+        before: undefined,
+        after: undefined,
       };
 
       this.$router.push({ query });
@@ -65,6 +69,8 @@ export default {
     ),
   },
   REPORT_TAB,
+  DEVELOPMENT_TAB_INDEX,
+  OPERATIONAL_TAB_INDEX,
 };
 </script>
 
@@ -86,6 +92,7 @@ export default {
         <slot name="header-development"></slot>
 
         <vulnerability-report
+          :is-active="tabIndex === $options.DEVELOPMENT_TAB_INDEX"
           :type="$options.REPORT_TAB.DEVELOPMENT"
           :query="query"
           :show-project-filter="showProjectFilter"
@@ -106,6 +113,7 @@ export default {
         <slot name="header-operational"></slot>
 
         <vulnerability-report
+          :is-active="tabIndex === $options.OPERATIONAL_TAB_INDEX"
           :type="$options.REPORT_TAB.OPERATIONAL"
           :query="query"
           :show-project-filter="showProjectFilter"

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

 #import "../fragments/vulnerability.fragment.graphql"
+#import "~/graphql_shared/fragments/pageInfo.fragment.graphql"
 
 query groupVulnerabilities(
   $fullPath: ID!
+  $before: String
   $after: String
   $first: Int = 20
+  $last: Int
   $projectId: [ID!]
   $severity: [VulnerabilitySeverity!]
   $reportType: [VulnerabilityReportType!]
@@ -18,8 +21,10 @@ query groupVulnerabilities(
   group(fullPath: $fullPath) {
     id
     vulnerabilities(
+      before: $before
       after: $after
       first: $first
+      last: $last
       severity: $severity
       reportType: $reportType
       scanner: $scanner
@@ -34,8 +39,7 @@ query groupVulnerabilities(
         ...VulnerabilityFragment
       }
       pageInfo {
-        endCursor
-        hasNextPage
+        ...PageInfo
       }
     }
   }

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

 #import "../fragments/vulnerability.fragment.graphql"
+#import "~/graphql_shared/fragments/pageInfo.fragment.graphql"
 
 query instanceVulnerabilities(
+  $before: String
   $after: String
   $first: Int = 20
+  $last: Int
   $projectId: [ID!]
   $severity: [VulnerabilitySeverity!]
   $reportType: [VulnerabilityReportType!]
@@ -15,8 +18,10 @@ query instanceVulnerabilities(
   $vetEnabled: Boolean = false
 ) {
   vulnerabilities(
+    before: $before
     after: $after
     first: $first
+    last: $last
     severity: $severity
     reportType: $reportType
     state: $state
@@ -31,8 +36,7 @@ query instanceVulnerabilities(
       ...VulnerabilityFragment
     }
     pageInfo {
-      endCursor
-      hasNextPage
+      ...PageInfo
     }
   }
 }

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

 #import "../fragments/vulnerability.fragment.graphql"
+#import "~/graphql_shared/fragments/pageInfo.fragment.graphql"
 
 query projectVulnerabilities(
   $fullPath: ID!
+  $before: String
   $after: String
   $first: Int = 20
+  $last: Int
   $severity: [VulnerabilitySeverity!]
   $reportType: [VulnerabilityReportType!]
   $scanner: [String!]
@@ -19,8 +22,10 @@ query projectVulnerabilities(
   project(fullPath: $fullPath) {
     id
     vulnerabilities(
+      before: $before
       after: $after
       first: $first
+      last: $last
       severity: $severity
       reportType: $reportType
       scanner: $scanner
@@ -54,8 +59,7 @@ query projectVulnerabilities(
         }
       }
       pageInfo {
-        endCursor
-        hasNextPage
+        ...PageInfo
       }
     }
   }

ee/app/controllers/groups/security/vulnerabilities_controller.rb

@@ -7,6 +7,7 @@ module Groups
 
       before_action do
         push_frontend_feature_flag(:vulnerability_management_survey, type: :ops, default_enabled: :yaml)
+        push_frontend_feature_flag(:vulnerability_report_pagination, current_user, default_enabled: :yaml)
       end
 
       feature_category :vulnerability_management

ee/app/controllers/projects/security/vulnerability_report_controller.rb

@@ -9,6 +9,7 @@ module Projects
       before_action do
         push_frontend_feature_flag(:vulnerability_management_survey, type: :ops, default_enabled: :yaml)
         push_frontend_feature_flag(:secure_vulnerability_training, @project, default_enabled: :yaml)
+        push_frontend_feature_flag(:vulnerability_report_pagination, current_user, default_enabled: :yaml)
       end
 
       feature_category :vulnerability_management

ee/app/controllers/security/vulnerabilities_controller.rb

@@ -6,6 +6,7 @@ module Security
 
     before_action do
       push_frontend_feature_flag(:vulnerability_management_survey, type: :ops, default_enabled: :yaml)
+      push_frontend_feature_flag(:vulnerability_report_pagination, current_user, default_enabled: :yaml)
     end
   end
 end

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

@@ -2,6 +2,7 @@ import Vue, { nextTick } from 'vue';
 import VueApollo from 'vue-apollo';
 
 import { GlIntersectionObserver } from '@gitlab/ui';
+import VueRouter from 'vue-router';
 import VulnerabilityListGraphql from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue';
 import VulnerabilityList from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_list.vue';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
@@ -13,6 +14,8 @@ import createFlash from '~/flash';
 jest.mock('~/flash');
 
 Vue.use(VueApollo);
+Vue.use(VueRouter);
+const router = new VueRouter();
 
 const fullPath = 'path';
 const portalName = 'portal-name';
@@ -24,7 +27,13 @@ const createVulnerabilitiesRequestHandler = ({ hasNextPage }) =>
         id: 'group-1',
         vulnerabilities: {
           nodes: [],
-          pageInfo: { endCursor: 'abc', hasNextPage },
+          pageInfo: {
+            __typename: 'PageInfo',
+            startCursor: 'abc',
+            endCursor: 'def',
+            hasNextPage,
+            hasPreviousPage: false,
+          },
         },
       },
     },
@@ -44,6 +53,7 @@ describe('Vulnerability list GraphQL component', () => {
     fields = [],
   } = {}) => {
     wrapper = shallowMountExtended(VulnerabilityListGraphql, {
+      router,
       apolloProvider: createMockApollo([[vulnerabilitiesQuery, vulnerabilitiesHandler]]),
       provide: {
         fullPath,