Merge branch '284471-refactor-vulnerability-report-filters' into 'master'

Improve code readability and layout of vulnerability report filters

See merge request gitlab-org/gitlab!58488

ee/app/assets/javascripts/security_dashboard/components/filters/filter_body.vue

@@ -71,7 +71,7 @@ export default {
 </script>
 
 <template>
-  <div class="dashboard-filter">
+  <div>
     <strong data-testid="name">{{ name }}</strong>
     <gl-dropdown
       class="gl-mt-2 gl-w-full"

ee/app/assets/javascripts/security_dashboard/components/filters/standard_filter.vue

@@ -10,10 +10,11 @@ export default {
       type: Object,
       required: true,
     },
-    showSearchBox: {
-      type: Boolean,
+    // Number of options that must exist for the search box to show.
+    searchBoxShowThreshold: {
+      type: Number,
       required: false,
-      default: false,
+      default: 20,
     },
     loading: {
       type: Boolean,
@@ -67,6 +68,9 @@ export default {
 
       return hasAllId ? [] : this.filter.defaultOptions;
     },
+    showSearchBox() {
+      return this.options.length >= this.searchBoxShowThreshold;
+    },
   },
   watch: {
     selectedOptions() {

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

 <script>
 import { debounce } from 'lodash';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import {
   stateFilter,
   severityFilter,
@@ -11,9 +12,9 @@ import ActivityFilter from './filters/activity_filter.vue';
 import ScannerFilter from './filters/scanner_filter.vue';
 import StandardFilter from './filters/standard_filter.vue';
 
-const searchBoxOptionCount = 20; // Number of options before the search box is shown.
-
 export default {
+  components: { StandardFilter, ScannerFilter, ActivityFilter },
+  mixins: [glFeatureFlagsMixin()],
   props: {
     projects: { type: Array, required: false, default: undefined },
   },
@@ -23,14 +24,19 @@ export default {
     };
   },
   computed: {
-    filters() {
-      const filters = [stateFilter, severityFilter, scannerFilter, activityFilter];
-
-      if (this.projects) {
-        filters.push(getProjectFilter(this.projects));
-      }
-
-      return filters;
+    standardFilters() {
+      return this.shouldShowCustomScannerFilter
+        ? [stateFilter, severityFilter]
+        : [stateFilter, severityFilter, scannerFilter];
+    },
+    shouldShowProjectFilter() {
+      return Boolean(this.projects?.length);
+    },
+    shouldShowCustomScannerFilter() {
+      return this.glFeatures.customSecurityScanners;
+    },
+    projectFilter() {
+      return getProjectFilter(this.projects);
     },
   },
   methods: {
@@ -43,33 +49,34 @@ export default {
     emitFilterChange: debounce(function emit() {
       this.$emit('filterChange', this.filterQuery);
     }),
-    getFilterComponent({ id }) {
-      if (id === activityFilter.id) {
-        return ActivityFilter;
-      } else if (gon.features?.customSecurityScanners && id === scannerFilter.id) {
-        return ScannerFilter;
-      }
-
-      return StandardFilter;
-    },
   },
-  searchBoxOptionCount,
+  scannerFilter,
+  activityFilter,
 };
 </script>
 
 <template>
-  <div class="dashboard-filters border-bottom bg-gray-light">
-    <div class="row mx-0 p-2">
-      <component
-        :is="getFilterComponent(filter)"
-        v-for="filter in filters"
-        :key="filter.id"
-        class="col-sm-6 col-md-4 col-lg-2 p-2"
-        :filter="filter"
-        :data-testid="filter.id"
-        :show-search-box="filter.options.length >= $options.searchBoxOptionCount"
-        @filter-changed="updateFilterQuery"
-      />
-    </div>
+  <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"
+  >
+    <standard-filter
+      v-for="filter in standardFilters"
+      :key="filter.id"
+      :filter="filter"
+      :data-testid="filter.id"
+      @filter-changed="updateFilterQuery"
+    />
+    <scanner-filter
+      v-if="shouldShowCustomScannerFilter"
+      :filter="$options.scannerFilter"
+      @filter-changed="updateFilterQuery"
+    />
+    <activity-filter :filter="$options.activityFilter" @filter-changed="updateFilterQuery" />
+    <standard-filter
+      v-if="shouldShowProjectFilter"
+      :filter="projectFilter"
+      :data-testid="projectFilter.id"
+      @filter-changed="updateFilterQuery"
+    />
   </div>
 </template>

ee/app/assets/stylesheets/pages/security/vulnerability_report.scss

+.vulnerability-report-filters {
+  @include gl-display-grid;
+  grid-template-columns: repeat(auto-fill, minmax(12rem, 1fr));
+  grid-gap: $gl-spacing-scale-5;
+}

ee/spec/frontend/security_dashboard/components/filters/standard_filter_spec.js

@@ -96,13 +96,14 @@ describe('Standard Filter component', () => {
 
   describe('search box', () => {
     it.each`
-      phrase             | showSearchBox
-      ${'shows'}         | ${true}
-      ${'does not show'} | ${false}
-    `('$phrase search box if showSearchBox is $showSearchBox', ({ showSearchBox }) => {
-      createWrapper({}, { showSearchBox });
-
-      expect(filterBody().props('showSearchBox')).toBe(showSearchBox);
+      phrase     | count | searchBoxShowThreshold
+      ${'shows'} | ${5}  | ${5}
+      ${'hides'} | ${7}  | ${8}
+    `('$phrase search box if there are $count options', ({ count, searchBoxShowThreshold }) => {
+      createWrapper({ options: generateOptions(count) }, { searchBoxShowThreshold });
+      const shouldShow = count >= searchBoxShowThreshold;
+
+      expect(filterBody().props('showSearchBox')).toBe(shouldShow);
     });
 
     it('filters options when something is typed in the search box', async () => {

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

 import { shallowMount } from '@vue/test-utils';
+import ActivityFilter from 'ee/security_dashboard/components/filters/activity_filter.vue';
 import ScannerFilter from 'ee/security_dashboard/components/filters/scanner_filter.vue';
 import StandardFilter from 'ee/security_dashboard/components/filters/standard_filter.vue';
 import Filters from 'ee/security_dashboard/components/first_class_vulnerability_filters.vue';
+import { scannerFilter, getProjectFilter } from 'ee/security_dashboard/helpers';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
 describe('First class vulnerability filters component', () => {
   let wrapper;
@@ -11,35 +14,41 @@ describe('First class vulnerability filters component', () => {
     { id: 'gid://gitlab/Project/12', name: 'GitLab Com' },
   ];
 
-  const findFilters = () => wrapper.findAll(StandardFilter);
-  const findStateFilter = () => wrapper.find('[data-testid="state"]');
-  const findProjectFilter = () => wrapper.find('[data-testid="projectId"]');
-
-  const createComponent = (propsData) => {
-    return shallowMount(Filters, { propsData });
+  const findStandardFilters = () => wrapper.findAllComponents(StandardFilter);
+  const findStandardScannerFilter = () => wrapper.findByTestId(scannerFilter.id);
+  const findCustomScannerFilter = () => wrapper.findComponent(ScannerFilter);
+  const findActivityFilter = () => wrapper.findComponent(ActivityFilter);
+  const findProjectFilter = () => wrapper.findByTestId(getProjectFilter([]).id);
+
+  const createComponent = ({ props, provide } = {}) => {
+    return extendedWrapper(
+      shallowMount(Filters, {
+        propsData: props,
+        provide,
+      }),
+    );
   };
 
-  beforeEach(() => {
-    gon.features = {};
-  });
-
   afterEach(() => {
     wrapper.destroy();
     wrapper = null;
   });
 
   it.each`
-    flagValue | expectedComponent | expectedName
-    ${true}   | ${ScannerFilter}  | ${'ScannerFilter'}
-    ${false}  | ${StandardFilter} | ${'StandardFilter'}
+    flagValue | isStandardFilterShown | isCustomFilterShown
+    ${true}   | ${false}              | ${true}
+    ${false}  | ${true}               | ${false}
   `(
-    `renders $expectedName component when customSecurityScanners feature flag is $flagValue`,
-    ({ flagValue, expectedComponent }) => {
-      wrapper = createComponent();
-      const filter = { id: 'reportType' };
-      gon.features.customSecurityScanners = flagValue;
-
-      expect(wrapper.vm.getFilterComponent(filter)).toEqual(expectedComponent);
+    `renders correct scanner filter component when customSecurityScanners feature flag is $flagValue`,
+    ({ flagValue, isStandardFilterShown, isCustomFilterShown }) => {
+      wrapper = createComponent({
+        provide: {
+          glFeatures: { customSecurityScanners: flagValue },
+        },
+      });
+
+      expect(findCustomScannerFilter().exists()).toBe(isCustomFilterShown);
+      expect(findStandardScannerFilter().exists()).toBe(isStandardFilterShown);
     },
   );
 
@@ -49,31 +58,28 @@ describe('First class vulnerability filters component', () => {
     });
 
     it('should render the default filters', () => {
-      expect(findFilters()).toHaveLength(3);
+      expect(findStandardFilters()).toHaveLength(3);
+      expect(findActivityFilter().exists()).toBe(true);
+      expect(findProjectFilter().exists()).toBe(false);
     });
 
     it('should emit filterChange when a filter is changed', () => {
       const options = { foo: 'bar' };
-      findStateFilter().vm.$emit('filter-changed', options);
+      findActivityFilter().vm.$emit('filter-changed', options);
 
       expect(wrapper.emitted('filterChange')[0][0]).toEqual(options);
     });
   });
 
   describe('when project filter is populated dynamically', () => {
-    beforeEach(() => {
-      wrapper = createComponent();
-    });
+    it('should not render the project filter if there are no options', async () => {
+      wrapper = createComponent({ props: { projects: [] } });
 
-    it('should render the project filter with no options', async () => {
-      wrapper.setProps({ projects: [] });
-      await wrapper.vm.$nextTick();
-      expect(findProjectFilter().props('filter').options).toHaveLength(0);
+      expect(findProjectFilter().exists()).toBe(false);
     });
 
     it('should render the project filter with the expected options', async () => {
-      wrapper.setProps({ projects });
-      await wrapper.vm.$nextTick();
+      wrapper = createComponent({ props: { projects } });
 
       expect(findProjectFilter().props('filter').options).toEqual([
         { id: '11', name: projects[0].name },