Dashboard: Convert selected options into a Set

A Set is per definition unique, so it makes it a bit easier to work
with. Additionally it allows us to overwrite the complete Selection at
once without iterating over all the options.

Also add missing spec for the moderator.

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

@@ -26,6 +26,9 @@ export default {
     filter() {
       return this.getFilter(this.filterId);
     },
+    selection() {
+      return this.getFilter(this.filterId).selection;
+    },
     selectedOptionText() {
       return this.getSelectedOptionNames(this.filterId) || '-';
     },
@@ -39,6 +42,9 @@ export default {
       });
       this.$emit('change');
     },
+    isSelected(option) {
+      return this.selection.has(option.id);
+    },
   },
 };
 </script>
@@ -57,11 +63,11 @@ export default {
         @click="clickFilter(option)"
       >
         <icon
-          v-if="option.selected"
+          v-if="isSelected(option)"
           class="vertical-align-middle js-check"
           name="mobile-issue-close"
         />
-        <span class="vertical-align-middle" :class="{ 'prepend-left-20': !option.selected }">{{
+        <span class="vertical-align-middle" :class="{ 'prepend-left-20': !isSelected(option) }">{{
           option.name
         }}</span>
       </gl-dropdown-item>

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

 import Vue from 'vue';
 import GroupSecurityDashboardApp from './components/app.vue';
-import store from './store';
+import createStore from './store';
 
 export default () => {
   const el = document.getElementById('js-group-security-dashboard');
 
+  const store = createStore();
+
   return new Vue({
     el,
     store,

ee/app/assets/javascripts/security_dashboard/store/index.js

@@ -7,14 +7,17 @@ import vulnerabilities from './modules/vulnerabilities/index';
 
 Vue.use(Vuex);
 
-const store = new Vuex.Store({
-  modules: {
-    filters,
-    projects,
-    vulnerabilities,
-  },
-});
+export default () => {
+  const store = new Vuex.Store({
+    modules: {
+      filters,
+      projects,
+      vulnerabilities,
+    },
+  });
 
-configureModerator(store);
 
-export default () => store;
+  configureModerator(store);
+
+  return store;
+};

ee/app/assets/javascripts/security_dashboard/store/moderator.js

@@ -4,23 +4,21 @@ export default function configureModerator(store) {
   store.subscribe(({ type, payload }) => {
     switch (type) {
       case `projects/${projectsMutationTypes.RECEIVE_PROJECTS_SUCCESS}`:
-        return store.dispatch('filters/setFilterOptions', {
+        store.dispatch('filters/setFilterOptions', {
           filterId: 'project_id',
           options: [
             {
               name: 'All',
               id: 'all',
-              selected: true,
             },
             ...payload.projects.map(project => ({
               name: project.name,
               id: project.id.toString(),
-              selected: false,
             })),
           ],
         });
+        break;
       default:
-        return null;
     }
   });
 }

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

@@ -2,11 +2,10 @@ import { sprintf, __ } from '~/locale';
 
 export const getFilter = state => filterId => state.filters.find(filter => filter.id === filterId);
 
-export const getSelectedOptions = (state, getters) => filterId =>
-  getters.getFilter(filterId).options.filter(option => option.selected);
-
-export const getSelectedOptionIds = (state, getters) => filterId =>
-  getters.getSelectedOptions(filterId).map(option => option.id);
+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);
@@ -21,22 +20,17 @@ export const getSelectedOptionNames = (state, getters) => filterId => {
     : firstOption;
 };
 
-export const getFilterIds = state => state.filters.map(filter => filter.id);
-
 /**
  * Loops through all the filters and returns all the active ones
  * stripping out any that are set to 'all'
  * @returns Object
  * e.g. { type: ['sast'], severity: ['high', 'medium'] }
  */
-export const activeFilters = (state, getters) =>
-  getters.getFilterIds.reduce(
-    (result, filterId) => ({
-      ...result,
-      [filterId]: getters.getSelectedOptionIds(filterId).filter(option => option !== 'all'),
-    }),
-    {},
-  );
+export const activeFilters = state =>
+  state.filters.reduce((acc, filter) => {
+    acc[filter.id] = [...filter.selection].filter(option => option !== 'all');
+    return acc;
+  }, {});
 
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 // This is no longer needed after gitlab-ce#52179 is merged

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

@@ -6,39 +6,24 @@ export default {
     const activeFilter = state.filters.find(filter => filter.id === filterId);
 
     if (activeFilter) {
-      let activeOptions;
+      let selection = new Set(activeFilter.selection);
 
       if (optionId === 'all') {
-        activeOptions = activeFilter.options.map(option => ({
-          ...option,
-          selected: option.id === optionId,
-        }));
+        selection = new Set(['all']);
       } else {
-        activeOptions = activeFilter.options.map(option => {
-          if (option.id === 'all') {
-            return {
-              ...option,
-              selected: false,
-            };
-          }
-
-          if (option.id === optionId) {
-            return {
-              ...option,
-              selected: !option.selected,
-            };
-          }
-
-          return option;
-        });
+        selection.delete('all');
+        if (selection.has(optionId)) {
+          selection.delete(optionId);
+        } else {
+          selection.add(optionId);
+        }
       }
 
       // This prevents us from selecting nothing at all
-      if (!activeOptions.find(option => option.selected)) {
-        activeOptions[0].selected = true;
+      if (selection.size === 0) {
+        selection = new Set(['all']);
       }
-
-      activeFilter.options = activeOptions;
+      activeFilter.selection = selection;
     }
   },
   [types.SET_FILTER_OPTIONS](state, payload) {

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

@@ -9,13 +9,13 @@ export default () => ({
         {
           name: 'All',
           id: 'all',
-          selected: true,
         },
         ...Object.entries(SEVERITIES).map(severity => {
           const [id, name] = severity;
           return { id, name };
         }),
       ],
+      selection: new Set(['all']),
     },
     {
       name: 'Report type',
@@ -24,13 +24,13 @@ export default () => ({
         {
           name: 'All',
           id: 'all',
-          selected: true,
         },
         ...Object.entries(REPORT_TYPES).map(type => {
           const [id, name] = type;
           return { id, name };
         }),
       ],
+      selection: new Set(['all']),
     },
     {
       name: 'Project',
@@ -39,9 +39,9 @@ export default () => ({
         {
           name: 'All',
           id: 'all',
-          selected: true,
         },
       ],
+      selection: new Set(['all']),
     },
   ],
 });

ee/spec/javascripts/security_dashboard/store/filters/getters_spec.js

@@ -6,15 +6,10 @@ describe('filters module getters', () => {
     const getFilter = filterId => getters.getFilter(state)(filterId);
     const getSelectedOptions = filterId =>
       getters.getSelectedOptions(state, { getFilter })(filterId);
-    const getSelectedOptionIds = filterId =>
-      getters.getSelectedOptionIds(state, { getSelectedOptions })(filterId);
-    const getFilterIds = getters.getFilterIds(state);
 
     return {
       getFilter,
       getSelectedOptions,
-      getSelectedOptionIds,
-      getFilterIds,
     };
   };
   let state;
@@ -49,7 +44,8 @@ describe('filters module getters', () => {
           filters: [
             {
               id: 'severity',
-              options: [{ id: 'critical', selected: true }, { id: 'high', selected: true }],
+              options: [{ id: 'critical' }, { id: 'high' }],
+              selection: new Set(['critical', 'high']),
             },
           ],
         };
@@ -60,20 +56,6 @@ describe('filters module getters', () => {
     });
   });
 
-  describe('getSelectedOptionIds', () => {
-    it('should return "one" as the selcted dummy ID', () => {
-      const dummyFilter = {
-        id: 'dummy',
-        options: [{ id: 'one', selected: true }, { id: 'anotherone', selected: false }],
-      };
-      state.filters.push(dummyFilter);
-      const selectedOptionIds = getters.getSelectedOptionIds(state, mockedGetters(state))('dummy');
-
-      expect(selectedOptionIds).toHaveLength(1);
-      expect(selectedOptionIds[0]).toEqual('one');
-    });
-  });
-
   describe('getSelectedOptionNames', () => {
     it('should return "All" as the selected option', () => {
       const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))(
@@ -88,7 +70,8 @@ describe('filters module getters', () => {
         filters: [
           {
             id: 'severity',
-            options: [{ name: 'Critical', selected: true }, { name: 'High', selected: true }],
+            options: [{ name: 'Critical', id: 1 }, { name: 'High', id: 2 }],
+            selection: new Set([1, 2]),
           },
         ],
       };
@@ -110,7 +93,8 @@ describe('filters module getters', () => {
     it('should return multiple dummy filters"', () => {
       const dummyFilter = {
         id: 'dummy',
-        options: [{ id: 'one', selected: true }, { id: 'anotherone', selected: true }],
+        options: [{ id: 'one' }, { id: 'two' }],
+        selection: new Set(['one', 'two']),
       };
       state.filters.push(dummyFilter);
       const activeFilters = getters.activeFilters(state, mockedGetters(state));

ee/spec/javascripts/security_dashboard/store/filters/mutations_spec.js

@@ -3,17 +3,19 @@ import * as types from 'ee/security_dashboard/store/modules/filters/mutation_typ
 import mutations from 'ee/security_dashboard/store/modules/filters/mutations';
 
 describe('filters module mutations', () => {
-  describe('SET_FILTER', () => {
-    let state;
-    let severityFilter;
-    let criticalOption;
-    let highOption;
+  let state;
+  let severityFilter;
+  let criticalOption;
+  let highOption;
+
+  beforeEach(() => {
+    state = createState();
+    [severityFilter] = state.filters;
+    [, criticalOption, highOption] = severityFilter.options;
+  });
 
+  describe('SET_FILTER', () => {
     beforeEach(() => {
-      state = createState();
-      [severityFilter] = state.filters;
-      [, criticalOption, highOption] = severityFilter.options;
-
       mutations[types.SET_FILTER](state, {
         filterId: severityFilter.id,
         optionId: criticalOption.id,
@@ -21,11 +23,16 @@ describe('filters module mutations', () => {
     });
 
     it('should make critical the selected option', () => {
-      expect(state.filters[0].options[1].selected).toEqual(true);
+      expect(state.filters[0].selection).toEqual(new Set(['critical']));
     });
 
-    it('should remove ALL as the selected option', () => {
-      expect(state.filters[0].options[0].selected).toEqual(false);
+    it('should set to `all` if no option is selected', () => {
+      mutations[types.SET_FILTER](state, {
+        filterId: severityFilter.id,
+        optionId: criticalOption.id,
+      });
+
+      expect(state.filters[0].selection).toEqual(new Set(['all']));
     });
 
     describe('on subsequent changes', () => {
@@ -35,27 +42,22 @@ describe('filters module mutations', () => {
           optionId: highOption.id,
         });
 
-        expect(state.filters[0].options[1].selected).toEqual(true);
-        expect(state.filters[0].options[2].selected).toEqual(true);
+        expect(state.filters[0].selection).toEqual(new Set(['high', 'critical']));
       });
     });
   });
 
   describe('SET_FILTER_OPTIONS', () => {
-    let state;
-    let firstFilter;
     const options = [{ id: 0, name: 'c' }, { id: 3, name: 'c' }];
 
     beforeEach(() => {
-      state = createState();
-      [firstFilter] = state.filters;
-      const filterId = firstFilter.id;
+      const filterId = severityFilter.id;
 
       mutations[types.SET_FILTER_OPTIONS](state, { filterId, options });
     });
 
     it('should add all the options to the type filter', () => {
-      expect(firstFilter.options).toEqual(options);
+      expect(severityFilter.options).toEqual(options);
     });
   });
 });

ee/spec/javascripts/security_dashboard/store/moderator_spec.js

+import createStore from 'ee/security_dashboard/store/index';
+import * as projectsMutationTypes from 'ee/security_dashboard/store/modules/projects/mutation_types';
+
+describe('moderator', () => {
+  let store;
+
+  beforeEach(() => {
+    store = createStore();
+  });
+
+  it('sets project filter options after projects have been received', () => {
+    spyOn(store, 'dispatch').and.returnValue();
+
+    store.commit(`projects/${projectsMutationTypes.RECEIVE_PROJECTS_SUCCESS}`, {
+      projects: [{ name: 'foo', id: 1, otherProp: 'foobar' }],
+    });
+
+    expect(store.dispatch).toHaveBeenCalledWith(
+      'filters/setFilterOptions',
+      Object({
+        filterId: 'project_id',
+        options: [{ name: 'All', id: 'all' }, { name: 'foo', id: '1' }],
+      }),
+    );
+  });
+});