Merge branch '209993-use-note-header' into 'master'

Use note header component in event item component

See merge request gitlab-org/gitlab!29437

app/assets/javascripts/notes/components/note_header.vue

@@ -39,13 +39,18 @@ export default {
       required: false,
       default: true,
     },
+    showSpinner: {
+      type: Boolean,
+      required: false,
+      default: true,
+    },
   },
   computed: {
     toggleChevronClass() {
       return this.expanded ? 'fa-chevron-up' : 'fa-chevron-down';
     },
     noteTimestampLink() {
-      return `#note_${this.noteId}`;
+      return this.noteId ? `#note_${this.noteId}` : undefined;
     },
     hasAuthor() {
       return this.author && Object.keys(this.author).length;
@@ -60,7 +65,9 @@ export default {
       this.$emit('toggleHandler');
     },
     updateTargetNoteHash() {
-      this.setTargetNoteHash(this.noteTimestampLink);
+      if (this.$store) {
+        this.setTargetNoteHash(this.noteTimestampLink);
+      }
     },
   },
 };
@@ -101,16 +108,20 @@ export default {
           <template v-if="actionText">{{ actionText }}</template>
         </span>
         <a
-          ref="noteTimestamp"
+          v-if="noteTimestampLink"
+          ref="noteTimestampLink"
           :href="noteTimestampLink"
           class="note-timestamp system-note-separator"
           @click="updateTargetNoteHash"
         >
           <time-ago-tooltip :time="createdAt" tooltip-placement="bottom" />
         </a>
+        <time-ago-tooltip v-else ref="noteTimestamp" :time="createdAt" tooltip-placement="bottom" />
       </template>
       <slot name="extra-controls"></slot>
       <i
+        v-if="showSpinner"
+        ref="spinner"
         class="fa fa-spinner fa-spin editing-spinner"
         :aria-label="__('Comment is being updated')"
         aria-hidden="true"

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

 <script>
 import { GlTooltipDirective, GlDeprecatedButton } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
-import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import NoteHeader from '~/notes/components/note_header.vue';
 
 export default {
   name: 'EventItem',
   components: {
     Icon,
-    TimeAgoTooltip,
+    NoteHeader,
     GlDeprecatedButton,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
   },
   props: {
+    id: {
+      type: [String, Number],
+      required: false,
+      default: undefined,
+    },
     author: {
       type: Object,
       required: true,
@@ -49,39 +54,36 @@ export default {
       default: true,
     },
   },
+  computed: {
+    noteId() {
+      return this.id ? `note_${this.id}` : undefined;
+    },
+  },
 };
 </script>
 
 <template>
-  <div class="d-flex align-items-center">
+  <div :id="noteId" class="d-flex align-items-center">
     <div class="circle-icon-container" :class="iconClass">
       <icon :size="16" :name="iconName" />
     </div>
     <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 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>
-          <span class="note-headline-light">@{{ author.username }}</span>
-        </a>
-        <span class="note-headline-light note-headline-meta">
-          <template v-if="createdAt">
-            <span class="system-note-separator">·</span>
-            <time-ago-tooltip :time="createdAt" tooltip-placement="bottom" />
-          </template>
-        </span>
-      </div>
+      <note-header
+        :note-id="id"
+        :author="author"
+        :created-at="createdAt"
+        :show-spinner="false"
+        class="pb-0"
+      >
+        <slot name="header-message">&middot;</slot>
+      </note-header>
+
       <slot></slot>
     </div>
 
     <slot v-if="showRightSlot" name="right-content"></slot>
 
-    <div v-else-if="showActionButtons" class="align-self-start">
+    <div v-else-if="showActionButtons">
       <gl-deprecated-button
         v-for="button in actionButtons"
         :key="button.title"

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

@@ -40,8 +40,10 @@ export default {
   },
 
   created() {
+    // window.location.pathname is the URL without the protocol or hash/querystring
+    // i.e. http://server/url?query=string#note_123 -> /server/url
     axios
-      .get(joinPaths(window.location.href, 'discussions'))
+      .get(joinPaths(window.location.pathname, 'discussions'))
       .then(({ data }) => {
         this.discussions = data;
       })

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

 <script>
-import Icon from '~/vue_shared/components/icon.vue';
-import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
 
 export default {
-  components: { Icon, TimeAgoTooltip },
+  components: { EventItem },
   props: {
     discussion: {
       type: Object,
@@ -20,32 +19,15 @@ export default {
 
 <template>
   <li v-if="systemNote" class="card border-bottom system-note p-0">
-    <div class="note-header-info mx-3 my-4">
-      <div class="timeline-icon mr-0">
-        <icon ref="icon" :name="systemNote.system_note_icon_name" />
-      </div>
-
-      <a
-        :href="systemNote.author.path"
-        class="js-user-link ml-3"
-        :data-user-id="systemNote.author.id"
-      >
-        <strong ref="authorName" class="note-header-author-name">
-          {{ systemNote.author.name }}
-        </strong>
-        <span
-          v-if="systemNote.author.status_tooltip_html"
-          ref="authorStatus"
-          v-html="systemNote.author.status_tooltip_html"
-        ></span>
-        <span ref="authorUsername" class="note-headline-light">
-          @{{ systemNote.author.username }}
-        </span>
-      </a>
-      <span ref="stateChangeMessage" class="note-headline-light">
-        {{ systemNote.note }}
-        <time-ago-tooltip :time="systemNote.created_at" />
-      </span>
-    </div>
+    <event-item
+      :id="systemNote.id"
+      :author="systemNote.author"
+      :created-at="systemNote.created_at"
+      :icon-name="systemNote.system_note_icon_name"
+      icon-class="timeline-icon m-0"
+      class="m-3"
+    >
+      <template #header-message>{{ systemNote.note }}</template>
+    </event-item>
   </li>
 </template>

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';
+import NoteHeader from '~/notes/components/note_header.vue';
 
 describe('Event Item', () => {
   let wrapper;
@@ -9,8 +10,13 @@ describe('Event Item', () => {
     wrapper = mountFn(Component, options);
   };
 
+  const noteHeader = () => wrapper.find(NoteHeader);
+
   describe('initial state', () => {
     const propsData = {
+      id: 123,
+      createdAt: 'createdAt',
+      headerMessage: 'header message',
       author: {
         name: 'Tanuki',
         username: 'gitlab',
@@ -25,12 +31,13 @@ describe('Event Item', () => {
       mountComponent({ propsData });
     });
 
-    it('uses the author name', () => {
-      expect(wrapper.find('.js-author').text()).toContain(propsData.author.name);
-    });
-
-    it('uses the author username', () => {
-      expect(wrapper.find('.js-author').text()).toContain(`@${propsData.author.username}`);
+    it('passes the expected values to the note header component', () => {
+      expect(noteHeader().props()).toMatchObject({
+        noteId: propsData.id,
+        author: propsData.author,
+        createdAt: propsData.createdAt,
+        showSpinner: false,
+      });
     });
 
     it('uses the fallback icon', () => {

ee/spec/frontend/vulnerabilities/footer_spec.js

@@ -96,7 +96,7 @@ describe('Vulnerability Footer', () => {
   });
 
   describe('state history', () => {
-    const discussionUrl = 'http://localhost/discussions';
+    const discussionUrl = '/discussions';
 
     const historyList = () => wrapper.find({ ref: 'historyList' });
     const historyEntries = () => wrapper.findAll(HistoryEntry);
@@ -107,7 +107,7 @@ describe('Vulnerability Footer', () => {
       expect(historyList().exists()).toBe(false);
     });
 
-    it('does render the history list if there are history items', () => {
+    it('renders the history list if there are history items', () => {
       // The shape of this object doesn't matter for this test, we just need to verify that it's passed to the history
       // entry.
       const historyItems = [{ id: 1, note: 'some note' }, { id: 2, note: 'another note' }];

ee/spec/frontend/vulnerabilities/history_entry_spec.js

-import { shallowMount } from '@vue/test-utils';
-import Icon from '~/vue_shared/components/icon.vue';
-import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+import { mount } from '@vue/test-utils';
 import HistoryEntry from 'ee/vulnerabilities/components/history_entry.vue';
+import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue';
 
 describe('History Entry', () => {
   let wrapper;
 
   const note = {
     system: true,
+    id: 123,
     note: 'changed vulnerability status to dismissed',
     system_note_icon_name: 'cancel',
-    created_at: 'created_at_timestamp',
+    created_at: new Date().toISOString(),
     author: {
       name: 'author name',
       username: 'author username',
@@ -19,7 +19,7 @@ describe('History Entry', () => {
   };
 
   const createWrapper = options => {
-    wrapper = shallowMount(HistoryEntry, {
+    wrapper = mount(HistoryEntry, {
       propsData: {
         discussion: {
           notes: [{ ...note, ...options }],
@@ -28,52 +28,24 @@ describe('History Entry', () => {
     });
   };
 
-  const icon = () => wrapper.find(Icon);
-  const authorName = () => wrapper.find({ ref: 'authorName' });
-  const authorUsername = () => wrapper.find({ ref: 'authorUsername' });
-  const authorStatus = () => wrapper.find({ ref: 'authorStatus' });
-  const stateChangeMessage = () => wrapper.find({ ref: 'stateChangeMessage' });
-  const timeAgoTooltip = () => wrapper.find(TimeAgoTooltip);
+  const eventItem = () => wrapper.find(EventItem);
 
   afterEach(() => wrapper.destroy());
 
-  describe('default wrapper tests', () => {
-    beforeEach(() => createWrapper());
+  it('passes the expected values to the event item component', () => {
+    createWrapper();
 
-    it('shows the correct icon', () => {
-      expect(icon().exists()).toBe(true);
-      expect(icon().attributes('name')).toBe(note.system_note_icon_name);
-    });
-
-    it('shows the correct user', () => {
-      expect(authorName().text()).toBe(note.author.name);
-      expect(authorUsername().text()).toBe(`@${note.author.username}`);
-    });
-
-    it('shows the correct status if the user has a status set', () => {
-      expect(authorStatus().exists()).toBe(true);
-      expect(authorStatus().element.innerHTML).toBe(note.author.status_tooltip_html);
-    });
-
-    it('shows the state change message', () => {
-      expect(stateChangeMessage().text()).toBe(note.note);
-    });
-
-    it('shows the time ago tooltip', () => {
-      expect(timeAgoTooltip().exists()).toBe(true);
-      expect(timeAgoTooltip().attributes('time')).toBe(note.created_at);
+    expect(eventItem().text()).toContain(note.note);
+    expect(eventItem().props()).toMatchObject({
+      id: note.id,
+      author: note.author,
+      createdAt: note.created_at,
+      iconName: note.system_note_icon_name,
     });
   });
 
-  describe('custom wrapper tests', () => {
-    it('does not show the user status if user has no status set', () => {
-      createWrapper({ author: { status_tooltip_html: undefined } });
-      expect(authorStatus().exists()).toBe(false);
-    });
-
-    it('does not render anything if there is no system note', () => {
-      createWrapper({ system: false });
-      expect(wrapper.html()).toBeFalsy();
-    });
+  it('does not render anything if there is no system note', () => {
+    createWrapper({ system: false });
+    expect(wrapper.html()).toBeFalsy();
   });
 });

spec/frontend/notes/components/note_header_spec.js

@@ -16,7 +16,9 @@ describe('NoteHeader component', () => {
   const findActionsWrapper = () => wrapper.find({ ref: 'discussionActions' });
   const findChevronIcon = () => wrapper.find({ ref: 'chevronIcon' });
   const findActionText = () => wrapper.find({ ref: 'actionText' });
+  const findTimestampLink = () => wrapper.find({ ref: 'noteTimestampLink' });
   const findTimestamp = () => wrapper.find({ ref: 'noteTimestamp' });
+  const findSpinner = () => wrapper.find({ ref: 'spinner' });
 
   const author = {
     avatar_url: null,
@@ -33,11 +35,7 @@ describe('NoteHeader component', () => {
       store: new Vuex.Store({
         actions,
       }),
-      propsData: {
-        ...props,
-        actionTextHtml: '',
-        noteId: '1394',
-      },
+      propsData: { ...props },
     });
   };
 
@@ -108,17 +106,18 @@ describe('NoteHeader component', () => {
     createComponent();
 
     expect(findActionText().exists()).toBe(false);
-    expect(findTimestamp().exists()).toBe(false);
+    expect(findTimestampLink().exists()).toBe(false);
   });
 
   describe('when createdAt is passed as a prop', () => {
     it('renders action text and a timestamp', () => {
       createComponent({
         createdAt: '2017-08-02T10:51:58.559Z',
+        noteId: 123,
       });
 
       expect(findActionText().exists()).toBe(true);
-      expect(findTimestamp().exists()).toBe(true);
+      expect(findTimestampLink().exists()).toBe(true);
     });
 
     it('renders correct actionText if passed', () => {
@@ -133,8 +132,9 @@ describe('NoteHeader component', () => {
     it('calls an action when timestamp is clicked', () => {
       createComponent({
         createdAt: '2017-08-02T10:51:58.559Z',
+        noteId: 123,
       });
-      findTimestamp().trigger('click');
+      findTimestampLink().trigger('click');
 
       expect(actions.setTargetNoteHash).toHaveBeenCalled();
     });
@@ -153,4 +153,30 @@ describe('NoteHeader component', () => {
       expect(wrapper.find(GitlabTeamMemberBadge).exists()).toBe(expected);
     },
   );
+
+  describe('loading spinner', () => {
+    it('shows spinner when showSpinner is true', () => {
+      createComponent();
+      expect(findSpinner().exists()).toBe(true);
+    });
+
+    it('does not show spinner when showSpinner is false', () => {
+      createComponent({ showSpinner: false });
+      expect(findSpinner().exists()).toBe(false);
+    });
+  });
+
+  describe('timestamp', () => {
+    it('shows timestamp as a link if a noteId was provided', () => {
+      createComponent({ createdAt: new Date().toISOString(), noteId: 123 });
+      expect(findTimestampLink().exists()).toBe(true);
+      expect(findTimestamp().exists()).toBe(false);
+    });
+
+    it('shows timestamp as plain text if a noteId was not provided', () => {
+      createComponent({ createdAt: new Date().toISOString() });
+      expect(findTimestampLink().exists()).toBe(false);
+      expect(findTimestamp().exists()).toBe(true);
+    });
+  });
 });