Commit 45625d54 authored by Martin Wortschack's avatar Martin Wortschack

Merge branch '216300-allow-for-mr-creation' into 'master'

Add split button for new vulnerability actions

See merge request gitlab-org/gitlab!31620
parents 1aba995b a134363a
---
title: Add ability to create merge request from vulnerability page
merge_request: 31620
author:
type: added
...@@ -8,7 +8,7 @@ function createHeaderApp() { ...@@ -8,7 +8,7 @@ function createHeaderApp() {
const pipeline = JSON.parse(el.dataset.pipelineJson); const pipeline = JSON.parse(el.dataset.pipelineJson);
const finding = JSON.parse(el.dataset.findingJson); const finding = JSON.parse(el.dataset.findingJson);
const { projectFingerprint, createIssueUrl } = el.dataset; const { projectFingerprint, createIssueUrl, createMrUrl } = el.dataset;
return new Vue({ return new Vue({
el, el,
...@@ -16,6 +16,7 @@ function createHeaderApp() { ...@@ -16,6 +16,7 @@ function createHeaderApp() {
render: h => render: h =>
h(HeaderApp, { h(HeaderApp, {
props: { props: {
createMrUrl,
initialVulnerability, initialVulnerability,
finding, finding,
pipeline, pipeline,
......
...@@ -9,8 +9,9 @@ import UsersCache from '~/lib/utils/users_cache'; ...@@ -9,8 +9,9 @@ import UsersCache from '~/lib/utils/users_cache';
import ResolutionAlert from './resolution_alert.vue'; import ResolutionAlert from './resolution_alert.vue';
import VulnerabilityStateDropdown from './vulnerability_state_dropdown.vue'; import VulnerabilityStateDropdown from './vulnerability_state_dropdown.vue';
import StatusDescription from './status_description.vue'; import StatusDescription from './status_description.vue';
import { VULNERABILITY_STATE_OBJECTS, HEADER_ACTION_BUTTONS } from '../constants'; import { VULNERABILITY_STATE_OBJECTS, FEEDBACK_TYPES, HEADER_ACTION_BUTTONS } from '../constants';
import VulnerabilitiesEventBus from './vulnerabilities_event_bus'; import VulnerabilitiesEventBus from './vulnerabilities_event_bus';
import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue';
export default { export default {
name: 'VulnerabilityHeader', name: 'VulnerabilityHeader',
...@@ -19,10 +20,15 @@ export default { ...@@ -19,10 +20,15 @@ export default {
GlLoadingIcon, GlLoadingIcon,
ResolutionAlert, ResolutionAlert,
VulnerabilityStateDropdown, VulnerabilityStateDropdown,
SplitButton,
StatusDescription, StatusDescription,
}, },
props: { props: {
createMrUrl: {
type: String,
required: true,
},
initialVulnerability: { initialVulnerability: {
type: Object, type: Object,
required: true, required: true,
...@@ -59,6 +65,10 @@ export default { ...@@ -59,6 +65,10 @@ export default {
actionButtons() { actionButtons() {
const buttons = []; const buttons = [];
if (this.canCreateMergeRequest) {
buttons.push(HEADER_ACTION_BUTTONS.mergeRequestCreation);
}
if (!this.hasIssue) { if (!this.hasIssue) {
buttons.push(HEADER_ACTION_BUTTONS.issueCreation); buttons.push(HEADER_ACTION_BUTTONS.issueCreation);
} }
...@@ -68,6 +78,17 @@ export default { ...@@ -68,6 +78,17 @@ export default {
hasIssue() { hasIssue() {
return Boolean(this.finding.issue_feedback?.issue_iid); return Boolean(this.finding.issue_feedback?.issue_iid);
}, },
hasRemediation() {
const { remediations } = this.finding;
return Boolean(remediations && remediations[0]?.diff?.length > 0);
},
canCreateMergeRequest() {
return (
!this.finding.merge_request_feedback?.merge_request_path &&
Boolean(this.createMrUrl) &&
this.hasRemediation
);
},
statusBoxStyle() { statusBoxStyle() {
// Get the badge variant based on the vulnerability state, defaulting to 'expired'. // Get the badge variant based on the vulnerability state, defaulting to 'expired'.
return VULNERABILITY_STATE_OBJECTS[this.vulnerability.state]?.statusBoxStyle || 'expired'; return VULNERABILITY_STATE_OBJECTS[this.vulnerability.state]?.statusBoxStyle || 'expired';
...@@ -132,7 +153,7 @@ export default { ...@@ -132,7 +153,7 @@ export default {
axios axios
.post(this.createIssueUrl, { .post(this.createIssueUrl, {
vulnerability_feedback: { vulnerability_feedback: {
feedback_type: 'issue', feedback_type: FEEDBACK_TYPES.ISSUE,
category: this.vulnerability.report_type, category: this.vulnerability.report_type,
project_fingerprint: this.projectFingerprint, project_fingerprint: this.projectFingerprint,
vulnerability_data: { vulnerability_data: {
...@@ -153,6 +174,32 @@ export default { ...@@ -153,6 +174,32 @@ export default {
); );
}); });
}, },
createMergeRequest() {
this.isProcessingAction = true;
axios
.post(this.createMrUrl, {
vulnerability_feedback: {
feedback_type: FEEDBACK_TYPES.MERGE_REQUEST,
category: this.vulnerability.report_type,
project_fingerprint: this.projectFingerprint,
vulnerability_data: {
...this.vulnerability,
...this.finding,
category: this.vulnerability.report_type,
target_branch: this.pipeline.sourceBranch,
},
},
})
.then(({ data: { merge_request_path } }) => {
redirectTo(merge_request_path);
})
.catch(() => {
this.isProcessingAction = false;
createFlash(
s__('ciReport|There was an error creating the merge request. Please try again.'),
);
});
},
}, },
}; };
</script> </script>
...@@ -194,8 +241,16 @@ export default { ...@@ -194,8 +241,16 @@ export default {
:initial-state="vulnerability.state" :initial-state="vulnerability.state"
@change="changeVulnerabilityState" @change="changeVulnerabilityState"
/> />
<split-button
v-if="actionButtons.length > 1"
:buttons="actionButtons"
:disabled="isProcessingAction"
class="js-split-button"
@createMergeRequest="createMergeRequest"
@createIssue="createIssue"
/>
<gl-deprecated-button <gl-deprecated-button
v-if="actionButtons.length > 0" v-else-if="actionButtons.length > 0"
class="ml-2" class="ml-2"
variant="success" variant="success"
category="secondary" category="secondary"
......
...@@ -33,6 +33,17 @@ export const VULNERABILITIES_PER_PAGE = 20; ...@@ -33,6 +33,17 @@ export const VULNERABILITIES_PER_PAGE = 20;
export const HEADER_ACTION_BUTTONS = { export const HEADER_ACTION_BUTTONS = {
issueCreation: { issueCreation: {
name: s__('ciReport|Create issue'), name: s__('ciReport|Create issue'),
tagline: s__('ciReport|Investigate this vulnerability by creating an issue'),
action: 'createIssue', action: 'createIssue',
}, },
mergeRequestCreation: {
name: s__('ciReport|Resolve with merge request'),
tagline: s__('ciReport|Automatically apply the patch in a new branch'),
action: 'createMergeRequest',
},
};
export const FEEDBACK_TYPES = {
ISSUE: 'issue',
MERGE_REQUEST: 'merge_request',
}; };
...@@ -10,9 +10,10 @@ import createFlash from '~/flash'; ...@@ -10,9 +10,10 @@ import createFlash from '~/flash';
import Header from 'ee/vulnerabilities/components/header.vue'; import Header from 'ee/vulnerabilities/components/header.vue';
import StatusDescription from 'ee/vulnerabilities/components/status_description.vue'; import StatusDescription from 'ee/vulnerabilities/components/status_description.vue';
import ResolutionAlert from 'ee/vulnerabilities/components/resolution_alert.vue'; import ResolutionAlert from 'ee/vulnerabilities/components/resolution_alert.vue';
import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue';
import VulnerabilityStateDropdown from 'ee/vulnerabilities/components/vulnerability_state_dropdown.vue'; import VulnerabilityStateDropdown from 'ee/vulnerabilities/components/vulnerability_state_dropdown.vue';
import VulnerabilitiesEventBus from 'ee/vulnerabilities/components/vulnerabilities_event_bus'; import VulnerabilitiesEventBus from 'ee/vulnerabilities/components/vulnerabilities_event_bus';
import { VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants'; import { FEEDBACK_TYPES, VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
const vulnerabilityStateEntries = Object.entries(VULNERABILITY_STATE_OBJECTS); const vulnerabilityStateEntries = Object.entries(VULNERABILITY_STATE_OBJECTS);
const mockAxios = new MockAdapter(axios); const mockAxios = new MockAdapter(axios);
...@@ -28,32 +29,35 @@ describe('Vulnerability Header', () => { ...@@ -28,32 +29,35 @@ describe('Vulnerability Header', () => {
state: 'detected', state: 'detected',
}; };
const findingWithIssue = { const diff = 'some diff to download';
const getFinding = ({
shouldShowCreateIssueButton = false,
shouldShowMergeRequestButton = false,
}) => {
return {
description: 'description', description: 'description',
identifiers: 'identifiers', identifiers: 'identifiers',
links: 'links', links: 'links',
location: 'location', location: 'location',
name: 'name', name: 'name',
issue_feedback: { issue_feedback: shouldShowCreateIssueButton ? null : { issue_iid: 12 },
issue_iid: 12, remediations: shouldShowMergeRequestButton ? [{ diff }] : null,
merge_request_feedback: {
merge_request_path: shouldShowMergeRequestButton ? null : 'some path',
}, },
}; };
const findingWithoutIssue = {
description: 'description',
identifiers: 'identifiers',
links: 'links',
location: 'location',
name: 'name',
}; };
const dataset = { const dataset = {
createIssueUrl: 'create_issue_url', createMrUrl: '/create_mr_url',
createIssueUrl: '/create_issue_url',
projectFingerprint: 'abc123', projectFingerprint: 'abc123',
pipeline: { pipeline: {
id: 2, id: 2,
created_at: new Date().toISOString(), created_at: new Date().toISOString(),
url: 'pipeline_url', url: 'pipeline_url',
sourceBranch: 'master',
}, },
}; };
...@@ -66,11 +70,12 @@ describe('Vulnerability Header', () => { ...@@ -66,11 +70,12 @@ describe('Vulnerability Header', () => {
}; };
const findGlDeprecatedButton = () => wrapper.find(GlDeprecatedButton); const findGlDeprecatedButton = () => wrapper.find(GlDeprecatedButton);
const findSplitButton = () => wrapper.find(SplitButton);
const findBadge = () => wrapper.find({ ref: 'badge' }); const findBadge = () => wrapper.find({ ref: 'badge' });
const findResolutionAlert = () => wrapper.find(ResolutionAlert); const findResolutionAlert = () => wrapper.find(ResolutionAlert);
const findStatusDescription = () => wrapper.find(StatusDescription); const findStatusDescription = () => wrapper.find(StatusDescription);
const createWrapper = (vulnerability = {}, finding = findingWithoutIssue) => { const createWrapper = ({ vulnerability = {}, finding = getFinding({}) }) => {
wrapper = shallowMount(Header, { wrapper = shallowMount(Header, {
propsData: { propsData: {
...dataset, ...dataset,
...@@ -88,7 +93,7 @@ describe('Vulnerability Header', () => { ...@@ -88,7 +93,7 @@ describe('Vulnerability Header', () => {
}); });
describe('state dropdown', () => { describe('state dropdown', () => {
beforeEach(createWrapper); beforeEach(() => createWrapper({}));
it('the vulnerability state dropdown is rendered', () => { it('the vulnerability state dropdown is rendered', () => {
expect(wrapper.find(VulnerabilityStateDropdown).exists()).toBe(true); expect(wrapper.find(VulnerabilityStateDropdown).exists()).toBe(true);
...@@ -148,14 +153,37 @@ describe('Vulnerability Header', () => { ...@@ -148,14 +153,37 @@ describe('Vulnerability Header', () => {
}); });
}); });
describe('split button', () => {
it('does render the create merge request and issue button as a split button', () => {
createWrapper({
finding: getFinding({
shouldShowCreateIssueButton: true,
shouldShowMergeRequestButton: true,
}),
});
expect(findSplitButton().exists()).toBe(true);
const buttons = findSplitButton().props('buttons');
expect(buttons).toHaveLength(2);
expect(buttons[0].name).toBe('Resolve with merge request');
expect(buttons[1].name).toBe('Create issue');
});
it('does not render the split button if there is only one action', () => {
createWrapper({ finding: getFinding({ shouldShowCreateIssueButton: true }) });
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({}, findingWithIssue); createWrapper({});
expect(findGlDeprecatedButton().exists()).toBe(false); expect(findGlDeprecatedButton().exists()).toBe(false);
}); });
describe('create issue', () => { describe('create issue', () => {
beforeEach(createWrapper); beforeEach(() =>
createWrapper({ finding: getFinding({ shouldShowCreateIssueButton: true }) }),
);
it('does display if there is only one action and not an issue already created', () => { it('does display if there is only one action and not an issue already created', () => {
expect(findGlDeprecatedButton().exists()).toBe(true); expect(findGlDeprecatedButton().exists()).toBe(true);
...@@ -175,12 +203,12 @@ describe('Vulnerability Header', () => { ...@@ -175,12 +203,12 @@ describe('Vulnerability Header', () => {
expect(postRequest.url).toBe(dataset.createIssueUrl); expect(postRequest.url).toBe(dataset.createIssueUrl);
expect(JSON.parse(postRequest.data)).toMatchObject({ expect(JSON.parse(postRequest.data)).toMatchObject({
vulnerability_feedback: { vulnerability_feedback: {
feedback_type: 'issue', feedback_type: FEEDBACK_TYPES.ISSUE,
category: defaultVulnerability.report_type, category: defaultVulnerability.report_type,
project_fingerprint: dataset.projectFingerprint, project_fingerprint: dataset.projectFingerprint,
vulnerability_data: { vulnerability_data: {
...defaultVulnerability, ...defaultVulnerability,
...findingWithoutIssue, ...getFinding({ shouldShowCreateIssueButton: true }),
category: defaultVulnerability.report_type, category: defaultVulnerability.report_type,
vulnerability_id: defaultVulnerability.id, vulnerability_id: defaultVulnerability.id,
}, },
...@@ -201,13 +229,66 @@ describe('Vulnerability Header', () => { ...@@ -201,13 +229,66 @@ describe('Vulnerability Header', () => {
}); });
}); });
}); });
describe('create merge request', () => {
beforeEach(() => {
createWrapper({
vulnerability: { state: 'resolved' },
finding: getFinding({ shouldShowMergeRequestButton: true }),
});
});
it('only renders the create merge request button', () => {
expect(findGlDeprecatedButton().exists()).toBe(true);
expect(findGlDeprecatedButton().text()).toBe('Resolve with merge request');
});
it('emits createMergeRequest when create merge request button is clicked', () => {
const mergeRequestPath = '/group/project/merge_request/123';
const spy = jest.spyOn(urlUtility, 'redirectTo');
mockAxios.onPost(dataset.createMRUrl).reply(200, {
merge_request_path: mergeRequestPath,
});
findGlDeprecatedButton().vm.$emit('click');
return waitForPromises().then(() => {
expect(mockAxios.history.post).toHaveLength(1);
const [postRequest] = mockAxios.history.post;
expect(postRequest.url).toBe(dataset.createMrUrl);
expect(JSON.parse(postRequest.data)).toMatchObject({
vulnerability_feedback: {
feedback_type: FEEDBACK_TYPES.MERGE_REQUEST,
category: defaultVulnerability.report_type,
project_fingerprint: dataset.projectFingerprint,
vulnerability_data: {
...defaultVulnerability,
...getFinding({ shouldShowMergeRequestButton: true }),
category: defaultVulnerability.report_type,
state: 'resolved',
},
},
});
expect(spy).toHaveBeenCalledWith(mergeRequestPath);
});
});
it('shows an error message when merge request creation fails', () => {
mockAxios.onPost(dataset.createMRUrl).reply(500);
findGlDeprecatedButton().vm.$emit('click');
return waitForPromises().then(() => {
expect(mockAxios.history.post).toHaveLength(1);
expect(createFlash).toHaveBeenCalledWith(
'There was an error creating the merge request. Please try again.',
);
});
});
});
}); });
describe('state badge', () => { describe('state badge', () => {
test.each(vulnerabilityStateEntries)( test.each(vulnerabilityStateEntries)(
'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, stateObject) => { (state, stateObject) => {
createWrapper({ state }); createWrapper({ vulnerability: { state } });
expect(findBadge().classes()).toContain(`status-box-${stateObject.statusBoxStyle}`); expect(findBadge().classes()).toContain(`status-box-${stateObject.statusBoxStyle}`);
expect(findBadge().text()).toBe(state); expect(findBadge().text()).toBe(state);
...@@ -223,7 +304,7 @@ describe('Vulnerability Header', () => { ...@@ -223,7 +304,7 @@ describe('Vulnerability Header', () => {
...{ state: 'confirmed', confirmed_by_id: user.id }, ...{ state: 'confirmed', confirmed_by_id: user.id },
}; };
createWrapper(vulnerability); createWrapper({ vulnerability });
return waitForPromises().then(() => { return waitForPromises().then(() => {
expect(findStatusDescription().exists()).toBe(true); expect(findStatusDescription().exists()).toBe(true);
...@@ -243,8 +324,10 @@ describe('Vulnerability Header', () => { ...@@ -243,8 +324,10 @@ describe('Vulnerability Header', () => {
beforeEach(() => { beforeEach(() => {
createWrapper({ createWrapper({
vulnerability: {
resolved_on_default_branch: true, resolved_on_default_branch: true,
project_default_branch: branchName, project_default_branch: branchName,
},
}); });
}); });
...@@ -263,8 +346,10 @@ describe('Vulnerability Header', () => { ...@@ -263,8 +346,10 @@ describe('Vulnerability Header', () => {
describe('when the vulnerability is already resolved', () => { describe('when the vulnerability is already resolved', () => {
beforeEach(() => { beforeEach(() => {
createWrapper({ createWrapper({
vulnerability: {
resolved_on_default_branch: true, resolved_on_default_branch: true,
state: 'resolved', state: 'resolved',
},
}); });
}); });
...@@ -281,7 +366,7 @@ describe('Vulnerability Header', () => { ...@@ -281,7 +366,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}_by_id`]: user.id }); createWrapper({ vulnerability: { state, [`${state}_by_id`]: user.id } });
return waitForPromises().then(() => { return waitForPromises().then(() => {
expect(mockAxios.history.get).toHaveLength(1); expect(mockAxios.history.get).toHaveLength(1);
...@@ -291,7 +376,7 @@ describe('Vulnerability Header', () => { ...@@ -291,7 +376,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);
...@@ -300,7 +385,7 @@ describe('Vulnerability Header', () => { ...@@ -300,7 +385,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', confirmed_by_id: 1 }); createWrapper({ vulnerability: { state: 'confirmed', confirmed_by_id: 1 } });
mockAxios.onGet().replyOnce(500); mockAxios.onGet().replyOnce(500);
...@@ -312,7 +397,7 @@ describe('Vulnerability Header', () => { ...@@ -312,7 +397,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', confirmed_by_id: user.id }); createWrapper({ vulnerability: { state: 'confirmed', confirmed_by_id: 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