Merge branch...

Merge branch '196725-add-vulnerability-status-text-to-the-standalone-vulnerability-page' into 'master'

Add reactive vulnerability status description

See merge request gitlab-org/gitlab!25971

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

@@ -45,7 +45,7 @@ function createFooterApp() {
 
 function createHeaderApp() {
   const el = document.getElementById('js-vulnerability-management-app');
-  const vulnerability = JSON.parse(el.dataset.vulnerabilityJson);
+  const initialVulnerability = JSON.parse(el.dataset.vulnerabilityJson);
   const pipeline = JSON.parse(el.dataset.pipelineJson);
 
   const { projectFingerprint, createIssueUrl } = el.dataset;
@@ -56,7 +56,7 @@ function createHeaderApp() {
     render: h =>
       h(HeaderApp, {
         props: {
-          vulnerability,
+          initialVulnerability,
           pipeline,
           projectFingerprint,
           createIssueUrl,

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

 <script>
-import { GlButton, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
+import { GlButton, GlLoadingIcon } from '@gitlab/ui';
 import Api from 'ee/api';
 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 UsersCache from '~/lib/utils/users_cache';
 import ResolutionAlert from './resolution_alert.vue';
 import VulnerabilityStateDropdown from './vulnerability_state_dropdown.vue';
+import StatusDescription from './status_description.vue';
 import { VULNERABILITY_STATES } from '../constants';
 
 export default {
   name: 'VulnerabilityManagementApp',
   components: {
     GlButton,
-    GlLink,
     GlLoadingIcon,
-    GlSprintf,
     ResolutionAlert,
-    TimeAgoTooltip,
     VulnerabilityStateDropdown,
+    StatusDescription,
   },
 
   props: {
-    vulnerability: {
+    initialVulnerability: {
       type: Object,
       required: true,
     },
     pipeline: {
       type: Object,
-      required: false,
-      default: undefined,
+      required: true,
     },
     createIssueUrl: {
       type: String,
@@ -46,27 +44,55 @@ export default {
     return {
       isLoadingVulnerability: false,
       isCreatingIssue: false,
-      state: this.vulnerability.state,
+      isLoadingUser: false,
+      vulnerability: this.initialVulnerability,
+      user: undefined,
     };
   },
 
   computed: {
     statusBoxStyle() {
       // Get the badge variant based on the vulnerability state, defaulting to 'expired'.
-      return VULNERABILITY_STATES[this.state]?.statusBoxStyle || 'expired';
+      return VULNERABILITY_STATES[this.vulnerability.state]?.statusBoxStyle || 'expired';
     },
     showResolutionAlert() {
-      return this.vulnerability.resolved_on_default_branch && this.state !== 'resolved';
+      return (
+        this.vulnerability.resolved_on_default_branch && this.vulnerability.state !== 'resolved'
+      );
+    },
+  },
+
+  watch: {
+    'vulnerability.state': {
+      immediate: true,
+      handler(state) {
+        const id = this.vulnerability[`${state}_by_id`];
+
+        if (id === undefined) return; // Don't do anything if there's no ID.
+
+        this.isLoadingUser = true;
+
+        UsersCache.retrieveById(id)
+          .then(userData => {
+            this.user = userData;
+          })
+          .catch(() => {
+            createFlash(s__('VulnerabilityManagement|Something went wrong, could not get user.'));
+          })
+          .finally(() => {
+            this.isLoadingUser = false;
+          });
+      },
     },
   },
 
   methods: {
-    onVulnerabilityStateChange(newState) {
+    changeVulnerabilityState(newState) {
       this.isLoadingVulnerability = true;
 
       Api.changeVulnerabilityState(this.vulnerability.id, newState)
         .then(({ data }) => {
-          this.state = data.state;
+          Object.assign(this.vulnerability, data);
         })
         .catch(() => {
           createFlash(
@@ -115,7 +141,7 @@ export default {
       :default-branch-name="vulnerability.default_branch_name"
     />
     <div class="detail-page-header">
-      <div class="detail-page-header-body lh-4 align-items-center">
+      <div class="detail-page-header-body align-items-center">
         <gl-loading-icon v-if="isLoadingVulnerability" class="mr-2" />
         <span
           v-else
@@ -124,21 +150,17 @@ export default {
             `text-capitalize align-self-center issuable-status-box status-box status-box-${statusBoxStyle}`
           "
         >
-          {{ state }}
+          {{ vulnerability.state }}
         </span>
 
-        <span v-if="pipeline" class="issuable-meta">
-          <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" class="link" target="_blank">{{ pipeline.id }}</gl-link>
-            </template>
-          </gl-sprintf>
-        </span>
-
-        <time-ago-tooltip v-else class="issuable-meta" :time="vulnerability.created_at" />
+        <status-description
+          class="issuable-meta"
+          :vulnerability="vulnerability"
+          :pipeline="pipeline"
+          :user="user"
+          :is-loading-vulnerability="isLoadingVulnerability"
+          :is-loading-user="isLoadingUser"
+        />
       </div>
 
       <div class="detail-page-header-actions align-items-center">
@@ -146,8 +168,8 @@ export default {
         <gl-loading-icon v-if="isLoadingVulnerability" class="d-inline" />
         <vulnerability-state-dropdown
           v-else
-          :initial-state="state"
-          @change="onVulnerabilityStateChange"
+          :initial-state="vulnerability.state"
+          @change="changeVulnerabilityState"
         />
         <gl-button
           ref="create-issue-btn"

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

+<script>
+import { GlLink, GlSprintf, GlSkeletonLoading, GlLoadingIcon } from '@gitlab/ui';
+import { s__ } from '~/locale';
+import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
+
+export default {
+  components: {
+    GlLink,
+    GlSprintf,
+    TimeAgoTooltip,
+    GlSkeletonLoading,
+    GlLoadingIcon,
+    UserAvatarLink,
+  },
+
+  props: {
+    vulnerability: {
+      type: Object,
+      required: true,
+    },
+    pipeline: {
+      type: Object,
+      required: true,
+    },
+    user: {
+      type: Object,
+      required: false,
+      default: undefined,
+    },
+    isLoadingVulnerability: {
+      type: Boolean,
+      required: true,
+    },
+    isLoadingUser: {
+      type: Boolean,
+      required: true,
+    },
+  },
+
+  computed: {
+    time() {
+      const { state } = this.vulnerability;
+      return state === 'detected'
+        ? this.pipeline.created_at
+        : this.vulnerability[`${this.vulnerability.state}_at`];
+    },
+
+    statusText() {
+      const { state } = this.vulnerability;
+
+      switch (state) {
+        case 'detected':
+          return s__('VulnerabilityManagement|Detected %{timeago} in pipeline %{pipelineLink}');
+        case 'confirmed':
+          return s__('VulnerabilityManagement|Confirmed %{timeago} by %{user}');
+        case 'dismissed':
+          return s__('VulnerabilityManagement|Dismissed %{timeago} by %{user}');
+        case 'resolved':
+          return s__('VulnerabilityManagement|Resolved %{timeago} by %{user}');
+        default:
+          return '%timeago';
+      }
+    },
+  },
+};
+</script>
+
+<template>
+  <span>
+    <gl-skeleton-loading v-if="isLoadingVulnerability" :lines="2" class="h-auto" />
+    <gl-sprintf v-else :message="statusText">
+      <template #timeago>
+        <time-ago-tooltip ref="timeAgo" :time="time" />
+      </template>
+      <template #user>
+        <gl-loading-icon v-if="isLoadingUser" class="d-inline ml-1" />
+        <user-avatar-link
+          v-else-if="user"
+          :link-href="user.user_path"
+          :img-src="user.avatar_url"
+          :img-size="24"
+          :username="user.name"
+          :data-user="user.id"
+          class="font-weight-bold js-user-link"
+          img-css-classes="avatar-inline"
+        />
+      </template>
+      <template v-if="pipeline" #pipelineLink>
+        <gl-link :href="pipeline.url" target="_blank" class="link">
+          {{ pipeline.id }}
+        </gl-link>
+      </template>
+    </gl-sprintf>
+  </span>
+</template>

ee/spec/frontend/vulnerabilities/app_spec.js

 import { shallowMount } from '@vue/test-utils';
 import MockAdapter from 'axios-mock-adapter';
+import waitForPromises from 'helpers/wait_for_promises';
+import UsersMockHelper from 'helpers/user_mock_data_helper';
+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 waitForPromises from 'helpers/wait_for_promises';
+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';
 import { VULNERABILITY_STATES } from 'ee/vulnerabilities/constants';
@@ -34,18 +36,24 @@ describe('Vulnerability management app', () => {
     },
   };
 
+  const createRandomUser = () => {
+    const user = UsersMockHelper.createRandomUser();
+    const url = Api.buildUrl(Api.userPath).replace(':id', user.id);
+    mockAxios.onGet(url).replyOnce(200, user);
+
+    return user;
+  };
+
   const findCreateIssueButton = () => wrapper.find({ ref: 'create-issue-btn' });
   const findBadge = () => wrapper.find({ ref: 'badge' });
   const findResolutionAlert = () => wrapper.find(ResolutionAlert);
+  const findStatusDescription = () => wrapper.find(StatusDescription);
 
   const createWrapper = (vulnerability = {}) => {
     wrapper = shallowMount(App, {
       propsData: {
         ...dataset,
-        vulnerability: {
-          ...defaultVulnerability,
-          ...vulnerability,
-        },
+        initialVulnerability: { ...defaultVulnerability, ...vulnerability },
       },
     });
   };
@@ -145,7 +153,30 @@ describe('Vulnerability management app', () => {
     );
   });
 
-  describe('when the vulnerability is no-longer detected on the default branch', () => {
+  describe('status description', () => {
+    it('the status description is rendered and passed the correct data', () => {
+      const user = createRandomUser();
+      const vulnerability = {
+        ...defaultVulnerability,
+        ...{ state: 'confirmed', confirmed_by_id: user.id },
+      };
+
+      createWrapper(vulnerability);
+
+      return waitForPromises().then(() => {
+        expect(findStatusDescription().exists()).toBe(true);
+        expect(findStatusDescription().props()).toEqual({
+          vulnerability,
+          pipeline: dataset.pipeline,
+          user,
+          isLoadingVulnerability: wrapper.vm.isLoadingVulnerability,
+          isLoadingUser: wrapper.vm.isLoadingUser,
+        });
+      });
+    });
+  });
+
+  describe('when the vulnerability is no longer detected on the default branch', () => {
     const branchName = 'master';
 
     beforeEach(() => {
@@ -182,4 +213,51 @@ describe('Vulnerability management app', () => {
       });
     });
   });
+
+  describe('vulnerability user watcher', () => {
+    it.each(vulnerabilityStateEntries)(
+      `loads the correct user for the vulnerability state "%s"`,
+      state => {
+        const user = createRandomUser();
+        createWrapper({ state, [`${state}_by_id`]: user.id });
+
+        return waitForPromises().then(() => {
+          expect(mockAxios.history.get.length).toBe(1);
+          expect(findStatusDescription().props('user')).toEqual(user);
+        });
+      },
+    );
+
+    it('does not load a user if there is no user ID', () => {
+      createWrapper({ state: 'detected' });
+
+      return waitForPromises().then(() => {
+        expect(mockAxios.history.get.length).toBe(0);
+        expect(findStatusDescription().props('user')).toBeUndefined();
+      });
+    });
+
+    it('will show an error when the user cannot be loaded', () => {
+      createWrapper({ state: 'confirmed', confirmed_by_id: 1 });
+
+      mockAxios.onGet().replyOnce(500);
+
+      return waitForPromises().then(() => {
+        expect(createFlash).toHaveBeenCalledTimes(1);
+        expect(mockAxios.history.get.length).toBe(1);
+      });
+    });
+
+    it('will set the isLoadingUser property correctly when the user is loading and finished loading', () => {
+      const user = createRandomUser();
+      createWrapper({ state: 'confirmed', confirmed_by_id: user.id });
+
+      expect(findStatusDescription().props('isLoadingUser')).toBe(true);
+
+      return waitForPromises().then(() => {
+        expect(mockAxios.history.get.length).toBe(1);
+        expect(findStatusDescription().props('isLoadingUser')).toBe(false);
+      });
+    });
+  });
 });

ee/spec/frontend/vulnerabilities/status_description_spec.js

+import { mount } from '@vue/test-utils';
+import { GlLink, GlSkeletonLoading, GlLoadingIcon } from '@gitlab/ui';
+import { capitalize } from 'lodash';
+import UsersMockHelper from 'helpers/user_mock_data_helper';
+import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue';
+import StatusText from 'ee/vulnerabilities/components/status_description.vue';
+import { VULNERABILITY_STATES } from 'ee/vulnerabilities/constants';
+
+const NON_DETECTED_STATES = Object.keys(VULNERABILITY_STATES);
+const ALL_STATES = ['detected', ...NON_DETECTED_STATES];
+
+describe('Vulnerability status description component', () => {
+  let wrapper;
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  const timeAgo = () => wrapper.find(TimeAgoTooltip);
+  const pipelineLink = () => wrapper.find(GlLink);
+  const userAvatar = () => wrapper.find(UserAvatarLink);
+  const userLoadingIcon = () => wrapper.find(GlLoadingIcon);
+  const skeletonLoader = () => wrapper.find(GlSkeletonLoading);
+
+  // Create a date using the passed-in string, or just use the current time if nothing was passed in.
+  const createDate = value => (value ? new Date(value) : new Date()).toISOString();
+
+  const createWrapper = ({
+    vulnerability = {},
+    pipeline = {},
+    user,
+    isLoadingVulnerability = false,
+    isLoadingUser = false,
+  } = {}) => {
+    const v = vulnerability;
+    const p = pipeline;
+
+    // Automatically create the ${v.state}_at property if it doesn't exist. Otherwise, every test would need to create
+    // it manually for the component to mount properly.
+    if (v.state === 'detected') {
+      p.created_at = p.created_at || createDate();
+    } else {
+      const propertyName = `${v.state}_at`;
+      v[propertyName] = v[propertyName] || createDate();
+    }
+
+    wrapper = mount(StatusText, {
+      propsData: { vulnerability, pipeline, user, isLoadingVulnerability, isLoadingUser },
+    });
+  };
+
+  describe('state text', () => {
+    it.each(ALL_STATES)('shows the correct string for the vulnerability state "%s"', state => {
+      createWrapper({ vulnerability: { state } });
+
+      expect(wrapper.text()).toMatch(new RegExp(`^${capitalize(state)}`));
+    });
+  });
+
+  describe('time ago', () => {
+    it('uses the pipeline created date when the vulnerability state is "detected"', () => {
+      const pipelineDateString = createDate('2001');
+      createWrapper({
+        vulnerability: { state: 'detected' },
+        pipeline: { created_at: pipelineDateString },
+      });
+
+      expect(timeAgo().props('time')).toBe(pipelineDateString);
+    });
+
+    // The .map() is used to output the correct test name by doubling up the parameter, i.e. 'detected' -> ['detected', 'detected'].
+    it.each(NON_DETECTED_STATES.map(x => [x, x]))(
+      'uses the "%s_at" property when the vulnerability state is "%s"',
+      state => {
+        const expectedDate = createDate();
+        createWrapper({
+          vulnerability: { state, [`${state}_at`]: expectedDate },
+          pipeline: { created_at: 'pipeline_created_at' },
+        });
+
+        expect(timeAgo().props('time')).toBe(expectedDate);
+      },
+    );
+  });
+
+  describe('pipeline link', () => {
+    it('shows the pipeline link when the vulnerability state is "detected"', () => {
+      createWrapper({
+        vulnerability: { state: 'detected' },
+        pipeline: { url: 'pipeline/url' },
+      });
+
+      expect(pipelineLink().attributes('href')).toBe('pipeline/url');
+    });
+
+    it.each(NON_DETECTED_STATES)(
+      'does not show the pipeline link when the vulnerability state is "%s"',
+      state => {
+        createWrapper({
+          vulnerability: { state },
+          pipeline: { url: 'pipeline/url' },
+        });
+
+        expect(pipelineLink().exists()).toBe(false); // The user avatar should be shown instead, those tests are handled separately.
+      },
+    );
+  });
+
+  describe('user', () => {
+    it('shows a loading icon when the user is loading', () => {
+      createWrapper({
+        vulnerability: { state: 'dismissed' },
+        isLoadingUser: true,
+        user: UsersMockHelper.createRandomUser(), // Create a user so we can verify that the loading icon and the user is not shown at the same time.
+      });
+
+      expect(userLoadingIcon().exists()).toBe(true);
+      expect(userAvatar().exists()).toBe(false);
+    });
+
+    it('shows the user when it exists and is not loading', () => {
+      const user = UsersMockHelper.createRandomUser();
+      createWrapper({
+        vulnerability: { state: 'resolved' },
+        user,
+      });
+
+      expect(userLoadingIcon().exists()).toBe(false);
+      expect(userAvatar().props()).toMatchObject({
+        linkHref: user.user_path,
+        imgSrc: user.avatar_url,
+        username: user.name,
+      });
+    });
+
+    it('does not show the user when it does not exist and is not loading', () => {
+      createWrapper();
+
+      expect(userLoadingIcon().exists()).toBe(false);
+      expect(userAvatar().exists()).toBe(false);
+    });
+  });
+
+  describe('skeleton loader', () => {
+    it('shows a skeleton loader and does not show anything else when the vulnerability is loading', () => {
+      createWrapper({ isLoadingVulnerability: true });
+
+      expect(skeletonLoader().exists()).toBe(true);
+      expect(timeAgo().exists()).toBe(false);
+      expect(pipelineLink().exists()).toBe(false);
+    });
+
+    it('hides the skeleton loader and shows everything else when the vulnerability is not loading', () => {
+      createWrapper({ vulnerability: { state: 'detected' } });
+
+      expect(skeletonLoader().exists()).toBe(false);
+      expect(timeAgo().exists()).toBe(true);
+      expect(pipelineLink().exists()).toBe(true);
+    });
+  });
+});

locale/gitlab.pot

@@ -6901,9 +6901,6 @@ msgstr ""
 msgid "Detect host keys"
 msgstr ""
 
-msgid "Detected %{timeago} in pipeline %{pipelineLink}"
-msgstr ""
-
 msgid "DevOps Score"
 msgstr ""
 
@@ -22580,18 +22577,33 @@ msgstr ""
 msgid "VulnerabilityManagement|Confirm"
 msgstr ""
 
+msgid "VulnerabilityManagement|Confirmed %{timeago} by %{user}"
+msgstr ""
+
 msgid "VulnerabilityManagement|Create issue"
 msgstr ""
 
+msgid "VulnerabilityManagement|Detected %{timeago} in pipeline %{pipelineLink}"
+msgstr ""
+
 msgid "VulnerabilityManagement|Dismiss"
 msgstr ""
 
+msgid "VulnerabilityManagement|Dismissed %{timeago} by %{user}"
+msgstr ""
+
 msgid "VulnerabilityManagement|Resolved"
 msgstr ""
 
+msgid "VulnerabilityManagement|Resolved %{timeago} by %{user}"
+msgstr ""
+
 msgid "VulnerabilityManagement|Something went wrong, could not create an issue."
 msgstr ""
 
+msgid "VulnerabilityManagement|Something went wrong, could not get user."
+msgstr ""
+
 msgid "VulnerabilityManagement|Something went wrong, could not update vulnerability state."
 msgstr ""
 

spec/frontend/helpers/user_mock_data_helper.js

+let id = 1;
+
+// Code taken from: https://gist.github.com/6174/6062387
+const getRandomString = () =>
+  Math.random()
+    .toString(36)
+    .substring(2, 15) +
+  Math.random()
+    .toString(36)
+    .substring(2, 15);
+
+const getRandomUrl = () => `https://${getRandomString()}.com/${getRandomString()}`;
+
 export default {
   createNumberRandomUsers(numberUsers) {
     const users = [];
     for (let i = 0; i < numberUsers; i += 1) {
       users.push({
-        avatar: 'https://gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon',
-        id: i + 1,
-        name: `GitLab User ${i}`,
-        username: `gitlab${i}`,
+        avatar_url: getRandomUrl(),
+        id: id + 1,
+        name: getRandomString(),
+        username: getRandomString(),
+        user_path: getRandomUrl(),
       });
+
+      id += 1;
     }
     return users;
   },
+
+  createRandomUser() {
+    return this.createNumberRandomUsers(1)[0];
+  },
 };

spec/frontend/sidebar/assignees_spec.js

@@ -101,14 +101,14 @@ describe('Assignee component', () => {
 
       const first = collapsedChildren.at(0);
 
-      expect(first.find('.avatar').attributes('src')).toBe(users[0].avatar);
+      expect(first.find('.avatar').attributes('src')).toBe(users[0].avatar_url);
       expect(first.find('.avatar').attributes('alt')).toBe(`${users[0].name}'s avatar`);
 
       expect(trimText(first.find('.author').text())).toBe(users[0].name);
 
       const second = collapsedChildren.at(1);
 
-      expect(second.find('.avatar').attributes('src')).toBe(users[1].avatar);
+      expect(second.find('.avatar').attributes('src')).toBe(users[1].avatar_url);
       expect(second.find('.avatar').attributes('alt')).toBe(`${users[1].name}'s avatar`);
 
       expect(trimText(second.find('.author').text())).toBe(users[1].name);
@@ -127,7 +127,7 @@ describe('Assignee component', () => {
 
       const first = collapsedChildren.at(0);
 
-      expect(first.find('.avatar').attributes('src')).toBe(users[0].avatar);
+      expect(first.find('.avatar').attributes('src')).toBe(users[0].avatar_url);
       expect(first.find('.avatar').attributes('alt')).toBe(`${users[0].name}'s avatar`);
 
       expect(trimText(first.find('.author').text())).toBe(users[0].name);