Commit 64081ae6 authored by Savas Vedova's avatar Savas Vedova

Migrate vulnerability state management to GraphQL

- As of today, we're using a POST request to update the vulnerability
state. With these changes, the state change will be managed by
GraphQL.
- Add changelog
parent 18937395
import vulnerabilityConfirm from './vulnerability_confirm.mutation.graphql';
import vulnerabilityDismiss from './vulnerability_dismiss.mutation.graphql';
import vulnerabilityResolve from './vulnerability_resolve.mutation.graphql';
import vulnerabilityRevertToDetected from './vulnerability_revert_to_detected.mutation.graphql';
export default {
revert: vulnerabilityRevertToDetected,
dismiss: vulnerabilityDismiss,
confirm: vulnerabilityConfirm,
resolve: vulnerabilityResolve,
};
mutation($id: VulnerabilityID!) {
vulnerabilityConfirm(input: { id: $id }) {
errors
vulnerability {
id
state
confirmedAt
}
}
}
mutation($id: ID!, $comment: String!) { mutation($id: VulnerabilityID!, $comment: String!) {
vulnerabilityDismiss(input: { id: $id, comment: $comment }) { vulnerabilityDismiss(input: { id: $id, comment: $comment }) {
errors errors
vulnerability { vulnerability {
id id
state state
dismissedAt
} }
} }
} }
mutation($id: VulnerabilityID!) {
vulnerabilityResolve(input: { id: $id }) {
errors
vulnerability {
id
state
resolvedAt
}
}
}
mutation($id: VulnerabilityID!) {
vulnerabilityRevertToDetected(input: { id: $id }) {
errors
vulnerability {
id
state
detectedAt
}
}
}
...@@ -3,12 +3,10 @@ import { GlLoadingIcon, GlButton, GlBadge } from '@gitlab/ui'; ...@@ -3,12 +3,10 @@ import { GlLoadingIcon, GlButton, GlBadge } from '@gitlab/ui';
import Api from 'ee/api'; import Api from 'ee/api';
import { CancelToken } from 'axios'; import { CancelToken } from 'axios';
import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue'; import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue';
import vulnerabilityStateMutations from 'ee/security_dashboard/graphql/mutate_vulnerability_state';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
import download from '~/lib/utils/downloader'; import download from '~/lib/utils/downloader';
import { import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
convertObjectPropsToSnakeCase,
convertObjectPropsToCamelCase,
} from '~/lib/utils/common_utils';
import { redirectTo } from '~/lib/utils/url_utility'; import { redirectTo } from '~/lib/utils/url_utility';
import { deprecatedCreateFlash as createFlash } from '~/flash'; import { deprecatedCreateFlash as createFlash } from '~/flash';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
...@@ -18,6 +16,8 @@ import VulnerabilityStateDropdown from './vulnerability_state_dropdown.vue'; ...@@ -18,6 +16,8 @@ import VulnerabilityStateDropdown from './vulnerability_state_dropdown.vue';
import StatusDescription from './status_description.vue'; import StatusDescription from './status_description.vue';
import { VULNERABILITY_STATE_OBJECTS, FEEDBACK_TYPES, HEADER_ACTION_BUTTONS } from '../constants'; import { VULNERABILITY_STATE_OBJECTS, FEEDBACK_TYPES, HEADER_ACTION_BUTTONS } from '../constants';
const gidPrefix = 'gid://gitlab/Vulnerability/';
export default { export default {
name: 'VulnerabilityHeader', name: 'VulnerabilityHeader',
...@@ -109,7 +109,9 @@ export default { ...@@ -109,7 +109,9 @@ export default {
handler(state) { handler(state) {
const id = this.vulnerability[`${state}ById`]; const id = this.vulnerability[`${state}ById`];
if (id === undefined) return; // Don't do anything if there's no ID. if (!id) {
return;
}
this.isLoadingUser = true; this.isLoadingUser = true;
...@@ -132,25 +134,39 @@ export default { ...@@ -132,25 +134,39 @@ export default {
const fn = this[action]; const fn = this[action];
if (typeof fn === 'function') fn(); if (typeof fn === 'function') fn();
}, },
changeVulnerabilityState(newState) {
async changeVulnerabilityState({ action, payload }) {
this.isLoadingVulnerability = true; this.isLoadingVulnerability = true;
Api.changeVulnerabilityState(this.vulnerability.id, newState) try {
.then(({ data }) => { const { data } = await this.$apollo.mutate({
Object.assign(this.vulnerability, convertObjectPropsToCamelCase(data)); mutation: vulnerabilityStateMutations[action],
this.$emit('vulnerability-state-change'); variables: { id: `${gidPrefix}${this.vulnerability.id}`, ...payload },
})
.catch(() => {
createFlash(
s__(
'VulnerabilityManagement|Something went wrong, could not update vulnerability state.',
),
);
})
.finally(() => {
this.isLoadingVulnerability = false;
}); });
const [queryName] = Object.keys(data);
const { vulnerability } = data[queryName];
vulnerability.id = vulnerability.id.replace(gidPrefix, '');
vulnerability.state = vulnerability.state.toLowerCase();
this.vulnerability = {
...this.vulnerability,
...vulnerability,
};
this.$emit('vulnerability-state-change');
} catch (error) {
createFlash({
error,
captureError: true,
message: s__(
'VulnerabilityManagement|Something went wrong, could not update vulnerability state.',
),
});
} finally {
this.isLoadingVulnerability = false;
}
}, },
createMergeRequest() { createMergeRequest() {
this.isProcessingAction = true; this.isProcessingAction = true;
......
...@@ -48,7 +48,7 @@ export default { ...@@ -48,7 +48,7 @@ export default {
}, },
saveState(selectedState) { saveState(selectedState) {
this.$emit('change', selectedState.action); this.$emit('change', selectedState);
this.closeDropdown(); this.closeDropdown();
}, },
}, },
......
...@@ -4,6 +4,8 @@ import { ...@@ -4,6 +4,8 @@ import {
FEEDBACK_TYPE_MERGE_REQUEST, FEEDBACK_TYPE_MERGE_REQUEST,
} from '~/vue_shared/security_reports/constants'; } from '~/vue_shared/security_reports/constants';
const falsePositiveMessage = s__('VulnerabilityManagement|Will not fix or a false-positive');
export const VULNERABILITY_STATE_OBJECTS = { export const VULNERABILITY_STATE_OBJECTS = {
detected: { detected: {
action: 'revert', action: 'revert',
...@@ -16,7 +18,10 @@ export const VULNERABILITY_STATE_OBJECTS = { ...@@ -16,7 +18,10 @@ export const VULNERABILITY_STATE_OBJECTS = {
action: 'dismiss', action: 'dismiss',
state: 'dismissed', state: 'dismissed',
displayName: s__('Dismiss'), displayName: s__('Dismiss'),
description: s__('VulnerabilityManagement|Will not fix or a false-positive'), description: falsePositiveMessage,
payload: {
comment: falsePositiveMessage,
},
}, },
confirmed: { confirmed: {
action: 'confirm', action: 'confirm',
......
import Vue from 'vue'; import Vue from 'vue';
import App from 'ee/vulnerabilities/components/vulnerability.vue'; import App from 'ee/vulnerabilities/components/vulnerability.vue';
import apolloProvider from 'ee/security_dashboard/graphql/provider';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
export default el => { export default el => {
if (!el) { if (!el) {
return null; return null;
} }
const vulnerability = convertObjectPropsToCamelCase(JSON.parse(el.dataset.vulnerability), { const vulnerability = convertObjectPropsToCamelCase(JSON.parse(el.dataset.vulnerability), {
deep: true, deep: true,
}); });
return new Vue({ return new Vue({
el, el,
apolloProvider,
provide: { provide: {
reportType: vulnerability.reportType, reportType: vulnerability.reportType,
newIssueUrl: vulnerability.newIssueUrl, newIssueUrl: vulnerability.newIssueUrl,
...@@ -21,7 +23,6 @@ export default el => { ...@@ -21,7 +23,6 @@ export default el => {
issueTrackingHelpPath: vulnerability.issueTrackingHelpPath, issueTrackingHelpPath: vulnerability.issueTrackingHelpPath,
permissionsHelpPath: vulnerability.permissionsHelpPath, permissionsHelpPath: vulnerability.permissionsHelpPath,
}, },
render: h => render: h =>
h(App, { h(App, {
props: { vulnerability }, props: { vulnerability },
......
---
title: Migrate vulnerability state management to GraphQL
merge_request: 50034
author:
type: changed
import { GlButton, GlBadge } from '@gitlab/ui'; import { GlButton, GlBadge } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMount, createLocalVue } from '@vue/test-utils';
import VueApollo from 'vue-apollo';
import MockAdapter from 'axios-mock-adapter'; import MockAdapter from 'axios-mock-adapter';
import Api from 'ee/api'; import Api from 'ee/api';
import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue'; import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue';
...@@ -7,15 +8,20 @@ import Header from 'ee/vulnerabilities/components/header.vue'; ...@@ -7,15 +8,20 @@ import Header from 'ee/vulnerabilities/components/header.vue';
import ResolutionAlert from 'ee/vulnerabilities/components/resolution_alert.vue'; import ResolutionAlert from 'ee/vulnerabilities/components/resolution_alert.vue';
import StatusDescription from 'ee/vulnerabilities/components/status_description.vue'; import StatusDescription from 'ee/vulnerabilities/components/status_description.vue';
import VulnerabilityStateDropdown from 'ee/vulnerabilities/components/vulnerability_state_dropdown.vue'; import VulnerabilityStateDropdown from 'ee/vulnerabilities/components/vulnerability_state_dropdown.vue';
import vulnerabilityStateMutations from 'ee/security_dashboard/graphql/mutate_vulnerability_state';
import { FEEDBACK_TYPES, VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants'; import { FEEDBACK_TYPES, VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
import UsersMockHelper from 'helpers/user_mock_data_helper'; import UsersMockHelper from 'helpers/user_mock_data_helper';
import waitForPromises from 'helpers/wait_for_promises'; import waitForPromises from 'helpers/wait_for_promises';
import createMockApollo from 'jest/helpers/mock_apollo_helper';
import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToSnakeCase } from '~/lib/utils/common_utils';
import { deprecatedCreateFlash as createFlash } from '~/flash'; import { deprecatedCreateFlash as createFlash } from '~/flash';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
import download from '~/lib/utils/downloader'; import download from '~/lib/utils/downloader';
import * as urlUtility from '~/lib/utils/url_utility'; import * as urlUtility from '~/lib/utils/url_utility';
const localVue = createLocalVue();
localVue.use(VueApollo);
const vulnerabilityStateEntries = Object.entries(VULNERABILITY_STATE_OBJECTS); const vulnerabilityStateEntries = Object.entries(VULNERABILITY_STATE_OBJECTS);
const mockAxios = new MockAdapter(axios); const mockAxios = new MockAdapter(axios);
jest.mock('~/flash'); jest.mock('~/flash');
...@@ -60,6 +66,10 @@ describe('Vulnerability Header', () => { ...@@ -60,6 +66,10 @@ describe('Vulnerability Header', () => {
}; };
}; };
const createApolloProvider = (...queries) => {
return createMockApollo([...queries]);
};
const createRandomUser = () => { const createRandomUser = () => {
const user = UsersMockHelper.createRandomUser(); const user = UsersMockHelper.createRandomUser();
const url = Api.buildUrl(Api.userPath).replace(':id', user.id); const url = Api.buildUrl(Api.userPath).replace(':id', user.id);
...@@ -74,10 +84,15 @@ describe('Vulnerability Header', () => { ...@@ -74,10 +84,15 @@ describe('Vulnerability Header', () => {
const findResolutionAlert = () => wrapper.find(ResolutionAlert); const findResolutionAlert = () => wrapper.find(ResolutionAlert);
const findStatusDescription = () => wrapper.find(StatusDescription); const findStatusDescription = () => wrapper.find(StatusDescription);
const createWrapper = (vulnerability = {}) => { const createWrapper = ({ vulnerability = {}, apolloProvider }) => {
wrapper = shallowMount(Header, { wrapper = shallowMount(Header, {
localVue,
apolloProvider,
propsData: { propsData: {
initialVulnerability: { ...defaultVulnerability, ...vulnerability }, initialVulnerability: {
...defaultVulnerability,
...vulnerability,
},
}, },
stubs: { stubs: {
GlBadge, GlBadge,
...@@ -92,60 +107,73 @@ describe('Vulnerability Header', () => { ...@@ -92,60 +107,73 @@ describe('Vulnerability Header', () => {
createFlash.mockReset(); createFlash.mockReset();
}); });
describe('state dropdown', () => { describe.each`
beforeEach(() => createWrapper()); action | queryName | expected
${'dismiss'} | ${'vulnerabilityDismiss'} | ${'dismissed'}
it('the vulnerability state dropdown is rendered', () => { ${'confirm'} | ${'vulnerabilityConfirm'} | ${'confirmed'}
expect(wrapper.find(VulnerabilityStateDropdown).exists()).toBe(true); ${'resolve'} | ${'vulnerabilityResolve'} | ${'resolved'}
}); ${'revert'} | ${'vulnerabilityRevertToDetected'} | ${'detected'}
`('state dropdown change', ({ action, queryName, expected }) => {
it('when the vulnerability state dropdown emits a change event, a POST API call is made', () => { describe('when API call is successful', () => {
const dropdown = wrapper.find(VulnerabilityStateDropdown); beforeEach(() => {
mockAxios.onPost().reply(201); const apolloProvider = createApolloProvider([
vulnerabilityStateMutations[action],
dropdown.vm.$emit('change'); jest.fn().mockResolvedValue({
data: {
[queryName]: {
errors: [],
vulnerability: {
id: 'gid://gitlab/Vulnerability/54',
[`${expected}At`]: '2020-09-16T11:13:26Z',
state: expected.toUpperCase(),
},
},
},
}),
]);
return waitForPromises().then(() => { createWrapper({ apolloProvider });
expect(mockAxios.history.post).toHaveLength(1); // Check that a POST request was made.
}); });
});
it('when the vulnerability state dropdown emits a change event, the state badge updates', () => {
const newState = 'dismissed';
mockAxios.onPost().reply(201, { state: newState });
expect(findBadge().text()).not.toBe(newState);
const dropdown = wrapper.find(VulnerabilityStateDropdown);
dropdown.vm.$emit('change'); it(`updates the state properly - ${action}`, async () => {
const dropdown = wrapper.find(VulnerabilityStateDropdown);
dropdown.vm.$emit('change', { action });
return waitForPromises().then(() => { await waitForPromises();
expect(findBadge().text()).toBe(newState); expect(findBadge().text()).toBe(expected);
}); });
});
it('when the vulnerability state dropdown emits a change event, the vulnerabilities event bus event is emitted with the proper event', () => {
const newState = 'dismissed';
mockAxios.onPost().reply(201, { state: newState });
expect(findBadge().text()).not.toBe(newState);
const dropdown = wrapper.find(VulnerabilityStateDropdown);
dropdown.vm.$emit('change'); it(`emits an event when the state is changed - ${action}`, async () => {
const dropdown = wrapper.find(VulnerabilityStateDropdown);
dropdown.vm.$emit('change', { action });
return waitForPromises().then(() => { await waitForPromises();
expect(wrapper.emitted()['vulnerability-state-change']).toBeTruthy(); expect(wrapper.emitted()['vulnerability-state-change']).toBeTruthy();
}); });
}); });
it('when the vulnerability state changes but the API call fails, an error message is displayed', () => { describe('when API call is failed', () => {
const dropdown = wrapper.find(VulnerabilityStateDropdown); beforeEach(() => {
mockAxios.onPost().reply(400); const apolloProvider = createApolloProvider([
vulnerabilityStateMutations[action],
jest.fn().mockRejectedValue({
data: {
[queryName]: {
errors: [{ message: 'Something went wrong' }],
vulnerability: {},
},
},
}),
]);
dropdown.vm.$emit('change', 'dismissed'); createWrapper({ apolloProvider });
});
return waitForPromises().then(() => { it('when the vulnerability state changes but the API call fails, an error message is displayed', async () => {
expect(mockAxios.history.post).toHaveLength(1); const dropdown = wrapper.find(VulnerabilityStateDropdown);
dropdown.vm.$emit('change', { action });
await waitForPromises();
expect(createFlash).toHaveBeenCalledTimes(1); expect(createFlash).toHaveBeenCalledTimes(1);
}); });
}); });
...@@ -153,7 +181,7 @@ describe('Vulnerability Header', () => { ...@@ -153,7 +181,7 @@ describe('Vulnerability Header', () => {
describe('split button', () => { describe('split button', () => {
it('does render the create merge request and issue button as a split button', () => { it('does render the create merge request and issue button as a split button', () => {
createWrapper(getVulnerability({ shouldShowMergeRequestButton: true })); createWrapper({ vulnerability: getVulnerability({ shouldShowMergeRequestButton: true }) });
expect(findSplitButton().exists()).toBe(true); expect(findSplitButton().exists()).toBe(true);
const buttons = findSplitButton().props('buttons'); const buttons = findSplitButton().props('buttons');
expect(buttons).toHaveLength(2); expect(buttons).toHaveLength(2);
...@@ -162,30 +190,32 @@ describe('Vulnerability Header', () => { ...@@ -162,30 +190,32 @@ describe('Vulnerability Header', () => {
}); });
it('does not render the split button if there is only one action', () => { it('does not render the split button if there is only one action', () => {
createWrapper( createWrapper({
getVulnerability({ vulnerability: getVulnerability({
shouldShowMergeRequestButton: true, shouldShowMergeRequestButton: true,
shouldShowDownloadPatchButton: false, shouldShowDownloadPatchButton: false,
}), }),
); });
expect(findSplitButton().exists()).toBe(false); expect(findSplitButton().exists()).toBe(false);
}); });
}); });
describe('single action button', () => { describe('single action button', () => {
it('does not display if there are no actions', () => { it('does not display if there are no actions', () => {
createWrapper(getVulnerability({})); createWrapper({ vulnerability: getVulnerability({}) });
expect(findGlButton().exists()).toBe(false); expect(findGlButton().exists()).toBe(false);
}); });
describe('create merge request', () => { describe('create merge request', () => {
beforeEach(() => { beforeEach(() => {
createWrapper({ createWrapper({
...getVulnerability({ vulnerability: {
shouldShowMergeRequestButton: true, ...getVulnerability({
shouldShowDownloadPatchButton: false, shouldShowMergeRequestButton: true,
}), shouldShowDownloadPatchButton: false,
state: 'resolved', }),
state: 'resolved',
},
}); });
}); });
...@@ -239,8 +269,10 @@ describe('Vulnerability Header', () => { ...@@ -239,8 +269,10 @@ describe('Vulnerability Header', () => {
describe('can download patch', () => { describe('can download patch', () => {
beforeEach(() => { beforeEach(() => {
createWrapper({ createWrapper({
...getVulnerability({ shouldShowMergeRequestButton: true }), vulnerability: {
createMrUrl: '', ...getVulnerability({ shouldShowMergeRequestButton: true }),
createMrUrl: '',
},
}); });
}); });
...@@ -269,7 +301,7 @@ describe('Vulnerability Header', () => { ...@@ -269,7 +301,7 @@ describe('Vulnerability Header', () => {
it.each(Object.entries(badgeVariants))( it.each(Object.entries(badgeVariants))(
'the vulnerability state badge has the correct style for the %s state', 'the vulnerability state badge has the correct style for the %s state',
(state, variant) => { (state, variant) => {
createWrapper({ state }); createWrapper({ vulnerability: { state } });
expect(findBadge().props('variant')).toBe(variant); expect(findBadge().props('variant')).toBe(variant);
expect(findBadge().text()).toBe(state); expect(findBadge().text()).toBe(state);
...@@ -278,16 +310,22 @@ describe('Vulnerability Header', () => { ...@@ -278,16 +310,22 @@ describe('Vulnerability Header', () => {
}); });
describe('status description', () => { describe('status description', () => {
it('the status description is rendered and passed the correct data', () => { let vulnerability;
const user = createRandomUser(); let user;
const vulnerability = {
beforeEach(() => {
user = createRandomUser();
vulnerability = {
...defaultVulnerability, ...defaultVulnerability,
state: 'confirmed', state: 'confirmed',
confirmedById: user.id, confirmedById: user.id,
}; };
createWrapper(vulnerability); createWrapper({ vulnerability });
});
it('the status description is rendered and passed the correct data', () => {
return waitForPromises().then(() => { return waitForPromises().then(() => {
expect(findStatusDescription().exists()).toBe(true); expect(findStatusDescription().exists()).toBe(true);
expect(findStatusDescription().props()).toEqual({ expect(findStatusDescription().props()).toEqual({
...@@ -306,8 +344,10 @@ describe('Vulnerability Header', () => { ...@@ -306,8 +344,10 @@ describe('Vulnerability Header', () => {
beforeEach(() => { beforeEach(() => {
createWrapper({ createWrapper({
resolvedOnDefaultBranch: true, vulnerability: {
projectDefaultBranch: branchName, resolvedOnDefaultBranch: true,
projectDefaultBranch: branchName,
},
}); });
}); });
...@@ -337,7 +377,7 @@ describe('Vulnerability Header', () => { ...@@ -337,7 +377,7 @@ describe('Vulnerability Header', () => {
`loads the correct user for the vulnerability state "%s"`, `loads the correct user for the vulnerability state "%s"`,
state => { state => {
const user = createRandomUser(); const user = createRandomUser();
createWrapper({ state, [`${state}ById`]: user.id }); createWrapper({ vulnerability: { state, [`${state}ById`]: user.id } });
return waitForPromises().then(() => { return waitForPromises().then(() => {
expect(mockAxios.history.get).toHaveLength(1); expect(mockAxios.history.get).toHaveLength(1);
...@@ -347,7 +387,7 @@ describe('Vulnerability Header', () => { ...@@ -347,7 +387,7 @@ describe('Vulnerability Header', () => {
); );
it('does not load a user if there is no user ID', () => { it('does not load a user if there is no user ID', () => {
createWrapper({ state: 'detected' }); createWrapper({ vulnerability: { state: 'detected' } });
return waitForPromises().then(() => { return waitForPromises().then(() => {
expect(mockAxios.history.get).toHaveLength(0); expect(mockAxios.history.get).toHaveLength(0);
...@@ -356,7 +396,7 @@ describe('Vulnerability Header', () => { ...@@ -356,7 +396,7 @@ describe('Vulnerability Header', () => {
}); });
it('will show an error when the user cannot be loaded', () => { it('will show an error when the user cannot be loaded', () => {
createWrapper({ state: 'confirmed', confirmedById: 1 }); createWrapper({ vulnerability: { state: 'confirmed', confirmedById: 1 } });
mockAxios.onGet().replyOnce(500); mockAxios.onGet().replyOnce(500);
...@@ -368,7 +408,7 @@ describe('Vulnerability Header', () => { ...@@ -368,7 +408,7 @@ describe('Vulnerability Header', () => {
it('will set the isLoadingUser property correctly when the user is loading and finished loading', () => { it('will set the isLoadingUser property correctly when the user is loading and finished loading', () => {
const user = createRandomUser(); const user = createRandomUser();
createWrapper({ state: 'confirmed', confirmedById: user.id }); createWrapper({ vulnerability: { state: 'confirmed', confirmedById: user.id } });
expect(findStatusDescription().props('isLoadingUser')).toBe(true); expect(findStatusDescription().props('isLoadingUser')).toBe(true);
......
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