Merge branch '12530-clarify-detected-license-results-in-mr-add-report-groups' into 'master'

Add status groups to license-compliance MR widget

See merge request gitlab-org/gitlab!28631

app/assets/stylesheets/pages/reports.scss

@@ -63,15 +63,6 @@
   list-style: none;
   padding: 0 1px;
   margin: 0;
-
-  .license-item {
-    line-height: $gl-padding-32;
-
-    .license-packages {
-      font-size: $label-font-size;
-    }
-
-  }
 }
 
 .report-block-list-icon {

ee/app/assets/javascripts/vue_shared/license_compliance/components/license_packages.vue

@@ -38,7 +38,7 @@ export default {
 };
 </script>
 <template>
-  <div class="license-packages d-inline">
+  <div class="license-packages d-inline gl-font-size-12">
     <div class="js-license-dependencies d-inline">{{ packageString }}</div>
     <button
       v-if="!showAllPackages && remainingPackages"

ee/app/assets/javascripts/vue_shared/license_compliance/constants.js

-/* eslint-disable @gitlab/require-i18n-strings */
+import { __, s__ } from '~/locale';
+
+import { STATUS_FAILED, STATUS_NEUTRAL, STATUS_SUCCESS } from '~/reports/constants';
 
 /*
  * Endpoint still returns 'approved' & 'blacklisted'
@@ -14,6 +16,7 @@ export const LICENSE_APPROVAL_ACTION = {
   DENY: 'deny',
 };
 
+/* eslint-disable @gitlab/require-i18n-strings */
 export const KNOWN_LICENSES = [
   'AGPL-1.0',
   'AGPL-3.0',
@@ -41,3 +44,22 @@ export const KNOWN_LICENSES = [
   'WTFPL',
   'Zlib',
 ];
+/* eslint-enable @gitlab/require-i18n-strings */
+
+export const REPORT_GROUPS = [
+  {
+    name: s__('LicenseManagement|Denied'),
+    description: __("Out-of-compliance with this project's policies and should be removed"),
+    status: STATUS_FAILED,
+  },
+  {
+    name: s__('LicenseManagement|Uncategorized'),
+    description: __('No policy matches this license'),
+    status: STATUS_NEUTRAL,
+  },
+  {
+    name: s__('LicenseManagement|Allowed'),
+    description: __('Acceptable for use in this project'),
+    status: STATUS_SUCCESS,
+  },
+];

ee/app/assets/javascripts/vue_shared/license_compliance/mr_widget_license_report.vue

@@ -2,13 +2,13 @@
 import { mapState, mapGetters, mapActions } from 'vuex';
 import { GlLink } from '@gitlab/ui';
 import reportsMixin from 'ee/vue_shared/security_reports/mixins/reports_mixin';
+import ReportItem from '~/reports/components/report_item.vue';
+import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
 import SetLicenseApprovalModal from 'ee/vue_shared/license_compliance/components/set_approval_status_modal.vue';
 import { componentNames } from 'ee/reports/components/issue_body';
 import Icon from '~/vue_shared/components/icon.vue';
 import ReportSection from '~/reports/components/report_section.vue';
-
 import { LICENSE_MANAGEMENT } from 'ee/vue_shared/license_compliance/store/constants';
-
 import createStore from './store';
 
 const store = createStore();
@@ -19,8 +19,10 @@ export default {
   store,
   components: {
     GlLink,
+    ReportItem,
     ReportSection,
     SetLicenseApprovalModal,
+    SmartVirtualList,
     Icon,
   },
   mixins: [reportsMixin],
@@ -64,6 +66,8 @@ export default {
       default: '',
     },
   },
+  typicalReportItemHeight: 26,
+  maxShownReportItems: 20,
   computed: {
     ...mapState(LICENSE_MANAGEMENT, ['loadLicenseReportError']),
     ...mapGetters(LICENSE_MANAGEMENT, [
@@ -71,6 +75,7 @@ export default {
       'isLoading',
       'licenseSummaryText',
       'reportContainsBlacklistedLicense',
+      'licenseReportGroups',
     ]),
     hasLicenseReportIssues() {
       const { licenseReport } = this;
@@ -119,6 +124,38 @@ export default {
       class="license-report-widget mr-report"
       data-qa-selector="license_report_widget"
     >
+      <template #body>
+        <smart-virtual-list
+          ref="reportSectionBody"
+          :size="$options.typicalReportItemHeight"
+          :length="licenseReport.length"
+          :remain="$options.maxShownReportItems"
+          class="report-block-container"
+          wtag="ul"
+          wclass="report-block-list my-1"
+        >
+          <template v-for="(licenseReportGroup, index) in licenseReportGroups">
+            <li
+              ref="reportHeading"
+              :key="licenseReportGroup.name"
+              :class="{ 'mt-3': index > 0 }"
+              class="mx-1 mb-1"
+            >
+              <h2 class="h5 m-0">{{ licenseReportGroup.name }}</h2>
+              <p class="m-0">{{ licenseReportGroup.description }}</p>
+            </li>
+            <report-item
+              v-for="license in licenseReportGroup.licenses"
+              :key="license.name"
+              :issue="license"
+              :status="license.status"
+              :component="$options.componentNames.LicenseIssueBody"
+              :show-report-section-status-icon="true"
+              class="my-1"
+            />
+          </template>
+        </smart-virtual-list>
+      </template>
       <template #success>
         <div class="pr-3">
           {{ licenseSummaryText }}

ee/app/assets/javascripts/vue_shared/license_compliance/store/getters.js

 import { n__, s__, sprintf } from '~/locale';
-import { LICENSE_APPROVAL_STATUS } from '../constants';
+import { addLicensesMatchingReportGroupStatus, reportGroupHasAtLeastOneLicense } from './utils';
+import { LICENSE_APPROVAL_STATUS, REPORT_GROUPS } from '../constants';
 
 export const isLoading = state => state.isLoadingManagedLicenses || state.isLoadingLicenseReport;
 
@@ -11,6 +12,11 @@ export const hasPendingLicenses = state => state.pendingLicenses.length > 0;
 
 export const licenseReport = state => state.newLicenses;
 
+export const licenseReportGroups = state =>
+  REPORT_GROUPS.map(addLicensesMatchingReportGroupStatus(state.newLicenses)).filter(
+    reportGroupHasAtLeastOneLicense,
+  );
+
 export const licenseSummaryText = (state, getters) => {
   const hasReportItems = getters.licenseReport && getters.licenseReport.length;
   const baseReportHasLicenses = state.existingLicenses.length;
@@ -66,7 +72,7 @@ export const licenseSummaryText = (state, getters) => {
   return s__('LicenseCompliance|License Compliance detected no new licenses');
 };
 
-export const reportContainsBlacklistedLicense = (_state, getters) =>
+export const reportContainsBlacklistedLicense = (_, getters) =>
   (getters.licenseReport || []).some(
     license => license.approvalStatus === LICENSE_APPROVAL_STATUS.DENIED,
   );

ee/app/assets/javascripts/vue_shared/license_compliance/store/utils.js

+import { groupBy } from 'lodash';
 import { LICENSE_APPROVAL_STATUS } from 'ee/vue_shared/license_compliance/constants';
 import { s__, n__, sprintf } from '~/locale';
 import { STATUS_FAILED, STATUS_NEUTRAL, STATUS_SUCCESS } from '~/reports/constants';
@@ -93,3 +94,30 @@ export const convertToOldReportFormat = license => {
     status: getIssueStatusFromLicenseStatus(approvalStatus),
   };
 };
+
+/**
+ * Takes an array of licenses and returns a function that takes an report-group objects
+ *
+ * It returns a fresh object, containing all properties of the original report-group and added "license" property,
+ * containing an array of licenses, matching the report-group's status
+ *
+ * @param {Array} licenses
+ * @returns {function(*): {licenses: (*|*[])}}
+ */
+export const addLicensesMatchingReportGroupStatus = licenses => {
+  const licensesGroupedByStatus = groupBy(licenses, 'status');
+
+  return reportGroup => ({
+    ...reportGroup,
+    licenses: licensesGroupedByStatus[reportGroup.status] || [],
+  });
+};
+
+/**
+ * Returns true of the given object has a "license" property, containing an array with at least licenses. Otherwise false.
+ *
+ *
+ * @param {Object}
+ * @returns {boolean}
+ */
+export const reportGroupHasAtLeastOneLicense = ({ licenses }) => licenses?.length > 0;

ee/changelogs/unreleased/12530-clarify-detected-license-results-in-mr.yml

+---
+title: 'Clarify detected license results in merge request: Group licenses by status'
+merge_request: 28631
+author:
+type: changed

ee/spec/frontend/vue_shared/license_compliance/__snapshots__/mr_widget_license_report_spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`License Report MR Widget report section report body should render correctly 1`] = `
+<smart-virtual-list-stub
+  class="report-block-container"
+  length="1"
+  remain="20"
+  rtag="div"
+  size="26"
+  wclass="report-block-list my-1"
+  wtag="ul"
+>
+  <li
+    class="mx-1 mb-1"
+  >
+    <h2
+      class="h5 m-0"
+    >
+      some-status group-name
+    </h2>
+     
+    <p
+      class="m-0"
+    >
+      some-status group-description
+    </p>
+  </li>
+   
+</smart-virtual-list-stub>
+`;
+
+exports[`License Report MR Widget report section should render correctly 1`] = `
+<report-section-stub
+  class="license-report-widget mr-report"
+  component="LicenseIssueBody"
+  data-qa-selector="license_report_widget"
+  errortext="FOO"
+  hasissues="true"
+  loadingtext="FOO"
+  neutralissues="[object Object]"
+  popoveroptions="[object Object]"
+  resolvedissues=""
+  showreportsectionstatusicon="true"
+  status="SUCCESS"
+  successtext=""
+  unresolvedissues=""
+>
+    
+  <div
+    class="append-right-default"
+  >
+    <a
+      class="btn btn-default btn-sm js-manage-licenses append-right-8"
+      href="http://test.host/lm_settings"
+    >
+      
+        Manage licenses
+      
+    </a>
+     
+    <a
+      class="btn btn-default btn-sm js-full-report"
+      href="http://test.host/path/to/the/full/report"
+      target="_blank"
+    >
+      
+        View full report 
+      <icon-stub
+        name="external-link"
+        size="16"
+      />
+    </a>
+  </div>
+</report-section-stub>
+`;

ee/spec/frontend/vue_shared/license_compliance/mock_data.js

+import { range } from 'lodash';
 import { LICENSE_APPROVAL_STATUS } from 'ee/vue_shared/license_compliance/constants';
 
 export const approvedLicense = {
@@ -57,4 +58,14 @@ export const licenseReport = [
   },
 ];
 
+export const generateReportGroup = ({ status = 'some-status', numberOfLicenses = 0 } = {}) => ({
+  status,
+  name: `${status} group-name`,
+  description: `${status} group-description`,
+  licenses: range(numberOfLicenses).map(i => ({
+    name: `${status} license-name-${i}`,
+    status,
+  })),
+});
+
 export default () => {};

ee/spec/frontend/vue_shared/license_compliance/store/getters_spec.js

@@ -91,6 +91,59 @@ describe('getters', () => {
     });
   });
 
+  describe('licenseReportGroups', () => {
+    it('returns an array of objects containing information about the group and licenses', () => {
+      const licensesSuccess = [
+        { status: 'success', value: 'foo' },
+        { status: 'success', value: 'bar' },
+      ];
+      const licensesNeutral = [
+        { status: 'neutral', value: 'foo' },
+        { status: 'neutral', value: 'bar' },
+      ];
+      const licensesFailed = [
+        { status: 'failed', value: 'foo' },
+        { status: 'failed', value: 'bar' },
+      ];
+      const newLicenses = [...licensesSuccess, ...licensesNeutral, ...licensesFailed];
+
+      expect(getters.licenseReportGroups({ newLicenses })).toEqual([
+        {
+          name: 'Denied',
+          description: `Out-of-compliance with this project's policies and should be removed`,
+          status: 'failed',
+          licenses: licensesFailed,
+        },
+        {
+          name: 'Uncategorized',
+          description: 'No policy matches this license',
+          status: 'neutral',
+          licenses: licensesNeutral,
+        },
+        {
+          name: 'Allowed',
+          description: 'Acceptable for use in this project',
+          status: 'success',
+          licenses: licensesSuccess,
+        },
+      ]);
+    });
+
+    it.each(['failed', 'neutral', 'success'])(
+      `it filters report-groups that don't have the given status: %s`,
+      status => {
+        const newLicenses = [{ status }];
+
+        expect(getters.licenseReportGroups({ newLicenses })).toEqual([
+          expect.objectContaining({
+            status,
+            licenses: newLicenses,
+          }),
+        ]);
+      },
+    );
+  });
+
   describe('licenseSummaryText', () => {
     describe('when licenses exist on both the HEAD and the BASE', () => {
       beforeEach(() => {

ee/spec/frontend/vue_shared/license_compliance/store/utils_spec.js

@@ -4,6 +4,8 @@ import {
   getStatusTranslationsFromLicenseStatus,
   getIssueStatusFromLicenseStatus,
   convertToOldReportFormat,
+  addLicensesMatchingReportGroupStatus,
+  reportGroupHasAtLeastOneLicense,
 } from 'ee/vue_shared/license_compliance/store/utils';
 import { LICENSE_APPROVAL_STATUS } from 'ee/vue_shared/license_compliance/constants';
 import { licenseReport } from '../mock_data';
@@ -111,4 +113,55 @@ describe('utils', () => {
       expect(parsedLicense.name).toEqual(rawLicense.name);
     });
   });
+
+  describe('addLicensesMatchingReportGroupStatus', () => {
+    describe('with matching licenses', () => {
+      it(`adds a "licenses" property containing an array of licenses matching the report's status to the report object`, () => {
+        const licenses = [
+          { status: 'match' },
+          { status: 'no-match' },
+          { status: 'match' },
+          { status: 'no-match' },
+        ];
+        const reportGroup = { description: 'description', status: 'match' };
+
+        expect(addLicensesMatchingReportGroupStatus(licenses)(reportGroup)).toEqual({
+          ...reportGroup,
+          licenses: [licenses[0], licenses[2]],
+        });
+      });
+    });
+
+    describe('without matching licenses', () => {
+      it('adds a "licenses" property containing an empty array to the report object', () => {
+        const licenses = [
+          { status: 'no-match' },
+          { status: 'no-match' },
+          { status: 'no-match' },
+          { status: 'no-match' },
+        ];
+        const reportGroup = { description: 'description', status: 'match' };
+
+        expect(addLicensesMatchingReportGroupStatus(licenses)(reportGroup)).toEqual({
+          ...reportGroup,
+          licenses: [],
+        });
+      });
+    });
+  });
+
+  describe('reportGroupHasAtLeastOneLicense', () => {
+    it.each`
+      givenReportGroup                   | expected
+      ${{ licenses: [{ foo: 'foo ' }] }} | ${true}
+      ${{ licenses: [] }}                | ${false}
+      ${{ licenses: null }}              | ${false}
+      ${{ licenses: undefined }}         | ${false}
+    `(
+      'returns "$expected" if the given report-group contains $licenses.length licenses',
+      ({ givenReportGroup, expected }) => {
+        expect(reportGroupHasAtLeastOneLicense(givenReportGroup)).toBe(expected);
+      },
+    );
+  });
 });

locale/gitlab.pot

@@ -931,6 +931,9 @@ msgstr ""
 msgid "Accept terms"
 msgstr ""
 
+msgid "Acceptable for use in this project"
+msgstr ""
+
 msgid "Accepted MR"
 msgstr ""
 
@@ -12014,6 +12017,15 @@ msgstr ""
 msgid "LicenseCompliance|You are about to remove the license, %{name}, from this project."
 msgstr ""
 
+msgid "LicenseManagement|Allowed"
+msgstr ""
+
+msgid "LicenseManagement|Denied"
+msgstr ""
+
+msgid "LicenseManagement|Uncategorized"
+msgstr ""
+
 msgid "Licensed Features"
 msgstr ""
 
@@ -13576,6 +13588,9 @@ msgstr ""
 msgid "No pods available"
 msgstr ""
 
+msgid "No policy matches this license"
+msgstr ""
+
 msgid "No preview for this file type"
 msgstr ""
 
@@ -14106,6 +14121,9 @@ msgstr ""
 msgid "Other visibility settings have been disabled by the administrator."
 msgstr ""
 
+msgid "Out-of-compliance with this project's policies and should be removed"
+msgstr ""
+
 msgid "Outbound requests"
 msgstr ""