Commit be4969af authored by Sam Beckham's avatar Sam Beckham Committed by Fatih Acet

Makes dismissed issues a first class citezen

- Adds `dismissedIssues` to the Vuex store
- Re-works the mutation so dismissed, new items are labelled as
  dismissed rather than `new`
- Renders these dismissed items at the bottom of the list of vulns
- Allows dismissed issues to switch lists in real time
- Refactors a lot of security reports mutations to allow for DRYer code
parent c37bc72f
......@@ -273,7 +273,6 @@ export default {
v-if="dependencyScanning.newIssues.length || dependencyScanning.resolvedIssues.length"
:unresolved-issues="dependencyScanning.newIssues"
:resolved-issues="dependencyScanning.resolvedIssues"
:all-issues="dependencyScanning.allIssues"
:component="$options.componentNames.SastIssueBody"
class="js-dss-issue-list report-block-group-list"
/>
......
import { s__, sprintf } from '~/locale';
import { groupedTextBuilder, statusIcon } from './utils';
import { countIssues, groupedTextBuilder, statusIcon } from './utils';
import { LOADING, ERROR, SUCCESS } from './constants';
import messages from './messages';
const groupedReportText = (report, name, errorMessage, loadingMessage) => {
const groupedReportText = (report, reportType, errorMessage, loadingMessage) => {
const { paths } = report;
if (report.hasError) {
return errorMessage;
}
......@@ -12,13 +14,11 @@ const groupedReportText = (report, name, errorMessage, loadingMessage) => {
return loadingMessage;
}
return groupedTextBuilder(
name,
report.paths,
(report.newIssues || []).length,
(report.resolvedIssues || []).length,
(report.allIssues || []).length,
);
return groupedTextBuilder({
...countIssues(report),
reportType,
paths,
});
};
export const groupedSastText = ({ sast }) =>
......@@ -43,6 +43,19 @@ export const groupedDependencyText = ({ dependencyScanning }) =>
messages.DEPENDENCY_SCANNING_IS_LOADING,
);
export const summaryCounts = state =>
[state.sast, state.sastContainer, state.dast, state.dependencyScanning].reduce(
(acc, report) => {
const curr = countIssues(report);
acc.added += curr.added;
acc.dismissed += curr.dismissed;
acc.fixed += curr.fixed;
acc.existing += curr.existing;
return acc;
},
{ added: 0, dismissed: 0, fixed: 0, existing: 0 },
);
export const groupedSummaryText = (state, getters) => {
const reportType = s__('ciReport|Security scanning');
......@@ -56,7 +69,7 @@ export const groupedSummaryText = (state, getters) => {
return s__('ciReport|Security scanning failed loading any results');
}
const { added, fixed, existing } = state.summaryCounts;
const { added, fixed, existing, dismissed } = getters.summaryCounts;
let status = '';
......@@ -74,7 +87,7 @@ export const groupedSummaryText = (state, getters) => {
*/
const paths = { head: true, base: !getters.noBaseInAllReports };
return groupedTextBuilder(reportType, paths, added, fixed, existing, status);
return groupedTextBuilder({ reportType, paths, added, fixed, existing, dismissed, status });
};
export const summaryStatus = (state, getters) => {
......
......@@ -81,17 +81,11 @@ export default {
Vue.set(state.sast, 'resolvedIssues', resolvedIssues);
Vue.set(state.sast, 'allIssues', allIssues);
Vue.set(state.sast, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
state.summaryCounts.fixed += resolvedIssues.length;
state.summaryCounts.existing += allIssues.length;
} else if (reports.head && !reports.base) {
const newIssues = parseSastIssues(reports.head, reports.enrichData, state.blobPath.head);
Vue.set(state.sast, 'newIssues', newIssues);
Vue.set(state.sast, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
}
},
......@@ -134,9 +128,6 @@ export default {
Vue.set(state.sastContainer, 'newIssues', newIssues);
Vue.set(state.sastContainer, 'resolvedIssues', resolvedIssues);
Vue.set(state.sastContainer, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
state.summaryCounts.fixed += resolvedIssues.length;
} else if (reports.head && !reports.base) {
const newIssues = getUnapprovedVulnerabilities(
parseSastContainer(reports.head.vulnerabilities, reports.enrichData),
......@@ -145,8 +136,6 @@ export default {
Vue.set(state.sastContainer, 'newIssues', newIssues);
Vue.set(state.sastContainer, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
}
},
......@@ -180,16 +169,11 @@ export default {
Vue.set(state.dast, 'newIssues', newIssues);
Vue.set(state.dast, 'resolvedIssues', resolvedIssues);
Vue.set(state.dast, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
state.summaryCounts.fixed += resolvedIssues.length;
} else if (reports.head && reports.head.site && !reports.base) {
const newIssues = parseDastIssues(reports.head.site.alerts, reports.enrichData);
Vue.set(state.dast, 'newIssues', newIssues);
Vue.set(state.dast, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
}
},
......@@ -249,10 +233,6 @@ export default {
Vue.set(state.dependencyScanning, 'resolvedIssues', resolvedIssues);
Vue.set(state.dependencyScanning, 'allIssues', allIssues);
Vue.set(state.dependencyScanning, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
state.summaryCounts.fixed += resolvedIssues.length;
state.summaryCounts.existing += allIssues.length;
}
if (reports.head && !reports.base) {
......@@ -263,8 +243,6 @@ export default {
);
Vue.set(state.dependencyScanning, 'newIssues', newIssues);
Vue.set(state.dependencyScanning, 'isLoading', false);
state.summaryCounts.added += newIssues.length;
}
},
......
import { s__ } from '~/locale';
export default () => ({
summaryCounts: {
added: 0,
fixed: 0,
existing: 0,
},
blobPath: {
head: null,
base: null,
......
......@@ -311,46 +311,86 @@ export const filterByKey = (firstArray = [], secondArray = [], key = '') =>
export const getUnapprovedVulnerabilities = (issues = [], unapproved = []) =>
issues.filter(item => unapproved.find(el => el === item.vulnerability));
export const groupedTextBuilder = (
export const groupedTextBuilder = ({
reportType = '',
paths = {},
newIssues = 0,
resolvedIssues = 0,
allIssues = 0,
added = 0,
fixed = 0,
existing = 0,
dismissed = 0,
status = '',
) => {
}) => {
let baseString = '';
if (!paths.base) {
if (newIssues > 0) {
if (added && !dismissed) {
// added
baseString = n__(
'ciReport|%{reportType} %{status} detected %{newCount} vulnerability for the source branch only',
'ciReport|%{reportType} %{status} detected %{newCount} vulnerabilities for the source branch only',
newIssues,
added,
);
} else if (!added && dismissed) {
// dismissed
baseString = n__(
'ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerability for the source branch only',
'ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerabilities for the source branch only',
dismissed,
);
} else if (added && dismissed) {
// added & dismissed
baseString = s__(
'ciReport|%{reportType} %{status} detected %{newCount} new, and %{dismissedCount} dismissed vulnerabilities for the source branch only',
);
} else {
// no vulnerabilities
baseString = s__(
'ciReport|%{reportType} %{status} detected no vulnerabilities for the source branch only',
);
}
} else if (paths.base && paths.head) {
if (newIssues > 0 && resolvedIssues > 0) {
baseString = s__(
'ciReport|%{reportType} %{status} detected %{newCount} new, and %{fixedCount} fixed vulnerabilities',
);
} else if (newIssues > 0 && resolvedIssues === 0) {
} else if (paths.head) {
if (added && !fixed && !dismissed) {
// added
baseString = n__(
'ciReport|%{reportType} %{status} detected %{newCount} new vulnerability',
'ciReport|%{reportType} %{status} detected %{newCount} new vulnerabilities',
newIssues,
added,
);
} else if (newIssues === 0 && resolvedIssues > 0) {
} else if (!added && fixed && !dismissed) {
// fixed
baseString = n__(
'ciReport|%{reportType} %{status} detected %{fixedCount} fixed vulnerability',
'ciReport|%{reportType} %{status} detected %{fixedCount} fixed vulnerabilities',
resolvedIssues,
fixed,
);
} else if (!added && !fixed && dismissed) {
// dismissed
baseString = n__(
'ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerability',
'ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerabilities',
dismissed,
);
} else if (added && fixed && !dismissed) {
// added & fixed
baseString = s__(
'ciReport|%{reportType} %{status} detected %{newCount} new, and %{fixedCount} fixed vulnerabilities',
);
} else if (allIssues > 0) {
} else if (added && !fixed && dismissed) {
// added & dismissed
baseString = s__(
'ciReport|%{reportType} %{status} detected %{newCount} new, and %{dismissedCount} dismissed vulnerabilities',
);
} else if (!added && fixed && dismissed) {
// fixed & dismissed
baseString = s__(
'ciReport|%{reportType} %{status} detected %{fixedCount} fixed, and %{dismissedCount} dismissed vulnerabilities',
);
} else if (added && fixed && dismissed) {
// added & fixed & dismissed
baseString = s__(
'ciReport|%{reportType} %{status} detected %{newCount} new, %{fixedCount} fixed, and %{dismissedCount} dismissed vulnerabilities',
);
} else if (existing) {
baseString = s__('ciReport|%{reportType} %{status} detected no new vulnerabilities');
} else {
baseString = s__('ciReport|%{reportType} %{status} detected no vulnerabilities');
......@@ -364,8 +404,9 @@ export const groupedTextBuilder = (
return sprintf(baseString, {
status,
reportType,
newCount: newIssues,
fixedCount: resolvedIssues,
newCount: added,
fixedCount: fixed,
dismissedCount: dismissed,
});
};
......@@ -380,3 +421,23 @@ export const statusIcon = (loading = false, failed = false, newIssues = 0, neutr
return 'success';
};
/**
* Counts issues. Simply returns the amount of existing and fixed Issues.
* New Issues are divided into dismissed and added.
*
* @param newIssues
* @param resolvedIssues
* @param allIssues
* @returns {{existing: number, added: number, dismissed: number, fixed: number}}
*/
export const countIssues = ({ newIssues = [], resolvedIssues = [], allIssues = [] } = {}) => {
const dismissed = newIssues.reduce((sum, issue) => (issue.isDismissed ? sum + 1 : sum), 0);
return {
added: newIssues.length - dismissed,
dismissed,
existing: allIssues.length,
fixed: resolvedIssues.length,
};
};
---
title: Consider dismissed items in security reports summary
merge_request: 9275
author:
type: changed
......@@ -124,7 +124,6 @@ describe('security reports mutations', () => {
expect(stateCopy.sast.isLoading).toEqual(false);
expect(stateCopy.sast.newIssues).toEqual(parsedSastIssuesHead);
expect(stateCopy.sast.resolvedIssues).toEqual(parsedSastBaseStore);
expect(stateCopy.summaryCounts).toEqual({ added: 2, fixed: 1, existing: 1 });
});
});
......@@ -137,7 +136,6 @@ describe('security reports mutations', () => {
expect(stateCopy.sast.isLoading).toEqual(false);
expect(stateCopy.sast.newIssues).toEqual(parsedSastIssuesStore);
expect(stateCopy.summaryCounts).toEqual({ added: 3, fixed: 0, existing: 0 });
});
});
});
......@@ -186,7 +184,6 @@ describe('security reports mutations', () => {
expect(stateCopy.sastContainer.isLoading).toEqual(false);
expect(stateCopy.sastContainer.newIssues).toEqual(dockerNewIssues);
expect(stateCopy.sastContainer.resolvedIssues).toEqual(parsedSastContainerBaseStore);
expect(stateCopy.summaryCounts).toEqual({ added: 1, fixed: 1, existing: 0 });
});
});
......@@ -198,7 +195,6 @@ describe('security reports mutations', () => {
expect(stateCopy.sastContainer.isLoading).toEqual(false);
expect(stateCopy.sastContainer.newIssues).toEqual(dockerOnlyHeadParsed);
expect(stateCopy.summaryCounts).toEqual({ added: 2, fixed: 0, existing: 0 });
});
});
});
......@@ -248,7 +244,6 @@ describe('security reports mutations', () => {
expect(stateCopy.dast.newIssues).toEqual(parsedDastNewIssues);
expect(stateCopy.dast.resolvedIssues).toEqual([]);
expect(stateCopy.summaryCounts).toEqual({ added: 1, fixed: 0, existing: 0 });
});
});
......@@ -260,7 +255,6 @@ describe('security reports mutations', () => {
expect(stateCopy.dast.isLoading).toEqual(false);
expect(stateCopy.dast.newIssues).toEqual(parsedDast);
expect(stateCopy.summaryCounts).toEqual({ added: 2, fixed: 0, existing: 0 });
});
});
});
......@@ -313,8 +307,6 @@ describe('security reports mutations', () => {
expect(stateCopy.dependencyScanning.resolvedIssues).toEqual(
parsedDependencyScanningBaseStore,
);
expect(stateCopy.summaryCounts).toEqual({ added: 2, fixed: 1, existing: 1 });
});
});
......@@ -327,7 +319,6 @@ describe('security reports mutations', () => {
expect(stateCopy.dependencyScanning.isLoading).toEqual(false);
expect(stateCopy.dependencyScanning.newIssues).toEqual(parsedDependencyScanningIssuesStore);
expect(stateCopy.summaryCounts).toEqual({ added: 3, fixed: 0, existing: 0 });
});
});
});
......@@ -566,9 +557,9 @@ describe('security reports mutations', () => {
});
it('updates issue in the resolved issues list', () => {
stateCopy.sast.newIssues = [];
stateCopy.sast.resolvedIssues = parsedDependencyScanningIssuesHead;
stateCopy.sast.allIssues = [];
stateCopy.dependencyScanning.newIssues = [];
stateCopy.dependencyScanning.resolvedIssues = parsedDependencyScanningIssuesHead;
stateCopy.dependencyScanning.allIssues = [];
const updatedIssue = {
...parsedDependencyScanningIssuesHead[0],
foo: 'bar',
......@@ -576,7 +567,7 @@ describe('security reports mutations', () => {
mutations[types.UPDATE_DEPENDENCY_SCANNING_ISSUE](stateCopy, updatedIssue);
expect(stateCopy.sast.resolvedIssues[0]).toEqual(updatedIssue);
expect(stateCopy.dependencyScanning.resolvedIssues[0]).toEqual(updatedIssue);
});
it('updates issue in the all issues list', () => {
......
......@@ -10,6 +10,7 @@ import {
getUnapprovedVulnerabilities,
groupedTextBuilder,
statusIcon,
countIssues,
} from 'ee/vue_shared/security_reports/store/utils';
import {
oldSastIssues,
......@@ -292,44 +293,56 @@ describe('security reports utils', () => {
});
describe('textBuilder', () => {
describe('with no issues', () => {
it('should return no vulnerabiltities text', () => {
expect(groupedTextBuilder('', { head: 'foo', base: 'bar' }, 0, 0, 0)).toEqual(
' detected no vulnerabilities',
describe('with only the head', () => {
const paths = { head: 'foo' };
it('should return unable to compare text', () => {
expect(groupedTextBuilder({ paths, added: 1 })).toEqual(
' detected 1 vulnerability for the source branch only',
);
});
});
describe('with only `all` issues', () => {
it('should return no new vulnerabiltities text', () => {
expect(groupedTextBuilder('', { head: 'foo', base: 'bar' }, 0, 0, 1)).toEqual(
' detected no new vulnerabilities',
it('should return unable to compare text with no vulnerability', () => {
expect(groupedTextBuilder({ paths })).toEqual(
' detected no vulnerabilities for the source branch only',
);
});
});
describe('with new issues and without base', () => {
it('should return unable to compare text', () => {
expect(groupedTextBuilder('', { head: 'foo' }, 1, 0, 0)).toEqual(
' detected 1 vulnerability for the source branch only',
it('should return dismissed text', () => {
expect(groupedTextBuilder({ paths, dismissed: 2 })).toEqual(
' detected 2 dismissed vulnerabilities for the source branch only',
);
});
it('should return unable to compare text with no vulnerability', () => {
expect(groupedTextBuilder('', { head: 'foo' }, 0, 0, 0)).toEqual(
' detected no vulnerabilities for the source branch only',
it('should return new and dismissed text', () => {
expect(groupedTextBuilder({ paths, added: 1, dismissed: 2 })).toEqual(
' detected 1 new, and 2 dismissed vulnerabilities for the source branch only',
);
});
});
describe('with base and head', () => {
const paths = { head: 'foo', base: 'foo' };
describe('with no issues', () => {
it('should return no vulnerabiltities text', () => {
expect(groupedTextBuilder({ paths })).toEqual(' detected no vulnerabilities');
});
});
describe('with only `all` issues', () => {
it('should return no new vulnerabiltities text', () => {
expect(groupedTextBuilder({ paths, existing: 1 })).toEqual(
' detected no new vulnerabilities',
);
});
});
describe('with only new issues', () => {
it('should return new issues text', () => {
expect(groupedTextBuilder('', { head: 'foo', base: 'foo' }, 1, 0, 0)).toEqual(
' detected 1 new vulnerability',
);
expect(groupedTextBuilder({ paths, added: 1 })).toEqual(' detected 1 new vulnerability');
expect(groupedTextBuilder('', { head: 'foo', base: 'foo' }, 2, 0, 0)).toEqual(
expect(groupedTextBuilder({ paths, added: 2 })).toEqual(
' detected 2 new vulnerabilities',
);
});
......@@ -337,27 +350,53 @@ describe('security reports utils', () => {
describe('with new and resolved issues', () => {
it('should return new and fixed issues text', () => {
expect(
groupedTextBuilder('', { head: 'foo', base: 'foo' }, 1, 1, 0).replace(/\n+\s+/m, ' '),
).toEqual(' detected 1 new, and 1 fixed vulnerabilities');
expect(groupedTextBuilder({ paths, added: 1, fixed: 1 }).replace(/\n+\s+/m, ' ')).toEqual(
' detected 1 new, and 1 fixed vulnerabilities',
);
expect(
groupedTextBuilder('', { head: 'foo', base: 'foo' }, 2, 2, 0).replace(/\n+\s+/m, ' '),
).toEqual(' detected 2 new, and 2 fixed vulnerabilities');
expect(groupedTextBuilder({ paths, added: 2, fixed: 2 }).replace(/\n+\s+/m, ' ')).toEqual(
' detected 2 new, and 2 fixed vulnerabilities',
);
});
});
describe('with only resolved issues', () => {
it('should return fixed issues text', () => {
expect(groupedTextBuilder('', { head: 'foo', base: 'foo' }, 0, 1, 0)).toEqual(
expect(groupedTextBuilder({ paths, fixed: 1 })).toEqual(
' detected 1 fixed vulnerability',
);
expect(groupedTextBuilder('', { head: 'foo', base: 'foo' }, 0, 2, 0)).toEqual(
expect(groupedTextBuilder({ paths, fixed: 2 })).toEqual(
' detected 2 fixed vulnerabilities',
);
});
});
describe('with dismissed issues', () => {
it('should return dismissed text', () => {
expect(groupedTextBuilder({ paths, dismissed: 2 })).toEqual(
' detected 2 dismissed vulnerabilities',
);
});
it('should return new and dismissed text', () => {
expect(groupedTextBuilder({ paths, added: 1, dismissed: 2 })).toEqual(
' detected 1 new, and 2 dismissed vulnerabilities',
);
});
it('should return fixed and dismissed text', () => {
expect(groupedTextBuilder({ paths, fixed: 1, dismissed: 2 })).toEqual(
' detected 1 fixed, and 2 dismissed vulnerabilities',
);
});
it('should return new, fixed and dismissed text', () => {
expect(groupedTextBuilder({ paths, fixed: 1, added: 1, dismissed: 2 })).toEqual(
' detected 1 new, 1 fixed, and 2 dismissed vulnerabilities',
);
});
});
});
});
......@@ -386,4 +425,67 @@ describe('security reports utils', () => {
});
});
});
describe('countIssues', () => {
const allIssues = [{}];
const resolvedIssues = [{}];
const dismissedIssues = [{ isDismissed: true }];
const addedIssues = [{ isDismissed: false }];
it('returns 0 for all counts if everything is empty', () => {
expect(countIssues()).toEqual({
added: 0,
dismissed: 0,
existing: 0,
fixed: 0,
});
});
it('counts `allIssues` as existing', () => {
expect(countIssues({ allIssues })).toEqual({
added: 0,
dismissed: 0,
existing: 1,
fixed: 0,
});
});
it('counts `resolvedIssues` as fixed', () => {
expect(countIssues({ resolvedIssues })).toEqual({
added: 0,
dismissed: 0,
existing: 0,
fixed: 1,
});
});
it('counts `newIssues` which are dismissed as dismissed', () => {
expect(countIssues({ newIssues: dismissedIssues })).toEqual({
added: 0,
dismissed: 1,
existing: 0,
fixed: 0,
});
});
it('counts `newIssues` which are not dismissed as added', () => {
expect(countIssues({ newIssues: addedIssues })).toEqual({
added: 1,
dismissed: 0,
existing: 0,
fixed: 0,
});
});
it('counts everything', () => {
expect(
countIssues({ newIssues: [...addedIssues, ...dismissedIssues], resolvedIssues, allIssues }),
).toEqual({
added: 1,
dismissed: 1,
existing: 1,
fixed: 1,
});
});
});
});
......@@ -11558,16 +11558,38 @@ msgstr ""
msgid "ciReport|%{remainingPackagesCount} more"
msgstr ""
msgid "ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerability"
msgid_plural "ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerabilities"
msgstr[0] ""
msgstr[1] ""
msgid "ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerability for the source branch only"
msgid_plural "ciReport|%{reportType} %{status} detected %{dismissedCount} dismissed vulnerabilities for the source branch only"
msgstr[0] ""
msgstr[1] ""
msgid "ciReport|%{reportType} %{status} detected %{fixedCount} fixed vulnerability"
msgid_plural "ciReport|%{reportType} %{status} detected %{fixedCount} fixed vulnerabilities"
msgstr[0] ""
msgstr[1] ""
msgid "ciReport|%{reportType} %{status} detected %{fixedCount} fixed, and %{dismissedCount} dismissed vulnerabilities"
msgstr ""
msgid "ciReport|%{reportType} %{status} detected %{newCount} new vulnerability"
msgid_plural "ciReport|%{reportType} %{status} detected %{newCount} new vulnerabilities"
msgstr[0] ""
msgstr[1] ""
msgid "ciReport|%{reportType} %{status} detected %{newCount} new, %{fixedCount} fixed, and %{dismissedCount} dismissed vulnerabilities"
msgstr ""
msgid "ciReport|%{reportType} %{status} detected %{newCount} new, and %{dismissedCount} dismissed vulnerabilities"
msgstr ""
msgid "ciReport|%{reportType} %{status} detected %{newCount} new, and %{dismissedCount} dismissed vulnerabilities for the source branch only"
msgstr ""
msgid "ciReport|%{reportType} %{status} detected %{newCount} new, and %{fixedCount} fixed vulnerabilities"
msgstr ""
......
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