License-Check approval UI support

Add missing import

Make tooltip unique

Add support for reports_approval type in UI

Additional UI

- Show report_approval type of approval rules in MR iwdget text
* Add toolips to report approval types in the approvals section
* Add tooltip help links from rails view helper

Refactor approver check popover to use in MR widget

* Decouple from project list vue component

* Refactor approver check component and decouple it from

Add tooltips to MR approvals widget

Add path helper for approval rules docs

Run prettier, linter, add changelog

Update Pot file and fix unit test errors

Add additional unit tests

Fix haml linter errors

Tweak tooltip layout

Update approval_check_popover to use config

* Refactor to use config object instead of several computed props

Refactor popover to not use internal _uuid

Run prettier

app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -168,6 +168,7 @@ export default class MergeRequestStore {
     this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path;
     this.conflictsDocsPath = data.conflicts_docs_path;
     this.ciEnvironmentsStatusPath = data.ci_environments_status_path;
+    this.securityApprovalsHelpPagePath = data.security_approvals_help_page_path;
   }
 
   get isNothingToMergeState() {

app/views/projects/merge_requests/show.html.haml

@@ -24,6 +24,7 @@
 
         window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
         window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/index.md', anchor: 'troubleshooting')}';
+        window.gl.mrWidgetData.security_approvals_help_page_path = '#{help_page_path('user/application_security/index.html', anchor: 'security-approvals-in-merge-requests-ultimate')}';
 
     #js-vue-mr-widget.mr-widget
 

ee/app/assets/javascripts/approvals/components/vulnerability_check_popover.vue → ee/app/assets/javascripts/approvals/components/approval_check_popover.vue

@@ -3,7 +3,7 @@ import { GlLink, GlPopover } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
 
 export default {
-  name: 'VulnerabilityCheckPopover',
+  name: 'ApprovalCheckPopover',
   components: {
     GlLink,
     GlPopover,
@@ -15,25 +15,42 @@ export default {
       required: false,
       default: '',
     },
+    documentationText: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    title: {
+      type: String,
+      required: true,
+    },
+    text: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
 };
 </script>
 
 <template>
   <span class="vertical-align-middle text-muted js-help">
-    <icon id="reports-info" name="question" :aria-label="__('help')" :size="14" />
+    <icon
+      ref="reportInfo"
+      name="question"
+      css-classes="text-info"
+      :aria-label="__('help')"
+      :size="14"
+    />
     <gl-popover
-      target="reports-info"
+      :target="() => $refs.reportInfo"
       placement="top"
-      triggers="click"
-      :title="
-        __(
-          'Vulnerability-Check requires one or more merge request approvals only if high or critical security vulnerabilities are detected.',
-        )
-      "
+      triggers="click blur"
+      :title="title"
     >
+      <div class="mb-2">{{ text }}</div>
       <gl-link v-if="documentationLink" target="_blank" :href="documentationLink">
-        <span class="vertical-align-middle">{{ __('Learn more about vulnerability check') }}</span>
+        <span class="vertical-align-middle">{{ documentationText }}</span>
         <icon name="external-link" class="vertical-align-middle" />
       </gl-link>
     </gl-popover>

ee/app/assets/javascripts/approvals/components/approval_check_rule_popover.vue

+<script>
+import ApprovalCheckPopover from './approval_check_popover.vue';
+import { VULNERABILITY_CHECK_NAME, LICENSE_CHECK_NAME, APPROVAL_RULE_CONFIGS } from '../constants';
+
+export default {
+  name: 'ApprovalCheckRulePopover',
+  components: {
+    ApprovalCheckPopover,
+  },
+  props: {
+    rule: {
+      type: Object,
+      required: true,
+    },
+    securityApprovalsHelpPagePath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  computed: {
+    showVulnerabilityCheckPopover() {
+      return this.rule.name === VULNERABILITY_CHECK_NAME;
+    },
+    showLicenseCheckPopover() {
+      return this.rule.name === LICENSE_CHECK_NAME;
+    },
+    showApprovalCheckPopover() {
+      return this.showVulnerabilityCheckPopover || this.showLicenseCheckPopover;
+    },
+    approvalRuleConfig() {
+      return APPROVAL_RULE_CONFIGS[this.rule.name];
+    },
+    documentationLink() {
+      /*
+       * The docs for these two rules have the same url & anchor
+       * We get the path from a rails view helper
+       */
+      if (this.showLicenseCheckPopover || this.showApprovalCheckPopover) {
+        return this.securityApprovalsHelpPagePath;
+      }
+      return '';
+    },
+  },
+};
+</script>
+
+<template>
+  <approval-check-popover
+    v-if="showApprovalCheckPopover"
+    :title="approvalRuleConfig.title"
+    :text="approvalRuleConfig.popoverText"
+    :documentation-link="documentationLink"
+    :documentation-text="approvalRuleConfig.documentationText"
+  />
+</template>

ee/app/assets/javascripts/approvals/components/project_settings/project_rules.vue

@@ -3,19 +3,17 @@ import { mapState } from 'vuex';
 import { n__, sprintf } from '~/locale';
 import Icon from '~/vue_shared/components/icon.vue';
 import UserAvatarList from '~/vue_shared/components/user_avatar/user_avatar_list.vue';
-import VulnerabilityCheckPopover from '../vulnerability_check_popover.vue';
+import ApprovalCheckRulePopover from '../approval_check_rule_popover.vue';
 import Rules from '../rules.vue';
 import RuleControls from '../rule_controls.vue';
 
-const VULNERABILITY_CHECK_NAME = 'Vulnerability-Check';
-
 export default {
   components: {
     Icon,
     RuleControls,
     Rules,
     UserAvatarList,
-    VulnerabilityCheckPopover,
+    ApprovalCheckRulePopover,
   },
   computed: {
     ...mapState(['settings']),
@@ -58,9 +56,6 @@ export default {
         { name: rule.name, count: rule.approvalsRequired },
       );
     },
-    showVulnerabilityCheckPopover(rule) {
-      return rule.name === VULNERABILITY_CHECK_NAME;
-    },
   },
 };
 </script>
@@ -69,7 +64,7 @@ export default {
   <rules :rules="rules">
     <template slot="thead" slot-scope="{ name, members, approvalsRequired }">
       <tr class="d-none d-sm-table-row">
-        <th v-if="settings.allowMultiRule">{{ name }}</th>
+        <th v-if="settings.allowMultiRule" class="w-25">{{ name }}</th>
         <th class="w-50">{{ members }}</th>
         <th>{{ approvalsRequired }}</th>
         <th></th>
@@ -79,7 +74,10 @@ export default {
       <td class="d-table-cell d-sm-none js-summary">{{ summaryText(rule) }}</td>
       <td v-if="settings.allowMultiRule" class="d-none d-sm-table-cell js-name">
         {{ rule.name }}
-        <vulnerability-check-popover v-if="showVulnerabilityCheckPopover(rule)" />
+        <approval-check-rule-popover
+          :rule="rule"
+          :security-approvals-help-page-path="settings.securityApprovalsHelpPagePath"
+        />
       </td>
       <td class="d-none d-sm-table-cell js-members">
         <user-avatar-list :items="rule.approvers" :img-size="24" />

ee/app/assets/javascripts/approvals/constants.js

+import { __ } from '~/locale';
+
 export const TYPE_USER = 'user';
 export const TYPE_GROUP = 'group';
 export const TYPE_HIDDEN_GROUPS = 'hidden_groups';
 
 export const RULE_TYPE_FALLBACK = 'fallback';
 export const RULE_TYPE_REGULAR = 'regular';
+export const RULE_TYPE_REPORT_APPROVER = 'report_approver';
 export const RULE_TYPE_CODE_OWNER = 'code_owner';
+
+export const VULNERABILITY_CHECK_NAME = 'Vulnerability-Check';
+export const LICENSE_CHECK_NAME = 'License-Check';
+
+export const APPROVAL_RULE_CONFIGS = {
+  [VULNERABILITY_CHECK_NAME]: {
+    title: __('Vulnerability-Check'),
+    popoverText: __(
+      'A merge request approval is required when a security report contains a new vulnerability of high, critical, or unknown severity.',
+    ),
+    documentationText: __('Learn more about Vulnerability-Check'),
+  },
+  [LICENSE_CHECK_NAME]: {
+    title: __('License-Check'),
+    popoverText: __(
+      'A merge request approval is required when the license compliance report contains a blacklisted license.',
+    ),
+    documentationText: __('Learn more about License-Check'),
+  },
+};

ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue

@@ -242,6 +242,7 @@ export default {
       :suggested-approvers="approvals.suggested_approvers"
       :approval-rules="mr.approvalRules"
       :is-loading-rules="isLoadingRules"
+      :security-approvals-help-page-path="mr.securityApprovalsHelpPagePath"
     />
   </mr-widget-container>
 </template>

ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_footer.vue

@@ -32,6 +32,11 @@ export default {
       required: false,
       default: false,
     },
+    securityApprovalsHelpPagePath: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     isCollapsed() {
@@ -76,7 +81,10 @@ export default {
       </template>
     </div>
     <div v-if="!isCollapsed && approvalRules.length" class="border-top">
-      <approvals-list :approval-rules="approvalRules" />
+      <approvals-list
+        :approval-rules="approvalRules"
+        :security-approvals-help-page-path="securityApprovalsHelpPagePath"
+      />
     </div>
   </div>
 </template>

ee/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_list.vue

@@ -2,6 +2,7 @@
 import _ from 'underscore';
 import { sprintf, __ } from '~/locale';
 import UserAvatarList from '~/vue_shared/components/user_avatar/user_avatar_list.vue';
+import ApprovalCheckRulePopover from 'ee/approvals/components/approval_check_rule_popover.vue';
 import { RULE_TYPE_CODE_OWNER } from 'ee/approvals/constants';
 import ApprovedIcon from './approved_icon.vue';
 
@@ -9,12 +10,18 @@ export default {
   components: {
     UserAvatarList,
     ApprovedIcon,
+    ApprovalCheckRulePopover,
   },
   props: {
     approvalRules: {
       type: Array,
       required: true,
     },
+    securityApprovalsHelpPagePath: {
+      type: String,
+      required: false,
+      default: '',
+    },
   },
   computed: {
     sections() {
@@ -72,7 +79,7 @@ export default {
     <thead class="thead-white text-nowrap">
       <tr class="d-none d-sm-table-row">
         <th class="w-0"></th>
-        <th>{{ s__('MRApprovals|Approvers') }}</th>
+        <th class="w-25">{{ s__('MRApprovals|Approvers') }}</th>
         <th class="w-50"></th>
         <th>{{ s__('MRApprovals|Pending approvals') }}</th>
         <th>{{ s__('MRApprovals|Approved by') }}</th>
@@ -88,7 +95,13 @@ export default {
       <tr v-for="rule in rules" :key="rule.id">
         <td class="w-0"><approved-icon :is-approved="rule.approved" /></td>
         <td :colspan="rule.fallback ? 2 : 1">
-          <div class="d-none d-sm-block js-name" :class="rule.nameClass">{{ rule.name }}</div>
+          <div class="d-none d-sm-block js-name" :class="rule.nameClass">
+            {{ rule.name }}
+            <approval-check-rule-popover
+              :rule="rule"
+              :security-approvals-help-page-path="securityApprovalsHelpPagePath"
+            />
+          </div>
           <div class="d-flex d-sm-none flex-column js-summary">
             <span>{{ summaryText(rule) }}</span>
             <user-avatar-list

ee/app/assets/javascripts/vue_merge_request_widget/mappers.js

@@ -4,6 +4,7 @@ import {
   RULE_TYPE_REGULAR,
   RULE_TYPE_FALLBACK,
   RULE_TYPE_CODE_OWNER,
+  RULE_TYPE_REPORT_APPROVER,
 } from 'ee/approvals/constants';
 
 function mapApprovalRule(rule, settings) {
@@ -35,11 +36,14 @@ function getApprovalRuleNamesLeft(data) {
   // Filter out empty names (fallback rule has no name) because the empties would look weird.
   const regularRules = (rulesLeft[RULE_TYPE_REGULAR] || []).map(x => x.name).filter(x => x);
 
+  // Report Approvals
+  const reportApprovalRules = (rulesLeft[RULE_TYPE_REPORT_APPROVER] || []).map(x => x.name);
+
   // If there are code owners that need to approve, only mention that once.
   // As the names of code owner rules are patterns that don't mean much out of context.
   const codeOwnerRules = rulesLeft[RULE_TYPE_CODE_OWNER] ? [__('Code Owners')] : [];
 
-  return [...regularRules, ...codeOwnerRules];
+  return [...regularRules, ...reportApprovalRules, ...codeOwnerRules];
 }
 
 /**

ee/app/views/projects/_merge_request_approvals_settings_form.html.haml

@@ -7,7 +7,8 @@
     'project_path': expose_path(api_v4_projects_path(id: @project.id)),
     'settings_path': expose_path(api_v4_projects_approval_settings_path(id: @project.id)),
     'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: @project.id)),
-    'allow_multi_rule': @project.multiple_approval_rules_available?.to_s } }
+    'allow_multi_rule': @project.multiple_approval_rules_available?.to_s,
+    'security_approvals_help_page_path': help_page_path('user/application_security/index.html', anchor: 'security-approvals-in-merge-requests-ultimate')} }
     .text-center.prepend-top-default
       = sprite_icon('spinner', size: 24, css_class: 'gl-spinner')
 

ee/changelogs/unreleased/blacklist-license-approval.yml

+---
+title: Add License-Check approval UI
+merge_request: 16371
+author:
+type: added

ee/spec/frontend/approvals/components/approval_check_rule_popover_spec.js

+import Vue from 'vue';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import { TEST_HOST } from 'helpers/test_constants';
+import component from 'ee/approvals/components/approval_check_rule_popover.vue';
+import {
+  VULNERABILITY_CHECK_NAME,
+  LICENSE_CHECK_NAME,
+  APPROVAL_RULE_CONFIGS,
+} from 'ee/approvals/constants';
+
+describe('Approval Check Popover', () => {
+  let wrapper;
+
+  beforeEach(() => {
+    const localVue = createLocalVue();
+    wrapper = shallowMount(component, {
+      localVue,
+      propsData: { rule: {} },
+      sync: false,
+    });
+  });
+
+  describe('computed props', () => {
+    const securityApprovalsHelpPagePath = `${TEST_HOST}/documentation`;
+
+    beforeEach(done => {
+      wrapper.setProps({ securityApprovalsHelpPagePath });
+      Vue.nextTick(done);
+    });
+
+    describe('showVulnerabilityCheckPopover', () => {
+      it('return true if the rule type is "Vulnerability-Check"', done => {
+        wrapper.setProps({ rule: { name: VULNERABILITY_CHECK_NAME } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.showVulnerabilityCheckPopover).toBe(true);
+          done();
+        });
+      });
+      it('return false if the rule type is "Vulnerability-Check"', done => {
+        wrapper.setProps({ rule: { name: 'FooRule' } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.showVulnerabilityCheckPopover).toBe(false);
+          done();
+        });
+      });
+    });
+
+    describe('showLicenseCheckPopover', () => {
+      it('return true if the rule type is "License-Check"', done => {
+        wrapper.setProps({ rule: { name: LICENSE_CHECK_NAME } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.showLicenseCheckPopover).toBe(true);
+          done();
+        });
+      });
+      it('return false if the rule type is "License-Check"', done => {
+        wrapper.setProps({ rule: { name: 'FooRule' } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.showLicenseCheckPopover).toBe(false);
+          done();
+        });
+      });
+    });
+
+    describe('approvalConfig', () => {
+      it('returns "Vulberability-Check" config', done => {
+        wrapper.setProps({ rule: { name: VULNERABILITY_CHECK_NAME } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.approvalRuleConfig.title).toBe(
+            APPROVAL_RULE_CONFIGS[VULNERABILITY_CHECK_NAME].title,
+          );
+          expect(wrapper.vm.approvalRuleConfig.popoverText).toBe(
+            APPROVAL_RULE_CONFIGS[VULNERABILITY_CHECK_NAME].popoverText,
+          );
+          expect(wrapper.vm.approvalRuleConfig.documentationText).toBe(
+            APPROVAL_RULE_CONFIGS[VULNERABILITY_CHECK_NAME].documentationText,
+          );
+          done();
+        });
+      });
+      it('returns "License-Check" config', done => {
+        wrapper.setProps({ rule: { name: LICENSE_CHECK_NAME } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.approvalRuleConfig.title).toBe(
+            APPROVAL_RULE_CONFIGS[LICENSE_CHECK_NAME].title,
+          );
+          expect(wrapper.vm.approvalRuleConfig.popoverText).toBe(
+            APPROVAL_RULE_CONFIGS[LICENSE_CHECK_NAME].popoverText,
+          );
+          expect(wrapper.vm.approvalRuleConfig.documentationText).toBe(
+            APPROVAL_RULE_CONFIGS[LICENSE_CHECK_NAME].documentationText,
+          );
+          done();
+        });
+      });
+      it('returns an undefined config', done => {
+        wrapper.setProps({ rule: { name: 'FooRule' } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.approvalConfig).toBe(undefined);
+          done();
+        });
+      });
+    });
+
+    describe('documentationLink', () => {
+      it('returns documentation link for "License-Check"', done => {
+        wrapper.setProps({ rule: { name: 'License-Check' } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.documentationLink).toBe(securityApprovalsHelpPagePath);
+          done();
+        });
+      });
+      it('returns documentation link for "Vulnerability-Check"', done => {
+        wrapper.setProps({ rule: { name: 'Vulnerability-Check' } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.documentationLink).toBe(securityApprovalsHelpPagePath);
+          done();
+        });
+      });
+      it('returns empty text', done => {
+        const text = '';
+        wrapper.setProps({ rule: { name: 'FooRule' } });
+        Vue.nextTick(() => {
+          expect(wrapper.vm.documentationLink).toBe(text);
+          done();
+        });
+      });
+    });
+  });
+});

ee/spec/frontend/vue_mr_widget/mappers_spec.js

 import { mapApprovalsResponse } from 'ee/vue_merge_request_widget/mappers';
-import { RULE_TYPE_REGULAR, RULE_TYPE_CODE_OWNER } from 'ee/approvals/constants';
+import {
+  RULE_TYPE_REGULAR,
+  RULE_TYPE_CODE_OWNER,
+  RULE_TYPE_REPORT_APPROVER,
+} from 'ee/approvals/constants';
 
 describe('EE MR Widget mappers', () => {
   let data;
@@ -30,6 +34,21 @@ describe('EE MR Widget mappers', () => {
         );
       });
 
+      it('approvalRuleNamesLeft includes report approvers', () => {
+        data.approval_rules_left.push(
+          { name: 'License-Check', rule_type: RULE_TYPE_REPORT_APPROVER },
+          { name: 'Vulnerability-Check', rule_type: RULE_TYPE_REPORT_APPROVER },
+        );
+
+        const result = mapApprovalsResponse(data);
+
+        expect(result).toEqual(
+          jasmine.objectContaining({
+            approvalRuleNamesLeft: ['Lorem', 'Ipsum', 'License-Check', 'Vulnerability-Check'],
+          }),
+        );
+      });
+
       it('approvalRuleNamesLeft includes "Code Owners" if any', () => {
         data.approval_rules_left.push(
           { name: 'src/foo', rule_type: RULE_TYPE_CODE_OWNER },

ee/spec/javascripts/approvals/components/vulnerability_check_popover_spec.js → ee/spec/javascripts/approvals/components/approval_check_popover_spec.js

@@ -2,15 +2,18 @@ import Vue from 'vue';
 import { mount, createLocalVue } from '@vue/test-utils';
 import { GlPopover } from '@gitlab/ui';
 import { TEST_HOST } from 'spec/test_constants';
-import component from 'ee/approvals/components/vulnerability_check_popover.vue';
+import component from 'ee/approvals/components/approval_check_popover.vue';
 
-describe('Vulnerability Check Popover', () => {
+describe('Approval Check Popover', () => {
   let wrapper;
 
   beforeEach(() => {
     const localVue = createLocalVue();
     wrapper = mount(component, {
       localVue,
+      propsData: {
+        title: 'Title',
+      },
       sync: false,
     });
   });

locale/gitlab.pot

@@ -653,6 +653,12 @@ msgstr ""
 msgid "A member of the abuse team will review your report as soon as possible."
 msgstr ""
 
+msgid "A merge request approval is required when a security report contains a new vulnerability of high, critical, or unknown severity."
+msgstr ""
+
+msgid "A merge request approval is required when the license compliance report contains a blacklisted license."
+msgstr ""
+
 msgid "A new branch will be created in your fork and a new merge request will be started."
 msgstr ""
 
@@ -9419,6 +9425,12 @@ msgstr ""
 msgid "Learn more about Kubernetes"
 msgstr ""
 
+msgid "Learn more about License-Check"
+msgstr ""
+
+msgid "Learn more about Vulnerability-Check"
+msgstr ""
+
 msgid "Learn more about Web Terminal"
 msgstr ""
 
@@ -9443,9 +9455,6 @@ msgstr ""
 msgid "Learn more about the dependency list"
 msgstr ""
 
-msgid "Learn more about vulnerability check"
-msgstr ""
-
 msgid "Learn more in the"
 msgstr ""
 
@@ -9482,6 +9491,9 @@ msgstr ""
 msgid "License Compliance"
 msgstr ""
 
+msgid "License-Check"
+msgstr ""
+
 msgid "LicenseCompliance|Add a license"
 msgstr ""
 
@@ -18151,7 +18163,7 @@ msgstr ""
 msgid "Vulnerabilities over time"
 msgstr ""
 
-msgid "Vulnerability-Check requires one or more merge request approvals only if high or critical security vulnerabilities are detected."
+msgid "Vulnerability-Check"
 msgstr ""
 
 msgid "VulnerabilityChart|%{formattedStartDate} to today"