Moved most of the data handling into discussion & notes models

Reduced some duplicated code with compiling components
Fixed bug with resolve button tooltip not updating after resolving discussion

12 files changed, 141 insertions, 134 deletions

diff --git a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6
index a0c859ab7f6..3c30f9df058 100644
--- a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6
+++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6
@@ -10,40 +10,31 @@
     },
     computed: {
       allResolved: function () {
-        let allResolved = true;
-        for (const discussionId in this.discussions) {
-          const discussion = this.discussions[discussionId];
-
-          for (const noteId in discussion) {
-            const note = discussion[noteId];
-
-            if (!note.resolved) {
-              allResolved = false;
-            }
-          }
-        }
-
-        return allResolved;
+        const discussion = this.discussions[discussionId];
+        return discussion.isResolved();
       }
     },
     methods: {
       jumpToNextUnresolvedDiscussion: function () {
-        let nextUnresolvedDiscussionId;
+        let nextUnresolvedDiscussionId,
+            firstUnresolvedDiscussionId;
 
         if (!this.discussionId) {
+          let i = 0;
           for (const discussionId in this.discussions) {
             const discussion = this.discussions[discussionId];
+            const isResolved = discussion.isResolved();
 
-            for (const noteId in discussion) {
-              const note = discussion[noteId];
+            if (!firstUnresolvedDiscussionId && !isResolved) {
+              firstUnresolvedDiscussionId = discussionId;
+            }
 
-              if (!note.resolved) {
-                nextUnresolvedDiscussionId = discussionId;
-                break;
-              }
+            if (!isResolved) {
+              nextUnresolvedDiscussionId = discussionId;
+              break;
             }
 
-            if (nextUnresolvedDiscussionId) break;
+            i++;
           }
         } else {
           const discussionKeys = Object.keys(this.discussions),
@@ -52,9 +43,16 @@
 
           if (nextDiscussionId) {
             nextUnresolvedDiscussionId = nextDiscussionId;
+          } else {
+            firstUnresolvedDiscussionId = discussionKeys[0];
           }
         }
 
+        if (firstUnresolvedDiscussionId) {
+          // Jump to first unresolved discussion
+          nextUnresolvedDiscussionId = firstUnresolvedDiscussionId;
+        }
+
         if (nextUnresolvedDiscussionId) {
           $.scrollTo(`.discussion[data-discussion-id="${nextUnresolvedDiscussionId}"]`, {
             offset: -($('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight())
diff --git a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6
index e077c3ba395..c8cf6b8ad3a 100644
--- a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6
+++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6
@@ -18,7 +18,16 @@
         loading: false
       };
     },
+    watch: {
+      'discussions': {
+        handler: 'updateTooltip',
+        deep: true
+      }
+    },
     computed: {
+      note: function () {
+        return CommentsStore.get(this.discussionId, this.noteId);
+      },
       buttonText: function () {
         if (this.isResolved) {
           return `Resolved by ${this.resolvedByName}`;
@@ -26,8 +35,12 @@
           return 'Mark as resolved';
         }
       },
-      isResolved: function () { return CommentsStore.get(this.discussionId, this.noteId).resolved; },
-      resolvedByName: function () { return CommentsStore.get(this.discussionId, this.noteId).user; },
+      isResolved: function () {
+        return this.note.resolved;
+      },
+      resolvedByName: function () {
+        return this.note.user;
+      },
     },
     methods: {
       updateTooltip: function () {
diff --git a/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6
index 56514afa6ce..5832435dcfe 100644
--- a/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6
+++ b/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6
@@ -5,20 +5,10 @@
     },
     computed: {
       isDiscussionResolved: function () {
-        const notes = CommentsStore.notesForDiscussion(this.discussionId),
-              discussion = CommentsStore.state[this.discussionId];
-        let allResolved = true;
+        const discussion = CommentsStore.state[this.discussionId],
+              notes = CommentsStore.notesForDiscussion(this.discussionId);
 
-        for (let i = 0; i < notes.length; i++) {
-          const noteId = notes[i];
-          const note = discussion[noteId];
-
-          if (!note.resolved) {
-            allResolved = false;
-          }
-        }
-
-        return allResolved;
+        return discussion.isResolved();
       },
       buttonText: function () {
         if (this.isDiscussionResolved) {
diff --git a/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6
index b8df89491ad..4d9e1dd8df8 100644
--- a/app/assets/javascripts/diff_notes/components/resolve_count.js.es6
+++ b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6
@@ -11,18 +11,9 @@
         let resolvedCount = 0;
 
         for (const discussionId in this.discussions) {
-          const comments = this.discussions[discussionId];
-          let resolved = true;
+          const discussion = this.discussions[discussionId];
 
-          for (const noteId in comments) {
-            const commentResolved = comments[noteId].resolved;
-
-            if (!commentResolved) {
-              resolved = false;
-            }
-          }
-
-          if (resolved) {
+          if (discussion.isResolved()) {
             resolvedCount++;
           }
         }
diff --git a/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6
index 543ed8625f3..47a317a07c4 100644
--- a/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6
+++ b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6
@@ -1,5 +1,5 @@
 ((w) => {
-  w.ResolveAllBtn = Vue.extend({
+  w.ResolveDiscussionBtn = Vue.extend({
     mixins: [
       ButtonMixins
     ],
@@ -17,15 +17,7 @@
     },
     computed: {
       allResolved: function () {
-        let isResolved = true;
-        for (const noteId in this.discussions[this.discussionId]) {
-          const resolved = this.discussions[this.discussionId][noteId].resolved;
-
-          if (!resolved) {
-            isResolved = false;
-          }
-        }
-        return isResolved;
+        return this.discussions[this.discussionId].isResolved();
       },
       buttonText: function () {
         if (this.allResolved) {
diff --git a/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6
index b856c718034..33b7648f0b6 100644
--- a/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6
+++ b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6
@@ -1,5 +1,6 @@
 //= require vue
 //= require vue-resource
+//= require_directory ./models
 //= require_directory ./stores
 //= require_directory ./services
 //= require_directory ./mixins
@@ -10,8 +11,8 @@ $(() => {
     el: '#diff-notes-app',
     components: {
       'resolve-btn': ResolveBtn,
-      'resolve-all-btn': ResolveAllBtn,
-      'resolve-comment-btn': ResolveCommentBtn,
+      'resolve-discussion-btn': ResolveDiscussionBtn,
+      'resolve-comment-btn': ResolveCommentBtn
     }
   });
 
@@ -21,4 +22,13 @@ $(() => {
       'resolve-count': ResolveCount
     }
   });
+
+  window.compileVueComponentsForDiffNotes = function () {
+    const $components = $('resolve-btn, resolve-discussion-btn, jump-to-discussion');
+    if ($components.length) {
+      $components.each(function () {
+        DiffNotesApp.$compile($(this).get(0))
+      });
+    }
+  }
 });
diff --git a/app/assets/javascripts/diff_notes/models/disucssion.js.es6 b/app/assets/javascripts/diff_notes/models/disucssion.js.es6
new file mode 100644
index 00000000000..bbc940214dd
--- /dev/null
+++ b/app/assets/javascripts/diff_notes/models/disucssion.js.es6
@@ -0,0 +1,51 @@
+class DiscussionModel {
+  constructor (discussionId) {
+    this.discussionId = discussionId;
+    this.notes = {};
+  }
+
+  createNote (noteId, resolved, user) {
+    Vue.set(this.notes, noteId, new NoteModel(this.discussionId, noteId, resolved, user));
+  }
+
+  deleteNote (noteId) {
+    Vue.delete(this.notes, noteId);
+  }
+
+  getNote (noteId) {
+    return this.notes[noteId];
+  }
+
+  isResolved () {
+    for (const noteId in this.notes) {
+      const note = this.notes[noteId];
+
+      if (!note.resolved) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  resolveAllNotes (user) {
+    for (const noteId in this.notes) {
+      const note = this.notes[noteId];
+
+      if (!note.resolved) {
+        note.resolved = true;
+        note.user = user;
+      }
+    }
+  }
+
+  unResolveAllNotes (user) {
+    for (const noteId in this.notes) {
+      const note = this.notes[noteId];
+
+      if (note.resolved) {
+        note.resolved = false;
+        note.user = null;
+      }
+    }
+  }
+}
diff --git a/app/assets/javascripts/diff_notes/models/note.js b/app/assets/javascripts/diff_notes/models/note.js
new file mode 100644
index 00000000000..2772991ce8b
--- /dev/null
+++ b/app/assets/javascripts/diff_notes/models/note.js
@@ -0,0 +1,8 @@
+class NoteModel {
+  constructor (discussionId, noteId, resolved, user) {
+    this.discussionId = discussionId;
+    this.noteId = noteId;
+    this.resolved = resolved;
+    this.user = user;
+  }
+}
diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6
index 50392e76fbe..0be938448b2 100644
--- a/app/assets/javascripts/diff_notes/services/resolve.js.es6
+++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6
@@ -24,17 +24,7 @@
     }
 
     toggleResolveForDiscussion(namespace, mergeRequestId, discussionId) {
-      const noteIds = CommentsStore.notesForDiscussion(discussionId);
-      let isResolved = true;
-
-      for (let i = 0; i < noteIds.length; i++) {
-        const noteId = noteIds[i];
-        const resolved = CommentsStore.state[discussionId][noteId].resolved;
-
-        if (!resolved) {
-          isResolved = false;
-        }
-      }
+      const isResolved = CommentsStore.state[discussionId].isResolved();
 
       if (isResolved) {
         return this.unResolveAll(namespace, mergeRequestId, discussionId);
@@ -55,9 +45,11 @@
       }, {}).then((response) => {
         const data = response.data;
         const user = data ? data.resolved_by : null;
+        const discussion = CommentsStore.state[discussionId];
+        discussion.resolveAllNotes(user);
+
         CommentsStore.loading[discussionId] = false;
 
-        CommentsStore.updateCommentsForDiscussion(discussionId, true, user);
 
         this.updateUpdatedHtml(discussionId, data);
       });
@@ -74,9 +66,10 @@
         discussionId
       }, {}).then((response) => {
         const data = response.data;
-        CommentsStore.loading[discussionId] = false;
+        const discussion = CommentsStore.state[discussionId];
+        discussion.unResolveAllNotes();
 
-        CommentsStore.updateCommentsForDiscussion(discussionId, false);
+        CommentsStore.loading[discussionId] = false;
 
         this.updateUpdatedHtml(discussionId, data);
       });
diff --git a/app/assets/javascripts/diff_notes/stores/comments.js.es6 b/app/assets/javascripts/diff_notes/stores/comments.js.es6
index 6e2040144a6..fbb71ca30af 100644
--- a/app/assets/javascripts/diff_notes/stores/comments.js.es6
+++ b/app/assets/javascripts/diff_notes/stores/comments.js.es6
@@ -3,57 +3,32 @@
     state: {},
     loading: {},
     get: function (discussionId, noteId) {
-      return this.state[discussionId][noteId];
+      return this.state[discussionId].getNote(noteId);
     },
     create: function (discussionId, noteId, resolved, user) {
+      let discussion = this.state[discussionId];
       if (!this.state[discussionId]) {
-        Vue.set(this.state, discussionId, {});
+        discussion = new DiscussionModel(discussionId);
+        Vue.set(this.state, discussionId, discussion);
         Vue.set(this.loading, discussionId, false);
       }
 
-      Vue.set(this.state[discussionId], noteId, { resolved, user});
+      discussion.createNote(noteId, resolved, user);
     },
     update: function (discussionId, noteId, resolved, user) {
-      this.state[discussionId][noteId].resolved = resolved;
-      this.state[discussionId][noteId].user = user;
+      const discussion = this.state[discussionId];
+      const note = discussion.getNote(noteId);
+      note.resolved = resolved;
+      note.user = user;
     },
     delete: function (discussionId, noteId) {
-      Vue.delete(this.state[discussionId], noteId);
+      const discussion = this.state[discussionId];
+      discussion.deleteNote(noteId);
 
-      if (Object.keys(this.state[discussionId]).length === 0) {
+      if (Object.keys(discussion.notes).length === 0) {
         Vue.delete(this.state, discussionId);
         Vue.delete(this.loading, discussionId);
       }
-    },
-    updateCommentsForDiscussion: function (discussionId, resolve, user) {
-      const noteIds = CommentsStore.resolvedNotesForDiscussion(discussionId, resolve);
-
-      for (let i = 0; i < noteIds.length; i++) {
-        const noteId = noteIds[i];
-        CommentsStore.update(discussionId, noteId, resolve, user);
-      }
-    },
-    notesForDiscussion: function (discussionId) {
-      let ids = [];
-
-      for (const noteId in CommentsStore.state[discussionId]) {
-        ids.push(noteId);
-      }
-
-      return ids;
-    },
-    resolvedNotesForDiscussion: function (discussionId, resolve) {
-      let ids = [];
-
-      for (const noteId in CommentsStore.state[discussionId]) {
-        const resolved = CommentsStore.state[discussionId][noteId].resolved;
-
-        if (resolved !== resolve) {
-          ids.push(noteId);
-        }
-      }
-
-      return ids;
     }
   };
 })(window);
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 2343ace3b68..8574a4e3370 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -120,10 +120,8 @@
           return function(data) {
             $('#diffs').html(data.html);
 
-            if ($('resolve-btn, resolve-all-btn, jump-to-discussion').length && (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null)) {
-              $('resolve-btn, resolve-all-btn, jump-to-discussion').each(function () {
-                DiffNotesApp.$compile($(this).get(0))
-              });
+            if (compileVueComponentsForDiffNotes) {
+              compileVueComponentsForDiffNotes();
             }
 
             gl.utils.localTimeAgo($('.js-timeago', 'div#diffs'));
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index aeaba5ae03e..0ffd3e0882b 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -300,10 +300,8 @@
         discussionContainer.append(note_html);
       }
 
-      if ($('resolve-btn, resolve-all-btn, jump-to-discussion').length && (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null)) {
-        $('resolve-btn, resolve-all-btn, jump-to-discussion').each(function () {
-          DiffNotesApp.$compile($(this).get(0))
-        });
+      if (compileVueComponentsForDiffNotes) {
+        compileVueComponentsForDiffNotes();
       }
 
       gl.utils.localTimeAgo($('.js-timeago', note_html), false);
@@ -402,7 +400,7 @@
         var namespacePath = $form.attr('data-namespace-path'),
             projectPath = $form.attr('data-project-path')
             discussionId = $form.attr('data-discussion-id'),
-            mergeRequestId = $('input[name="noteable_iid"]', $form).val(),
+            mergeRequestId = $form.attr('data-noteable-iid'),
             namespace = namespacePath + '/' + projectPath;
 
         if (ResolveService != null) {
@@ -429,20 +427,10 @@
       $html.find('.js-task-list-container').taskList('enable');
       $note_li = $('.note-row-' + note.id);
 
-      if (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null) {
-        ref = DiffNotesApp.$refs['' + note.id + ''];
-
-        if (ref) {
-          ref.$destroy(true);
-        }
-      }
-
       $note_li.replaceWith($html);
 
-      if ($('resolve-btn, resolve-all-btn, jump-to-discussion').length && (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null)) {
-        $('resolve-btn, resolve-all-btn, jump-to-discussion').each(function () {
-          DiffNotesApp.$compile($(this).get(0))
-        });
+      if (compileVueComponentsForDiffNotes) {
+        compileVueComponentsForDiffNotes();
       }
     };
 
@@ -589,7 +577,7 @@
      */
 
     Notes.prototype.setupDiscussionNoteForm = function(dataHolder, form) {
-      var canResolve = dataHolder.attr('data-resolvable');
+      var canResolve = dataHolder.attr('data-can-resolve');
       form.attr('id', "new-discussion-note-form-" + (dataHolder.data("discussionId")));
       form.attr("data-line-code", dataHolder.data("lineCode"));
       form.find("#note_type").val(dataHolder.data("noteType"));
@@ -604,7 +592,7 @@
 
       if (canResolve === 'false') {
         form.find('resolve-comment-btn').remove();
-      } else if (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null) {
+      } else if (DiffNotesApp) {
         var $commentBtn = form.find('resolve-comment-btn');
         $commentBtn
           .attr(':discussion-id', "'" + dataHolder.data('discussionId') + "'");