Extract DiffDiscussionHeader to a new component

Since the Noteable Discussion already had pretty robust test 
coverage of the header, this is basically just moving the code 
for the component and the tests that cover it to a 
separate file.

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

+<script>
+import { mapActions } from 'vuex';
+import _ from 'underscore';
+
+import { s__, __, sprintf } from '~/locale';
+import { truncateSha } from '~/lib/utils/text_utility';
+
+import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
+import noteEditedText from './note_edited_text.vue';
+import noteHeader from './note_header.vue';
+
+export default {
+  name: 'DiffDiscussionHeader',
+  components: {
+    userAvatarLink,
+    noteEditedText,
+    noteHeader,
+  },
+  props: {
+    discussion: {
+      type: Object,
+      required: true,
+    },
+  },
+  computed: {
+    notes() {
+      return this.discussion.notes;
+    },
+    firstNote() {
+      return this.notes[0];
+    },
+    lastNote() {
+      return this.notes[this.notes.length - 1];
+    },
+    author() {
+      return this.firstNote.author;
+    },
+    resolvedText() {
+      return this.discussion.resolved_by_push ? __('Automatically resolved') : __('Resolved');
+    },
+    lastUpdatedBy() {
+      return this.notes.length > 1 ? this.lastNote.author : null;
+    },
+    lastUpdatedAt() {
+      return this.notes.length > 1 ? this.lastNote.created_at : null;
+    },
+    headerText() {
+      const linkStart = `<a href="${_.escape(this.discussion.discussion_path)}">`;
+      const linkEnd = '</a>';
+
+      const { commit_id: commitId } = this.discussion;
+      let commitDisplay = commitId;
+
+      if (commitId) {
+        commitDisplay = `<span class="commit-sha">${truncateSha(commitId)}</span>`;
+      }
+
+      const {
+        for_commit: isForCommit,
+        diff_discussion: isDiffDiscussion,
+        active: isActive,
+      } = this.discussion;
+
+      let text = s__('MergeRequests|started a thread');
+      if (isForCommit) {
+        text = s__(
+          'MergeRequests|started a thread on commit %{linkStart}%{commitDisplay}%{linkEnd}',
+        );
+      } else if (isDiffDiscussion && commitId) {
+        text = isActive
+          ? s__('MergeRequests|started a thread on commit %{linkStart}%{commitDisplay}%{linkEnd}')
+          : s__(
+              'MergeRequests|started a thread on an outdated change in commit %{linkStart}%{commitDisplay}%{linkEnd}',
+            );
+      } else if (isDiffDiscussion) {
+        text = isActive
+          ? s__('MergeRequests|started a thread on %{linkStart}the diff%{linkEnd}')
+          : s__(
+              'MergeRequests|started a thread on %{linkStart}an old version of the diff%{linkEnd}',
+            );
+      }
+
+      return sprintf(text, { commitDisplay, linkStart, linkEnd }, false);
+    },
+  },
+  methods: {
+    ...mapActions(['toggleDiscussion']),
+    toggleDiscussionHandler() {
+      this.toggleDiscussion({ discussionId: this.discussion.id });
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="discussion-header note-wrapper">
+    <div v-once class="timeline-icon align-self-start flex-shrink-0">
+      <user-avatar-link
+        v-if="author"
+        :link-href="author.path"
+        :img-src="author.avatar_url"
+        :img-alt="author.name"
+        :img-size="40"
+      />
+    </div>
+    <div class="timeline-content w-100">
+      <note-header
+        :author="author"
+        :created-at="firstNote.created_at"
+        :note-id="firstNote.id"
+        :include-toggle="true"
+        :expanded="discussion.expanded"
+        @toggleHandler="toggleDiscussionHandler"
+      >
+        <span v-html="headerText"></span>
+      </note-header>
+      <note-edited-text
+        v-if="discussion.resolved"
+        :edited-at="discussion.resolved_at"
+        :edited-by="discussion.resolved_by"
+        :action-text="resolvedText"
+        class-name="discussion-headline-light js-discussion-headline"
+      />
+      <note-edited-text
+        v-else-if="lastUpdatedAt"
+        :edited-at="lastUpdatedAt"
+        :edited-by="lastUpdatedBy"
+        :action-text="__('Last updated')"
+        class-name="discussion-headline-light js-discussion-headline"
+      />
+    </div>
+  </div>
+</template>

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

 <script>
-import _ from 'underscore';
 import { mapActions, mapGetters } from 'vuex';
 import { GlTooltipDirective } from '@gitlab/ui';
-import { truncateSha } from '~/lib/utils/text_utility';
-import { s__, __, sprintf } from '~/locale';
+import { s__, __ } from '~/locale';
 import { clearDraft, getDiscussionReplyKey } from '~/lib/utils/autosave';
 import icon from '~/vue_shared/components/icon.vue';
 import diffLineNoteFormMixin from 'ee_else_ce/notes/mixins/diff_line_note_form';
 import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
 import Flash from '../../flash';
 import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
-import noteHeader from './note_header.vue';
+import diffDiscussionHeader from './diff_discussion_header.vue';
 import noteSignedOutWidget from './note_signed_out_widget.vue';
-import noteEditedText from './note_edited_text.vue';
 import noteForm from './note_form.vue';
 import diffWithNote from './diff_with_note.vue';
 import noteable from '../mixins/noteable';
@@ -27,9 +24,8 @@ export default {
   components: {
     icon,
     userAvatarLink,
-    noteHeader,
+    diffDiscussionHeader,
     noteSignedOutWidget,
-    noteEditedText,
     noteForm,
     DraftNote: () => import('ee_component/batch_comments/components/draft_note.vue'),
     TimelineEntryItem,
@@ -92,9 +88,6 @@ export default {
     currentUser() {
       return this.getUserData;
     },
-    author() {
-      return this.firstNote.author;
-    },
     autosaveKey() {
       return getDiscussionReplyKey(this.firstNote.noteable_type, this.discussion.id);
     },
@@ -104,27 +97,6 @@ export default {
     firstNote() {
       return this.discussion.notes.slice(0, 1)[0];
     },
-    lastUpdatedBy() {
-      const { notes } = this.discussion;
-
-      if (notes.length > 1) {
-        return notes[notes.length - 1].author;
-      }
-
-      return null;
-    },
-    lastUpdatedAt() {
-      const { notes } = this.discussion;
-
-      if (notes.length > 1) {
-        return notes[notes.length - 1].created_at;
-      }
-
-      return null;
-    },
-    resolvedText() {
-      return this.discussion.resolved_by_push ? __('Automatically resolved') : __('Resolved');
-    },
     shouldShowJumpToNextDiscussion() {
       return this.showJumpToNextDiscussion(this.discussionsByDiffOrder ? 'diff' : 'discussion');
     },
@@ -150,40 +122,6 @@ export default {
     shouldHideDiscussionBody() {
       return this.shouldRenderDiffs && !this.isExpanded;
     },
-    actionText() {
-      const linkStart = `<a href="${_.escape(this.discussion.discussion_path)}">`;
-      const linkEnd = '</a>';
-
-      let { commit_id: commitId } = this.discussion;
-      if (commitId) {
-        commitId = `<span class="commit-sha">${truncateSha(commitId)}</span>`;
-      }
-
-      const {
-        for_commit: isForCommit,
-        diff_discussion: isDiffDiscussion,
-        active: isActive,
-      } = this.discussion;
-
-      let text = s__('MergeRequests|started a thread');
-      if (isForCommit) {
-        text = s__('MergeRequests|started a thread on commit %{linkStart}%{commitId}%{linkEnd}');
-      } else if (isDiffDiscussion && commitId) {
-        text = isActive
-          ? s__('MergeRequests|started a thread on commit %{linkStart}%{commitId}%{linkEnd}')
-          : s__(
-              'MergeRequests|started a thread on an outdated change in commit %{linkStart}%{commitId}%{linkEnd}',
-            );
-      } else if (isDiffDiscussion) {
-        text = isActive
-          ? s__('MergeRequests|started a thread on %{linkStart}the diff%{linkEnd}')
-          : s__(
-              'MergeRequests|started a thread on %{linkStart}an old version of the diff%{linkEnd}',
-            );
-      }
-
-      return sprintf(text, { commitId, linkStart, linkEnd }, false);
-    },
     diffLine() {
       if (this.line) {
         return this.line;
@@ -208,16 +146,11 @@ export default {
   methods: {
     ...mapActions([
       'saveNote',
-      'toggleDiscussion',
       'removePlaceholderNotes',
       'toggleResolveNote',
       'expandDiscussion',
       'removeConvertedDiscussion',
     ]),
-    truncateSha,
-    toggleDiscussionHandler() {
-      this.toggleDiscussion({ discussionId: this.discussion.id });
-    },
     showReplyForm() {
       this.isReplying = true;
     },
@@ -311,43 +244,7 @@ export default {
         class="discussion js-discussion-container"
         data-qa-selector="discussion_content"
       >
-        <div v-if="shouldRenderDiffs" class="discussion-header note-wrapper">
-          <div v-once class="timeline-icon align-self-start flex-shrink-0">
-            <user-avatar-link
-              v-if="author"
-              :link-href="author.path"
-              :img-src="author.avatar_url"
-              :img-alt="author.name"
-              :img-size="40"
-            />
-          </div>
-          <div class="timeline-content w-100">
-            <note-header
-              :author="author"
-              :created-at="firstNote.created_at"
-              :note-id="firstNote.id"
-              :include-toggle="true"
-              :expanded="discussion.expanded"
-              @toggleHandler="toggleDiscussionHandler"
-            >
-              <span v-html="actionText"></span>
-            </note-header>
-            <note-edited-text
-              v-if="discussion.resolved"
-              :edited-at="discussion.resolved_at"
-              :edited-by="discussion.resolved_by"
-              :action-text="resolvedText"
-              class-name="discussion-headline-light js-discussion-headline"
-            />
-            <note-edited-text
-              v-else-if="lastUpdatedAt"
-              :edited-at="lastUpdatedAt"
-              :edited-by="lastUpdatedBy"
-              action-text="Last updated"
-              class-name="discussion-headline-light js-discussion-headline"
-            />
-          </div>
-        </div>
+        <diff-discussion-header v-if="shouldRenderDiffs" :discussion="discussion" />
         <div v-if="!shouldHideDiscussionBody" class="discussion-body">
           <component
             :is="wrapperComponent"

locale/gitlab.pot

@@ -10660,10 +10660,10 @@ msgstr ""
 msgid "MergeRequests|started a thread on %{linkStart}the diff%{linkEnd}"
 msgstr ""
 
-msgid "MergeRequests|started a thread on an outdated change in commit %{linkStart}%{commitId}%{linkEnd}"
+msgid "MergeRequests|started a thread on an outdated change in commit %{linkStart}%{commitDisplay}%{linkEnd}"
 msgstr ""
 
-msgid "MergeRequests|started a thread on commit %{linkStart}%{commitId}%{linkEnd}"
+msgid "MergeRequests|started a thread on commit %{linkStart}%{commitDisplay}%{linkEnd}"
 msgstr ""
 
 msgid "MergeRequest| %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}"

spec/javascripts/notes/components/diff_discussion_header_spec.js

+import { mount, createLocalVue } from '@vue/test-utils';
+import createStore from '~/notes/stores';
+import diffDiscussionHeader from '~/notes/components/diff_discussion_header.vue';
+import { discussionMock } from '../mock_data';
+import mockDiffFile from '../../diffs/mock_data/diff_file';
+
+const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
+
+describe('diff_discussion_header component', () => {
+  let store;
+  let wrapper;
+
+  preloadFixtures(discussionWithTwoUnresolvedNotes);
+
+  beforeEach(() => {
+    window.mrTabs = {};
+    store = createStore();
+
+    const localVue = createLocalVue();
+    wrapper = mount(diffDiscussionHeader, {
+      store,
+      propsData: { discussion: discussionMock },
+      localVue,
+      sync: false,
+    });
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('should render user avatar', () => {
+    const discussion = { ...discussionMock };
+    discussion.diff_file = mockDiffFile;
+    discussion.diff_discussion = true;
+
+    wrapper.setProps({ discussion });
+
+    expect(wrapper.find('.user-avatar-link').exists()).toBe(true);
+  });
+
+  describe('action text', () => {
+    const commitId = 'razupaltuff';
+    const truncatedCommitId = commitId.substr(0, 8);
+    let commitElement;
+
+    beforeEach(done => {
+      store.state.diffs = {
+        projectPath: 'something',
+      };
+
+      wrapper.setProps({
+        discussion: {
+          ...discussionMock,
+          for_commit: true,
+          commit_id: commitId,
+          diff_discussion: true,
+          diff_file: {
+            ...mockDiffFile,
+          },
+        },
+      });
+
+      wrapper.vm
+        .$nextTick()
+        .then(() => {
+          commitElement = wrapper.find('.commit-sha');
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    describe('for diff threads without a commit id', () => {
+      it('should show started a thread on the diff text', done => {
+        Object.assign(wrapper.vm.discussion, {
+          for_commit: false,
+          commit_id: null,
+        });
+
+        wrapper.vm.$nextTick(() => {
+          expect(wrapper.text()).toContain('started a thread on the diff');
+
+          done();
+        });
+      });
+
+      it('should show thread on older version text', done => {
+        Object.assign(wrapper.vm.discussion, {
+          for_commit: false,
+          commit_id: null,
+          active: false,
+        });
+
+        wrapper.vm.$nextTick(() => {
+          expect(wrapper.text()).toContain('started a thread on an old version of the diff');
+
+          done();
+        });
+      });
+    });
+
+    describe('for commit threads', () => {
+      it('should display a monospace started a thread on commit', () => {
+        expect(wrapper.text()).toContain(`started a thread on commit ${truncatedCommitId}`);
+        expect(commitElement.exists()).toBe(true);
+        expect(commitElement.text()).toContain(truncatedCommitId);
+      });
+    });
+
+    describe('for diff thread with a commit id', () => {
+      it('should display started thread on commit header', done => {
+        wrapper.vm.discussion.for_commit = false;
+
+        wrapper.vm.$nextTick(() => {
+          expect(wrapper.text()).toContain(`started a thread on commit ${truncatedCommitId}`);
+
+          expect(commitElement).not.toBe(null);
+
+          done();
+        });
+      });
+
+      it('should display outdated change on commit header', done => {
+        wrapper.vm.discussion.for_commit = false;
+        wrapper.vm.discussion.active = false;
+
+        wrapper.vm.$nextTick(() => {
+          expect(wrapper.text()).toContain(
+            `started a thread on an outdated change in commit ${truncatedCommitId}`,
+          );
+
+          expect(commitElement).not.toBe(null);
+
+          done();
+        });
+      });
+    });
+  });
+});

spec/javascripts/notes/components/noteable_discussion_spec.js

-import { shallowMount, createLocalVue } from '@vue/test-utils';
+import { mount, createLocalVue } from '@vue/test-utils';
 import createStore from '~/notes/stores';
 import noteableDiscussion from '~/notes/components/noteable_discussion.vue';
 import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue';
@@ -23,7 +23,7 @@ describe('noteable_discussion component', () => {
     store.dispatch('setNotesData', notesDataMock);
 
     const localVue = createLocalVue();
-    wrapper = shallowMount(noteableDiscussion, {
+    wrapper = mount(noteableDiscussion, {
       store,
       propsData: { discussion: discussionMock },
       localVue,
@@ -35,16 +35,6 @@ describe('noteable_discussion component', () => {
     wrapper.destroy();
   });
 
-  it('should render user avatar', () => {
-    const discussion = { ...discussionMock };
-    discussion.diff_file = mockDiffFile;
-    discussion.diff_discussion = true;
-
-    wrapper.setProps({ discussion, renderDiffFile: true });
-
-    expect(wrapper.find('.user-avatar-link').exists()).toBe(true);
-  });
-
   it('should not render thread header for non diff threads', () => {
     expect(wrapper.find('.discussion-header').exists()).toBe(false);
   });
@@ -134,105 +124,6 @@ describe('noteable_discussion component', () => {
     });
   });
 
-  describe('action text', () => {
-    const commitId = 'razupaltuff';
-    const truncatedCommitId = commitId.substr(0, 8);
-    let commitElement;
-
-    beforeEach(done => {
-      store.state.diffs = {
-        projectPath: 'something',
-      };
-
-      wrapper.setProps({
-        discussion: {
-          ...discussionMock,
-          for_commit: true,
-          commit_id: commitId,
-          diff_discussion: true,
-          diff_file: {
-            ...mockDiffFile,
-          },
-        },
-        renderDiffFile: true,
-      });
-
-      wrapper.vm
-        .$nextTick()
-        .then(() => {
-          commitElement = wrapper.find('.commit-sha');
-        })
-        .then(done)
-        .catch(done.fail);
-    });
-
-    describe('for commit threads', () => {
-      it('should display a monospace started a thread on commit', () => {
-        expect(wrapper.text()).toContain(`started a thread on commit ${truncatedCommitId}`);
-        expect(commitElement.exists()).toBe(true);
-        expect(commitElement.text()).toContain(truncatedCommitId);
-      });
-    });
-
-    describe('for diff thread with a commit id', () => {
-      it('should display started thread on commit header', done => {
-        wrapper.vm.discussion.for_commit = false;
-
-        wrapper.vm.$nextTick(() => {
-          expect(wrapper.text()).toContain(`started a thread on commit ${truncatedCommitId}`);
-
-          expect(commitElement).not.toBe(null);
-
-          done();
-        });
-      });
-
-      it('should display outdated change on commit header', done => {
-        wrapper.vm.discussion.for_commit = false;
-        wrapper.vm.discussion.active = false;
-
-        wrapper.vm.$nextTick(() => {
-          expect(wrapper.text()).toContain(
-            `started a thread on an outdated change in commit ${truncatedCommitId}`,
-          );
-
-          expect(commitElement).not.toBe(null);
-
-          done();
-        });
-      });
-    });
-
-    describe('for diff threads without a commit id', () => {
-      it('should show started a thread on the diff text', done => {
-        Object.assign(wrapper.vm.discussion, {
-          for_commit: false,
-          commit_id: null,
-        });
-
-        wrapper.vm.$nextTick(() => {
-          expect(wrapper.text()).toContain('started a thread on the diff');
-
-          done();
-        });
-      });
-
-      it('should show thread on older version text', done => {
-        Object.assign(wrapper.vm.discussion, {
-          for_commit: false,
-          commit_id: null,
-          active: false,
-        });
-
-        wrapper.vm.$nextTick(() => {
-          expect(wrapper.text()).toContain('started a thread on an old version of the diff');
-
-          done();
-        });
-      });
-    });
-  });
-
   describe('for resolved thread', () => {
     beforeEach(() => {
       const discussion = getJSONFixture(discussionWithTwoUnresolvedNotes)[0];
@@ -262,6 +153,7 @@ describe('noteable_discussion component', () => {
       }));
 
       wrapper.setProps({ discussion });
+
       wrapper.vm
         .$nextTick()
         .then(done)