Commit 66eb8aff authored by Daniel Tian's avatar Daniel Tian Committed by Illya Klymov

Fix simultaneous requests for vulnerabilities on pipeline security tab

Fix multiple simultaneous requests for vulnerabilities on pipeline
security tab, leading to results inconsistent with selected filters
parent 53b3c8d5
---
title: Fix multiple simultaneous requests for vulnerabilities on pipeline security tab
merge_request: 48426
author:
type: fixed
......@@ -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);
}),
......
......@@ -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',
......
......@@ -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) => {
// Don't show an error message if the request was cancelled through the cancel token.
if (!axios.isCancel(error)) {
dispatch('receiveVulnerabilitiesError', error?.response?.status);
}
});
};
......
......@@ -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);
}
});
};
......@@ -32,7 +32,7 @@ describe('Filter component', () => {
wrapper = null;
});
describe('severity', () => {
describe('filters', () => {
beforeEach(() => {
createWrapper();
});
......
......@@ -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;
......
......@@ -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();
expect(state).toMatchObject({
isLoadingVulnerabilities: true,
errorLoadingVulnerabilities: false,
loadingVulnerabilitiesErrorCode: null,
});
it('should set `errorLoadingVulnerabilities` to `false`', () => {
expect(state.errorLoadingVulnerabilities).toBeFalsy();
});
it('should reset `loadingVulnerabilitiesErrorCode`', () => {
expect(state.loadingVulnerabilitiesErrorCode).toBe(null);
});
});
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment