Merge branch '196724-create-and-style-the-vulnerability-status-badge' into 'master'

Create and style the vulnerability status badge

See merge request gitlab-org/gitlab!24847

ee/app/assets/javascripts/api.js

@@ -29,6 +29,7 @@ export default {
   paymentFormPath: '/-/subscriptions/payment_form',
   paymentMethodPath: '/-/subscriptions/payment_method',
   confirmOrderPath: '/-/subscriptions',
+  vulnerabilitiesActionPath: '/api/:version/vulnerabilities/:id/:action',
 
   userSubscription(namespaceId) {
     const url = Api.buildUrl(this.subscriptionPath).replace(':id', encodeURIComponent(namespaceId));
@@ -254,4 +255,12 @@ export default {
     const url = Api.buildUrl(this.confirmOrderPath);
     return axios.post(url, params);
   },
+
+  changeVulnerabilityState(id, state) {
+    const url = Api.buildUrl(this.vulnerabilitiesActionPath)
+      .replace(':id', id)
+      .replace(':action', state);
+
+    return axios.post(url);
+  },
 };

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

@@ -37,10 +37,11 @@ function createSolutionCardApp() {
 }
 
 function createHeaderApp() {
-  const el = document.getElementById('js-vulnerability-show-header');
-  const { createIssueUrl } = el.dataset;
-  const vulnerability = JSON.parse(el.dataset.vulnerability);
-  const finding = JSON.parse(el.dataset.finding);
+  const el = document.getElementById('js-vulnerability-management-app');
+  const vulnerability = JSON.parse(el.dataset.vulnerabilityJson);
+  const pipeline = JSON.parse(el.dataset.pipelineJson);
+
+  const { projectFingerprint, createIssueUrl } = el.dataset;
 
   return new Vue({
     el,
@@ -49,7 +50,8 @@ function createHeaderApp() {
       h(HeaderApp, {
         props: {
           vulnerability,
-          finding,
+          pipeline,
+          projectFingerprint,
           createIssueUrl,
         },
       }),

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

 <script>
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlBadge, GlLink, GlSprintf } from '@gitlab/ui';
+import Api from 'ee/api';
 import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import axios from '~/lib/utils/axios_utils';
 import { redirectTo } from '~/lib/utils/url_utility';
 import createFlash from '~/flash';
 import { s__ } from '~/locale';
+import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import VulnerabilityStateDropdown from './vulnerability_state_dropdown.vue';
+import { VULNERABILITY_STATES } from '../constants';
 
 export default {
+  name: 'VulnerabilityManagementApp',
   components: {
     GlLoadingIcon,
+    GlBadge,
+    GlLink,
+    GlSprintf,
+    TimeAgoTooltip,
     VulnerabilityStateDropdown,
     LoadingButton,
   },
@@ -19,29 +27,44 @@ export default {
       type: Object,
       required: true,
     },
-    finding: {
+    pipeline: {
       type: Object,
-      required: true,
+      required: false,
+      default: undefined,
     },
     createIssueUrl: {
       type: String,
       required: true,
     },
+    projectFingerprint: {
+      type: String,
+      required: true,
+    },
   },
 
-  data: () => ({
-    isLoadingVulnerability: false,
-    isCreatingIssue: false,
-  }),
+  data() {
+    return {
+      isLoadingVulnerability: false,
+      isCreatingIssue: false,
+      state: this.vulnerability.state,
+    };
+  },
+
+  computed: {
+    variant() {
+      // Get the badge variant based on the vulnerability state, defaulting to 'warning'.
+      return VULNERABILITY_STATES[this.state]?.variant || 'warning';
+    },
+  },
 
   methods: {
     onVulnerabilityStateChange(newState) {
       this.isLoadingVulnerability = true;
 
-      axios
-        .post(`/api/v4/vulnerabilities/${this.vulnerability.id}/${newState}`)
-        // Reload the page for now since the rest of the page is still a static haml file.
-        .then(() => window.location.reload(true))
+      Api.changeVulnerabilityState(this.vulnerability.id, newState)
+        .then(({ data }) => {
+          this.state = data.state;
+        })
         .catch(() => {
           createFlash(
             s__(
@@ -60,7 +83,7 @@ export default {
           vulnerability_feedback: {
             feedback_type: 'issue',
             category: this.vulnerability.report_type,
-            project_fingerprint: this.finding.project_fingerprint,
+            project_fingerprint: this.projectFingerprint,
             vulnerability_data: { ...this.vulnerability, category: this.vulnerability.report_type },
           },
         })
@@ -79,16 +102,33 @@ export default {
 </script>
 
 <template>
-  <div>
+  <div class="d-flex align-items-center border-bottom pt-2 pb-2">
+    <gl-loading-icon v-if="isLoadingVulnerability" />
+    <gl-badge v-else ref="badge" class="text-capitalize" :variant="variant">{{ state }}</gl-badge>
+
+    <span v-if="pipeline" class="mx-2">
+      <gl-sprintf :message="__('Detected %{timeago} in pipeline %{pipelineLink}')">
+        <template #timeago>
+          <time-ago-tooltip :time="pipeline.created_at" />
+        </template>
+        <template v-if="pipeline.id" #pipelineLink>
+          <gl-link :href="pipeline.url" target="_blank">{{ pipeline.id }}</gl-link>
+        </template>
+      </gl-sprintf>
+    </span>
+
+    <time-ago-tooltip v-else class="ml-2" :time="vulnerability.created_at" />
+
+    <label class="mb-0 ml-auto mr-2">{{ __('Status') }}</label>
     <gl-loading-icon v-if="isLoadingVulnerability" />
     <vulnerability-state-dropdown
       v-else
-      :state="vulnerability.state"
+      :initial-state="state"
       @change="onVulnerabilityStateChange"
     />
     <loading-button
       ref="create-issue-btn"
-      class="align-items-center d-inline-flex"
+      class="align-items-center d-inline-flex ml-2"
       :loading="isCreatingIssue"
       :label="s__('VulnerabilityManagement|Create issue')"
       container-class="btn btn-success btn-inverted"

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

@@ -3,27 +3,33 @@ import { GlDropdown, GlIcon, GlButton } from '@gitlab/ui';
 import { VULNERABILITY_STATES } from '../constants';
 
 export default {
+  states: Object.values(VULNERABILITY_STATES),
   components: { GlDropdown, GlIcon, GlButton },
 
   props: {
     // Initial vulnerability state from the parent. This is used to disable the Change Status button
     // if the selected value is the initial value, and also used to reset the dropdown back to the
     // initial value if the user closed the dropdown without saving it.
-    state: { type: String, required: true },
+    initialState: { type: String, required: true },
   },
 
   data() {
     return {
-      states: Object.values(VULNERABILITY_STATES),
       // Vulnerability state that's picked in the dropdown. Defaults to the passed-in state.
-      selected: VULNERABILITY_STATES[this.state],
+      selected: VULNERABILITY_STATES[this.initialState],
     };
   },
 
   computed: {
-    // Alias for this.state, since using 'state' can get confusing within this component.
-    initialState() {
-      return VULNERABILITY_STATES[this.state];
+    initialStateItem() {
+      return VULNERABILITY_STATES[this.initialState];
+    },
+  },
+
+  watch: {
+    // If the initial state was changed by the parent component, re-select the correct state object.
+    initialStateItem(newItem) {
+      this.selected = newItem;
     },
   },
 
@@ -38,7 +44,7 @@ export default {
 
     // Reset the selected dropdown item to what was passed in by the parent.
     resetDropdown() {
-      this.selected = this.initialState;
+      this.selected = this.initialStateItem;
     },
 
     saveState(selectedState) {
@@ -54,15 +60,15 @@ export default {
     ref="dropdown"
     menu-class="p-0"
     toggle-class="text-capitalize"
-    :text="state"
+    :text="initialState"
     :right="true"
     @hide="resetDropdown"
   >
     <li
-      v-for="stateItem in states"
+      v-for="stateItem in $options.states"
       :key="stateItem.action"
       class="py-3 px-2 dropdown-item cursor-pointer border-bottom"
-      :class="[stateItem.action, { selected: selected === stateItem }]"
+      :class="{ selected: selected === stateItem }"
       @click="changeSelectedState(stateItem)"
     >
       <div class="d-flex align-items-center">
@@ -78,15 +84,16 @@ export default {
     </li>
 
     <div class="text-right p-3">
-      <gl-button ref="cancel-button" class="mr-2" @click="closeDropdown">{{
-        __('Cancel')
-      }}</gl-button>
+      <gl-button ref="cancel-button" class="mr-2" @click="closeDropdown">
+        {{ __('Cancel') }}
+      </gl-button>
       <gl-button
         ref="save-button"
         variant="success"
-        :disabled="selected === initialState"
+        :disabled="selected === initialStateItem"
         @click="saveState(selected)"
-        >{{ s__('VulnerabilityManagement|Change status') }}
+      >
+        {{ s__('VulnerabilityManagement|Change status') }}
       </gl-button>
     </div>
   </gl-dropdown>

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

@@ -4,16 +4,19 @@ import { s__ } from '~/locale';
 export const VULNERABILITY_STATES = {
   dismissed: {
     action: 'dismiss',
+    variant: 'light',
     displayName: s__('VulnerabilityManagement|Dismiss'),
     description: s__('VulnerabilityManagement|Will not fix or a false-positive'),
   },
   confirmed: {
     action: 'confirm',
+    variant: 'danger',
     displayName: s__('VulnerabilityManagement|Confirm'),
     description: s__('VulnerabilityManagement|A true-positive and will fix'),
   },
   resolved: {
     action: 'resolve',
+    variant: 'success',
     displayName: s__('VulnerabilityManagement|Resolved'),
     description: s__('VulnerabilityManagement|Verified as fixed or mitigated'),
   },

ee/app/helpers/vulnerabilities_helper.rb

+# frozen_string_literal: true
+
+module VulnerabilitiesHelper
+  def vulnerability_data(vulnerability, pipeline)
+    return unless vulnerability
+
+    {
+      vulnerability_json: vulnerability.to_json,
+      project_fingerprint: vulnerability.finding.project_fingerprint,
+      create_issue_url: create_vulnerability_feedback_issue_path(vulnerability.finding.project),
+      pipeline_json: vulnerability_pipeline_data(pipeline).to_json
+    }
+  end
+
+  def vulnerability_pipeline_data(pipeline)
+    return unless pipeline
+
+    {
+      id: pipeline.id,
+      created_at: pipeline.created_at.iso8601,
+      url: pipeline_path(pipeline)
+    }
+  end
+end

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

@@ -4,22 +4,7 @@
 - page_title @vulnerability.title
 - page_description @vulnerability.description
 
-.detail-page-header.align-items-center
-  .detail-page-header-body
-    .issuable-status-box.status-box.status-box-open.closed
-      %span= @vulnerability.state
-    - if @pipeline
-      %span#js-pipeline-created
-        - timeago = time_ago_with_tooltip(@pipeline.created_at)
-        - pipeline_link = '<a href="%{url}">%{id}</a>'.html_safe % { url: pipeline_url(@pipeline), id: @pipeline.id }
-        = _('Detected %{timeago} in pipeline %{pipeline_link}').html_safe % { pipeline_link: pipeline_link, timeago: timeago }
-    - else
-      %span#js-vulnerability-created
-        = time_ago_with_tooltip(@vulnerability.created_at)
-  %label.mb-0.mr-2= _('Status')
-  #js-vulnerability-show-header{ data: { vulnerability: @vulnerability.to_json,
-    finding: @vulnerability.finding.to_json,
-    create_issue_url: create_vulnerability_feedback_issue_path(@vulnerability.finding.project) } }
+#js-vulnerability-management-app{ data: vulnerability_data(@vulnerability, @pipeline) }
 
 .issue-details.issuable-details
   .detail-page-description.content-block

ee/spec/controllers/projects/security/vulnerabilities_controller_spec.rb

@@ -77,12 +77,6 @@ describe Projects::Security::VulnerabilitiesController do
         expect(response.body).to have_text(vulnerability.title)
       end
 
-      it 'renders the time pipeline ran' do
-        show_vulnerability
-
-        expect(response.body).to have_css("#js-pipeline-created")
-      end
-
       it 'renders the solution card' do
         show_vulnerability
 
@@ -93,16 +87,12 @@ describe Projects::Security::VulnerabilitiesController do
     context "when there's no attached pipeline" do
       let_it_be(:finding) { create(:vulnerabilities_occurrence, vulnerability: vulnerability) }
 
-      it 'renders the time the vulnerability was created' do
-        show_vulnerability
-
-        expect(response.body).to have_css("#js-vulnerability-created")
-      end
-
-      it 'renders the solution card' do
+      it 'renders the vulnerability page' do
         show_vulnerability
 
-        expect(response.body).to have_css("#js-vulnerability-solution")
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template(:show)
+        expect(response.body).to have_text(vulnerability.title)
       end
     end
 

ee/spec/frontend/api_spec.js

@@ -622,4 +622,23 @@ describe('Api', () => {
       });
     });
   });
+
+  describe('changeVulnerabilityState', () => {
+    it.each`
+      id    | action
+      ${5}  | ${'dismiss'}
+      ${7}  | ${'confirm'}
+      ${38} | ${'resolve'}
+    `('POSTS to correct endpoint ($id, $action)', ({ id, action }) => {
+      const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/vulnerabilities/${id}/${action}`;
+      const expectedResponse = { id, action, test: 'test' };
+
+      mock.onPost(expectedUrl).replyOnce(200, expectedResponse);
+
+      return Api.changeVulnerabilityState(id, action).then(({ data }) => {
+        expect(mock.history.post).toContainEqual(expect.objectContaining({ url: expectedUrl }));
+        expect(data).toEqual(expectedResponse);
+      });
+    });
+  });
 });

ee/spec/frontend/vulnerabilities/app_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { GlBadge } from '@gitlab/ui';
+import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
-import { redirectTo } from '~/lib/utils/url_utility';
+import * as urlUtility from '~/lib/utils/url_utility';
 
 import createFlash from '~/flash';
-import MockAdapter from 'axios-mock-adapter';
 import App from 'ee/vulnerabilities/components/app.vue';
 import waitForPromises from 'helpers/wait_for_promises';
 import VulnerabilityStateDropdown from 'ee/vulnerabilities/components/vulnerability_state_dropdown.vue';
+import { VULNERABILITY_STATES } from 'ee/vulnerabilities/constants';
 
+const vulnerabilityStateEntries = Object.entries(VULNERABILITY_STATES);
 const mockAxios = new MockAdapter(axios);
 jest.mock('~/flash');
-jest.mock('~/lib/utils/url_utility');
 
 describe('Vulnerability management app', () => {
   let wrapper;
 
   const vulnerability = {
     id: 1,
-    state: 'doesnt matter',
+    created_at: new Date().toISOString(),
     report_type: 'sast',
   };
-  const finding = {
-    project_fingerprint: 'abc123',
-    report_type: 'sast',
+
+  const dataset = {
+    createIssueUrl: 'create_issue_url',
+    projectFingerprint: 'abc123',
+    pipeline: {
+      id: 2,
+      created_at: new Date().toISOString(),
+      url: 'pipeline_url',
+    },
   };
-  const createIssueUrl = 'create_issue_path';
 
   const findCreateIssueButton = () => wrapper.find({ ref: 'create-issue-btn' });
 
-  beforeEach(() => {
+  const createWrapper = (state = 'detected') => {
     wrapper = shallowMount(App, {
       propsData: {
-        vulnerability,
-        finding,
-        createIssueUrl,
+        vulnerability: Object.assign({ state }, vulnerability),
+        ...dataset,
       },
     });
-  });
+  };
 
   afterEach(() => {
     wrapper.destroy();
@@ -45,6 +51,8 @@ describe('Vulnerability management app', () => {
   });
 
   describe('state dropdown', () => {
+    beforeEach(createWrapper);
+
     it('the vulnerability state dropdown is rendered', () => {
       expect(wrapper.find(VulnerabilityStateDropdown).exists()).toBe(true);
     });
@@ -74,34 +82,37 @@ describe('Vulnerability management app', () => {
   });
 
   describe('create issue button', () => {
+    beforeEach(createWrapper);
+
     it('renders properly', () => {
       expect(findCreateIssueButton().exists()).toBe(true);
     });
 
     it('calls create issue endpoint on click and redirects to new issue', () => {
       const issueUrl = '/group/project/issues/123';
-      mockAxios.onPost(createIssueUrl).reply(200, {
+      const spy = jest.spyOn(urlUtility, 'redirectTo');
+      mockAxios.onPost(dataset.createIssueUrl).reply(200, {
         issue_url: issueUrl,
       });
       findCreateIssueButton().vm.$emit('click');
       return waitForPromises().then(() => {
         expect(mockAxios.history.post).toHaveLength(1);
         const [postRequest] = mockAxios.history.post;
-        expect(postRequest.url).toBe(createIssueUrl);
+        expect(postRequest.url).toBe(dataset.createIssueUrl);
         expect(JSON.parse(postRequest.data)).toMatchObject({
           vulnerability_feedback: {
             feedback_type: 'issue',
             category: vulnerability.report_type,
-            project_fingerprint: finding.project_fingerprint,
+            project_fingerprint: dataset.projectFingerprint,
             vulnerability_data: { ...vulnerability, category: vulnerability.report_type },
           },
         });
-        expect(redirectTo).toHaveBeenCalledWith(issueUrl);
+        expect(spy).toHaveBeenCalledWith(issueUrl);
       });
     });
 
     it('shows an error message when issue creation fails', () => {
-      mockAxios.onPost(createIssueUrl).reply(500);
+      mockAxios.onPost(dataset.createIssueUrl).reply(500);
       findCreateIssueButton().vm.$emit('click');
       return waitForPromises().then(() => {
         expect(mockAxios.history.post).toHaveLength(1);
@@ -111,4 +122,17 @@ describe('Vulnerability management app', () => {
       });
     });
   });
+
+  describe('state badge', () => {
+    test.each(vulnerabilityStateEntries)(
+      'the vulnerability state badge has the correct variant for the %s state',
+      (stateString, stateObject) => {
+        createWrapper(stateString);
+        const badge = wrapper.find(GlBadge);
+
+        expect(badge.attributes('variant')).toBe(stateObject.variant);
+        expect(badge.text()).toBe(stateString);
+      },
+    );
+  });
 });

ee/spec/helpers/ee/vulnerabilities_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe VulnerabilitiesHelper do
+  RSpec.shared_examples 'vulnerability properties' do
+    it 'has expected vulnerability properties' do
+      expect(subject).to include(
+        vulnerability_json: vulnerability.to_json,
+        project_fingerprint: vulnerability.finding.project_fingerprint,
+        create_issue_url: be_present
+      )
+    end
+  end
+
+  before do
+    allow(helper).to receive(:can?).and_return(true)
+    allow(helper).to receive(:current_user).and_return(user)
+  end
+
+  let(:user) { build(:user) }
+
+  describe '#vulnerability_data' do
+    let(:vulnerability) { create(:vulnerability, :with_findings) }
+
+    subject { helper.vulnerability_data(vulnerability, pipeline) }
+
+    describe 'when pipeline exists' do
+      let(:pipeline) { create(:ci_pipeline) }
+      let(:pipelineData) { JSON.parse(subject[:pipeline_json]) }
+
+      include_examples 'vulnerability properties'
+
+      it 'returns expected pipeline data' do
+        expect(pipelineData).to include(
+          'id' => pipeline.id,
+          'created_at' => pipeline.created_at.iso8601,
+          'url' => be_present
+        )
+      end
+    end
+
+    describe 'when pipeline is nil' do
+      let(:pipeline) { nil }
+
+      include_examples 'vulnerability properties'
+
+      it 'returns no pipeline data' do
+        expect(subject[:pipeline]).to be_nil
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -6842,7 +6842,7 @@ msgstr ""
 msgid "Detect host keys"
 msgstr ""
 
-msgid "Detected %{timeago} in pipeline %{pipeline_link}"
+msgid "Detected %{timeago} in pipeline %{pipelineLink}"
 msgstr ""
 
 msgid "DevOps Score"