Commit 1d471fdc authored by David Pisek's avatar David Pisek Committed by Phil Hughes

Clarify security report findings in MR widget

* Removes icons from security report items
* Adds new component that renders a grouped list of report items
  (groups: New and Fixed)
* Fixes spacing issue between report items on smaller screens
parent f8e8d27a
<script>
import { s__ } from '~/locale';
import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
import ReportItem from '~/reports/components/report_item.vue';
export default {
components: {
ReportItem,
SmartVirtualList,
},
props: {
component: {
type: String,
required: false,
default: '',
},
resolvedIssues: {
type: Array,
required: false,
default: () => [],
},
unresolvedIssues: {
type: Array,
required: false,
default: () => [],
},
resolvedHeading: {
type: String,
required: false,
default: s__('ciReport|Fixed'),
},
unresolvedHeading: {
type: String,
required: false,
default: s__('ciReport|New'),
},
},
groups: ['unresolved', 'resolved'],
typicalReportItemHeight: 32,
maxShownReportItems: 20,
computed: {
groups() {
return this.$options.groups
.map(group => ({
name: group,
issues: this[`${group}Issues`],
heading: this[`${group}Heading`],
}))
.filter(({ issues }) => issues.length > 0);
},
listLength() {
// every group has a header which is rendered as a list item
const groupsCount = this.groups.length;
const issuesCount = this.groups.reduce(
(totalIssues, { issues }) => totalIssues + issues.length,
0,
);
return groupsCount + issuesCount;
},
},
};
</script>
<template>
<smart-virtual-list
:length="listLength"
:remain="$options.maxShownReportItems"
:size="$options.typicalReportItemHeight"
class="report-block-container"
wtag="ul"
wclass="report-block-list"
>
<template v-for="(group, groupIndex) in groups">
<h2
:key="group.name"
:data-testid="`${group.name}Heading`"
:class="[groupIndex > 0 ? 'mt-2' : 'mt-0']"
class="h5 mb-1"
>
{{ group.heading }}
</h2>
<report-item
v-for="(issue, issueIndex) in group.issues"
:key="`${group.name}-${issue.name}-${group.name}-${issueIndex}`"
:issue="issue"
:show-report-section-status-icon="false"
:component="component"
status="none"
/>
</template>
</smart-virtual-list>
</template>
...@@ -22,7 +22,7 @@ GitLab checks the Container Scanning report, compares the found vulnerabilities ...@@ -22,7 +22,7 @@ GitLab checks the Container Scanning report, compares the found vulnerabilities
between the source and target branches, and shows the information right on the between the source and target branches, and shows the information right on the
merge request. merge request.
![Container Scanning Widget](img/container_scanning_v12_9.png) ![Container Scanning Widget](img/container_scanning_v13_0.png)
## Contribute your scanner ## Contribute your scanner
......
...@@ -33,7 +33,7 @@ NOTE: **Note:** ...@@ -33,7 +33,7 @@ NOTE: **Note:**
This comparison logic uses only the latest pipeline executed for the target branch's base commit. This comparison logic uses only the latest pipeline executed for the target branch's base commit.
Running the pipeline on any other commit has no effect on the merge request. Running the pipeline on any other commit has no effect on the merge request.
![DAST Widget](img/dast_all_v12_9.png) ![DAST Widget](img/dast_all_v13_0.png)
By clicking on one of the detected linked vulnerabilities, you can By clicking on one of the detected linked vulnerabilities, you can
see the details and the URL(s) affected. see the details and the URL(s) affected.
......
...@@ -24,7 +24,7 @@ GitLab checks the Dependency Scanning report, compares the found vulnerabilities ...@@ -24,7 +24,7 @@ GitLab checks the Dependency Scanning report, compares the found vulnerabilities
between the source and target branches, and shows the information on the between the source and target branches, and shows the information on the
merge request. merge request.
![Dependency Scanning Widget](img/dependency_scanning.png) ![Dependency Scanning Widget](img/dependency_scanning_v13_0.png)
The results are sorted by the severity of the vulnerability: The results are sorted by the severity of the vulnerability:
......
...@@ -25,7 +25,7 @@ You can take advantage of SAST by doing one of the following: ...@@ -25,7 +25,7 @@ You can take advantage of SAST by doing one of the following:
GitLab checks the SAST report, compares the found vulnerabilities between the GitLab checks the SAST report, compares the found vulnerabilities between the
source and target branches, and shows the information right on the merge request. source and target branches, and shows the information right on the merge request.
![SAST Widget](img/sast_v12_9.png) ![SAST Widget](img/sast_v13_0.png)
The results are sorted by the priority of the vulnerability: The results are sorted by the priority of the vulnerability:
......
...@@ -4,7 +4,7 @@ import { componentNames } from 'ee/reports/components/issue_body'; ...@@ -4,7 +4,7 @@ import { componentNames } from 'ee/reports/components/issue_body';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import ReportSection from '~/reports/components/report_section.vue'; import ReportSection from '~/reports/components/report_section.vue';
import SummaryRow from '~/reports/components/summary_row.vue'; import SummaryRow from '~/reports/components/summary_row.vue';
import IssuesList from '~/reports/components/issues_list.vue'; import GroupedIssuesList from '~/reports/components/grouped_issues_list.vue';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import IssueModal from './components/modal.vue'; import IssueModal from './components/modal.vue';
import securityReportsMixin from './mixins/security_report_mixin'; import securityReportsMixin from './mixins/security_report_mixin';
...@@ -15,9 +15,9 @@ import { mrStates } from '~/mr_popover/constants'; ...@@ -15,9 +15,9 @@ import { mrStates } from '~/mr_popover/constants';
export default { export default {
store: createStore(), store: createStore(),
components: { components: {
GroupedIssuesList,
ReportSection, ReportSection,
SummaryRow, SummaryRow,
IssuesList,
IssueModal, IssueModal,
Icon, Icon,
GlSprintf, GlSprintf,
...@@ -346,13 +346,13 @@ export default { ...@@ -346,13 +346,13 @@ export default {
data-qa-selector="sast_scan_report" data-qa-selector="sast_scan_report"
/> />
<issues-list <grouped-issues-list
v-if="sast.newIssues.length || sast.resolvedIssues.length" v-if="sast.newIssues.length || sast.resolvedIssues.length"
:unresolved-issues="sast.newIssues" :unresolved-issues="sast.newIssues"
:resolved-issues="sast.resolvedIssues" :resolved-issues="sast.resolvedIssues"
:all-issues="sast.allIssues"
:component="$options.componentNames.SecurityIssueBody" :component="$options.componentNames.SecurityIssueBody"
class="js-sast-issue-list report-block-group-list" class="report-block-group-list"
data-testid="sast-issues-list"
/> />
</template> </template>
...@@ -365,12 +365,13 @@ export default { ...@@ -365,12 +365,13 @@ export default {
data-qa-selector="dependency_scan_report" data-qa-selector="dependency_scan_report"
/> />
<issues-list <grouped-issues-list
v-if="dependencyScanning.newIssues.length || dependencyScanning.resolvedIssues.length" v-if="dependencyScanning.newIssues.length || dependencyScanning.resolvedIssues.length"
:unresolved-issues="dependencyScanning.newIssues" :unresolved-issues="dependencyScanning.newIssues"
:resolved-issues="dependencyScanning.resolvedIssues" :resolved-issues="dependencyScanning.resolvedIssues"
:component="$options.componentNames.SecurityIssueBody" :component="$options.componentNames.SecurityIssueBody"
class="js-dss-issue-list report-block-group-list" class="report-block-group-list"
data-testid="dependency-scanning-issues-list"
/> />
</template> </template>
...@@ -383,12 +384,13 @@ export default { ...@@ -383,12 +384,13 @@ export default {
data-qa-selector="container_scan_report" data-qa-selector="container_scan_report"
/> />
<issues-list <grouped-issues-list
v-if="containerScanning.newIssues.length || containerScanning.resolvedIssues.length" v-if="containerScanning.newIssues.length || containerScanning.resolvedIssues.length"
:unresolved-issues="containerScanning.newIssues" :unresolved-issues="containerScanning.newIssues"
:resolved-issues="containerScanning.resolvedIssues" :resolved-issues="containerScanning.resolvedIssues"
:component="$options.componentNames.SecurityIssueBody" :component="$options.componentNames.SecurityIssueBody"
class="report-block-group-list" class="report-block-group-list"
data-testid="container-scanning-issues-list"
/> />
</template> </template>
...@@ -414,12 +416,13 @@ export default { ...@@ -414,12 +416,13 @@ export default {
</template> </template>
</summary-row> </summary-row>
<issues-list <grouped-issues-list
v-if="dast.newIssues.length || dast.resolvedIssues.length" v-if="dast.newIssues.length || dast.resolvedIssues.length"
:unresolved-issues="dast.newIssues" :unresolved-issues="dast.newIssues"
:resolved-issues="dast.resolvedIssues" :resolved-issues="dast.resolvedIssues"
:component="$options.componentNames.SecurityIssueBody" :component="$options.componentNames.SecurityIssueBody"
class="report-block-group-list" class="report-block-group-list"
data-testid="dast-issues-list"
/> />
</template> </template>
...@@ -432,12 +435,13 @@ export default { ...@@ -432,12 +435,13 @@ export default {
data-qa-selector="secret_scan_report" data-qa-selector="secret_scan_report"
/> />
<issues-list <grouped-issues-list
v-if="secretScanning.newIssues.length || secretScanning.resolvedIssues.length" v-if="secretScanning.newIssues.length || secretScanning.resolvedIssues.length"
:unresolved-issues="secretScanning.newIssues" :unresolved-issues="secretScanning.newIssues"
:resolved-issues="secretScanning.resolvedIssues" :resolved-issues="secretScanning.resolvedIssues"
:component="$options.componentNames.SecurityIssueBody" :component="$options.componentNames.SecurityIssueBody"
class="report-block-group-list" class="report-block-group-list"
data-testid="secret-scanning-issues-list"
/> />
</template> </template>
......
---
title: Clarify security report findings in merge request widget
merge_request: 30688
author:
type: changed
import Vue from 'vue'; import Vue from 'vue';
import MockAdapter from 'axios-mock-adapter'; import MockAdapter from 'axios-mock-adapter';
import GroupedIssuesList from '~/reports/components/grouped_issues_list.vue';
import GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_security_reports_app.vue'; import GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_security_reports_app.vue';
import state from 'ee/vue_shared/security_reports/store/state'; import state from 'ee/vue_shared/security_reports/store/state';
import * as types from 'ee/vue_shared/security_reports/store/mutation_types'; import * as types from 'ee/vue_shared/security_reports/store/mutation_types';
...@@ -229,13 +230,27 @@ describe('Grouped security reports app', () => { ...@@ -229,13 +230,27 @@ describe('Grouped security reports app', () => {
}); });
}); });
it('has the success icon for fixed vulnerabilities', () => { it.each`
const icon = wrapper.vm.$el.querySelector( reportType | resolvedIssues | unresolvedIssues
'.js-container-scanning~.js-plain-element .ic-status_success_borderless', ${'sast'} | ${sastDiffSuccessMock.fixed} | ${sastDiffSuccessMock.added}
); ${'dependency-scanning'} | ${dependencyScanningDiffSuccessMock.fixed} | ${dependencyScanningDiffSuccessMock.added}
${'container-scanning'} | ${containerScanningDiffSuccessMock.fixed} | ${containerScanningDiffSuccessMock.added}
expect(icon).not.toBeNull(); ${'dast'} | ${dastDiffSuccessMock.fixed} | ${dastDiffSuccessMock.added}
}); ${'secret-scanning'} | ${secretScanningDiffSuccessMock.fixed} | ${secretScanningDiffSuccessMock.added}
`(
'renders a grouped-issues-list with the correct props for "$reportType" issues',
({ reportType, resolvedIssues, unresolvedIssues }) => {
const issuesList = wrapper.find(`[data-testid="${reportType}-issues-list"]`);
expect(issuesList.is(GroupedIssuesList)).toBe(true);
expect(issuesList.props()).toMatchObject({
resolvedIssues,
unresolvedIssues,
component: 'SecurityIssueBody',
});
},
);
}); });
}); });
......
...@@ -24951,6 +24951,9 @@ msgstr "" ...@@ -24951,6 +24951,9 @@ msgstr ""
msgid "ciReport|Failed to load %{reportName} report" msgid "ciReport|Failed to load %{reportName} report"
msgstr "" msgstr ""
msgid "ciReport|Fixed"
msgstr ""
msgid "ciReport|Fixed:" msgid "ciReport|Fixed:"
msgstr "" msgstr ""
...@@ -24969,6 +24972,9 @@ msgstr "" ...@@ -24969,6 +24972,9 @@ msgstr ""
msgid "ciReport|Manage licenses" msgid "ciReport|Manage licenses"
msgstr "" msgstr ""
msgid "ciReport|New"
msgstr ""
msgid "ciReport|No changes to code quality" msgid "ciReport|No changes to code quality"
msgstr "" msgstr ""
......
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`Grouped Issues List renders a smart virtual list with the correct props 1`] = `
Object {
"length": 4,
"remain": 20,
"rtag": "div",
"size": 32,
"wclass": "report-block-list",
"wtag": "ul",
}
`;
exports[`Grouped Issues List with data renders a report item with the correct props 1`] = `
Object {
"component": "TestIssueBody",
"isNew": false,
"issue": Object {
"name": "foo",
},
"showReportSectionStatusIcon": false,
"status": "none",
"statusIconSize": 24,
}
`;
import { shallowMount } from '@vue/test-utils';
import GroupedIssuesList from '~/reports/components/grouped_issues_list.vue';
import ReportItem from '~/reports/components/report_item.vue';
import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
describe('Grouped Issues List', () => {
let wrapper;
const createComponent = ({ propsData = {}, stubs = {} } = {}) => {
wrapper = shallowMount(GroupedIssuesList, {
propsData,
stubs,
});
};
const findHeading = groupName => wrapper.find(`[data-testid="${groupName}Heading"`);
afterEach(() => {
wrapper.destroy();
wrapper = null;
});
it('renders a smart virtual list with the correct props', () => {
createComponent({
propsData: {
resolvedIssues: [{ name: 'foo' }],
unresolvedIssues: [{ name: 'bar' }],
},
stubs: {
SmartVirtualList,
},
});
expect(wrapper.find(SmartVirtualList).props()).toMatchSnapshot();
});
describe('without data', () => {
beforeEach(createComponent);
it.each(['unresolved', 'resolved'])('does not a render a header for %s issues', issueName => {
expect(findHeading(issueName).exists()).toBe(false);
});
it.each('resolved', 'unresolved')('does not render report items for %s issues', () => {
expect(wrapper.contains(ReportItem)).toBe(false);
});
});
describe('with data', () => {
it.each`
givenIssues | givenHeading | groupName
${[{ name: 'foo issue' }]} | ${'Foo Heading'} | ${'resolved'}
${[{ name: 'bar issue' }]} | ${'Bar Heading'} | ${'unresolved'}
`('renders the heading for $groupName issues', ({ givenIssues, givenHeading, groupName }) => {
createComponent({
propsData: { [`${groupName}Issues`]: givenIssues, [`${groupName}Heading`]: givenHeading },
});
expect(findHeading(groupName).text()).toBe(givenHeading);
});
it.each(['resolved', 'unresolved'])('renders all %s issues', issueName => {
const issues = [{ name: 'foo' }, { name: 'bar' }];
createComponent({
propsData: { [`${issueName}Issues`]: issues },
});
expect(wrapper.findAll(ReportItem)).toHaveLength(issues.length);
});
it('renders a report item with the correct props', () => {
createComponent({
propsData: {
resolvedIssues: [{ name: 'foo' }],
component: 'TestIssueBody',
},
stubs: {
ReportItem,
},
});
expect(wrapper.find(ReportItem).props()).toMatchSnapshot();
});
});
});
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