Merge branch '210334-refactor-vulnerability-filters' into 'master'

Move the vuex logic out of the filter component

Closes #210334

See merge request gitlab-org/gitlab!27216

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

 <script>
-import { mapGetters, mapActions } from 'vuex';
 import { GlDropdown, GlSearchBoxByType } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
 
@@ -10,8 +9,8 @@ export default {
     Icon,
   },
   props: {
-    filterId: {
-      type: String,
+    filter: {
+      type: Object,
       required: true,
     },
   },
@@ -21,15 +20,17 @@ export default {
     };
   },
   computed: {
-    ...mapGetters('filters', ['getFilter', 'getSelectedOptions', 'getSelectedOptionNames']),
-    filter() {
-      return this.getFilter(this.filterId);
+    filterId() {
+      return this.filter.id;
     },
     selection() {
-      return this.getFilter(this.filterId).selection;
+      return this.filter.selection;
     },
-    selectedOptionText() {
-      return this.getSelectedOptionNames(this.filterId) || '-';
+    firstSelectedOption() {
+      return this.filter.options.find(option => this.selection.has(option.id))?.name || '-';
+    },
+    extraOptionCount() {
+      return this.selection.size - 1;
     },
     filteredOptions() {
       return this.filter.options.filter(option =>
@@ -41,12 +42,8 @@ export default {
     },
   },
   methods: {
-    ...mapActions('filters', ['setFilter']),
     clickFilter(option) {
-      this.setFilter({
-        filterId: this.filterId,
-        optionId: option.id,
-      });
+      this.$emit('setFilter', { filterId: this.filterId, optionId: option.id });
     },
     isSelected(option) {
       return this.selection.has(option.id);
@@ -64,12 +61,11 @@ export default {
     <gl-dropdown ref="dropdown" class="d-block mt-1" menu-class="dropdown-extended-height">
       <template slot="button-content">
         <span class="text-truncate" :data-qa-selector="qaSelector">
-          {{ selectedOptionText.firstOption }}
+          {{ firstSelectedOption }}
         </span>
-        <span v-if="selectedOptionText.extraOptionCount" class="flex-grow-1 ml-1">
-          {{ selectedOptionText.extraOptionCount }}
+        <span v-if="extraOptionCount" class="flex-grow-1 ml-1">
+          {{ n__('+%d more', '+%d more', extraOptionCount) }}
         </span>
-
         <i class="fa fa-chevron-down" aria-hidden="true"></i>
       </template>
 

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

 <script>
-import { mapGetters } from 'vuex';
+import { mapGetters, mapActions } from 'vuex';
 import DashboardFilter from './filter.vue';
 import GlToggleVuex from '~/vue_shared/components/gl_toggle_vuex.vue';
 
@@ -9,9 +9,10 @@ export default {
     GlToggleVuex,
   },
   computed: {
-    ...mapGetters({
-      filters: 'filters/visibleFilters',
-    }),
+    ...mapGetters('filters', ['visibleFilters']),
+  },
+  methods: {
+    ...mapActions('filters', ['setFilter']),
   },
 };
 </script>
@@ -20,10 +21,11 @@ export default {
   <div class="dashboard-filters border-bottom bg-gray-light">
     <div class="row mx-0 p-2">
       <dashboard-filter
-        v-for="filter in filters"
+        v-for="filter in visibleFilters"
         :key="filter.id"
         class="col-sm-6 col-md-4 col-lg-2 p-2 js-filter"
-        :filter-id="filter.id"
+        :filter="filter"
+        @setFilter="setFilter"
       />
       <div class="ml-lg-auto p-2">
         <strong>{{ s__('SecurityDashboard|Hide dismissed') }}</strong>

ee/app/assets/javascripts/security_dashboard/store/modules/filters/getters.js

-import { sprintf, __ } from '~/locale';
 import { isBaseFilterOption } from './utils';
 
-export const getFilter = state => filterId => state.filters.find(filter => filter.id === filterId);
-
-export const getSelectedOptions = (state, getters) => filterId => {
-  const filter = getters.getFilter(filterId);
-  return filter.options.filter(option => filter.selection.has(option.id));
-};
-
-export const getSelectedOptionNames = (state, getters) => filterId => {
-  const selectedOptions = getters.getSelectedOptions(filterId);
-  const extraOptionCount = selectedOptions.length - 1;
-  const firstOption = selectedOptions.map(option => option.name)[0];
-
-  return {
-    firstOption,
-    extraOptionCount: extraOptionCount
-      ? sprintf(__('+%{extraOptionCount} more'), { extraOptionCount })
-      : '',
-  };
-};
-
 /**
  * Loops through all the filters and returns all the active ones
  * stripping out base filter options.

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

-import Vuex from 'vuex';
 import Filter from 'ee/security_dashboard/components/filter.vue';
-import createStore from 'ee/security_dashboard/store';
-import { mount, createLocalVue } from '@vue/test-utils';
+import { mount } from '@vue/test-utils';
 import stubChildren from 'helpers/stub_children';
+import { trimText } from 'helpers/text_helper';
 
-const localVue = createLocalVue();
-localVue.use(Vuex);
+const generateOption = index => ({
+  name: `Option ${index}`,
+  id: `option-${index}`,
+});
+
+const generateOptions = length => {
+  return Array.from({ length }).map((_, i) => generateOption(i));
+};
 
 describe('Filter component', () => {
   let wrapper;
-  let store;
 
   const createWrapper = propsData => {
     wrapper = mount(Filter, {
@@ -19,7 +23,6 @@ describe('Filter component', () => {
         GlSearchBoxByType: false,
       },
       propsData,
-      store,
       attachToDocument: true,
     });
   };
@@ -34,37 +37,36 @@ describe('Filter component', () => {
     return toggleButton.attributes('aria-expanded') === 'true';
   }
 
-  function setProjectsCount(count) {
-    const projects = new Array(count).fill(null).map((_, i) => ({
-      name: i.toString(),
-      id: i.toString(),
-    }));
-
-    store.dispatch('filters/setFilterOptions', {
-      filterId: 'project_id',
-      options: projects,
-    });
-  }
-
-  beforeEach(() => {
-    store = createStore();
-  });
-
   afterEach(() => {
     wrapper.destroy();
   });
 
   describe('severity', () => {
+    let options;
+
     beforeEach(() => {
-      createWrapper({ filterId: 'severity' });
+      options = generateOptions(8);
+      const filter = {
+        name: 'Severity',
+        id: 'severity',
+        options,
+        selection: new Set([options[0].id, options[1].id, options[2].id]),
+      };
+      createWrapper({ filter });
     });
 
     it('should display all 8 severity options', () => {
       expect(dropdownItemsCount()).toEqual(8);
     });
 
-    it('should display a check next to only the selected item', () => {
-      expect(wrapper.findAll('.dropdown-item .js-check').length).toEqual(1);
+    it('should display a check next to only the selected items', () => {
+      expect(wrapper.findAll('.dropdown-item .js-check').length).toEqual(3);
+    });
+
+    it('should correctly display the selected text', () => {
+      const selectedText = trimText(wrapper.find('.dropdown-toggle').text());
+
+      expect(selectedText).toBe(`${options[0].name} +2 more`);
     });
 
     it('should display "Severity" as the option name', () => {
@@ -107,11 +109,18 @@ describe('Filter component', () => {
 
   describe('Project', () => {
     describe('when there are lots of projects', () => {
-      const lots = 30;
+      const LOTS = 30;
+
       beforeEach(() => {
-        createWrapper({ filterId: 'project_id', dashboardDocumentation: '' });
-        setProjectsCount(lots);
-        return wrapper.vm.$nextTick();
+        const options = generateOptions(LOTS);
+        const filter = {
+          name: 'Project',
+          id: 'project',
+          options,
+          selection: new Set([options[0].id]),
+        };
+
+        createWrapper({ filter });
       });
 
       it('should display a search box', () => {
@@ -119,7 +128,7 @@ describe('Filter component', () => {
       });
 
       it(`should show all projects`, () => {
-        expect(dropdownItemsCount()).toBe(lots);
+        expect(dropdownItemsCount()).toBe(LOTS);
       });
 
       it('should show only matching projects when a search term is entered', () => {

ee/spec/frontend/security_dashboard/store/modules/filters/getters_spec.js

 import createState from 'ee/security_dashboard/store/modules/filters/state';
 import * as getters from 'ee/security_dashboard/store/modules/filters/getters';
-import { BASE_FILTERS } from 'ee/security_dashboard/store/modules/filters/constants';
 
 describe('filters module getters', () => {
-  const mockedGetters = state => {
-    const getFilter = filterId => getters.getFilter(state)(filterId);
-    const getSelectedOptions = filterId =>
-      getters.getSelectedOptions(state, { getFilter })(filterId);
-
-    return {
-      getFilter,
-      getSelectedOptions,
-    };
-  };
   let state;
 
   beforeEach(() => {
     state = createState();
   });
 
-  describe('getFilter', () => {
-    it('should return the type filter information', () => {
-      const typeFilter = getters.getFilter(state)('report_type');
-
-      expect(typeFilter.name).toEqual('Report type');
-    });
-  });
-
-  describe('getSelectedOptions', () => {
-    describe('with one selected option', () => {
-      it('should return the base filter as the selected option', () => {
-        const selectedOptions = getters.getSelectedOptions(state, mockedGetters(state))(
-          'report_type',
-        );
-
-        expect(selectedOptions).toHaveLength(1);
-        expect(selectedOptions[0].name).toBe(BASE_FILTERS.report_type.name);
-      });
-    });
-
-    describe('with multiple selected options', () => {
-      it('should return both "High" and "Critical" ', () => {
-        state = {
-          filters: [
-            {
-              id: 'severity',
-              options: [{ id: 'critical' }, { id: 'high' }],
-              selection: new Set(['critical', 'high']),
-            },
-          ],
-        };
-        const selectedOptions = getters.getSelectedOptions(state, mockedGetters(state))('severity');
-
-        expect(selectedOptions).toHaveLength(2);
-      });
-    });
-  });
-
-  describe('getSelectedOptionNames', () => {
-    it('should return the base filter as the selected option', () => {
-      const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))(
-        'severity',
-      );
-
-      expect(selectedOptionNames.firstOption).toBe(BASE_FILTERS.severity.name);
-      expect(selectedOptionNames.extraOptionCount).toBe('');
-    });
-
-    it('should return the correct message when multiple filters are selected', () => {
-      state = {
-        filters: [
-          {
-            id: 'severity',
-            options: [{ name: 'Critical', id: 1 }, { name: 'High', id: 2 }],
-            selection: new Set([1, 2]),
-          },
-        ],
-      };
-      const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))(
-        'severity',
-      );
-
-      expect(selectedOptionNames).toEqual({ firstOption: 'Critical', extraOptionCount: '+1 more' });
-    });
-  });
-
   describe('activeFilters', () => {
     it('should return no severity filters', () => {
-      const activeFilters = getters.activeFilters(state, mockedGetters(state));
+      const activeFilters = getters.activeFilters(state);
 
       expect(activeFilters.severity).toHaveLength(0);
     });
@@ -99,7 +22,7 @@ describe('filters module getters', () => {
         selection: new Set(['one', 'two']),
       };
       state.filters.push(dummyFilter);
-      const activeFilters = getters.activeFilters(state, mockedGetters(state));
+      const activeFilters = getters.activeFilters(state);
 
       expect(activeFilters.dummy).toHaveLength(2);
     });

locale/gitlab.pot

@@ -581,10 +581,12 @@ msgstr ""
 msgid "+ %{numberOfHiddenAssignees} more"
 msgstr ""
 
-msgid "+%{approvers} more approvers"
-msgstr ""
+msgid "+%d more"
+msgid_plural "+%d more"
+msgstr[0] ""
+msgstr[1] ""
 
-msgid "+%{extraOptionCount} more"
+msgid "+%{approvers} more approvers"
 msgstr ""
 
 msgid "+%{tags} more"