fix discussion comment specspsimyn-resolve-component

simplify DOM for diff discussions
split discussion reply area to its own component

4 files changed, 298 insertions, 12 deletions

diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue
index 0c726f8c6fa..2056329d922 100644
--- a/app/assets/javascripts/diffs/components/diff_discussions.vue
+++ b/app/assets/javascripts/diffs/components/diff_discussions.vue
@@ -1,9 +1,11 @@
 <script>
-import noteableDiscussion from '../../notes/components/noteable_discussion.vue';
+import noteableNote from '~/notes/components/noteable_note.vue';
+import discussionReply from '~/notes/components/discussion_reply.vue';
 
 export default {
   components: {
-    noteableDiscussion,
+    discussionReply,
+    noteableNote,
   },
   props: {
     notes: {
@@ -11,6 +13,11 @@ export default {
       required: true,
     },
   },
+  methods: {
+    componentData(note) {
+      return note.isPlaceholderNote ? this.note.notes[0] : note;
+    },
+  },
 };
 </script>
 
@@ -19,21 +26,23 @@ export default {
     v-if="notes.length"
   >
     <div
-      v-for="notesArr in notes"
-      :key="notesArr.id"
+      v-for="discussion in notes"
+      :key="discussion.id"
       class="discussion-notes diff-discussions"
     >
       <ul
         class="notes"
-        :data-discussion-id="notesArr.id"
+        :data-discussion-id="discussion.id"
       >
-        <noteable-discussion
-          :note="notesArr"
-          :render-header="false"
-          :render-diff-file="false"
-          :always-expanded="true"
+        <noteable-note
+          v-for="note in discussion.notes"
+          :note="componentData(note)"
+          :key="note.id"
         />
       </ul>
+      <discussion-reply
+        :note="discussion"
+      />
     </div>
   </div>
 </template>
diff --git a/app/assets/javascripts/notes/components/discussion_reply.vue b/app/assets/javascripts/notes/components/discussion_reply.vue
new file mode 100644
index 00000000000..239746d4d71
--- /dev/null
+++ b/app/assets/javascripts/notes/components/discussion_reply.vue
@@ -0,0 +1,277 @@
+<script>
+import _ from 'underscore';
+import { mapActions, mapGetters } from 'vuex';
+import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg';
+import nextDiscussionsSvg from 'icons/_next_discussion.svg';
+import Flash from '../../flash';
+import { SYSTEM_NOTE } from '../constants';
+import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
+import noteableNote from './noteable_note.vue';
+import noteHeader from './note_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 placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
+import placeholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue';
+import autosave from '../mixins/autosave';
+import noteable from '../mixins/noteable';
+import resolvable from '../mixins/resolvable';
+import tooltip from '../../vue_shared/directives/tooltip';
+import { scrollToElement } from '../../lib/utils/common_utils';
+
+export default {
+  components: {
+    noteableNote,
+    diffWithNote,
+    userAvatarLink,
+    noteHeader,
+    noteSignedOutWidget,
+    noteEditedText,
+    noteForm,
+    placeholderNote,
+    placeholderSystemNote,
+  },
+  directives: {
+    tooltip,
+  },
+  mixins: [autosave, noteable, resolvable],
+  props: {
+    note: {
+      type: Object,
+      required: true,
+    },
+  },
+  data() {
+    return {
+      isReplying: false,
+      isResolving: false,
+    };
+  },
+  computed: {
+    ...mapGetters([
+      'getNoteableData',
+      'discussionCount',
+      'resolvedDiscussionCount',
+      'allDiscussions',
+      'unresolvedDiscussions',
+    ]),
+    discussion() {
+      return {
+        ...this.note.notes[0],
+        truncatedDiffLines: this.note.truncated_diff_lines || [],
+        truncatedDiffLinesPath: this.note.truncated_diff_lines_path,
+        diffFile: this.note.diff_file,
+        diffDiscussion: this.note.diff_discussion,
+        imageDiffHtml: this.note.image_diff_html,
+        active: this.note.active,
+        discussionPath: this.note.discussion_path,
+        resolved: this.note.resolved,
+        resolvedBy: this.note.resolved_by,
+        resolvedByPush: this.note.resolved_by_push,
+        resolvedAt: this.note.resolved_at,
+      };
+    },
+    canReply() {
+      return this.getNoteableData.current_user.can_create_note;
+    },
+    newNotePath() {
+      return this.getNoteableData.create_note_path;
+    },
+    hasMultipleUnresolvedDiscussions() {
+      return this.unresolvedDiscussions.length > 1;
+    },
+  },
+  mounted() {
+    if (this.isReplying) {
+      this.initAutoSave(this.discussion.noteable_type);
+    }
+  },
+  updated() {
+    if (this.isReplying) {
+      if (!this.autosave) {
+        this.initAutoSave(this.discussion.noteable_type);
+      } else {
+        this.setAutoSave();
+      }
+    }
+  },
+  created() {
+    this.resolveDiscussionsSvg = resolveDiscussionsSvg;
+    this.nextDiscussionsSvg = nextDiscussionsSvg;
+  },
+  methods: {
+    ...mapActions([
+      'saveNote',
+      'toggleDiscussion',
+      'removePlaceholderNotes',
+      'toggleResolveNote',
+      'expandDiscussion',
+    ]),
+    componentName(note) {
+      if (note.isPlaceholderNote) {
+        if (note.placeholderType === SYSTEM_NOTE) {
+          return placeholderSystemNote;
+        }
+        return placeholderNote;
+      }
+
+      return noteableNote;
+    },
+    componentData(note) {
+      return note.isPlaceholderNote ? this.note.notes[0] : note;
+    },
+    toggleDiscussionHandler() {
+      this.toggleDiscussion({ discussionId: this.note.id });
+    },
+    showReplyForm() {
+      this.isReplying = true;
+    },
+    cancelReplyForm(shouldConfirm) {
+      if (shouldConfirm && this.$refs.noteForm.isDirty) {
+        // eslint-disable-next-line no-alert
+        if (!confirm('Are you sure you want to cancel creating this comment?')) {
+          return;
+        }
+      }
+
+      this.resetAutoSave();
+      this.isReplying = false;
+    },
+    saveReply(noteText, form, callback) {
+      const replyData = {
+        endpoint: this.newNotePath,
+        flashContainer: this.$el,
+        data: {
+          in_reply_to_discussion_id: this.note.reply_id,
+          target_type: this.getNoteableData.targetType,
+          target_id: this.discussion.noteable_id,
+          // head_commit_sha:
+          note: { note: noteText },
+        },
+      };
+      this.isReplying = false;
+
+      this.saveNote(replyData)
+        .then(() => {
+          this.resetAutoSave();
+          callback();
+        })
+        .catch(err => {
+          this.removePlaceholderNotes();
+          this.isReplying = true;
+          this.$nextTick(() => {
+            const msg = `Your comment could not be submitted!
+Please check your network connection and try again.`;
+            Flash(msg, 'alert', this.$el);
+            this.$refs.noteForm.note = noteText;
+            callback(err);
+          });
+        });
+    },
+    jumpToNextDiscussion() {
+      const discussionIds = this.allDiscussions.map(d => d.id);
+      const unresolvedIds = this.unresolvedDiscussions.map(d => d.id);
+      const currentIndex = discussionIds.indexOf(this.note.id);
+      const remainingAfterCurrent = discussionIds.slice(currentIndex + 1);
+      const nextIndex = _.findIndex(remainingAfterCurrent, id => unresolvedIds.indexOf(id) > -1);
+
+      if (nextIndex > -1) {
+        const nextId = remainingAfterCurrent[nextIndex];
+        const el = document.querySelector(`[data-discussion-id="${nextId}"]`);
+
+        if (el) {
+          this.expandDiscussion({ discussionId: nextId });
+          scrollToElement(el);
+        }
+      }
+    },
+  },
+};
+</script>
+
+<template>
+  <div
+    :class="{ 'is-replying': isReplying }"
+    class="discussion-reply-holder"
+  >
+    <template v-if="!isReplying && canReply">
+      <div
+        class="btn-group-justified discussion-with-resolve-btn"
+        role="group"
+      >
+        <div
+          class="btn-group"
+          role="group">
+          <button
+            @click="showReplyForm"
+            type="button"
+            class="js-vue-discussion-reply btn btn-text-field"
+            title="Add a reply">Reply...</button>
+        </div>
+        <div
+          v-if="note.resolvable"
+          class="btn-group"
+          role="group">
+          <button
+            @click="resolveHandler()"
+            type="button"
+            class="btn btn-default"
+          >
+            <i
+              v-if="isResolving"
+              aria-hidden="true"
+              class="fa fa-spinner fa-spin"
+            ></i>
+            {{ resolveButtonTitle }}
+          </button>
+        </div>
+        <div
+          v-if="note.resolvable"
+          class="btn-group discussion-actions"
+          role="group"
+        >
+          <div
+            v-if="!discussionResolved"
+            class="btn-group"
+            role="group">
+            <a
+              :href="note.resolve_with_issue_path"
+              v-tooltip
+              class="new-issue-for-discussion btn
+                btn-default discussion-create-issue-btn"
+              title="Resolve this discussion in a new issue"
+              data-container="body"
+            >
+              <span v-html="resolveDiscussionsSvg"></span>
+            </a>
+          </div>
+          <div
+            v-if="hasMultipleUnresolvedDiscussions"
+            class="btn-group"
+            role="group">
+            <button
+              @click="jumpToNextDiscussion"
+              v-tooltip
+              class="btn btn-default discussion-next-btn"
+              title="Jump to next unresolved discussion"
+              data-container="body"
+            >
+              <span v-html="nextDiscussionsSvg"></span>
+            </button>
+          </div>
+        </div>
+      </div>
+    </template>
+    <note-form
+      v-if="isReplying"
+      save-button-title="Comment"
+      :note="note"
+      :is-editing="false"
+      @handleFormUpdate="saveReply"
+      @cancelForm="cancelReplyForm"
+      ref="noteForm"
+    />
+    <note-signed-out-widget v-if="!canReply" />
+  </div>
+</template>
diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb
index 3af086300a2..5568a3821cc 100644
--- a/spec/features/merge_request/user_posts_diff_notes_spec.rb
+++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb
@@ -185,7 +185,7 @@ describe 'Merge request > User posts diff notes', :js do
     end
 
     describe 'posting a note' do
-      xit 'adds as discussion' do
+      it 'adds as discussion' do
         expect(page).to have_css('.js-temp-notes-holder', count: 2)
 
         should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false)
diff --git a/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb b/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb
index 102396486dc..3efd4ddbe86 100644
--- a/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb
+++ b/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb
@@ -112,7 +112,7 @@ describe 'User comments on a diff', :js do
   end
 
   context 'when editing comments' do
-    xit 'edits a comment' do
+    it 'edits a comment' do
       click_diff_line(find("[id='#{sample_commit.line_code}']"))
 
       page.within('.js-discussion-note-form') do