Merge branch '209990-refactor-vulnerability-header-footer' into 'master'

Refactor vulnerability header and footer

See merge request gitlab-org/gitlab!28651

ee/app/assets/javascripts/pages/projects/security/vulnerabilities/show/index.js

 import Vue from 'vue';
-import HeaderApp from 'ee/vulnerabilities/components/app.vue';
+import HeaderApp from 'ee/vulnerabilities/components/header.vue';
 import FooterApp from 'ee/vulnerabilities/components/footer.vue';
 
+function createHeaderApp() {
+  const el = document.getElementById('js-vulnerability-header');
+  const initialVulnerability = JSON.parse(el.dataset.vulnerabilityJson);
+  const pipeline = JSON.parse(el.dataset.pipelineJson);
+  const finding = JSON.parse(el.dataset.findingJson);
+
+  const { projectFingerprint, createIssueUrl } = el.dataset;
+
+  return new Vue({
+    el,
+
+    render: h =>
+      h(HeaderApp, {
+        props: {
+          initialVulnerability,
+          finding,
+          pipeline,
+          projectFingerprint,
+          createIssueUrl,
+        },
+      }),
+  });
+}
+
 function createFooterApp() {
   const el = document.getElementById('js-vulnerability-footer');
 
@@ -43,30 +67,6 @@ function createFooterApp() {
   });
 }
 
-function createHeaderApp() {
-  const el = document.getElementById('js-vulnerability-management-app');
-  const initialVulnerability = JSON.parse(el.dataset.vulnerabilityJson);
-  const pipeline = JSON.parse(el.dataset.pipelineJson);
-  const finding = JSON.parse(el.dataset.findingJson);
-
-  const { projectFingerprint, createIssueUrl } = el.dataset;
-
-  return new Vue({
-    el,
-
-    render: h =>
-      h(HeaderApp, {
-        props: {
-          initialVulnerability,
-          finding,
-          pipeline,
-          projectFingerprint,
-          createIssueUrl,
-        },
-      }),
-  });
-}
-
 window.addEventListener('DOMContentLoaded', () => {
   createHeaderApp();
   createFooterApp();

ee/app/assets/javascripts/vue_shared/security_reports/components/dismissal_note.vue

@@ -77,12 +77,12 @@ export default {
       return [
         {
           iconName: 'pencil',
-          emit: 'editVulnerabilityDismissalComment',
+          onClick: () => this.$emit('editVulnerabilityDismissalComment'),
           title: __('Edit Comment'),
         },
         {
           iconName: 'remove',
-          emit: 'showDismissalDeleteButtons',
+          onClick: () => this.$emit('showDismissalDeleteButtons'),
           title: __('Delete Comment'),
         },
       ];
@@ -97,7 +97,7 @@ export default {
       :author="feedback.author"
       :created-at="feedback.created_at"
       icon-name="cancel"
-      icon-style="ci-status-icon-pending"
+      icon-class="ci-status-icon-pending"
     >
       <div v-if="feedback.created_at" v-html="eventText"></div>
     </event-item>
@@ -110,15 +110,11 @@ export default {
         :show-right-slot="isShowingDeleteButtons"
         :show-action-buttons="showDismissalCommentActions"
         icon-name="comment"
-        icon-style="ci-status-icon-pending"
-        @editVulnerabilityDismissalComment="$emit('editVulnerabilityDismissalComment')"
-        @showDismissalDeleteButtons="$emit('showDismissalDeleteButtons')"
-        @hideDismissalDeleteButtons="$emit('hideDismissalDeleteButtons')"
-        @deleteDismissalComment="$emit('deleteDismissalComment')"
+        icon-class="ci-status-icon-pending"
       >
         {{ commentDetails.comment }}
 
-        <template v-slot:right-content>
+        <template #right-content>
           <div class="d-flex flex-grow-1 align-self-start flex-row-reverse">
             <loading-button
               :label="__('Delete comment')"

ee/app/assets/javascripts/vue_shared/security_reports/components/event_item.vue

@@ -28,7 +28,7 @@ export default {
       required: false,
       default: 'plus',
     },
-    iconStyle: {
+    iconClass: {
       type: String,
       required: false,
       default: 'ci-status-icon-success',
@@ -54,16 +54,16 @@ export default {
 
 <template>
   <div class="d-flex align-items-center">
-    <div class="circle-icon-container" :class="iconStyle">
+    <div class="circle-icon-container" :class="iconClass">
       <icon :size="16" :name="iconName" />
     </div>
-    <div class="ml-3" data-qa-selector="event_item_content">
+    <div class="ml-3 flex-grow-1" data-qa-selector="event_item_content">
       <div class="note-header-info pb-0">
         <a
           :href="author.path"
           :data-user-id="author.id"
           :data-username="author.username"
-          class="js-author"
+          class="js-author js-user-link"
         >
           <strong class="note-header-author-name">{{ author.name }}</strong>
           <span v-if="author.status_tooltip_html" v-html="author.status_tooltip_html"></span>
@@ -81,21 +81,18 @@ export default {
 
     <slot v-if="showRightSlot" name="right-content"></slot>
 
-    <div v-else class="d-flex flex-grow-1 align-self-start flex-row-reverse">
-      <div v-if="showActionButtons" class="action-buttons">
-        <gl-deprecated-button
-          v-for="button in actionButtons"
-          :key="button.title"
-          ref="button"
-          v-gl-tooltip
-          class="px-1"
-          variant="transparent"
-          :title="button.title"
-          @click="$emit(button.emit)"
-        >
-          <icon :name="button.iconName" class="link-highlight" />
-        </gl-deprecated-button>
-      </div>
+    <div v-else-if="showActionButtons" class="align-self-start">
+      <gl-deprecated-button
+        v-for="button in actionButtons"
+        :key="button.title"
+        v-gl-tooltip
+        class="px-1"
+        variant="transparent"
+        :title="button.title"
+        @click="button.onClick"
+      >
+        <icon :name="button.iconName" class="link-highlight" />
+      </gl-deprecated-button>
     </div>
   </div>
 </template>

ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue

@@ -75,11 +75,9 @@ export default {
       return Boolean(!this.isResolved && this.canDismissVulnerability);
     },
     canDownloadPatchForThisVulnerability() {
-      const remediationDiff = this.remediation && this.remediation.diff;
       return Boolean(
         !this.isResolved &&
-          remediationDiff &&
-          remediationDiff.length > 0 &&
+          this.remediation?.diff?.length > 0 &&
           (!this.vulnerability.hasMergeRequest && this.remediation),
       );
     },
@@ -89,47 +87,34 @@ export default {
     hasRemediation() {
       return Boolean(this.remediation);
     },
-    hasDismissedBy() {
-      return (
-        this.vulnerability &&
-        this.vulnerability.dismissalFeedback &&
-        this.vulnerability.dismissalFeedback.pipeline &&
-        this.vulnerability.dismissalFeedback.author
-      );
-    },
     project() {
       return this.modal.project;
     },
     solution() {
-      return this.vulnerability && this.vulnerability.solution;
+      return this.vulnerability?.solution;
     },
     remediation() {
-      return (
-        this.vulnerability && this.vulnerability.remediations && this.vulnerability.remediations[0]
-      );
+      return this.vulnerability?.remediations?.[0];
     },
     vulnerability() {
       return this.modal.vulnerability;
     },
     issueFeedback() {
-      return this.vulnerability && this.vulnerability.issue_feedback;
+      return this.vulnerability?.issue_feedback;
     },
     canReadIssueFeedback() {
-      return this.issueFeedback && this.issueFeedback.issue_url;
+      return this.issueFeedback?.issue_url;
     },
     mergeRequestFeedback() {
-      return this.vulnerability && this.vulnerability.merge_request_feedback;
+      return this.vulnerability?.merge_request_feedback;
     },
     canReadMergeRequestFeedback() {
-      return this.mergeRequestFeedback && this.mergeRequestFeedback.merge_request_path;
+      return this.mergeRequestFeedback?.merge_request_path;
     },
     dismissalFeedback() {
-      return (
-        this.vulnerability &&
-        // grouped security reports are populating `dismissalFeedback` and the dashboards `dismissal_feedback`
-        // https://gitlab.com/gitlab-org/gitlab/issues/207489 aims to use the same property in all cases
-        (this.vulnerability.dismissalFeedback || this.vulnerability.dismissal_feedback)
-      );
+      // grouped security reports are populating `dismissalFeedback` and the dashboards `dismissal_feedback`
+      // https://gitlab.com/gitlab-org/gitlab/issues/207489 aims to use the same property in all cases
+      return this.vulnerability?.dismissalFeedback || this.vulnerability?.dismissal_feedback;
     },
     isEditingExistingFeedback() {
       return this.dismissalFeedback && this.modal.isCommentingOnDismissal;
@@ -158,6 +143,24 @@ export default {
         },
       };
     },
+    dismissalFeedbackComment() {
+      return this.dismissalFeedback?.comment_details?.comment;
+    },
+    showFeedbackNotes() {
+      return (
+        (this.canReadIssueFeedback || this.canReadMergeRequestFeedback) &&
+        (this.issueFeedback || this.mergeRequestFeedback)
+      );
+    },
+    showDismissalCard() {
+      return this.dismissalFeedback || this.modal.isCommentingOnDismissal;
+    },
+    showDismissalCommentActions() {
+      return !this.dismissalFeedback?.comment_details || !this.isEditingExistingFeedback;
+    },
+    showDismissalCommentTextbox() {
+      return !this.dismissalFeedback?.comment_details || this.isEditingExistingFeedback;
+    },
   },
   methods: {
     handleDismissalCommentSubmission() {
@@ -209,51 +212,43 @@ export default {
         :vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
       />
 
-      <ul v-if="canReadIssueFeedback || canReadMergeRequestFeedback" class="notes card my-4">
-        <li v-if="issueFeedback" class="note">
-          <div class="card-body">
-            <issue-note :feedback="issueFeedback" :project="project" />
-          </div>
-        </li>
-        <li v-if="mergeRequestFeedback" class="note">
-          <div class="card-body">
-            <merge-request-note :feedback="mergeRequestFeedback" :project="project" />
-          </div>
-        </li>
-      </ul>
+      <div v-if="showFeedbackNotes" class="card my-4">
+        <issue-note
+          v-if="issueFeedback"
+          :feedback="issueFeedback"
+          :project="project"
+          class="card-body"
+        />
+        <merge-request-note
+          v-if="mergeRequestFeedback"
+          :feedback="mergeRequestFeedback"
+          :project="project"
+          class="card-body"
+        />
+      </div>
 
-      <div v-if="dismissalFeedback || modal.isCommentingOnDismissal" class="card my-4">
-        <div class="card-body">
-          <dismissal-note
-            :feedback="dismissalFeedbackObject"
-            :is-commenting-on-dismissal="modal.isCommentingOnDismissal"
-            :is-showing-delete-buttons="modal.isShowingDeleteButtons"
-            :project="project"
-            :show-dismissal-comment-actions="
-              !dismissalFeedback || !dismissalFeedback.comment_details || !isEditingExistingFeedback
-            "
-            @editVulnerabilityDismissalComment="$emit('editVulnerabilityDismissalComment')"
-            @showDismissalDeleteButtons="$emit('showDismissalDeleteButtons')"
-            @hideDismissalDeleteButtons="$emit('hideDismissalDeleteButtons')"
-            @deleteDismissalComment="$emit('deleteDismissalComment')"
-          />
-          <dismissal-comment-box-toggle
-            v-if="
-              !dismissalFeedback || !dismissalFeedback.comment_details || isEditingExistingFeedback
-            "
-            v-model="localDismissalComment"
-            :dismissal-comment="
-              dismissalFeedback &&
-                dismissalFeedback.comment_details &&
-                dismissalFeedback.comment_details.comment
-            "
-            :is-active="modal.isCommentingOnDismissal"
-            :error-message="dismissalCommentErrorMessage"
-            @openDismissalCommentBox="$emit('openDismissalCommentBox')"
-            @submit="handleDismissalCommentSubmission"
-            @clearError="clearDismissalError"
-          />
-        </div>
+      <div v-if="showDismissalCard" class="card card-body my-4">
+        <dismissal-note
+          :feedback="dismissalFeedbackObject"
+          :is-commenting-on-dismissal="modal.isCommentingOnDismissal"
+          :is-showing-delete-buttons="modal.isShowingDeleteButtons"
+          :project="project"
+          :show-dismissal-comment-actions="showDismissalCommentActions"
+          @editVulnerabilityDismissalComment="$emit('editVulnerabilityDismissalComment')"
+          @showDismissalDeleteButtons="$emit('showDismissalDeleteButtons')"
+          @hideDismissalDeleteButtons="$emit('hideDismissalDeleteButtons')"
+          @deleteDismissalComment="$emit('deleteDismissalComment')"
+        />
+        <dismissal-comment-box-toggle
+          v-if="showDismissalCommentTextbox"
+          v-model="localDismissalComment"
+          :dismissal-comment="dismissalFeedbackComment"
+          :is-active="modal.isCommentingOnDismissal"
+          :error-message="dismissalCommentErrorMessage"
+          @openDismissalCommentBox="$emit('openDismissalCommentBox')"
+          @submit="handleDismissalCommentSubmission"
+          @clearError="clearDismissalError"
+        />
       </div>
 
       <div v-if="modal.error" class="alert alert-danger">{{ modal.error }}</div>

ee/app/assets/javascripts/vulnerabilities/components/footer.vue

@@ -31,17 +31,11 @@ export default {
 };
 </script>
 <template>
-  <ul class="notes">
-    <li v-if="hasSolution" class="note">
-      <solution-card v-bind="solutionInfo" />
-    </li>
-    <li>
-      <hr />
-    </li>
-    <li v-if="hasIssue" class="note card my-4 border-bottom">
-      <div class="card-body">
-        <issue-note :feedback="feedback" :project="project" />
-      </div>
-    </li>
-  </ul>
+  <div>
+    <solution-card v-if="hasSolution" v-bind="solutionInfo" />
+    <div v-if="hasIssue" class="card">
+      <issue-note :feedback="feedback" :project="project" class="card-body" />
+    </div>
+    <hr />
+  </div>
 </template>

ee/app/assets/javascripts/vulnerabilities/components/app.vue → ee/app/assets/javascripts/vulnerabilities/components/header.vue

@@ -12,7 +12,7 @@ import StatusDescription from './status_description.vue';
 import { VULNERABILITY_STATE_OBJECTS } from '../constants';
 
 export default {
-  name: 'VulnerabilityManagementApp',
+  name: 'VulnerabilityHeader',
   components: {
     GlDeprecatedButton,
     GlLoadingIcon,

ee/app/views/projects/security/vulnerabilities/show.html.haml

@@ -6,10 +6,10 @@
 - finding = @vulnerability.finding
 - location = finding.location
 
-#js-vulnerability-management-app{ data: vulnerability_data(@vulnerability, @pipeline) }
+#js-vulnerability-header{ data: vulnerability_data(@vulnerability, @pipeline) }
 
 .issue-details.issuable-details
-  .detail-page-description
+  .detail-page-description.p-0.my-3
     %h2.title= @vulnerability.title
     .description
       .md

ee/spec/frontend/vue_shared/security_reports/components/event_item_spec.js

+import { GlDeprecatedButton } from '@gitlab/ui';
 import Component from 'ee/vue_shared/security_reports/components/event_item.vue';
 import { shallowMount, mount } from '@vue/test-utils';
 
@@ -37,7 +38,7 @@ describe('Event Item', () => {
     });
 
     it('uses the fallback icon class', () => {
-      expect(wrapper.props().iconStyle).toBe('ci-status-icon-success');
+      expect(wrapper.props().iconClass).toBe('ci-status-icon-success');
     });
 
     it('renders the action buttons tontainer', () => {
@@ -53,12 +54,12 @@ describe('Event Item', () => {
       actionButtons: [
         {
           iconName: 'pencil',
-          emit: 'fooEvent',
+          onClick: jest.fn(),
           title: 'Foo Action',
         },
         {
           iconName: 'remove',
-          emit: 'barEvent',
+          onClick: jest.fn(),
           title: 'Bar Action',
         },
       ],
@@ -77,12 +78,12 @@ describe('Event Item', () => {
     });
 
     it('renders the action buttons', () => {
-      expect(wrapper.findAll('.action-buttons > button').length).toBe(2);
+      expect(wrapper.findAll(GlDeprecatedButton).length).toBe(2);
       expect(wrapper).toMatchSnapshot();
     });
 
     it('emits the button events when clicked', () => {
-      const buttons = wrapper.findAll('.action-buttons > button');
+      const buttons = wrapper.findAll(GlDeprecatedButton);
       buttons.at(0).trigger('click');
       return wrapper.vm
         .$nextTick()
@@ -91,8 +92,8 @@ describe('Event Item', () => {
           return wrapper.vm.$nextTick();
         })
         .then(() => {
-          expect(wrapper.emitted().fooEvent.length).toEqual(1);
-          expect(wrapper.emitted().barEvent.length).toEqual(1);
+          expect(propsData.actionButtons[0].onClick).toHaveBeenCalledTimes(1);
+          expect(propsData.actionButtons[1].onClick).toHaveBeenCalledTimes(1);
         });
     });
   });

ee/spec/frontend/vue_shared/security_reports/components/modal_spec.js

@@ -2,6 +2,8 @@ import Vue from 'vue';
 import component from 'ee/vue_shared/security_reports/components/modal.vue';
 import createState from 'ee/vue_shared/security_reports/store/state';
 import SolutionCard from 'ee/vue_shared/security_reports/components/solution_card.vue';
+import IssueNote from 'ee/vue_shared/security_reports/components/issue_note.vue';
+import MergeRequestNote from 'ee/vue_shared/security_reports/components/merge_request_note.vue';
 import { mount, shallowMount } from '@vue/test-utils';
 
 describe('Security Reports modal', () => {
@@ -202,8 +204,7 @@ describe('Security Reports modal', () => {
       });
 
       it('displays a link to the issue', () => {
-        const notes = wrapper.find('.notes');
-        expect(notes.exists()).toBe(true);
+        expect(wrapper.contains(IssueNote)).toBe(true);
       });
     });
 
@@ -220,8 +221,8 @@ describe('Security Reports modal', () => {
       });
 
       it('hides the link to the issue', () => {
-        const notes = wrapper.find('.notes');
-        expect(notes.exists()).toBe(false);
+        const note = wrapper.find(IssueNote);
+        expect(note.exists()).toBe(false);
       });
     });
   });
@@ -240,8 +241,7 @@ describe('Security Reports modal', () => {
       });
 
       it('displays a link to the merge request', () => {
-        const notes = wrapper.find('.notes');
-        expect(notes.exists()).toBe(true);
+        expect(wrapper.contains(MergeRequestNote)).toBe(true);
       });
     });
 
@@ -258,8 +258,8 @@ describe('Security Reports modal', () => {
       });
 
       it('hides the link to the merge request', () => {
-        const notes = wrapper.find('.notes');
-        expect(notes.exists()).toBe(false);
+        const note = wrapper.find(MergeRequestNote);
+        expect(note.exists()).toBe(false);
       });
     });
   });

ee/spec/frontend/vulnerabilities/app_spec.js → ee/spec/frontend/vulnerabilities/header_spec.js

@@ -6,7 +6,7 @@ import Api from '~/api';
 import axios from '~/lib/utils/axios_utils';
 import * as urlUtility from '~/lib/utils/url_utility';
 import createFlash from '~/flash';
-import App from 'ee/vulnerabilities/components/app.vue';
+import Header from 'ee/vulnerabilities/components/header.vue';
 import StatusDescription from 'ee/vulnerabilities/components/status_description.vue';
 import ResolutionAlert from 'ee/vulnerabilities/components/resolution_alert.vue';
 import VulnerabilityStateDropdown from 'ee/vulnerabilities/components/vulnerability_state_dropdown.vue';
@@ -16,7 +16,7 @@ const vulnerabilityStateEntries = Object.entries(VULNERABILITY_STATE_OBJECTS);
 const mockAxios = new MockAdapter(axios);
 jest.mock('~/flash');
 
-describe('Vulnerability management app', () => {
+describe('Vulnerability Header', () => {
   let wrapper;
 
   const defaultVulnerability = {
@@ -69,7 +69,7 @@ describe('Vulnerability management app', () => {
   const findStatusDescription = () => wrapper.find(StatusDescription);
 
   const createWrapper = (vulnerability = {}, finding = findingWithoutIssue) => {
-    wrapper = shallowMount(App, {
+    wrapper = shallowMount(Header, {
       propsData: {
         ...dataset,
         initialVulnerability: { ...defaultVulnerability, ...vulnerability },