Merge branch '210327-fix-followup-issues' into 'master'

Improve pipeline security tab filter logic

See merge request gitlab-org/gitlab!47721

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

@@ -15,12 +15,17 @@ export default {
   }),
   computed: {
     ...mapState('filters', ['filters']),
-    hideDismissed() {
-      return this.filters.scope === DISMISSAL_STATES.DISMISSED;
+    hideDismissed: {
+      set(isHidden) {
+        this.setHideDismissed(isHidden);
+      },
+      get() {
+        return this.filters.scope === DISMISSAL_STATES.DISMISSED;
+      },
     },
   },
   methods: {
-    ...mapActions('filters', ['setFilter', 'toggleHideDismissed']),
+    ...mapActions('filters', ['setFilter', 'setHideDismissed']),
   },
 };
 </script>
@@ -39,11 +44,7 @@ export default {
         <slot name="buttons"></slot>
         <div class="pl-md-6">
           <strong>{{ s__('SecurityReports|Hide dismissed') }}</strong>
-          <gl-toggle
-            class="d-block mt-1 js-toggle"
-            :value="hideDismissed"
-            @change="toggleHideDismissed"
-          />
+          <gl-toggle v-model="hideDismissed" class="gl-mt-2 js-toggle" />
         </div>
       </div>
     </div>

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

@@ -83,9 +83,13 @@ export default {
       this.updateRouteQuery();
     },
     updateRouteQuery() {
+      if (!this.$router) {
+        return;
+      }
+
       const query = { query: { ...this.$route?.query, ...this.queryObject } };
       // To avoid a console error, don't update the querystring if it's the same as the current one.
-      if (this.$router && !isEqual(this.routeQueryIds, this.queryObject[this.filter.id])) {
+      if (!isEqual(this.routeQueryIds, this.queryObject[this.filter.id])) {
         this.$router.push(query);
       }
     },

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

@@ -31,7 +31,7 @@ export const severityFilter = {
 };
 
 export const scannerFilter = {
-  name: s__('Reports|Scanner'),
+  name: s__('SecurityReports|Scanner'),
   id: 'reportType',
   options: parseOptions(REPORT_TYPES),
   allOption: BASE_FILTERS.report_type,

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

-import * as types from './mutation_types';
+import { mapValues } from 'lodash';
+import Tracking from '~/tracking';
+import { SET_FILTER, SET_HIDE_DISMISSED } from './mutation_types';
+import { DISMISSAL_STATES } from './constants';
+import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
 
 export const setFilter = ({ commit }, filter) => {
-  commit(types.SET_FILTER, filter);
+  // Convert the filter key to snake case and the selected option IDs to lower case. The API
+  // endpoint needs them to be in this format.
+  const convertedFilter = mapValues(convertObjectPropsToSnakeCase(filter), array =>
+    array.map(element => element.toLowerCase()),
+  );
+
+  commit(SET_FILTER, convertedFilter);
+
+  const [label, value] = Object.values(filter);
+  Tracking.event(document.body.dataset.page, 'set_filter', { label, value });
 };
 
-export const toggleHideDismissed = ({ commit }) => {
-  commit(types.TOGGLE_HIDE_DISMISSED);
+export const setHideDismissed = ({ commit }, isHidden) => {
+  const value = isHidden ? DISMISSAL_STATES.DISMISSED : DISMISSAL_STATES.ALL;
+  commit(SET_HIDE_DISMISSED, value);
+
+  Tracking.event(document.body.dataset.page, 'set_toggle', { label: 'scope', value });
 };

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

 export const SET_FILTER = 'SET_FILTER';
-export const TOGGLE_HIDE_DISMISSED = 'TOGGLE_HIDE_DISMISSED';
+export const SET_HIDE_DISMISSED = 'SET_HIDE_DISMISSED';

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

-import { mapValues } from 'lodash';
-import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
-import * as types from './mutation_types';
-import { DISMISSAL_STATES } from './constants';
-import Tracking from '~/tracking';
+import { SET_FILTER, SET_HIDE_DISMISSED } from './mutation_types';
 
 export default {
-  [types.SET_FILTER](state, filter) {
-    // Convert the filter key to snake case and the selected option IDs to lower case. The API
-    // endpoint needs them to be in this format.
-    const convertedFilter = mapValues(convertObjectPropsToSnakeCase(filter), array =>
-      array.map(element => element.toLowerCase()),
-    );
-
-    state.filters = { ...state.filters, ...convertedFilter };
-
-    const [label, value] = Object.values(filter);
-    Tracking.event(document.body.dataset.page, 'set_filter', { label, value });
+  [SET_FILTER](state, filter) {
+    state.filters = { ...state.filters, ...filter };
   },
-  [types.TOGGLE_HIDE_DISMISSED](state) {
-    const scope =
-      state.filters.scope === DISMISSAL_STATES.DISMISSED
-        ? DISMISSAL_STATES.ALL
-        : DISMISSAL_STATES.DISMISSED;
-
+  [SET_HIDE_DISMISSED](state, scope) {
     state.filters = { ...state.filters, scope };
-
-    Tracking.event(document.body.dataset.page, 'set_toggle', { label: 'scope', value: scope });
   },
 };

ee/app/assets/javascripts/security_dashboard/store/plugins/mediator.js

-import { SET_FILTER, TOGGLE_HIDE_DISMISSED } from '../modules/filters/mutation_types';
+import { SET_FILTER, SET_HIDE_DISMISSED } from '../modules/filters/mutation_types';
 
-const refreshTypes = [`filters/${SET_FILTER}`, `filters/${TOGGLE_HIDE_DISMISSED}`];
+const refreshTypes = [`filters/${SET_FILTER}`, `filters/${SET_HIDE_DISMISSED}`];
 
 export default store => {
   const refreshVulnerabilities = payload => {

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

 import * as actions from 'ee/security_dashboard/store/modules/filters/actions';
 import * as types from 'ee/security_dashboard/store/modules/filters/mutation_types';
-import createState from 'ee/security_dashboard/store/modules/filters/state';
+import { DISMISSAL_STATES } from 'ee/security_dashboard/store/modules/filters/constants';
 import testAction from 'helpers/vuex_action_helper';
 import Tracking from '~/tracking';
 
@@ -14,28 +14,41 @@ describe('filters actions', () => {
   });
 
   describe('setFilter', () => {
-    it('should commit the SET_FILTER mutuation', () => {
-      const state = createState();
-      const payload = { reportType: ['sast'] };
+    it('should commit the SET_FILTER mutation with the right casing', () => {
+      const payload = {
+        oneWord: ['ABC', 'DEF'],
+        twoWords: ['123', '456'],
+        threeTotalWords: ['Abc123', 'dEF456'],
+      };
 
-      return testAction(actions.setFilter, payload, state, [
+      return testAction(actions.setFilter, payload, undefined, [
         {
           type: types.SET_FILTER,
-          payload,
+          payload: {
+            one_word: ['abc', 'def'],
+            two_words: ['123', '456'],
+            three_total_words: ['abc123', 'def456'],
+          },
         },
       ]);
     });
   });
 
-  describe('toggleHideDismissed', () => {
-    it('should commit the TOGGLE_HIDE_DISMISSED mutation', () => {
-      const state = createState();
-
-      return testAction(actions.toggleHideDismissed, undefined, state, [
-        {
-          type: types.TOGGLE_HIDE_DISMISSED,
-        },
-      ]);
-    });
+  describe('setHideDismissed', () => {
+    it.each`
+      isHidden | expected
+      ${true}  | ${DISMISSAL_STATES.DISMISSED}
+      ${false} | ${DISMISSAL_STATES.ALL}
+    `(
+      'should commit the SET_HIDE_DISMISSED mutation with "$expected" when called with $isHidden',
+      ({ isHidden, expected }) => {
+        return testAction(actions.setHideDismissed, isHidden, undefined, [
+          {
+            type: types.SET_HIDE_DISMISSED,
+            payload: expected,
+          },
+        ]);
+      },
+    );
   });
 });

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

 import { severityFilter } from 'ee/security_dashboard/helpers';
 import {
   SET_FILTER,
-  TOGGLE_HIDE_DISMISSED,
+  SET_HIDE_DISMISSED,
 } from 'ee/security_dashboard/store/modules/filters/mutation_types';
 import mutations from 'ee/security_dashboard/store/modules/filters/mutations';
 import createState from 'ee/security_dashboard/store/modules/filters/state';
+import { DISMISSAL_STATES } from 'ee/security_dashboard/store/modules/filters/constants';
 
 const criticalOption = severityFilter.options.find(x => x.id === 'CRITICAL');
 const highOption = severityFilter.options.find(x => x.id === 'HIGH');
 
+const existingFilters = {
+  filter1: ['some', 'value'],
+  filter2: ['other', 'values'],
+};
+
 describe('filters module mutations', () => {
   let state;
 
@@ -18,43 +24,36 @@ describe('filters module mutations', () => {
 
   describe('SET_FILTER', () => {
     it.each`
-      options                               | expected
-      ${[]}                                 | ${[]}
-      ${[criticalOption.id]}                | ${[criticalOption.id.toLowerCase()]}
-      ${[criticalOption.id, highOption.id]} | ${[criticalOption.id.toLowerCase(), highOption.id.toLowerCase()]}
-    `('sets the filter to $options', ({ options, expected }) => {
+      options
+      ${[]}
+      ${[criticalOption.id]}
+      ${[criticalOption.id, highOption.id]}
+    `('sets the filter to $options', ({ options }) => {
       mutations[SET_FILTER](state, { [severityFilter.id]: options });
 
-      expect(state.filters[severityFilter.id]).toEqual(expected);
+      expect(state.filters[severityFilter.id]).toEqual(options);
     });
 
-    it('sets multiple filters correctly with the right casing', () => {
-      const filter1 = { oneWord: ['ABC', 'DEF'] };
-      const filter2 = { twoWords: ['123', '456'] };
-      const filter3 = { threeTotalWords: ['Abc123', 'dEF456'] };
-
-      mutations[SET_FILTER](state, filter1);
-      mutations[SET_FILTER](state, filter2);
-      mutations[SET_FILTER](state, filter3);
+    it('adds new filter to existing filters', () => {
+      const newFilter = { filter3: ['custom', 'filters'] };
+      state.filters = existingFilters;
+      mutations[SET_FILTER](state, newFilter);
 
-      expect(state.filters).toMatchObject({
-        one_word: ['abc', 'def'],
-        two_words: ['123', '456'],
-        three_total_words: ['abc123', 'def456'],
-      });
+      expect(state.filters).toEqual({ ...existingFilters, ...newFilter });
     });
   });
 
-  describe('TOGGLE_HIDE_DISMISSED', () => {
-    it('toggles scope filter', () => {
-      const toggleAndCheck = expected => {
-        mutations[TOGGLE_HIDE_DISMISSED](state);
-        expect(state.filters.scope).toBe(expected);
-      };
+  describe('SET_HIDE_DISMISSED', () => {
+    it.each(Object.values(DISMISSAL_STATES))(`sets scope filter to "%s"`, value => {
+      mutations[SET_HIDE_DISMISSED](state, value);
+      expect(state.filters.scope).toBe(value);
+    });
+
+    it('adds scope filter to existing filters', () => {
+      state.filters = existingFilters;
+      mutations[SET_HIDE_DISMISSED](state, 'dismissed');
 
-      toggleAndCheck('all');
-      toggleAndCheck('dismissed');
-      toggleAndCheck('all');
+      expect(state.filters).toEqual({ ...existingFilters, scope: 'dismissed' });
     });
   });
 });

ee/spec/frontend/security_dashboard/store/plugins/mediator_spec.js

 import createStore from 'ee/security_dashboard/store/index';
 import {
   SET_FILTER,
-  TOGGLE_HIDE_DISMISSED,
+  SET_HIDE_DISMISSED,
 } from 'ee/security_dashboard/store/modules/filters/mutation_types';
 
 function expectRefreshDispatches(store, payload) {
@@ -38,7 +38,7 @@ describe('mediator', () => {
   });
 
   it('triggers fetching vulnerabilities after "Hide dismissed" toggle changes', () => {
-    store.commit(`filters/${TOGGLE_HIDE_DISMISSED}`);
+    store.commit(`filters/${SET_HIDE_DISMISSED}`);
 
     expectRefreshDispatches(store, store.state.filters.filters);
   });

locale/gitlab.pot

@@ -24252,6 +24252,9 @@ msgstr ""
 msgid "SecurityReports|Scan details"
 msgstr ""
 
+msgid "SecurityReports|Scanner"
+msgstr ""
+
 msgid "SecurityReports|Security Dashboard"
 msgstr ""