Merge branch 'Clean-Up-Vulnerability-Footer-Props' into 'master'

Clean Up Vulnerability Footer Props

See merge request gitlab-org/gitlab!40796

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

@@ -4,6 +4,7 @@ import IssueNote from 'ee/vue_shared/security_reports/components/issue_note.vue'
 import SolutionCard from 'ee/vue_shared/security_reports/components/solution_card.vue';
 import MergeRequestNote from 'ee/vue_shared/security_reports/components/merge_request_note.vue';
 import Api from 'ee/api';
+import { VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
 import axios from '~/lib/utils/axios_utils';
 import Poll from '~/lib/utils/poll';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
@@ -16,42 +17,8 @@ export default {
   name: 'VulnerabilityFooter',
   components: { IssueNote, SolutionCard, MergeRequestNote, HistoryEntry, RelatedIssues },
   props: {
-    discussionsUrl: {
-      type: String,
-      required: true,
-    },
-    notesUrl: {
-      type: String,
-      required: true,
-    },
-    project: {
-      type: Object,
-      required: true,
-    },
-    solutionInfo: {
-      type: Object,
-      required: true,
-    },
-    issueFeedback: {
-      type: Object,
-      required: false,
-      default: () => null,
-    },
-    mergeRequestFeedback: {
+    vulnerability: {
       type: Object,
-      required: false,
-      default: () => null,
-    },
-    vulnerabilityId: {
-      type: Number,
-      required: true,
-    },
-    canModifyRelatedIssues: {
-      type: Boolean,
-      required: true,
-    },
-    relatedIssuesHelpPath: {
-      type: String,
       required: true,
     },
   },
@@ -73,11 +40,40 @@ export default {
           return acc;
         }, {});
     },
+    project() {
+      return {
+        url: this.vulnerability.project.full_path,
+        value: this.vulnerability.project.full_name,
+      };
+    },
+    solutionInfo() {
+      const {
+        solution,
+        has_mr: hasMr,
+        vulnerability_feedback_help_path: vulnerabilityFeedbackHelpPath,
+        remediations,
+        state,
+      } = this.vulnerability;
+
+      const remediation = remediations?.[0];
+      const hasDownload = Boolean(
+        state !== VULNERABILITY_STATE_OBJECTS.resolved.state && remediation?.diff?.length && !hasMr,
+      );
+
+      return {
+        solution,
+        remediation,
+        hasDownload,
+        hasMr,
+        vulnerabilityFeedbackHelpPath,
+        isStandaloneVulnerability: true,
+      };
+    },
     hasSolution() {
       return Boolean(this.solutionInfo.solution || this.solutionInfo.remediation);
     },
     issueLinksEndpoint() {
-      return Api.buildUrl(Api.vulnerabilityIssueLinksPath).replace(':id', this.vulnerabilityId);
+      return Api.buildUrl(Api.vulnerabilityIssueLinksPath).replace(':id', this.vulnerability.id);
     },
   },
 
@@ -101,7 +97,7 @@ export default {
     },
     fetchDiscussions() {
       axios
-        .get(this.discussionsUrl)
+        .get(this.vulnerability.discussions_url)
         .then(({ data, headers: { date } }) => {
           this.discussionsDictionary = data.reduce((acc, discussion) => {
             acc[discussion.id] = discussion;
@@ -137,7 +133,9 @@ export default {
       this.poll = new Poll({
         resource: {
           fetchNotes: () =>
-            axios.get(this.notesUrl, { headers: { 'X-Last-Fetched-At': this.lastFetchedAt } }),
+            axios.get(this.vulnerability.notes_url, {
+              headers: { 'X-Last-Fetched-At': this.lastFetchedAt },
+            }),
         },
         method: 'fetchNotes',
         successCallback: ({ data: { notes, last_fetched_at: lastFetchedAt } }) => {
@@ -194,16 +192,19 @@ export default {
   <div data-qa-selector="vulnerability_footer">
     <solution-card v-if="hasSolution" v-bind="solutionInfo" />
 
-    <div v-if="issueFeedback || mergeRequestFeedback" class="card gl-mt-5">
+    <div
+      v-if="vulnerability.issue_feedback || vulnerability.merge_request_feedback"
+      class="card gl-mt-5"
+    >
       <issue-note
-        v-if="issueFeedback"
-        :feedback="issueFeedback"
+        v-if="vulnerability.issue_feedback"
+        :feedback="vulnerability.issue_feedback"
         :project="project"
         class="card-body"
       />
       <merge-request-note
-        v-if="mergeRequestFeedback"
-        :feedback="mergeRequestFeedback"
+        v-if="vulnerability.merge_request_feedback"
+        :feedback="vulnerability.merge_request_feedback"
         :project="project"
         class="card-body"
       />
@@ -211,9 +212,9 @@ export default {
 
     <related-issues
       :endpoint="issueLinksEndpoint"
-      :can-modify-related-issues="canModifyRelatedIssues"
+      :can-modify-related-issues="vulnerability.can_modify_related_issues"
       :project-path="project.url"
-      :help-path="relatedIssuesHelpPath"
+      :help-path="vulnerability.related_issues_help_path"
     />
 
     <hr />
@@ -223,7 +224,7 @@ export default {
         v-for="discussion in discussions"
         :key="discussion.id"
         :discussion="discussion"
-        :notes-url="notesUrl"
+        :notes-url="vulnerability.notes_url"
       />
     </ul>
   </div>

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

@@ -38,7 +38,9 @@ export default {
       isProcessingAction: false,
       isLoadingVulnerability: false,
       isLoadingUser: false,
-      vulnerability: this.initialVulnerability,
+      // Spread operator because the header could modify the `project`
+      // prop leading to an error in the footer component.
+      vulnerability: { ...this.initialVulnerability },
       user: undefined,
       refreshVulnerabilitySource: undefined,
     };

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

 <script>
-import { VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
 import VulnerabilityHeader from './header.vue';
 import VulnerabilityDetails from './details.vue';
 import VulnerabilityFooter from './footer.vue';
-import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 
 export default {
   components: { VulnerabilityHeader, VulnerabilityDetails, VulnerabilityFooter },
@@ -15,62 +13,6 @@ export default {
     },
   },
 
-  computed: {
-    footerInfo() {
-      const {
-        vulnerabilityFeedbackHelpPath,
-        hasMr,
-        discussionsUrl,
-        createIssueUrl,
-        state,
-        issueFeedback,
-        mergeRequestFeedback,
-        notesUrl,
-        projectFingerprint,
-        remediations,
-        reportType,
-        solution,
-        id,
-        canModifyRelatedIssues,
-        relatedIssuesHelpPath,
-      } = convertObjectPropsToCamelCase(this.vulnerability);
-
-      const remediation = remediations?.length ? remediations[0] : null;
-      const hasDownload = Boolean(
-        state !== VULNERABILITY_STATE_OBJECTS.resolved.state && remediation?.diff?.length && !hasMr,
-      );
-      const hasRemediation = Boolean(remediation);
-
-      const props = {
-        vulnerabilityId: id,
-        discussionsUrl,
-        notesUrl,
-        projectFingerprint,
-        solutionInfo: {
-          solution,
-          remediation,
-          hasDownload,
-          hasMr,
-          hasRemediation,
-          vulnerabilityFeedbackHelpPath,
-          isStandaloneVulnerability: true,
-        },
-        createIssueUrl,
-        reportType,
-        issueFeedback,
-        mergeRequestFeedback,
-        canModifyRelatedIssues,
-        project: {
-          url: this.vulnerability.project.full_path,
-          value: this.vulnerability.project.full_name,
-        },
-        relatedIssuesHelpPath,
-      };
-
-      return props;
-    },
-  },
-
   methods: {
     refreshHeader() {
       this.$refs.header.refreshVulnerability();
@@ -92,7 +34,7 @@ export default {
     <vulnerability-details :vulnerability="vulnerability" />
     <vulnerability-footer
       ref="footer"
-      v-bind="footerInfo"
+      :vulnerability="vulnerability"
       @vulnerability-state-change="refreshHeader"
     />
   </div>

ee/spec/frontend/vulnerabilities/footer_spec.js

@@ -18,42 +18,23 @@ jest.mock('~/user_popovers');
 describe('Vulnerability Footer', () => {
   let wrapper;
 
-  const minimumProps = {
-    discussionsUrl: `/discussions`,
-    solutionInfo: {
-      hasDownload: false,
-      hasMr: false,
-      hasRemediation: false,
-      isStandaloneVulnerability: true,
-      remediation: null,
-      solution: undefined,
-      vulnerabilityFeedbackHelpPath:
-        '/help/user/application_security/index#interacting-with-the-vulnerabilities',
-    },
-    finding: {},
-    notesUrl: '/notes',
+  const vulnerability = {
+    id: 1,
+    discussions_url: '/discussions',
+    notes_url: '/notes',
     project: {
-      url: '/root/security-reports',
-      value: 'Administrator / Security Reports',
+      full_path: '/root/security-reports',
+      full_name: 'Administrator / Security Reports',
     },
-    vulnerabilityId: 1,
-    canModifyRelatedIssues: true,
-    relatedIssuesHelpPath: 'help/path',
-  };
-
-  const solutionInfoProp = {
-    hasDownload: true,
-    hasMr: false,
-    isStandaloneVulnerability: true,
-    remediation: {},
-    solution: 'Upgrade to fixed version.\n',
-    vulnerabilityFeedbackHelpPath:
-      '/help/user/application_security/index#interacting-with-the-vulnerabilities',
+    can_modify_related_issues: true,
+    related_issues_help_path: 'help/path',
+    has_mr: false,
+    vulnerability_feedback_help_path: 'feedback/help/path',
   };
 
-  const createWrapper = (props = minimumProps) => {
+  const createWrapper = (properties = {}) => {
     wrapper = shallowMount(VulnerabilityFooter, {
-      propsData: props,
+      propsData: { vulnerability: { ...vulnerability, ...properties } },
     });
   };
 
@@ -77,9 +58,18 @@ describe('Vulnerability Footer', () => {
 
   describe('solution card', () => {
     it('does show solution card when there is one', () => {
-      createWrapper({ ...minimumProps, solutionInfo: solutionInfoProp });
+      const properties = { remediations: [{ diff: [{}] }], solution: 'some solution' };
+      createWrapper(properties);
+
       expect(wrapper.find(SolutionCard).exists()).toBe(true);
-      expect(wrapper.find(SolutionCard).props()).toMatchObject(solutionInfoProp);
+      expect(wrapper.find(SolutionCard).props()).toEqual({
+        solution: properties.solution,
+        remediation: properties.remediations[0],
+        hasDownload: true,
+        hasMr: vulnerability.has_mr,
+        vulnerabilityFeedbackHelpPath: vulnerability.vulnerability_feedback_help_path,
+        isStandaloneVulnerability: true,
+      });
     });
 
     it('does not show solution card when there is not one', () => {
@@ -90,18 +80,16 @@ describe('Vulnerability Footer', () => {
 
   describe.each`
     type               | prop                        | component
-    ${'issue'}         | ${'issueFeedback'}        | ${IssueNote}
-    ${'merge request'} | ${'mergeRequestFeedback'} | ${MergeRequestNote}
+    ${'issue'}         | ${'issue_feedback'}         | ${IssueNote}
+    ${'merge request'} | ${'merge_request_feedback'} | ${MergeRequestNote}
   `('$type note', ({ prop, component }) => {
     // The object itself does not matter, we just want to make sure it's passed to the issue note.
     const feedback = {};
 
     it('shows issue note when an issue exists for the vulnerability', () => {
-      createWrapper({ ...minimumProps, [prop]: feedback });
+      createWrapper({ [prop]: feedback });
       expect(wrapper.find(component).exists()).toBe(true);
-      expect(wrapper.find(component).props()).toMatchObject({
-        feedback,
-      });
+      expect(wrapper.find(component).props('feedback')).toBe(feedback);
     });
 
     it('does not show issue note when there is no issue for the vulnerability', () => {
@@ -111,7 +99,7 @@ describe('Vulnerability Footer', () => {
   });
 
   describe('state history', () => {
-    const discussionUrl = '/discussions';
+    const discussionUrl = vulnerability.discussions_url;
 
     const historyList = () => wrapper.find({ ref: 'historyList' });
     const historyEntries = () => wrapper.findAll(HistoryEntry);
@@ -166,7 +154,7 @@ describe('Vulnerability Footer', () => {
       const getDiscussion = (entries, index) => entries.at(index).props('discussion');
       const createNotesRequest = (...notes) =>
         mockAxios
-          .onGet(minimumProps.notesUrl)
+          .onGet(vulnerability.notes_url)
           .replyOnce(200, { notes, last_fetched_at: Date.now() });
 
       // Following #217184 the vulnerability polling uses an initial timeout
@@ -243,7 +231,7 @@ describe('Vulnerability Footer', () => {
       });
 
       it('shows an error if the notes poll fails', () => {
-        mockAxios.onGet(minimumProps.notesUrl).replyOnce(500);
+        mockAxios.onGet(vulnerability.notes_url).replyOnce(500);
 
         return axios.waitForAll().then(async () => {
           await startTimeoutsAndAwaitRequests();
@@ -276,16 +264,16 @@ describe('Vulnerability Footer', () => {
     it('has the correct props', () => {
       const endpoint = Api.buildUrl(Api.vulnerabilityIssueLinksPath).replace(
         ':id',
-        minimumProps.vulnerabilityId,
+        vulnerability.id,
       );
       createWrapper();
 
       expect(relatedIssues().exists()).toBe(true);
       expect(relatedIssues().props()).toMatchObject({
         endpoint,
-        canModifyRelatedIssues: minimumProps.canModifyRelatedIssues,
-        projectPath: minimumProps.project.url,
-        helpPath: minimumProps.relatedIssuesHelpPath,
+        canModifyRelatedIssues: vulnerability.can_modify_related_issues,
+        projectPath: vulnerability.project.full_path,
+        helpPath: vulnerability.related_issues_help_path,
       });
     });
   });

ee/spec/frontend/vulnerabilities/vulnerability_spec.js

@@ -70,32 +70,9 @@ describe('Vulnerability', () => {
     });
 
     it('passes the correct properties to the children', () => {
-      expect(findHeader().props()).toMatchObject({
-        initialVulnerability: vulnerability,
-      });
-      expect(findDetails().props()).toMatchObject({ vulnerability });
-      expect(findFooter().props()).toMatchObject({
-        vulnerabilityId: vulnerability.id,
-        discussionsUrl: vulnerability.discussions_url,
-        notesUrl: vulnerability.notes_url,
-        solutionInfo: {
-          solution: vulnerability.solution,
-          remediation: vulnerability.remediation,
-          hasDownload: Boolean(vulnerability.has_download),
-          hasMr: vulnerability.has_mr,
-          hasRemediation: Boolean(vulnerability.has_remediation),
-          vulnerabilityFeedbackHelpPath: vulnerability.vulnerability_feedback_help_path,
-          isStandaloneVulnerability: true,
-        },
-        issueFeedback: vulnerability.issue_feedback,
-        mergeRequestFeedback: vulnerability.merge_request_feedback,
-        canModifyRelatedIssues: vulnerability.can_modify_related_issues,
-        project: {
-          url: vulnerability.project.full_path,
-          value: vulnerability.project.full_name,
-        },
-        relatedIssuesHelpPath: vulnerability.related_issues_help_path,
-      });
+      expect(findHeader().props('initialVulnerability')).toBe(vulnerability);
+      expect(findDetails().props('vulnerability')).toBe(vulnerability);
+      expect(findFooter().props('vulnerability')).toBe(vulnerability);
     });
   });