Merge branch '338790-more-flexible-filters' into 'master'

Make filters more reusable and use it on new group vulnerability report

See merge request gitlab-org/gitlab!73057

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

@@ -8,14 +8,14 @@ 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;
+import VulnerabilityFilters from '../shared/vulnerability_report/vulnerability_filters.vue';
+import { FIELDS, FILTERS } from '../shared/vulnerability_report/constants';
 
 export default {
   components: {
     VulnerabilityCounts,
     VulnerabilityList,
+    VulnerabilityFilters,
     GlIntersectionObserver,
     GlLoadingIcon,
     DashboardNotConfiguredGroup,
@@ -25,6 +25,7 @@ export default {
     return {
       counts: {},
       vulnerabilities: [],
+      filters: [],
       sort: undefined,
       pageInfo: undefined,
     };
@@ -37,6 +38,7 @@ export default {
         return {
           fullPath: this.groupFullPath,
           isGroup: true,
+          ...this.filters,
         };
       },
       update({ group }) {
@@ -58,6 +60,7 @@ export default {
           fullPath: this.groupFullPath,
           sort: this.sort,
           vetEnabled: this.canViewFalsePositive,
+          ...this.filters,
         };
       },
       update({ group }) {
@@ -91,14 +94,14 @@ export default {
     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,
+        ...[this.canAdminVulnerability ? FIELDS.CHECKBOX : []],
+        FIELDS.DETECTED,
+        FIELDS.STATUS,
+        FIELDS.SEVERITY,
+        FIELDS.DESCRIPTION,
+        FIELDS.IDENTIFIER,
+        FIELDS.TOOL,
+        FIELDS.ACTIVITY,
       ];
     },
   },
@@ -108,6 +111,11 @@ export default {
       this.vulnerabilities = [];
       this.sort = sort;
     },
+    updateFilters(filters) {
+      // Clear out the vulnerabilities so that the skeleton loader is shown.
+      this.vulnerabilities = [];
+      this.filters = filters;
+    },
     fetchNextPage() {
       this.$apollo.queries.vulnerabilities.fetchMore({
         variables: { after: this.pageInfo.endCursor },
@@ -122,6 +130,13 @@ export default {
       });
     },
   },
+  filtersToShow: [
+    FILTERS.STATUS,
+    FILTERS.SEVERITY,
+    FILTERS.TOOL_SIMPLE,
+    FILTERS.ACTIVITY,
+    FILTERS.PROJECT,
+  ],
 };
 </script>
 
@@ -131,6 +146,12 @@ export default {
   <div v-else class="gl-mt-5">
     <vulnerability-counts :counts="counts" :is-loading="isLoadingCounts" />
 
+    <vulnerability-filters
+      :filters="$options.filtersToShow"
+      class="security-dashboard-filters gl-mt-7"
+      @filters-changed="updateFilters"
+    />
+
     <vulnerability-list
       class="gl-mt-5"
       :is-loading="isLoadingInitialVulnerabilities"

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

 import { __, s__ } from '~/locale';
+import {
+  stateFilter,
+  severityFilter,
+  activityFilter,
+  simpleScannerFilter,
+  vendorScannerFilter,
+  getProjectFilter,
+} from 'ee/security_dashboard/helpers';
 
 export const FIELDS = {
   CHECKBOX: {
@@ -47,3 +55,12 @@ export const FIELDS = {
     class: 'activity',
   },
 };
+
+export const FILTERS = {
+  STATUS: stateFilter,
+  SEVERITY: severityFilter,
+  ACTIVITY: activityFilter,
+  TOOL_SIMPLE: simpleScannerFilter,
+  TOOL_VENDOR: vendorScannerFilter,
+  PROJECT: getProjectFilter(),
+};

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

 <script>
 import { debounce, cloneDeep, isEqual } from 'lodash';
-import {
-  stateFilter,
-  severityFilter,
-  vendorScannerFilter,
-  simpleScannerFilter,
-  activityFilter,
-  getProjectFilter,
-} from 'ee/security_dashboard/helpers';
-import { DASHBOARD_TYPES } from 'ee/security_dashboard/store/constants';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import ActivityFilter from '../filters/activity_filter.vue';
 import ProjectFilter from '../filters/project_filter.vue';
 import ProjectFilterDeprecated from '../filters/project_filter_deprecated.vue';
 import ScannerFilter from '../filters/scanner_filter.vue';
 import SimpleFilter from '../filters/simple_filter.vue';
+import { FILTERS } from './constants';
+
+const { ACTIVITY, TOOL_VENDOR, PROJECT } = FILTERS;
 
 export default {
   components: {
@@ -24,44 +17,34 @@ export default {
     ProjectFilter,
     ProjectFilterDeprecated,
   },
-  mixins: [glFeatureFlagsMixin()],
-  inject: ['dashboardType'],
   props: {
-    projects: { type: Array, required: false, default: undefined },
+    filters: {
+      type: Array,
+      required: true,
+    },
   },
   data() {
     return {
       filterQuery: {},
     };
   },
-  computed: {
-    isProjectDashboard() {
-      return this.dashboardType === DASHBOARD_TYPES.PROJECT;
-    },
-    isPipeline() {
-      return this.dashboardType === DASHBOARD_TYPES.PIPELINE;
-    },
-    isGroupDashboard() {
-      return this.dashboardType === DASHBOARD_TYPES.GROUP;
-    },
-    isInstanceDashboard() {
-      return this.dashboardType === DASHBOARD_TYPES.INSTANCE;
-    },
-    shouldShowProjectFilter() {
-      return this.isGroupDashboard || this.isInstanceDashboard;
-    },
-    shouldShowNewProjectFilter() {
-      return this.glFeatures.vulnReportNewProjectFilter && this.shouldShowProjectFilter;
-    },
-    projectFilter() {
-      return getProjectFilter(this.projects);
-    },
-  },
   methods: {
+    getComponentType(filter) {
+      switch (filter) {
+        case TOOL_VENDOR:
+          return ScannerFilter;
+        case ACTIVITY:
+          return ActivityFilter;
+        case PROJECT:
+          return ProjectFilter;
+        default:
+          return SimpleFilter;
+      }
+    },
     updateFilterQuery(query) {
       const oldQuery = cloneDeep(this.filterQuery);
       this.filterQuery = { ...this.filterQuery, ...query };
-
+      // Don't emit if the filters didn't change because it will trigger the GraphQL queries to run.
       if (!isEqual(oldQuery, this.filterQuery)) {
         this.emitFilterChange();
       }
@@ -70,13 +53,9 @@ export default {
     // the same time, which will trigger this method multiple times. We'll debounce it so that it
     // only runs once.
     emitFilterChange: debounce(function emit() {
-      this.$emit('filterChange', this.filterQuery);
+      this.$emit('filters-changed', this.filterQuery);
     }),
   },
-  simpleFilters: [stateFilter, severityFilter],
-  vendorScannerFilter,
-  simpleScannerFilter,
-  activityFilter,
 };
 </script>
 
@@ -84,42 +63,13 @@ export default {
   <div
     class="vulnerability-report-filters gl-p-5 gl-bg-gray-10 gl-border-b-1 gl-border-b-solid gl-border-b-gray-100"
   >
-    <simple-filter
-      v-for="filter in $options.simpleFilters"
+    <component
+      :is="getComponentType(filter)"
+      v-for="filter in filters"
       :key="filter.id"
       :filter="filter"
       :data-testid="filter.id"
       @filter-changed="updateFilterQuery"
     />
-
-    <scanner-filter
-      v-if="isProjectDashboard"
-      :filter="$options.vendorScannerFilter"
-      @filter-changed="updateFilterQuery"
-    />
-    <simple-filter
-      v-else
-      :filter="$options.simpleScannerFilter"
-      :data-testid="$options.simpleScannerFilter.id"
-      @filter-changed="updateFilterQuery"
-    />
-
-    <activity-filter
-      v-if="!isPipeline"
-      :filter="$options.activityFilter"
-      @filter-changed="updateFilterQuery"
-    />
-
-    <project-filter
-      v-if="shouldShowNewProjectFilter"
-      :filter="projectFilter"
-      @filter-changed="updateFilterQuery"
-    />
-    <project-filter-deprecated
-      v-else-if="shouldShowProjectFilter"
-      :filter="projectFilter"
-      :data-testid="projectFilter.id"
-      @filter-changed="updateFilterQuery"
-    />
   </div>
 </template>

ee/app/assets/stylesheets/components/vulnerability_list.scss

@@ -11,7 +11,9 @@ $selection-summary-with-error-height: 118px;
 }
 
 .security-dashboard-filters {
+  @include gl-sticky;
   @include sticky-top-positioning();
+  @include gl-z-index-1;
 }
 
 .vulnerability-list {

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

@@ -5,6 +5,7 @@ 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 VulnerabilityFilters from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_filters.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';
@@ -207,4 +208,24 @@ describe('Vulnerability counts component', () => {
       expect(findIntersectionObserver().exists()).toBe(false);
     });
   });
+
+  describe('vulnerability filters component', () => {
+    it('will pass data from filters-changed event to GraphQL queries', async () => {
+      const countsHandler = jest.fn().mockResolvedValue();
+      const vulnerabilitiesHandler = jest.fn().mockResolvedValue();
+      createWrapper({ countsHandler, vulnerabilitiesHandler });
+      // Sanity check, the report component will call these the first time it's mounted.
+      expect(countsHandler).toHaveBeenCalledTimes(1);
+      expect(vulnerabilitiesHandler).toHaveBeenCalledTimes(1);
+
+      const data = { a: 1 };
+      wrapper.findComponent(VulnerabilityFilters).vm.$emit('filters-changed', data);
+      await nextTick();
+
+      expect(countsHandler).toHaveBeenCalledTimes(2);
+      expect(countsHandler).toHaveBeenCalledWith(expect.objectContaining(data));
+      expect(vulnerabilitiesHandler).toHaveBeenCalledTimes(2);
+      expect(vulnerabilitiesHandler).toHaveBeenCalledWith(expect.objectContaining(data));
+    });
+  });
 });

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

-import { shallowMount } from '@vue/test-utils';
 import ActivityFilter from 'ee/security_dashboard/components/shared/filters/activity_filter.vue';
 import VulnerabilityFilters from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_filters.vue';
 import ProjectFilter from 'ee/security_dashboard/components/shared/filters/project_filter.vue';
 import ScannerFilter from 'ee/security_dashboard/components/shared/filters/scanner_filter.vue';
 import SimpleFilter from 'ee/security_dashboard/components/shared/filters/simple_filter.vue';
-import { getProjectFilter, simpleScannerFilter } from 'ee/security_dashboard/helpers';
-import { DASHBOARD_TYPES } from 'ee/security_dashboard/store/constants';
-import { extendedWrapper } from 'helpers/vue_test_utils_helper';
+import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import { FILTERS } from 'ee/security_dashboard/components/shared/vulnerability_report/constants';
 
-describe('First class vulnerability filters component', () => {
-  let wrapper;
-
-  const projects = [
-    { id: 'gid://gitlab/Project/11', name: 'GitLab Org' },
-    { id: 'gid://gitlab/Project/12', name: 'GitLab Com' },
-  ];
+const { ACTIVITY, PROJECT, SEVERITY, STATUS, TOOL_SIMPLE, TOOL_VENDOR } = FILTERS;
 
-  const findSimpleFilters = () => wrapper.findAllComponents(SimpleFilter);
-  const findSimpleScannerFilter = () => wrapper.findByTestId(simpleScannerFilter.id);
-  const findVendorScannerFilter = () => wrapper.findComponent(ScannerFilter);
-  const findActivityFilter = () => wrapper.findComponent(ActivityFilter);
-  const findProjectFilter = () => wrapper.findByTestId(getProjectFilter([]).id);
-  const findNewProjectFilter = () => wrapper.findComponent(ProjectFilter);
+describe('Vulnerability filters component', () => {
+  let wrapper;
 
-  const createComponent = ({ props, provide } = {}) => {
-    return extendedWrapper(
-      shallowMount(VulnerabilityFilters, {
-        propsData: props,
-        provide: {
-          dashboardType: DASHBOARD_TYPES.PROJECT,
-          ...provide,
-        },
-      }),
-    );
+  const createWrapper = ({ filters }) => {
+    wrapper = shallowMountExtended(VulnerabilityFilters, {
+      propsData: { filters },
+    });
   };
 
   afterEach(() => {
     wrapper.destroy();
   });
 
-  describe('on render without project filter', () => {
-    beforeEach(() => {
-      wrapper = createComponent();
-    });
-
-    it('should render the default filters', () => {
-      expect(findSimpleFilters()).toHaveLength(2);
-      expect(findActivityFilter().exists()).toBe(true);
-      expect(findProjectFilter().exists()).toBe(false);
-    });
-
-    it('should emit filterChange when a filter is changed', () => {
-      const options = { foo: 'bar' };
-      findActivityFilter().vm.$emit('filter-changed', options);
-
-      expect(wrapper.emitted('filterChange')[0][0]).toEqual(options);
-    });
-  });
-
-  describe('project filter', () => {
-    it.each`
-      dashboardType               | isShown
-      ${DASHBOARD_TYPES.PROJECT}  | ${false}
-      ${DASHBOARD_TYPES.PIPELINE} | ${false}
-      ${DASHBOARD_TYPES.GROUP}    | ${true}
-      ${DASHBOARD_TYPES.INSTANCE} | ${true}
-    `(
-      'on the $dashboardType report the project filter shown is $isShown',
-      ({ dashboardType, isShown }) => {
-        wrapper = createComponent({ provide: { dashboardType } });
-
-        expect(findProjectFilter().exists()).toBe(isShown);
-      },
-    );
-
-    it('should render the project filter with the expected options', () => {
-      wrapper = createComponent({
-        provide: { dashboardType: DASHBOARD_TYPES.GROUP },
-        props: { projects },
-      });
-
-      expect(findProjectFilter().props('filter').options).toEqual([
-        { id: '11', name: projects[0].name },
-        { id: '12', name: projects[1].name },
-      ]);
-    });
-
-    it.each`
-      featureFlag | isProjectFilterShown | isNewProjectFilterShown
-      ${false}    | ${true}              | ${false}
-      ${true}     | ${false}             | ${true}
-    `(
-      'should show the correct project filter when vulnReportNewProjectFilter feature flag is $featureFlag',
-      ({ featureFlag, isProjectFilterShown, isNewProjectFilterShown }) => {
-        wrapper = createComponent({
-          provide: {
-            dashboardType: DASHBOARD_TYPES.GROUP,
-            glFeatures: { vulnReportNewProjectFilter: featureFlag },
-          },
-        });
-
-        expect(findProjectFilter().exists()).toBe(isProjectFilterShown);
-        expect(findNewProjectFilter().exists()).toBe(isNewProjectFilterShown);
-      },
-    );
-  });
+  it('emits filters-changed event when filter is changed', () => {
+    createWrapper({ filters: [STATUS] });
+    const filter = wrapper.findComponent(SimpleFilter);
+    const data = { a: 1 };
+    filter.vm.$emit('filter-changed', data);
 
-  describe('activity filter', () => {
-    beforeEach(() => {
-      wrapper = createComponent({ provide: { dashboardType: DASHBOARD_TYPES.PIPELINE } });
+    expect(wrapper.emitted('filters-changed')[0][0]).toMatchObject(data);
   });
 
-    it('does not display on the pipeline dashboard', () => {
-      expect(findActivityFilter().exists()).toBe(false);
-    });
-  });
-
-  describe('scanner filter', () => {
   it.each`
-      type        | dashboardType
-      ${'vendor'} | ${DASHBOARD_TYPES.PROJECT}
-      ${'simple'} | ${DASHBOARD_TYPES.GROUP}
-      ${'simple'} | ${DASHBOARD_TYPES.INSTANCE}
-      ${'simple'} | ${DASHBOARD_TYPES.PIPELINE}
-    `('shows the $type scanner filter on the $dashboardType report', ({ type, dashboardType }) => {
-      wrapper = createComponent({ provide: { dashboardType } });
-
-      expect(findSimpleScannerFilter().exists()).toBe(type === 'simple');
-      expect(findVendorScannerFilter().exists()).toBe(type === 'vendor');
-    });
+    name             | filters          | expectedComponent
+    ${'activity'}    | ${[ACTIVITY]}    | ${ActivityFilter}
+    ${'project'}     | ${[PROJECT]}     | ${ProjectFilter}
+    ${'severity'}    | ${[SEVERITY]}    | ${SimpleFilter}
+    ${'status'}      | ${[STATUS]}      | ${SimpleFilter}
+    ${'tool_simple'} | ${[TOOL_SIMPLE]} | ${SimpleFilter}
+    ${'tool_vendor'} | ${[TOOL_VENDOR]} | ${ScannerFilter}
+  `(`shows the expected component for filter '$name'`, ({ filters, expectedComponent }) => {
+    createWrapper({ filters });
+
+    expect(wrapper.findComponent(expectedComponent).exists()).toBe(true);
   });
 });