Commit a134363a authored by Alexander Turinske's avatar Alexander Turinske

Add split button for new vulnerability actions

- add split button that allows
  for the creation of an issue and creation of a merge
  request
- add logic to conditionally render split button with single button
- add tests
- abstract out feedback types strings to the constants
  file to be more adjustable
- add changelog
parent bd056f74
---
title: Add ability to create merge request from vulnerability page
merge_request: 31620
author:
type: added
......@@ -8,7 +8,7 @@ function createHeaderApp() {
const pipeline = JSON.parse(el.dataset.pipelineJson);
const finding = JSON.parse(el.dataset.findingJson);
const { projectFingerprint, createIssueUrl } = el.dataset;
const { projectFingerprint, createIssueUrl, createMrUrl } = el.dataset;
return new Vue({
el,
......@@ -16,6 +16,7 @@ function createHeaderApp() {
render: h =>
h(HeaderApp, {
props: {
createMrUrl,
initialVulnerability,
finding,
pipeline,
......
......@@ -9,8 +9,9 @@ import UsersCache from '~/lib/utils/users_cache';
import ResolutionAlert from './resolution_alert.vue';
import VulnerabilityStateDropdown from './vulnerability_state_dropdown.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 SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue';
export default {
name: 'VulnerabilityHeader',
......@@ -19,10 +20,15 @@ export default {
GlLoadingIcon,
ResolutionAlert,
VulnerabilityStateDropdown,
SplitButton,
StatusDescription,
},
props: {
createMrUrl: {
type: String,
required: true,
},
initialVulnerability: {
type: Object,
required: true,
......@@ -59,6 +65,10 @@ export default {
actionButtons() {
const buttons = [];
if (this.canCreateMergeRequest) {
buttons.push(HEADER_ACTION_BUTTONS.mergeRequestCreation);
}
if (!this.hasIssue) {
buttons.push(HEADER_ACTION_BUTTONS.issueCreation);
}
......@@ -68,6 +78,17 @@ export default {
hasIssue() {
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() {
// Get the badge variant based on the vulnerability state, defaulting to 'expired'.
return VULNERABILITY_STATE_OBJECTS[this.vulnerability.state]?.statusBoxStyle || 'expired';
......@@ -132,7 +153,7 @@ export default {
axios
.post(this.createIssueUrl, {
vulnerability_feedback: {
feedback_type: 'issue',
feedback_type: FEEDBACK_TYPES.ISSUE,
category: this.vulnerability.report_type,
project_fingerprint: this.projectFingerprint,
vulnerability_data: {
......@@ -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>
......@@ -194,8 +241,16 @@ export default {
:initial-state="vulnerability.state"
@change="changeVulnerabilityState"
/>
<split-button
v-if="actionButtons.length > 1"
:buttons="actionButtons"
:disabled="isProcessingAction"
class="js-split-button"
@createMergeRequest="createMergeRequest"
@createIssue="createIssue"
/>
<gl-deprecated-button
v-if="actionButtons.length > 0"
v-else-if="actionButtons.length > 0"
class="ml-2"
variant="success"
category="secondary"
......
......@@ -33,6 +33,17 @@ export const VULNERABILITIES_PER_PAGE = 20;
export const HEADER_ACTION_BUTTONS = {
issueCreation: {
name: s__('ciReport|Create issue'),
tagline: s__('ciReport|Investigate this vulnerability by creating an issue'),
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';
import Header from 'ee/vulnerabilities/components/header.vue';
import StatusDescription from 'ee/vulnerabilities/components/status_description.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 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 mockAxios = new MockAdapter(axios);
......@@ -28,32 +29,35 @@ describe('Vulnerability Header', () => {
state: 'detected',
};
const findingWithIssue = {
description: 'description',
identifiers: 'identifiers',
links: 'links',
location: 'location',
name: 'name',
issue_feedback: {
issue_iid: 12,
},
};
const findingWithoutIssue = {
description: 'description',
identifiers: 'identifiers',
links: 'links',
location: 'location',
name: 'name',
const diff = 'some diff to download';
const getFinding = ({
shouldShowCreateIssueButton = false,
shouldShowMergeRequestButton = false,
}) => {
return {
description: 'description',
identifiers: 'identifiers',
links: 'links',
location: 'location',
name: 'name',
issue_feedback: shouldShowCreateIssueButton ? null : { issue_iid: 12 },
remediations: shouldShowMergeRequestButton ? [{ diff }] : null,
merge_request_feedback: {
merge_request_path: shouldShowMergeRequestButton ? null : 'some path',
},
};
};
const dataset = {
createIssueUrl: 'create_issue_url',
createMrUrl: '/create_mr_url',
createIssueUrl: '/create_issue_url',
projectFingerprint: 'abc123',
pipeline: {
id: 2,
created_at: new Date().toISOString(),
url: 'pipeline_url',
sourceBranch: 'master',
},
};
......@@ -66,11 +70,12 @@ describe('Vulnerability Header', () => {
};
const findGlDeprecatedButton = () => wrapper.find(GlDeprecatedButton);
const findSplitButton = () => wrapper.find(SplitButton);
const findBadge = () => wrapper.find({ ref: 'badge' });
const findResolutionAlert = () => wrapper.find(ResolutionAlert);
const findStatusDescription = () => wrapper.find(StatusDescription);
const createWrapper = (vulnerability = {}, finding = findingWithoutIssue) => {
const createWrapper = ({ vulnerability = {}, finding = getFinding({}) }) => {
wrapper = shallowMount(Header, {
propsData: {
...dataset,
......@@ -88,7 +93,7 @@ describe('Vulnerability Header', () => {
});
describe('state dropdown', () => {
beforeEach(createWrapper);
beforeEach(() => createWrapper({}));
it('the vulnerability state dropdown is rendered', () => {
expect(wrapper.find(VulnerabilityStateDropdown).exists()).toBe(true);
......@@ -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', () => {
it('does not display if there are no actions', () => {
createWrapper({}, findingWithIssue);
createWrapper({});
expect(findGlDeprecatedButton().exists()).toBe(false);
});
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', () => {
expect(findGlDeprecatedButton().exists()).toBe(true);
......@@ -175,12 +203,12 @@ describe('Vulnerability Header', () => {
expect(postRequest.url).toBe(dataset.createIssueUrl);
expect(JSON.parse(postRequest.data)).toMatchObject({
vulnerability_feedback: {
feedback_type: 'issue',
feedback_type: FEEDBACK_TYPES.ISSUE,
category: defaultVulnerability.report_type,
project_fingerprint: dataset.projectFingerprint,
vulnerability_data: {
...defaultVulnerability,
...findingWithoutIssue,
...getFinding({ shouldShowCreateIssueButton: true }),
category: defaultVulnerability.report_type,
vulnerability_id: defaultVulnerability.id,
},
......@@ -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', () => {
test.each(vulnerabilityStateEntries)(
'the vulnerability state badge has the correct style for the %s state',
(state, stateObject) => {
createWrapper({ state });
createWrapper({ vulnerability: { state } });
expect(findBadge().classes()).toContain(`status-box-${stateObject.statusBoxStyle}`);
expect(findBadge().text()).toBe(state);
......@@ -223,7 +304,7 @@ describe('Vulnerability Header', () => {
...{ state: 'confirmed', confirmed_by_id: user.id },
};
createWrapper(vulnerability);
createWrapper({ vulnerability });
return waitForPromises().then(() => {
expect(findStatusDescription().exists()).toBe(true);
......@@ -243,8 +324,10 @@ describe('Vulnerability Header', () => {
beforeEach(() => {
createWrapper({
resolved_on_default_branch: true,
project_default_branch: branchName,
vulnerability: {
resolved_on_default_branch: true,
project_default_branch: branchName,
},
});
});
......@@ -263,8 +346,10 @@ describe('Vulnerability Header', () => {
describe('when the vulnerability is already resolved', () => {
beforeEach(() => {
createWrapper({
resolved_on_default_branch: true,
state: 'resolved',
vulnerability: {
resolved_on_default_branch: true,
state: 'resolved',
},
});
});
......@@ -281,7 +366,7 @@ describe('Vulnerability Header', () => {
`loads the correct user for the vulnerability state "%s"`,
state => {
const user = createRandomUser();
createWrapper({ state, [`${state}_by_id`]: user.id });
createWrapper({ vulnerability: { state, [`${state}_by_id`]: user.id } });
return waitForPromises().then(() => {
expect(mockAxios.history.get).toHaveLength(1);
......@@ -291,7 +376,7 @@ describe('Vulnerability Header', () => {
);
it('does not load a user if there is no user ID', () => {
createWrapper({ state: 'detected' });
createWrapper({ vulnerability: { state: 'detected' } });
return waitForPromises().then(() => {
expect(mockAxios.history.get).toHaveLength(0);
......@@ -300,7 +385,7 @@ describe('Vulnerability Header', () => {
});
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);
......@@ -312,7 +397,7 @@ describe('Vulnerability Header', () => {
it('will set the isLoadingUser property correctly when the user is loading and finished loading', () => {
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);
......
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