Merge branch '214329-clarify-security-report-findings-in-merge-request-widget' into 'master'

Clarify security report findings in merge request widget

See merge request gitlab-org/gitlab!30688

app/assets/javascripts/reports/components/grouped_issues_list.vue

+<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>

doc/user/application_security/container_scanning/index.md

@@ -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
 merge request.
 
-![Container Scanning Widget](img/container_scanning_v12_9.png)
+![Container Scanning Widget](img/container_scanning_v13_0.png)
 
 ## Contribute your scanner
 

doc/user/application_security/dast/index.md

@@ -33,7 +33,7 @@ NOTE: **Note:**
 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.
 
-![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
 see the details and the URL(s) affected.

doc/user/application_security/dependency_scanning/index.md

@@ -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
 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:
 

doc/user/application_security/sast/index.md

@@ -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
 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:
 

ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue

@@ -4,7 +4,7 @@ import { componentNames } from 'ee/reports/components/issue_body';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import ReportSection from '~/reports/components/report_section.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 IssueModal from './components/modal.vue';
 import securityReportsMixin from './mixins/security_report_mixin';
@@ -15,9 +15,9 @@ import { mrStates } from '~/mr_popover/constants';
 export default {
   store: createStore(),
   components: {
+    GroupedIssuesList,
     ReportSection,
     SummaryRow,
-    IssuesList,
     IssueModal,
     Icon,
     GlSprintf,
@@ -346,13 +346,13 @@ export default {
             data-qa-selector="sast_scan_report"
           />
 
-          <issues-list
+          <grouped-issues-list
             v-if="sast.newIssues.length || sast.resolvedIssues.length"
             :unresolved-issues="sast.newIssues"
             :resolved-issues="sast.resolvedIssues"
-            :all-issues="sast.allIssues"
             :component="$options.componentNames.SecurityIssueBody"
-            class="js-sast-issue-list report-block-group-list"
+            class="report-block-group-list"
+            data-testid="sast-issues-list"
           />
         </template>
 
@@ -365,12 +365,13 @@ export default {
             data-qa-selector="dependency_scan_report"
           />
 
-          <issues-list
+          <grouped-issues-list
             v-if="dependencyScanning.newIssues.length || dependencyScanning.resolvedIssues.length"
             :unresolved-issues="dependencyScanning.newIssues"
             :resolved-issues="dependencyScanning.resolvedIssues"
             :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>
 
@@ -383,12 +384,13 @@ export default {
             data-qa-selector="container_scan_report"
           />
 
-          <issues-list
+          <grouped-issues-list
             v-if="containerScanning.newIssues.length || containerScanning.resolvedIssues.length"
             :unresolved-issues="containerScanning.newIssues"
             :resolved-issues="containerScanning.resolvedIssues"
             :component="$options.componentNames.SecurityIssueBody"
             class="report-block-group-list"
+            data-testid="container-scanning-issues-list"
           />
         </template>
 
@@ -414,12 +416,13 @@ export default {
             </template>
           </summary-row>
 
-          <issues-list
+          <grouped-issues-list
             v-if="dast.newIssues.length || dast.resolvedIssues.length"
             :unresolved-issues="dast.newIssues"
             :resolved-issues="dast.resolvedIssues"
             :component="$options.componentNames.SecurityIssueBody"
             class="report-block-group-list"
+            data-testid="dast-issues-list"
           />
         </template>
 
@@ -432,12 +435,13 @@ export default {
             data-qa-selector="secret_scan_report"
           />
 
-          <issues-list
+          <grouped-issues-list
             v-if="secretScanning.newIssues.length || secretScanning.resolvedIssues.length"
             :unresolved-issues="secretScanning.newIssues"
             :resolved-issues="secretScanning.resolvedIssues"
             :component="$options.componentNames.SecurityIssueBody"
             class="report-block-group-list"
+            data-testid="secret-scanning-issues-list"
           />
         </template>
 

ee/changelogs/unreleased/214329-clarify-security-report-findings-in-merge-request-widget.yml

+---
+title: Clarify security report findings in merge request widget
+merge_request: 30688
+author:
+type: changed

ee/spec/frontend/vue_shared/security_reports/grouped_security_reports_app_spec.js

 import Vue from 'vue';
 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 state from 'ee/vue_shared/security_reports/store/state';
 import * as types from 'ee/vue_shared/security_reports/store/mutation_types';
@@ -229,13 +230,27 @@ describe('Grouped security reports app', () => {
         });
       });
 
-      it('has the success icon for fixed vulnerabilities', () => {
-        const icon = wrapper.vm.$el.querySelector(
-          '.js-container-scanning~.js-plain-element .ic-status_success_borderless',
-        );
-
-        expect(icon).not.toBeNull();
-      });
+      it.each`
+        reportType               | resolvedIssues                             | unresolvedIssues
+        ${'sast'}                | ${sastDiffSuccessMock.fixed}               | ${sastDiffSuccessMock.added}
+        ${'dependency-scanning'} | ${dependencyScanningDiffSuccessMock.fixed} | ${dependencyScanningDiffSuccessMock.added}
+        ${'container-scanning'}  | ${containerScanningDiffSuccessMock.fixed}  | ${containerScanningDiffSuccessMock.added}
+        ${'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',
+          });
+        },
+      );
     });
   });
 

locale/gitlab.pot

@@ -24951,6 +24951,9 @@ msgstr ""
 msgid "ciReport|Failed to load %{reportName} report"
 msgstr ""
 
+msgid "ciReport|Fixed"
+msgstr ""
+
 msgid "ciReport|Fixed:"
 msgstr ""
 
@@ -24969,6 +24972,9 @@ msgstr ""
 msgid "ciReport|Manage licenses"
 msgstr ""
 
+msgid "ciReport|New"
+msgstr ""
+
 msgid "ciReport|No changes to code quality"
 msgstr ""
 

spec/frontend/reports/components/__snapshots__/grouped_issues_list_spec.js.snap

+// 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,
+}
+`;

spec/frontend/reports/components/grouped_issues_list_spec.js

+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();
+    });
+  });
+});