Merge branch '287762-fix-multiple-vulnerabilities-requests' into 'master'

Fix simultaneous requests for vulnerabilities on pipeline security tab

See merge request gitlab-org/gitlab!48426

changelogs/unreleased/287762-fix-multiple-vulnerabilities-requests.yml

+---
+title: Fix multiple simultaneous requests for vulnerabilities on pipeline security tab
+merge_request: 48426
+author:
+type: fixed

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

@@ -27,8 +27,8 @@ export default {
       this.filterQuery = { ...this.filterQuery, ...query };
       this.emitFilterChange();
     },
-    // All the filters will emit @filter-changed when the page is first loaded, which will trigger
-    // this method multiple times. We'll debounce it so that it only runs once.
+    // When this component is first shown, every filter will emit its own @filter-changed event at,
+    // 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);
     }),

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

@@ -66,7 +66,6 @@ export default {
   created() {
     this.setPipelineId(this.pipelineId);
     this.setVulnerabilitiesEndpoint(this.vulnerabilitiesEndpoint);
-    this.fetchVulnerabilities({ ...this.filters, page: this.pageInfo.page });
     this.fetchPipelineJobs();
   },
   methods: {
@@ -74,7 +73,6 @@ export default {
       'closeDismissalCommentBox',
       'createIssue',
       'createMergeRequest',
-      'fetchVulnerabilities',
       'openDismissalCommentBox',
       'setPipelineId',
       'setVulnerabilitiesEndpoint',

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

@@ -12,6 +12,8 @@ import {
 } from '~/vue_shared/security_reports/constants';
 import * as types from './mutation_types';
 
+let vulnerabilitiesSource;
+
 /**
  * A lot of this file has duplicate actions in
  * ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js
@@ -38,10 +40,17 @@ export const fetchVulnerabilities = ({ state, dispatch }, params = {}) => {
     return;
   }
   dispatch('requestVulnerabilities');
+  // Cancel a pending request if there is one.
+  if (vulnerabilitiesSource) {
+    vulnerabilitiesSource.cancel();
+  }
+
+  vulnerabilitiesSource = axios.CancelToken.source();
 
   axios({
     method: 'GET',
     url: state.vulnerabilitiesEndpoint,
+    cancelToken: vulnerabilitiesSource.token,
     params,
   })
     .then((response) => {
@@ -49,7 +58,10 @@ export const fetchVulnerabilities = ({ state, dispatch }, params = {}) => {
       dispatch('receiveVulnerabilitiesSuccess', { headers, data });
     })
     .catch((error) => {
-      dispatch('receiveVulnerabilitiesError', error?.response?.status);
+      // Don't show an error message if the request was cancelled through the cancel token.
+      if (!axios.isCancel(error)) {
+        dispatch('receiveVulnerabilitiesError', error?.response?.status);
+      }
     });
 };
 

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

@@ -3,13 +3,9 @@ import { SET_FILTER, SET_HIDE_DISMISSED } from '../modules/filters/mutation_type
 const refreshTypes = [`filters/${SET_FILTER}`, `filters/${SET_HIDE_DISMISSED}`];
 
 export default (store) => {
-  const refreshVulnerabilities = (payload) => {
-    store.dispatch('vulnerabilities/fetchVulnerabilities', payload);
-  };
-
   store.subscribe(({ type }) => {
     if (refreshTypes.includes(type)) {
-      refreshVulnerabilities(store.state.filters.filters);
+      store.dispatch('vulnerabilities/fetchVulnerabilities', store.state.filters.filters);
     }
   });
 };

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

@@ -32,7 +32,7 @@ describe('Filter component', () => {
     wrapper = null;
   });
 
-  describe('severity', () => {
+  describe('filters', () => {
     beforeEach(() => {
       createWrapper();
     });

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

@@ -164,6 +164,18 @@ describe('vulnerabilities actions', () => {
         return testAction(actions.fetchVulnerabilities, {}, state);
       });
     });
+
+    describe('pending request', () => {
+      it('cancels the pending request when a new one is made', () => {
+        const dispatch = jest.fn();
+        const cancel = jest.fn();
+        jest.spyOn(axios.CancelToken, 'source').mockImplementation(() => ({ cancel }));
+        actions.fetchVulnerabilities({ state, dispatch });
+        actions.fetchVulnerabilities({ state, dispatch });
+
+        expect(cancel).toHaveBeenCalledTimes(1);
+      });
+    });
   });
 
   describe('receiveVulnerabilitiesSuccess', () => {
@@ -590,7 +602,7 @@ describe('vulnerability dismissal', () => {
         );
       });
 
-      it('should load the previous page if there is no more vulnerabiliy on the current one and page > 1', () => {
+      it('should load the previous page if there are no more vulnerabilities on the current one and page > 1', () => {
         state.vulnerabilities = [mockDataVulnerabilities[0]];
         state.pageInfo.page = 3;
 

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

@@ -53,22 +53,16 @@ describe('vulnerabilities module mutations', () => {
   });
 
   describe('REQUEST_VULNERABILITIES', () => {
-    beforeEach(() => {
+    it('should set properties to expected values', () => {
       state.errorLoadingVulnerabilities = true;
       state.loadingVulnerabilitiesErrorCode = 403;
       mutations[types.REQUEST_VULNERABILITIES](state);
-    });
-
-    it('should set `isLoadingVulnerabilities` to `true`', () => {
-      expect(state.isLoadingVulnerabilities).toBeTruthy();
-    });
 
-    it('should set `errorLoadingVulnerabilities` to `false`', () => {
-      expect(state.errorLoadingVulnerabilities).toBeFalsy();
-    });
-
-    it('should reset `loadingVulnerabilitiesErrorCode`', () => {
-      expect(state.loadingVulnerabilitiesErrorCode).toBe(null);
+      expect(state).toMatchObject({
+        isLoadingVulnerabilities: true,
+        errorLoadingVulnerabilities: false,
+        loadingVulnerabilitiesErrorCode: null,
+      });
     });
   });