WIPnew-resolvable-discussion

44 files changed, 932 insertions, 577 deletions

diff --git a/app/assets/javascripts/behaviors/quick_submit.js b/app/assets/javascripts/behaviors/quick_submit.js
index 4edcaa15fe5..8d422e9366c 100644
--- a/app/assets/javascripts/behaviors/quick_submit.js
+++ b/app/assets/javascripts/behaviors/quick_submit.js
@@ -31,38 +31,64 @@
   };
 
   $(document).on('keydown.quick_submit', '.js-quick-submit', function(e) {
-    var $form, $submit_button;
+    var $form, $submitButton, $altSubmitButton;
     // Enter
     if (!keyCodeIs(e, 13)) {
       return;
     }
-    if (!((e.metaKey && !e.altKey && !e.ctrlKey && !e.shiftKey) || (e.ctrlKey && !e.altKey && !e.metaKey && !e.shiftKey))) {
+    if (e.shiftKey || (e.metaKey && e.ctrlKey)) {
+      return;
+    }
+    if (!(e.metaKey || e.ctrlKey)) {
       return;
     }
-    e.preventDefault();
     $form = $(e.target).closest('form');
-    $submit_button = $form.find('input[type=submit], button[type=submit]');
-    if ($submit_button.attr('disabled')) {
+    $submitButton = $form.find('input[type=submit], button[type=submit]');
+    if (e.altKey) {
+      $submitButton = $submitButton.filter('[data-quick-submit-alt]');
+      if ($submitButton.length === 0) {
+        return;
+      }
+    }
+    e.preventDefault();
+    if ($submitButton.attr('disabled')) {
       return;
     }
-    $submit_button.disable();
-    return $form.submit();
+     // Click button instead of submitting form, so that button name and value are sent along
+    $submitButton.click();
   });
 
   // If the user tabs to a submit button on a `js-quick-submit` form, display a
   // tooltip to let them know they could've used the hotkey
   $(document).on('keyup.quick_submit', '.js-quick-submit input[type=submit], .js-quick-submit button[type=submit]', function(e) {
-    var $this, title;
+    var $this, title, quickSubmitAlt;
     // Tab
     if (!keyCodeIs(e, 9)) {
       return;
     }
+
+    $this = $(this);
+
+    quickSubmitAlt = $this.data('quick-submit-alt');
+
     if (isMac()) {
-      title = "You can also press ⌘-Enter";
+      if (quickSubmitAlt) {
+        shortcut = "&#8997⌘⏎";
+      }
+      else {
+        shortcut = "⌘⏎";
+      }
     } else {
-      title = "You can also press Ctrl-Enter";
+      if (quickSubmitAlt) {
+        shortcut = "Ctrl-Alt-Enter";
+      }
+      else {
+        shortcut = "Ctrl-Enter";
+      }
     }
-    $this = $(this);
+
+    title = "You can also press " + shortcut;
+
     return $this.tooltip({
       container: 'body',
       html: 'true',
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 0ca0e255595..96cc6cd6f83 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -194,11 +194,7 @@
             _this.last_fetched_at = data.last_fetched_at;
             _this.setPollingInterval(data.notes.length);
             return $.each(notes, function(i, note) {
-              if (note.discussion_html != null) {
-                return _this.renderDiscussionNote(note);
-              } else {
-                return _this.renderNote(note);
-              }
+              _this.renderNote(note);
             });
           };
         })(this)
@@ -242,6 +238,10 @@
 
     Notes.prototype.renderNote = function(note) {
       var $notesList, votesBlock;
+      if (note.discussion_html != null) {
+        return this.renderDiscussionNote(note);
+      }
+
       if (!note.valid) {
         if (note.award) {
           new Flash('You have already awarded this emoji!', 'alert', this.parentTimeline);
@@ -390,6 +390,7 @@
       form.find("#note_line_code").remove();
       form.find("#note_position").remove();
       form.find("#note_type").remove();
+      form.find("#note_in_reply_to_discussion_id").remove();
       form.find('.js-comment-resolve-button').closest('comment-and-resolve-btn').remove();
       return this.parentTimeline = form.parents('.timeline');
     };
@@ -446,7 +447,7 @@
         }
       }
 
-      this.renderDiscussionNote(note);
+      this.renderNote(note);
       // cleanup after successfully creating a diff/discussion note
       this.removeDiscussionNoteForm($form);
     };
@@ -641,8 +642,10 @@
       form.find("#note_position").val(dataHolder.attr("data-position"));
       form.find("#note_noteable_type").val(dataHolder.data("noteableType"));
       form.find("#note_noteable_id").val(dataHolder.data("noteableId"));
+      form.find("#note_in_reply_to_discussion_id").val(dataHolder.data("discussionId"));
       form.find('.js-note-discard').show().removeClass('js-note-discard').addClass('js-close-discussion-note-form').text(form.find('.js-close-discussion-note-form').data('cancel-text'));
       form.find('.js-note-target-close').remove();
+      form.find('.js-note-new-discussion').remove();
       this.setupNoteForm(form);
 
       if (typeof gl.diffNotesCompileComponents !== 'undefined') {
diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
index d174e1145a7..a8b7267274f 100644
--- a/app/controllers/projects/discussions_controller.rb
+++ b/app/controllers/projects/discussions_controller.rb
@@ -30,7 +30,7 @@ class Projects::DiscussionsController < Projects::ApplicationController
   end
 
   def discussion
-    @discussion ||= @merge_request.find_diff_discussion(params[:id]) || render_404
+    @discussion ||= @merge_request.find_discussion(params[:id]) || render_404
   end
 
   def authorize_resolve_discussion!
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 15ca080c696..0661fd20946 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -6,13 +6,14 @@ class Projects::NotesController < Projects::ApplicationController
   before_action :authorize_create_note!, only: [:create]
   before_action :authorize_admin_note!, only: [:update, :destroy]
   before_action :authorize_resolve_note!, only: [:resolve, :unresolve]
-  before_action :find_current_user_notes, only: [:index]
 
   def index
     current_fetched_at = Time.now.to_i
 
     notes_json = { notes: [], last_fetched_at: current_fetched_at }
 
+    @notes = notes_finder.execute
+
     @notes.each do |note|
       next if note.cross_reference_not_visible_for?(current_user)
 
@@ -110,6 +111,17 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
+  def discussion_html(discussion)
+    return if discussion.single_note?
+
+    render_to_string(
+      "discussions/_discussion",
+      layout: false,
+      formats: [:html],
+      locals: { discussion: discussion }
+    )
+  end
+
   def diff_discussion_html(discussion)
     return unless discussion.diff_discussion?
 
@@ -134,17 +146,6 @@ class Projects::NotesController < Projects::ApplicationController
     )
   end
 
-  def discussion_html(discussion)
-    return unless discussion.diff_discussion?
-
-    render_to_string(
-      "discussions/_discussion",
-      layout: false,
-      formats: [:html],
-      locals: { discussion: discussion }
-    )
-  end
-
   def note_json(note)
     attrs = {
       award: false,
@@ -167,9 +168,8 @@ class Projects::NotesController < Projects::ApplicationController
         note: note.note
       )
 
-      if note.diff_note?
-        discussion = note.to_discussion
-
+      discussion = note.to_discussion
+      unless discussion.single_note?
         attrs.merge!(
           diff_discussion_html: diff_discussion_html(discussion),
           discussion_html: discussion_html(discussion)
@@ -212,11 +212,16 @@ class Projects::NotesController < Projects::ApplicationController
   def note_params
     params.require(:note).permit(
       :note, :noteable, :noteable_id, :noteable_type, :project_id,
-      :attachment, :line_code, :commit_id, :type, :position
+      :attachment, :line_code, :commit_id, :type, :position,
+      :in_reply_to_discussion_id, :new_discussion
     )
   end
 
-  def find_current_user_notes
-    @notes = NotesFinder.new.execute(project, current_user, params)
+  def notes_finder
+    @notes_finder ||= NotesFinder.new(project, current_user, params)
+  end
+
+  def target
+    notes_finder.target
   end
 end
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index a653a6d59c6..fc86acd9329 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -1,27 +1,41 @@
 class NotesFinder
   FETCH_OVERLAP = 5.seconds
 
-  def execute(project, current_user, params)
+  attr_accessor :project, :current_user, :params
+
+  def initialize(project, current_user, params)
+    @project, @current_user, @params = project, current_user, params
+  end
+
+  def execute
+    notes =
+      if target.respond_to?(:related_notes)
+        target.related_notes
+      else
+        target.notes
+      end
+
+    last_fetched_at = Time.at(params.fetch(:last_fetched_at, 0).to_i)
+    # Use overlapping intervals to avoid worrying about race conditions
+    notes.inc_author.where('updated_at > ?', last_fetched_at - FETCH_OVERLAP).fresh
+  end
+
+  def target
     target_type = params[:target_type]
     target_id   = params[:target_id]
-    # Default to 0 to remain compatible with old clients
-    last_fetched_at = Time.at(params.fetch(:last_fetched_at, 0).to_i)
 
     notes =
       case target_type
       when "commit"
-        project.notes.for_commit_id(target_id).non_diff_notes
+        project.commit(target_id)
       when "issue"
-        IssuesFinder.new(current_user, project_id: project.id).find(target_id).notes.inc_author
+        IssuesFinder.new(current_user, project_id: project.id).find(target_id)
       when "merge_request"
-        project.merge_requests.find(target_id).mr_and_commit_notes.inc_author
+        project.merge_requests.find(target_id)
       when "snippet", "project_snippet"
-        project.snippets.find(target_id).notes
+        project.snippets.find(target_id)
       else
         raise 'invalid target_type'
       end
-
-    # Use overlapping intervals to avoid worrying about race conditions
-    notes.where('updated_at > ?', last_fetched_at - FETCH_OVERLAP).fresh
   end
 end
diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb
index d64e48f774b..dea282de4b3 100644
--- a/app/mailers/emails/issues.rb
+++ b/app/mailers/emails/issues.rb
@@ -3,7 +3,7 @@ module Emails
     def new_issue_email(recipient_id, issue_id)
       setup_issue_mail(issue_id, recipient_id)
 
-      mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id))
+      mail_new_discussion(@issue, issue_thread_options(@issue.author_id, recipient_id))
     end
 
     def new_mention_in_issue_email(recipient_id, issue_id, updated_by_user_id)
diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index ec27ac517db..2a6b0371adb 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -3,7 +3,7 @@ module Emails
     def new_merge_request_email(recipient_id, merge_request_id)
       setup_merge_request_mail(merge_request_id, recipient_id)
 
-      mail_new_thread(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id))
+      mail_new_discussion(@merge_request, merge_request_thread_options(@merge_request.author_id, recipient_id))
     end
 
     def new_mention_in_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 0bc1c19e9cd..589ac2a9885 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -136,7 +136,7 @@ class Notify < BaseMailer
   # with headers suitable for grouping by thread in email clients.
   #
   # See: mail_answer_thread
-  def mail_new_thread(model, headers = {})
+  def mail_new_discussion(model, headers = {})
     headers['Message-ID'] = message_id(model)
 
     mail_thread(model, headers)
diff --git a/app/models/commit_discussion.rb b/app/models/commit_discussion.rb
new file mode 100644
index 00000000000..c6e026334bb
--- /dev/null
+++ b/app/models/commit_discussion.rb
@@ -0,0 +1,5 @@
+class CommitDiscussion < Discussion
+  def potentially_resolvable?
+    false
+  end
+end
diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb
index b8dd27a7afe..abf8ba1b1d8 100644
--- a/app/models/concerns/note_on_diff.rb
+++ b/app/models/concerns/note_on_diff.rb
@@ -5,6 +5,10 @@ module NoteOnDiff
     true
   end
 
+  def part_of_discussion?
+    true
+  end
+
   def diff_file
     raise NotImplementedError
   end
@@ -24,12 +28,4 @@ module NoteOnDiff
   def diff_attributes
     raise NotImplementedError
   end
-
-  def can_be_award_emoji?
-    false
-  end
-
-  def to_discussion
-    Discussion.new([self])
-  end
 end
diff --git a/app/models/concerns/resolvable_note.rb b/app/models/concerns/resolvable_note.rb
new file mode 100644
index 00000000000..d34d6e2b203
--- /dev/null
+++ b/app/models/concerns/resolvable_note.rb
@@ -0,0 +1,66 @@
+module ResolvableNote
+  extend ActiveSupport::Concern
+
+  included do
+    belongs_to :resolved_by, class_name: "User"
+
+    validates :resolved_by, presence: true, if: :resolved?
+
+    # Keep this scope in sync with the logic in `#resolvable?` in `Note` subclasses that are resolvable
+    scope :resolvable, -> { where(type: ['DiffNote', 'DiscussionNote']).user.where(noteable_type: 'MergeRequest') }
+    scope :resolved, -> { resolvable.where.not(resolved_at: nil) }
+    scope :unresolved, -> { resolvable.where(resolved_at: nil) }
+  end
+
+  module ClassMethods
+    # To be defined in any Note subclasses that are actually resolvable
+    # def resolvable?
+    #   true
+    # end
+
+    # This method must be kept in sync with `#resolve!`
+    def resolve!(current_user)
+      unresolved.update_all(resolved_at: Time.now, resolved_by_id: current_user.id)
+    end
+
+    # This method must be kept in sync with `#unresolve!`
+    def unresolve!
+      resolved.update_all(resolved_at: nil, resolved_by_id: nil)
+    end
+  end
+
+  # If you update this method remember to also update the scope `resolvable`
+  def resolvable?
+    self.class.resolvable? && !system?
+  end
+
+  def resolved?
+    return false unless resolvable?
+
+    self.resolved_at.present?
+  end
+
+  def to_be_resolved?
+    resolvable? && !resolved?
+  end
+
+  # If you update this method remember to also update `.resolve!`
+  def resolve!(current_user)
+    return unless resolvable?
+    return if resolved?
+
+    self.resolved_at = Time.now
+    self.resolved_by = current_user
+    save!
+  end
+
+  # If you update this method remember to also update `.unresolve!`
+  def unresolve!
+    return unless resolvable?
+    return unless resolved?
+
+    self.resolved_at = nil
+    self.resolved_by = nil
+    save!
+  end
+end
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb
new file mode 100644
index 00000000000..1b24b5d1faa
--- /dev/null
+++ b/app/models/diff_discussion.rb
@@ -0,0 +1,61 @@
+class DiffDiscussion < Discussion
+  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
+
+  delegate  :line_code,
+            :original_line_code,
+            :diff_file,
+            :for_line?,
+            :active?,
+
+            to: :first_note
+
+  delegate  :blob,
+            :highlighted_diff_lines,
+            :diff_lines,
+
+            to: :diff_file,
+            allow_nil: true
+
+  def diff_discussion?
+    true
+  end
+
+  def legacy_diff_discussion?
+    false
+  end
+
+  def potentially_resolvable?
+    first_note.for_merge_request?
+  end
+
+  def active?
+    return @active if @active.present?
+
+    @active = first_note.active?
+  end
+  MEMOIZED_VALUES << :active
+
+  def reply_attributes
+    super.merge(first_note.diff_attributes)
+  end
+
+  # Returns an array of at most 16 highlighted lines above a diff note
+  def truncated_diff_lines(highlight: true)
+    lines = highlight ? highlighted_diff_lines : diff_lines
+    prev_lines = []
+
+    lines.each do |line|
+      if line.meta?
+        prev_lines.clear
+      else
+        prev_lines << line
+
+        break if for_line?(line)
+
+        prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
+      end
+    end
+
+    prev_lines
+  end
+end
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 559b3075905..829c8516a5d 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -9,15 +9,9 @@ class DiffNote < Note
   validates :diff_line, presence: true
   validates :line_code, presence: true, line_code: true
   validates :noteable_type, inclusion: { in: ['Commit', 'MergeRequest'] }
-  validates :resolved_by, presence: true, if: :resolved?
   validate :positions_complete
   validate :verify_supported
 
-  # Keep this scope in sync with the logic in `#resolvable?`
-  scope :resolvable, -> { user.where(noteable_type: 'MergeRequest') }
-  scope :resolved, -> { resolvable.where.not(resolved_at: nil) }
-  scope :unresolved, -> { resolvable.where(resolved_at: nil) }
-
   after_initialize :ensure_original_discussion_id
   before_validation :set_original_position, :update_position, on: :create
   before_validation :set_line_code, :set_original_discussion_id
@@ -27,18 +21,12 @@ class DiffNote < Note
   after_save :keep_around_commits
 
   class << self
-    def build_discussion_id(noteable_type, noteable_id, position)
-      [super(noteable_type, noteable_id), *position.key].join("-")
+    def resolvable?
+      true
     end
 
-    # This method must be kept in sync with `#resolve!`
-    def resolve!(current_user)
-      unresolved.update_all(resolved_at: Time.now, resolved_by_id: current_user.id)
-    end
-
-    # This method must be kept in sync with `#unresolve!`
-    def unresolve!
-      resolved.update_all(resolved_at: nil, resolved_by_id: nil)
+    def build_discussion_id(noteable_type, noteable_id, position)
+      [super(noteable_type, noteable_id), *position.key].join("-")
     end
   end
 
@@ -46,6 +34,10 @@ class DiffNote < Note
     true
   end
 
+  def discussion_class
+    DiffDiscussion
+  end
+
   def diff_attributes
     { position: position.to_json }
   end
@@ -88,41 +80,8 @@ class DiffNote < Note
     self.position.diff_refs == diff_refs
   end
 
-  # If you update this method remember to also update the scope `resolvable`
   def resolvable?
-    !system? && for_merge_request?
-  end
-
-  def resolved?
-    return false unless resolvable?
-
-    self.resolved_at.present?
-  end
-
-  # If you update this method remember to also update `.resolve!`
-  def resolve!(current_user)
-    return unless resolvable?
-    return if resolved?
-
-    self.resolved_at = Time.now
-    self.resolved_by = current_user
-    save!
-  end
-
-  # If you update this method remember to also update `.unresolve!`
-  def unresolve!
-    return unless resolvable?
-    return unless resolved?
-
-    self.resolved_at = nil
-    self.resolved_by = nil
-    save!
-  end
-
-  def discussion
-    return unless resolvable?
-
-    self.noteable.find_diff_discussion(self.discussion_id)
+    super && for_merge_request?
   end
 
   private
diff --git a/app/models/discussion.rb b/app/models/discussion.rb
index 75a85563235..ceadaebe996 100644
--- a/app/models/discussion.rb
+++ b/app/models/discussion.rb
@@ -1,5 +1,5 @@
 class Discussion
-  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
+  MEMOIZED_VALUES = []
 
   attr_reader :notes
 
@@ -11,12 +11,6 @@ class Discussion
             :for_commit?,
             :for_merge_request?,
 
-            :line_code,
-            :original_line_code,
-            :diff_file,
-            :for_line?,
-            :active?,
-
             to: :first_note
 
   delegate  :resolved_at,
@@ -25,19 +19,12 @@ class Discussion
             to: :last_resolved_note,
             allow_nil: true
 
-  delegate  :blob,
-            :highlighted_diff_lines,
-            :diff_lines,
-
-            to: :diff_file,
-            allow_nil: true
-
-  def self.for_notes(notes)
-    notes.group_by(&:discussion_id).values.map { |notes| new(notes) }
+  def self.build(notes)
+    notes.first.discussion_class.new(notes)
   end
 
-  def self.for_diff_notes(notes)
-    notes.group_by(&:line_code).values.map { |notes| new(notes) }
+  def self.build_collection(notes)
+    notes.group_by(&:discussion_id).values.map { |notes| build(notes) }
   end
 
   def initialize(notes)
@@ -49,6 +36,7 @@ class Discussion
 
     @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last
   end
+  MEMOIZED_VALUES << :last_resolved_note
 
   def last_updated_at
     last_note.created_at
@@ -65,32 +53,40 @@ class Discussion
   alias_method :to_param, :id
 
   def diff_discussion?
-    first_note.diff_note?
+    false
   end
 
-  def legacy_diff_discussion?
-    notes.any?(&:legacy_diff_note?)
+  def potentially_resolvable?
+    true
+  end
+
+  def single_note?
+    false
   end
 
   def resolvable?
     return @resolvable if @resolvable.present?
 
-    @resolvable = diff_discussion? && notes.any?(&:resolvable?)
+    @resolvable = potentially_resolvable? && notes.any?(&:resolvable?)
   end
+  MEMOIZED_VALUES << :resolvable
 
   def resolved?
     return @resolved if @resolved.present?
 
     @resolved = resolvable? && notes.none?(&:to_be_resolved?)
   end
+  MEMOIZED_VALUES << :resolved
 
   def first_note
     @first_note ||= @notes.first
   end
+  MEMOIZED_VALUES << :first_note
 
   def last_note
     @last_note ||= @notes.last
   end
+  MEMOIZED_VALUES << :last_note
 
   def resolved_notes
     notes.select(&:resolved?)
@@ -120,25 +116,12 @@ class Discussion
     update { |notes| notes.unresolve! }
   end
 
-  def for_target?(target)
-    self.noteable == target && !diff_discussion?
-  end
-
-  def active?
-    return @active if @active.present?
-
-    @active = first_note.active?
-  end
-
   def collapsed?
-    return false unless diff_discussion?
-
     if resolvable?
       # New diff discussions only disappear once they are marked resolved
       resolved?
     else
-      # Old diff discussions disappear once they become outdated
-      !active?
+      false
     end
   end
 
@@ -147,52 +130,28 @@ class Discussion
   end
 
   def reply_attributes
-    data = {
+    {
       noteable_type: first_note.noteable_type,
       noteable_id:   first_note.noteable_id,
       commit_id:     first_note.commit_id,
       discussion_id: self.id,
+      note_type:     first_note.type
     }
-
-    if diff_discussion?
-      data[:note_type] = first_note.type
-
-      data.merge!(first_note.diff_attributes)
-    end
-
-    data
-  end
-
-  # Returns an array of at most 16 highlighted lines above a diff note
-  def truncated_diff_lines(highlight: true)
-    lines = highlight ? highlighted_diff_lines : diff_lines
-    prev_lines = []
-
-    lines.each do |line|
-      if line.meta?
-        prev_lines.clear
-      else
-        prev_lines << line
-
-        break if for_line?(line)
-
-        prev_lines.shift if prev_lines.length >= NUMBER_OF_TRUNCATED_DIFF_LINES
-      end
-    end
-
-    prev_lines
   end
 
   private
 
   def update
-    notes_relation = DiffNote.where(id: notes.map(&:id)).fresh
+    # Do not select `Note.resolvable`, so that system notes remain in the collection
+    notes_relation = Note.where(id: notes.map(&:id))
+
     yield(notes_relation)
 
     # Set the notes array to the updated notes
-    @notes = notes_relation.to_a
+    @notes = notes_relation.fresh.to_a
 
-    # Reset the memoized values
-    @last_resolved_note = @resolvable = @resolved = @first_note = @last_note = nil
+    MEMOIZED_VALUES.each do |var|
+      instance_variable_set(:"@#{var}", nil)
+    end
   end
 end
diff --git a/app/models/discussion_note.rb b/app/models/discussion_note.rb
new file mode 100644
index 00000000000..b1b4905a512
--- /dev/null
+++ b/app/models/discussion_note.rb
@@ -0,0 +1,27 @@
+class DiscussionNote < Note
+  validates :noteable_type, inclusion: { in: ['MergeRequest'] }
+
+  class << self
+    def resolvable?
+      true
+    end
+  end
+
+  def part_of_discussion?
+    true
+  end
+
+  def discussion_class
+    Discussion
+  end
+
+  private
+
+  def set_discussion_id
+    if in_reply_to_discussion_id
+      self.discussion_id = in_reply_to_discussion_id
+    else
+      super
+    end
+  end
+end
diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb
new file mode 100644
index 00000000000..59bc68da052
--- /dev/null
+++ b/app/models/legacy_diff_discussion.rb
@@ -0,0 +1,13 @@
+class LegacyDiffDiscussion < DiffDiscussion
+  def legacy_diff_discussion?
+    true
+  end
+
+  def potentially_resolvable?
+    false
+  end
+
+  def collapsed?
+    !active?
+  end
+end
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 40277a9b139..b71aa71b8fe 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -13,6 +13,10 @@ class LegacyDiffNote < Note
     end
   end
 
+  def discussion_class
+    LegacyDiffDiscussion
+  end
+
   def legacy_diff_note?
     true
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 38d8c15e6b0..4b64894f182 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -453,7 +453,7 @@ class MergeRequest < ActiveRecord::Base
     should_remove_source_branch? || force_remove_source_branch?
   end
 
-  def mr_and_commit_notes
+  def related_notes
     # Fetch comments only from last 100 commits
     commits_for_notes_limit = 100
     commit_ids = commits.last(commits_for_notes_limit).map(&:id)
@@ -469,29 +469,25 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def discussions
-    @discussions ||= self.mr_and_commit_notes.
+    @discussions ||= self.related_notes.
       inc_relations_for_view.
-      fresh.
       discussions
   end
 
-  def diff_discussions
-    @diff_discussions ||= self.notes.diff_notes.discussions
+  def find_discussion(discussion_id)
+    self.related_notes.find_discussion(discussion_id)
   end
 
-  def find_diff_discussion(discussion_id)
-    notes = self.notes.diff_notes.where(discussion_id: discussion_id).fresh.to_a
-    return if notes.empty?
-
-    Discussion.new(notes)
+  def resolvable_discussions
+    @resolvable_discussions ||= self.notes.resolvable.discussions
   end
 
   def discussions_resolvable?
-    diff_discussions.any?(&:resolvable?)
+    resolvable_discussions.any?(&:resolvable?)
   end
 
   def discussions_resolved?
-    discussions_resolvable? && diff_discussions.none?(&:to_be_resolved?)
+    discussions_resolvable? && resolvable_discussions.none?(&:to_be_resolved?)
   end
 
   def discussions_to_be_resolved?
diff --git a/app/models/note.rb b/app/models/note.rb
index 5b50ca285c3..3e8aa49a601 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -8,6 +8,7 @@ class Note < ActiveRecord::Base
   include FasterCacheKeys
   include CacheMarkdownField
   include AfterCommitQueue
+  include ResolvableNote
 
   cache_markdown_field :note, pipeline: :note
 
@@ -22,6 +23,9 @@ class Note < ActiveRecord::Base
   # Attribute used to store the attributes that have ben changed by slash commands.
   attr_accessor :commands_changes
 
+  # The discussion ID for the `DiscussionNote` thread being replied to
+  attr_accessor :in_reply_to_discussion_id
+
   default_value_for :system, false
 
   attr_mentionable :note, pipeline: :note
@@ -32,9 +36,6 @@ class Note < ActiveRecord::Base
   belongs_to :author, class_name: "User"
   belongs_to :updated_by, class_name: "User"
 
-  # Only used by DiffNote, but defined here so that it can be used in `Note.includes`
-  belongs_to :resolved_by, class_name: "User"
-
   has_many :todos, dependent: :destroy
   has_many :events, as: :target, dependent: :destroy
 
@@ -52,6 +53,7 @@ class Note < ActiveRecord::Base
   validates :noteable_id, presence: true, unless: [:for_commit?, :importing?]
   validates :commit_id, presence: true, if: :for_commit?
   validates :author, presence: true
+  validates :discussion_id, presence: true, format: { with: /\A\h{40}\z/ }
 
   validate unless: [:for_commit?, :importing?] do |note|
     unless note.noteable.try(:project) == note.project
@@ -71,8 +73,9 @@ class Note < ActiveRecord::Base
   scope :inc_author, ->{ includes(:author) }
   scope :inc_relations_for_view, ->{ includes(:project, :author, :updated_by, :resolved_by, :award_emoji) }
 
+  scope :discussion_notes, ->{ where(type: 'DiscussionNote') }
   scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) }
-  scope :non_diff_notes, ->{ where(type: ['Note', nil]) }
+  scope :non_diff_notes, ->{ where(type: ['Note', 'DiscussionNote', nil]) }
 
   scope :with_associations, -> do
     # FYI noteable cannot be loaded for LegacyDiffNote for commits
@@ -98,14 +101,28 @@ class Note < ActiveRecord::Base
       Digest::SHA1.hexdigest(build_discussion_id(*args))
     end
 
+    def resolvable?
+      false
+    end
+
     def discussions
-      Discussion.for_notes(all)
+      Discussion.build_collection(all.fresh)
+    end
+
+    def find_discussion(discussion_id)
+      notes = where(discussion_id: discussion_id).fresh.to_a
+      return if notes.empty?
+
+      Discussion.build(notes)
     end
 
     def grouped_diff_discussions
-      active_notes = diff_notes.fresh.select(&:active?)
-      Discussion.for_diff_notes(active_notes).
-        map { |d| [d.line_code, d] }.to_h
+      diff_notes.
+        fresh.
+        select(&:active?).
+        group_by(&:line_code).
+        map { |line_code, notes| [line_code, DiffDiscussion.build(notes)] }.
+        to_h
     end
 
     # Searches for notes matching the given query.
@@ -146,18 +163,10 @@ class Note < ActiveRecord::Base
     true
   end
 
-  def resolvable?
-    false
-  end
-
-  def resolved?
+  def part_of_discussion?
     false
   end
 
-  def to_be_resolved?
-    resolvable? && !resolved?
-  end
-
   def max_attachment_size
     current_application_settings.max_attachment_size.megabytes.to_i
   end
@@ -226,7 +235,7 @@ class Note < ActiveRecord::Base
   end
 
   def can_be_award_emoji?
-    noteable.is_a?(Awardable)
+    noteable.is_a?(Awardable) && !part_of_discussion?
   end
 
   def contains_emoji_only?
@@ -237,6 +246,29 @@ class Note < ActiveRecord::Base
     note.match(Banzai::Filter::EmojiFilter.emoji_pattern)[1]
   end
 
+  def discussion_class
+    if for_merge_request?
+      # Notes on merge requests are always in a discussion of their own
+      SingleNoteDiscussion
+    else
+      CommitDiscussion
+    end
+  end
+
+  # Returns a discussion containing just this note
+  def to_discussion
+    Discussion.build([self])
+  end
+
+  # Returns the entire discussion this note is part of
+  def discussion
+    if part_of_discussion?
+      self.noteable.notes.find_discussion(self.discussion_id)
+    else
+      to_discussion
+    end
+  end
+
   private
 
   def keep_around_commit
diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb
index f4bcb49b34d..558e4c1a7eb 100644
--- a/app/models/sent_notification.rb
+++ b/app/models/sent_notification.rb
@@ -5,10 +5,11 @@ class SentNotification < ActiveRecord::Base
   belongs_to :noteable, polymorphic: true
   belongs_to :recipient, class_name: "User"
 
-  validates :project, :recipient, :reply_key, presence: true
-  validates :reply_key, uniqueness: true
+  validates :project, :recipient, presence: true
+  validates :reply_key, presence: true, uniqueness: true
   validates :noteable_id, presence: true, unless: :for_commit?
   validates :commit_id, presence: true, if: :for_commit?
+  validates :in_reply_to_discussion_id, format: { with: /\A\h{40}\z/, if: :discussion_note? }
   validate :note_valid
 
   after_save :keep_around_commit
@@ -34,23 +35,24 @@ class SentNotification < ActiveRecord::Base
       end
 
       attrs.reverse_merge!(
-        project:        noteable.project,
-        noteable_type:  noteable.class.name,
-        noteable_id:    noteable_id,
-        commit_id:      commit_id,
-        recipient_id:   recipient_id,
-        reply_key:      reply_key
+        project: noteable.project,
+        noteable_type: noteable.class.name,
+        noteable_id: noteable_id,
+        commit_id: commit_id,
+        recipient_id: recipient_id,
+        reply_key: reply_key
       )
 
       create(attrs)
     end
 
     def record_note(note, recipient_id, reply_key, attrs = {})
-      if note.diff_note?
-        attrs[:note_type] = note.type
+      attrs.merge!(
+        note_type: note.type,
+        in_reply_to_discussion_id: note.discussion_id
+      )
 
-        attrs.merge!(note.diff_attributes)
-      end
+      attrs.merge!(note.diff_attributes) if note.diff_note?
 
       record(note.noteable, recipient_id, reply_key, attrs)
     end
@@ -64,6 +66,10 @@ class SentNotification < ActiveRecord::Base
     noteable_type == "Commit"
   end
 
+  def discussion_note?
+    note_type == DiscussionNote.name
+  end
+
   def noteable
     if for_commit?
       project.commit(commit_id) rescue nil
@@ -89,31 +95,38 @@ class SentNotification < ActiveRecord::Base
     self.reply_key
   end
 
-  def note_attributes
-    {
-      project:        self.project,
-      author:         self.recipient,
-      type:           self.note_type,
-      noteable_type:  self.noteable_type,
-      noteable_id:    self.noteable_id,
-      commit_id:      self.commit_id,
-      line_code:      self.line_code,
-      position:       self.position.to_json
-    }
-  end
-
   def create_note(note)
     Notes::CreateService.new(
       self.project,
       self.recipient,
-      self.note_attributes.merge(note: note)
+      self.note_params.merge(note: note)
     ).execute
   end
 
   private
 
+  def note_params
+    {
+      project: self.project,
+      author: self.recipient,
+      type: self.note_type,
+      noteable_type: self.noteable_type,
+      noteable_id: self.noteable_id,
+      commit_id: self.commit_id,
+
+      # LegacyDiffNote
+      line_code: self.line_code,
+
+      # DiffNote
+      position: self.position.to_json,
+
+      # DiscussionNote
+      in_reply_to_discussion_id: self.in_reply_to_discussion_id
+    }
+  end
+
   def note_valid
-    Note.new(note_attributes.merge(note: "Test")).valid?
+    Note.new(note_params.merge(note: "Test")).valid?
   end
 
   def keep_around_commit
diff --git a/app/models/single_note_discussion.rb b/app/models/single_note_discussion.rb
new file mode 100644
index 00000000000..ee1aecb6b96
--- /dev/null
+++ b/app/models/single_note_discussion.rb
@@ -0,0 +1,9 @@
+class SingleNoteDiscussion < Discussion
+  def potentially_resolvable?
+    false
+  end
+
+  def single_note?
+    true
+  end
+end
diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb
index d75592e31f3..82bde6f38bc 100644
--- a/app/services/notes/create_service.rb
+++ b/app/services/notes/create_service.rb
@@ -1,10 +1,19 @@
 module Notes
   class CreateService < BaseService
     def execute
+      in_reply_to_discussion_id = params.delete(:in_reply_to_discussion_id)
+      
+      params[:type] = DiscussionNote.name if params.delete(:new_discussion)
+
       note = project.notes.new(params)
       note.author = current_user
       note.system = false
 
+      if note.is_a?(DiscussionNote) && in_reply_to_discussion_id
+        discussion_exists = note.noteable.notes.discussion_notes.find_discussion(in_reply_to_discussion_id)
+        note.in_reply_to_discussion_id = in_reply_to_discussion_id if discussion_exists
+      end
+
       if note.award_emoji?
         noteable = note.noteable
         if noteable.user_can_award?(current_user, note.award_emoji_name)
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 2bce2780484..1a430f9810b 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -15,16 +15,19 @@
 
           .inline.discussion-headline-light
             = discussion.author.to_reference
-            started a discussion on
+            started a discussion
 
             - if discussion.for_commit?
+              on
               - commit = discussion.noteable
               - if commit
                 commit
-                = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code), class: 'monospace'
+                - anchor = discussion.line_code if discussion.diff_discussion?
+                = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor), class: 'monospace'
               - else
                 a deleted commit
-            - else
+            - elsif discussion.diff_discussion?
+              on
               - if discussion.active?
                 = link_to diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code) do
                   the diff
diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml
index dfdbdf1f969..819bf553f00 100644
--- a/app/views/discussions/_notes.html.haml
+++ b/app/views/discussions/_notes.html.haml
@@ -3,14 +3,13 @@
 
 - if current_user
   .discussion-reply-holder
-    - if discussion.diff_discussion?
+    - if discussion.potentially_resolvable?
       - line_type = local_assigns.fetch(:line_type, nil)
 
       .btn-group-justified.discussion-with-resolve-btn{ role: "group" }
         .btn-group{ role: "group" }
           = link_to_reply_discussion(discussion, line_type)
         = render "discussions/resolve_all", discussion: discussion
-        - if discussion.for_merge_request?
-          = render "discussions/jump_to_next", discussion: discussion
+        = render "discussions/jump_to_next", discussion: discussion
     - else
       = link_to_reply_discussion(discussion)
diff --git a/app/views/discussions/_resolve_all.html.haml b/app/views/discussions/_resolve_all.html.haml
index f0b61e0f7de..ebdc341d748 100644
--- a/app/views/discussions/_resolve_all.html.haml
+++ b/app/views/discussions/_resolve_all.html.haml
@@ -1,10 +1,9 @@
-- if discussion.for_merge_request?
-  %resolve-discussion-btn{ ":project-path" => "'#{project_path(discussion.project)}'",
-      ":discussion-id" => "'#{discussion.id}'",
-      ":merge-request-id" => discussion.noteable.iid,
-      ":can-resolve" => discussion.can_resolve?(current_user),
-      "inline-template" => true }
-    .btn-group{ role: "group", "v-if" => "showButton" }
-      %button.btn.btn-default{ type: "button", "@click" => "resolve", ":disabled" => "loading", "v-cloak" => "true" }
-        = icon("spinner spin", "v-show" => "loading")
-        {{ buttonText }}
+%resolve-discussion-btn{ ":project-path" => "'#{project_path(discussion.project)}'",
+    ":discussion-id" => "'#{discussion.id}'",
+    ":merge-request-id" => discussion.noteable.iid,
+    ":can-resolve" => discussion.can_resolve?(current_user),
+    "inline-template" => true }
+  .btn-group{ role: "group", "v-if" => "showButton" }
+    %button.btn.btn-default{ type: "button", "@click" => "resolve", ":disabled" => "loading", "v-cloak" => "true" }
+      = icon("spinner spin", "v-show" => "loading")
+      {{ buttonText }}
diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml
index cfb44bd206c..1f1d698202c 100644
--- a/app/views/projects/merge_requests/_discussion.html.haml
+++ b/app/views/projects/merge_requests/_discussion.html.haml
@@ -1,4 +1,6 @@
 - content_for :note_actions do
+  = submit_tag 'Start new discussion', name: 'note[new_discussion]', class: "btn btn-nr append-right-10 comment-btn js-comment-button btn-inverted js-note-new-discussion", data: { quick_submit_alt: true }
+
   - if can?(current_user, :update_merge_request, @merge_request)
     - if @merge_request.open?
       = link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"}
diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml
index 9ffcc48eb80..1cde318bbae 100644
--- a/app/views/projects/merge_requests/_merge_request.html.haml
+++ b/app/views/projects/merge_requests/_merge_request.html.haml
@@ -39,7 +39,7 @@
           = icon('thumbs-down')
           = downvotes
 
-      - note_count = merge_request.mr_and_commit_notes.user.count
+      - note_count = merge_request.related_notes.user.count
       %li
         = link_to merge_request_path(merge_request, anchor: 'notes'), class: ('no-comments' if note_count.zero?) do
           = icon('comments')
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index 0e2975bd551..306c754215a 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -53,7 +53,7 @@
             %li.notes-tab
               = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do
                 Discussion
-                %span.badge= @merge_request.mr_and_commit_notes.user.count
+                %span.badge= @merge_request.related_notes.user.count
             - if @merge_request.source_project
               %li.commits-tab
                 = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index 46b402545cd..8ac70cafd80 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -4,12 +4,13 @@
   = hidden_field_tag :view, diff_view
   = hidden_field_tag :line_type
   = note_target_fields(@note)
-  = f.hidden_field :commit_id
-  = f.hidden_field :line_code
-  = f.hidden_field :noteable_id
-  = f.hidden_field :noteable_type
   = f.hidden_field :type
+  = f.hidden_field :noteable_type
+  = f.hidden_field :noteable_id
+  = f.hidden_field :commit_id
   = f.hidden_field :position
+  = f.hidden_field :line_code
+  = f.hidden_field :in_reply_to_discussion_id
 
   = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do
     = render 'projects/zen', f: f,
diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml
index 022578bd6db..5232b1d07cd 100644
--- a/app/views/projects/notes/_notes.html.haml
+++ b/app/views/projects/notes/_notes.html.haml
@@ -1,6 +1,6 @@
 - if @discussions.present?
   - @discussions.each do |discussion|
-    - if discussion.for_target?(@noteable)
+    - if discussion.single_note?
       = render partial: "projects/notes/note", object: discussion.first_note, as: :note
     - else
       = render 'discussions/discussion', discussion: discussion
diff --git a/changelogs/unreleased/new-resolvable-discussion.yml b/changelogs/unreleased/new-resolvable-discussion.yml
new file mode 100644
index 00000000000..af1de6a45e7
--- /dev/null
+++ b/changelogs/unreleased/new-resolvable-discussion.yml
@@ -0,0 +1,4 @@
+---
+title: Add option to start a new resolvable discussion in an MR
+merge_request: 
+author: 
diff --git a/db/migrate/20161128095517_add_in_reply_to_discussion_id_to_sent_notifications.rb b/db/migrate/20161128095517_add_in_reply_to_discussion_id_to_sent_notifications.rb
new file mode 100644
index 00000000000..d56d83ca1d3
--- /dev/null
+++ b/db/migrate/20161128095517_add_in_reply_to_discussion_id_to_sent_notifications.rb
@@ -0,0 +1,29 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddInReplyToDiscussionIdToSentNotifications < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :sent_notifications, :in_reply_to_discussion_id, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 0d510c8a269..dc27c7450e9 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1001,6 +1001,7 @@ ActiveRecord::Schema.define(version: 20161128161412) do
     t.string "line_code"
     t.string "note_type"
     t.text "position"
+    t.string "in_reply_to_discussion_id"
   end
 
   add_index "sent_notifications", ["reply_key"], name: "index_sent_notifications_on_reply_key", unique: true, using: :btree
diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb
index ff617fea847..e91832459b6 100644
--- a/spec/controllers/projects/discussions_controller_spec.rb
+++ b/spec/controllers/projects/discussions_controller_spec.rb
@@ -4,7 +4,7 @@ describe Projects::DiscussionsController do
   let(:user)    { create(:user) }
   let(:project) { create(:project) }
   let(:merge_request) { create(:merge_request, source_project: project) }
-  let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) }
+  let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) }
   let(:discussion) { note.discussion }
 
   let(:request_params) do
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 92e38b02615..f6de25b7243 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -14,6 +14,16 @@ describe Projects::NotesController do
     }
   end
 
+  describe 'GET index' do
+    # It renders the discussion partial for any threaded note
+    # TODO: Test
+  end
+
+  describe 'POST create' do
+    # Test :type, :new_discussion, :in_reply_to_discussion_id (in_reply_to_id?)
+    # TODO: Test
+  end
+
   describe 'POST toggle_award_emoji' do
     before do
       sign_in(user)
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index 6919002dedc..2572eabe6be 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -15,6 +15,14 @@ FactoryGirl.define do
     factory :note_on_project_snippet,    traits: [:on_project_snippet]
     factory :system_note,                traits: [:system]
 
+
+    factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do
+      trait :resolved do
+        resolved_at { Time.now }
+        resolved_by { create(:user) }
+      end
+    end
+
     factory :legacy_diff_note_on_commit, traits: [:on_commit, :legacy_diff_note], class: LegacyDiffNote
     factory :legacy_diff_note_on_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote
 
diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb
index 7c6860372cc..0e1d98066dd 100644
--- a/spec/finders/notes_finder_spec.rb
+++ b/spec/finders/notes_finder_spec.rb
@@ -3,58 +3,74 @@ require 'spec_helper'
 describe NotesFinder do
   let(:user) { create :user }
   let(:project) { create :project }
-  let(:note1) { create :note_on_commit, project: project }
-  let(:note2) { create :note_on_commit, project: project }
+  let!(:note1) { create :note_on_commit, project: project }
+  let!(:note2) { create :note_on_commit, project: project }
   let(:commit) { note1.noteable }
+  subject { NotesFinder.new(project, user, params) }
 
   before do
-    project.team << [user, :master]
+    # project.team << [user, :master]
   end
 
   describe '#execute' do
     let(:params)  { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } }
 
-    before do
-      note1
-      note2
-    end
-
     it 'finds all notes' do
-      notes = NotesFinder.new.execute(project, user, params)
+      notes = subject.execute
       expect(notes.size).to eq(2)
     end
 
     it 'raises an exception for an invalid target_type' do
       params.merge!(target_type: 'invalid')
-      expect { NotesFinder.new.execute(project, user, params) }.to raise_error('invalid target_type')
+      expect { subject.execute }.to raise_error('invalid target_type')
     end
 
     it 'filters out old notes' do
       note2.update_attribute(:updated_at, 2.hours.ago)
-      notes = NotesFinder.new.execute(project, user, params)
+      notes = subject.execute
       expect(notes).to eq([note1])
     end
 
     context 'confidential issue notes' do
-      let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) }
+      let(:confidential_issue) { create(:issue, :confidential, project: project) }
       let!(:confidential_note) { create(:note, noteable: confidential_issue, project: confidential_issue.project) }
 
       let(:params) { { target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } }
 
-      it 'returns notes if user can see the issue' do
-        expect(NotesFinder.new.execute(project, user, params)).to eq([confidential_note])
+      context 'when user is the author of the issue' do
+        before do
+          confidential_issue.update(author: user)
+        end
+
+        it 'returns confidential notes' do
+          expect(subject.execute).to eq([confidential_note])
+        end
       end
 
-      it 'raises an error if user can not see the issue' do
-        user = create(:user)
-        expect { NotesFinder.new.execute(project, user, params) }.to raise_error(ActiveRecord::RecordNotFound)
+      context 'when user is a master on the team' do
+        before do
+          project.team << [user, :master]
+        end
+
+        it 'returns confidential notes' do
+          expect(subject.execute).to eq([confidential_note])
+        end
       end
 
-      it 'raises an error for project members with guest role' do
-        user = create(:user)
-        project.team << [user, :guest]
+      context 'when user is a guest on the team' do
+        before do
+          project.team << [user, :guest]
+        end
+
+        it 'raises an error' do
+          expect { subject.execute }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+      end
 
-        expect { NotesFinder.new.execute(project, user, params) }.to raise_error(ActiveRecord::RecordNotFound)
+      context 'when user is not a member of the team' do
+        it 'raises an error if user can not see the issue' do
+          expect { subject.execute }.to raise_error(ActiveRecord::RecordNotFound)
+        end
       end
     end
   end
diff --git a/spec/models/concerns/resolvable_note_spec.rb b/spec/models/concerns/resolvable_note_spec.rb
new file mode 100644
index 00000000000..f71704e6538
--- /dev/null
+++ b/spec/models/concerns/resolvable_note_spec.rb
@@ -0,0 +1,276 @@
+require 'spec_helper'
+
+describe Note, ResolvableNote, models: true do
+  subject { create(:discussion_note_on_merge_request) }
+
+  describe '.resolvable' do
+    # TODO: Test
+  end
+
+  describe '.resolved' do
+    # TODO: Test
+  end
+
+  describe '.unresolved' do
+    # TODO: Test
+  end
+
+  describe ".resolve!" do
+    let(:current_user) { create(:user) }
+    let!(:commit_note) { create(:diff_note_on_commit) }
+    let!(:resolved_note) { create(:discussion_note_on_merge_request, :resolved) }
+    let!(:unresolved_note) { create(:discussion_note_on_merge_request) }
+
+    before do
+      described_class.resolve!(current_user)
+
+      commit_note.reload
+      resolved_note.reload
+      unresolved_note.reload
+    end
+
+    it 'resolves only the resolvable, not yet resolved notes' do
+      expect(commit_note.resolved_at).to be_nil
+      expect(resolved_note.resolved_by).not_to eq(current_user)
+      expect(unresolved_note.resolved_at).not_to be_nil
+      expect(unresolved_note.resolved_by).to eq(current_user)
+    end
+  end
+
+  describe ".unresolve!" do
+    let!(:resolved_note) { create(:discussion_note_on_merge_request, :resolved) }
+
+    before do
+      described_class.unresolve!
+
+      resolved_note.reload
+    end
+
+    it 'unresolves the resolved notes' do
+      expect(resolved_note.resolved_by).to be_nil
+      expect(resolved_note.resolved_at).to be_nil
+    end
+  end
+
+  describe '#discussion_resolvable?' do
+    # TODO: Test
+  end
+
+  describe '#resolvable?' do
+    context "when potentially resolvable" do
+      before do
+        allow(subject).to receive(:discussion_resolvable?).and_return(true)
+      end
+
+      context "when a system note" do
+        before do
+          subject.system = true
+        end
+
+        it "returns false" do
+          expect(subject.resolvable?).to be false
+        end
+      end
+
+      context "when a regular note" do
+        it "returns true" do
+          expect(subject.resolvable?).to be true
+        end
+      end
+    end
+
+    context "when not potentially resolvable" do
+      before do
+        allow(subject).to receive(:discussion_resolvable?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.resolvable?).to be false
+      end
+    end
+  end
+
+  describe "#to_be_resolved?" do
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns false" do
+        expect(subject.to_be_resolved?).to be false
+      end
+    end
+
+    context "when resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when resolved" do
+        before do
+          allow(subject).to receive(:resolved?).and_return(true)
+        end
+
+        it "returns false" do
+          expect(subject.to_be_resolved?).to be false
+        end
+      end
+
+      context "when not resolved" do
+        before do
+          allow(subject).to receive(:resolved?).and_return(false)
+        end
+
+        it "returns true" do
+          expect(subject.to_be_resolved?).to be true
+        end
+      end
+    end
+  end
+
+  describe "#resolve!" do
+    let(:current_user) { create(:user) }
+
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns nil" do
+        expect(subject.resolve!(current_user)).to be_nil
+      end
+
+      it "doesn't set resolved_at" do
+        subject.resolve!(current_user)
+
+        expect(subject.resolved_at).to be_nil
+      end
+
+      it "doesn't set resolved_by" do
+        subject.resolve!(current_user)
+
+        expect(subject.resolved_by).to be_nil
+      end
+
+      it "doesn't mark as resolved" do
+        subject.resolve!(current_user)
+
+        expect(subject.resolved?).to be false
+      end
+    end
+
+    context "when resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when already resolved" do
+        let(:user) { create(:user) }
+
+        before do
+          subject.resolve!(user)
+        end
+
+        it "returns nil" do
+          expect(subject.resolve!(current_user)).to be_nil
+        end
+
+        it "doesn't change resolved_at" do
+          expect(subject.resolved_at).not_to be_nil
+
+          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at }
+        end
+
+        it "doesn't change resolved_by" do
+          expect(subject.resolved_by).to eq(user)
+
+          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by }
+        end
+
+        it "doesn't change resolved status" do
+          expect(subject.resolved?).to be true
+
+          expect { subject.resolve!(current_user) }.not_to change { subject.resolved? }
+        end
+      end
+
+      context "when not yet resolved" do
+        it "returns true" do
+          expect(subject.resolve!(current_user)).to be true
+        end
+
+        it "sets resolved_at" do
+          subject.resolve!(current_user)
+
+          expect(subject.resolved_at).not_to be_nil
+        end
+
+        it "sets resolved_by" do
+          subject.resolve!(current_user)
+
+          expect(subject.resolved_by).to eq(current_user)
+        end
+
+        it "marks as resolved" do
+          subject.resolve!(current_user)
+
+          expect(subject.resolved?).to be true
+        end
+      end
+    end
+  end
+
+  describe "#unresolve!" do
+    context "when not resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(false)
+      end
+
+      it "returns nil" do
+        expect(subject.unresolve!).to be_nil
+      end
+    end
+
+    context "when resolvable" do
+      before do
+        allow(subject).to receive(:resolvable?).and_return(true)
+      end
+
+      context "when resolved" do
+        let(:user) { create(:user) }
+
+        before do
+          subject.resolve!(user)
+        end
+
+        it "returns true" do
+          expect(subject.unresolve!).to be true
+        end
+
+        it "unsets resolved_at" do
+          subject.unresolve!
+
+          expect(subject.resolved_at).to be_nil
+        end
+
+        it "unsets resolved_by" do
+          subject.unresolve!
+
+          expect(subject.resolved_by).to be_nil
+        end
+
+        it "unmarks as resolved" do
+          subject.unresolve!
+
+          expect(subject.resolved?).to be false
+        end
+      end
+
+      context "when not resolved" do
+        it "returns nil" do
+          expect(subject.unresolve!).to be_nil
+        end
+      end
+    end
+  end
+end
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index 3db5937a4f3..00770d5bcd4 100644
--- a/spec/models/diff_note_spec.rb
+++ b/spec/models/diff_note_spec.rb
@@ -31,43 +31,6 @@ describe DiffNote, models: true do
 
   subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
 
-  describe ".resolve!" do
-    let(:current_user) { create(:user) }
-    let!(:commit_note) { create(:diff_note_on_commit) }
-    let!(:resolved_note) { create(:diff_note_on_merge_request, :resolved) }
-    let!(:unresolved_note) { create(:diff_note_on_merge_request) }
-
-    before do
-      described_class.resolve!(current_user)
-
-      commit_note.reload
-      resolved_note.reload
-      unresolved_note.reload
-    end
-
-    it 'resolves only the resolvable, not yet resolved notes' do
-      expect(commit_note.resolved_at).to be_nil
-      expect(resolved_note.resolved_by).not_to eq(current_user)
-      expect(unresolved_note.resolved_at).not_to be_nil
-      expect(unresolved_note.resolved_by).to eq(current_user)
-    end
-  end
-
-  describe ".unresolve!" do
-    let!(:resolved_note) { create(:diff_note_on_merge_request, :resolved) }
-
-    before do
-      described_class.unresolve!
-
-      resolved_note.reload
-    end
-
-    it 'unresolves the resolved notes' do
-      expect(resolved_note.resolved_by).to be_nil
-      expect(resolved_note.resolved_at).to be_nil
-    end
-  end
-
   describe "#position=" do
     context "when provided a string" do
       it "sets the position" do
@@ -226,248 +189,18 @@ describe DiffNote, models: true do
     end
   end
 
-  describe "#resolvable?" do
+  describe '#discussion_resolvable?' do
     context "when noteable is a commit" do
       subject { create(:diff_note_on_commit, project: project, position: position) }
 
       it "returns false" do
-        expect(subject.resolvable?).to be false
+        expect(subject.discussion_resolvable?).to be false
       end
     end
 
     context "when noteable is a merge request" do
-      context "when a system note" do
-        before do
-          subject.system = true
-        end
-
-        it "returns false" do
-          expect(subject.resolvable?).to be false
-        end
-      end
-
-      context "when a regular note" do
-        it "returns true" do
-          expect(subject.resolvable?).to be true
-        end
-      end
-    end
-  end
-
-  describe "#to_be_resolved?" do
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns false" do
-        expect(subject.to_be_resolved?).to be false
-      end
-    end
-
-    context "when resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when resolved" do
-        before do
-          allow(subject).to receive(:resolved?).and_return(true)
-        end
-
-        it "returns false" do
-          expect(subject.to_be_resolved?).to be false
-        end
-      end
-
-      context "when not resolved" do
-        before do
-          allow(subject).to receive(:resolved?).and_return(false)
-        end
-
-        it "returns true" do
-          expect(subject.to_be_resolved?).to be true
-        end
-      end
-    end
-  end
-
-  describe "#resolve!" do
-    let(:current_user) { create(:user) }
-
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns nil" do
-        expect(subject.resolve!(current_user)).to be_nil
-      end
-
-      it "doesn't set resolved_at" do
-        subject.resolve!(current_user)
-
-        expect(subject.resolved_at).to be_nil
-      end
-
-      it "doesn't set resolved_by" do
-        subject.resolve!(current_user)
-
-        expect(subject.resolved_by).to be_nil
-      end
-
-      it "doesn't mark as resolved" do
-        subject.resolve!(current_user)
-
-        expect(subject.resolved?).to be false
-      end
-    end
-
-    context "when resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when already resolved" do
-        let(:user) { create(:user) }
-
-        before do
-          subject.resolve!(user)
-        end
-
-        it "returns nil" do
-          expect(subject.resolve!(current_user)).to be_nil
-        end
-
-        it "doesn't change resolved_at" do
-          expect(subject.resolved_at).not_to be_nil
-
-          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at }
-        end
-
-        it "doesn't change resolved_by" do
-          expect(subject.resolved_by).to eq(user)
-
-          expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by }
-        end
-
-        it "doesn't change resolved status" do
-          expect(subject.resolved?).to be true
-
-          expect { subject.resolve!(current_user) }.not_to change { subject.resolved? }
-        end
-      end
-
-      context "when not yet resolved" do
-        it "returns true" do
-          expect(subject.resolve!(current_user)).to be true
-        end
-
-        it "sets resolved_at" do
-          subject.resolve!(current_user)
-
-          expect(subject.resolved_at).not_to be_nil
-        end
-
-        it "sets resolved_by" do
-          subject.resolve!(current_user)
-
-          expect(subject.resolved_by).to eq(current_user)
-        end
-
-        it "marks as resolved" do
-          subject.resolve!(current_user)
-
-          expect(subject.resolved?).to be true
-        end
-      end
-    end
-  end
-
-  describe "#unresolve!" do
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns nil" do
-        expect(subject.unresolve!).to be_nil
-      end
-    end
-
-    context "when resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(true)
-      end
-
-      context "when resolved" do
-        let(:user) { create(:user) }
-
-        before do
-          subject.resolve!(user)
-        end
-
-        it "returns true" do
-          expect(subject.unresolve!).to be true
-        end
-
-        it "unsets resolved_at" do
-          subject.unresolve!
-
-          expect(subject.resolved_at).to be_nil
-        end
-
-        it "unsets resolved_by" do
-          subject.unresolve!
-
-          expect(subject.resolved_by).to be_nil
-        end
-
-        it "unmarks as resolved" do
-          subject.unresolve!
-
-          expect(subject.resolved?).to be false
-        end
-      end
-
-      context "when not resolved" do
-        it "returns nil" do
-          expect(subject.unresolve!).to be_nil
-        end
-      end
-    end
-  end
-
-  describe "#discussion" do
-    context "when not resolvable" do
-      before do
-        allow(subject).to receive(:resolvable?).and_return(false)
-      end
-
-      it "returns nil" do
-        expect(subject.discussion).to be_nil
-      end
-    end
-
-    context "when resolvable" do
-      let!(:diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: subject.position) }
-      let!(:diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) }
-
-      let(:active_position2) do
-        Gitlab::Diff::Position.new(
-          old_path: "files/ruby/popen.rb",
-          new_path: "files/ruby/popen.rb",
-          old_line: 16,
-          new_line: 22,
-          diff_refs: merge_request.diff_refs
-        )
-      end
-
-      it "returns the discussion this note is in" do
-        discussion = subject.discussion
-
-        expect(discussion.id).to eq(subject.discussion_id)
-        expect(discussion.notes).to eq([subject, diff_note2])
+      it "returns true" do
+        expect(subject.discussion_resolvable?).to be true
       end
     end
   end
diff --git a/spec/models/discussion_note_spec.rb b/spec/models/discussion_note_spec.rb
new file mode 100644
index 00000000000..9b8534e1d61
--- /dev/null
+++ b/spec/models/discussion_note_spec.rb
@@ -0,0 +1,7 @@
+require 'spec_helper'
+
+describe DiscussionNote, models: true do
+  describe '#discussion_id' do
+    # Uses in_reply_to_discussion_id
+  end
+end
diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb
index 2a67c60b978..f42b01810b3 100644
--- a/spec/models/discussion_spec.rb
+++ b/spec/models/discussion_spec.rb
@@ -3,14 +3,14 @@ require 'spec_helper'
 describe Discussion, model: true do
   subject { described_class.new([first_note, second_note, third_note]) }
 
-  let(:first_note) { create(:diff_note_on_merge_request) }
-  let(:second_note) { create(:diff_note_on_merge_request) }
-  let(:third_note) { create(:diff_note_on_merge_request) }
+  let(:first_note) { create(:discussion_note_on_merge_request) }
+  let(:second_note) { create(:discussion_note_on_merge_request) }
+  let(:third_note) { create(:discussion_note_on_merge_request) }
 
   describe "#resolvable?" do
-    context "when a diff discussion" do
+    context "when potentially resolvable" do
       before do
-        allow(subject).to receive(:diff_discussion?).and_return(true)
+        allow(subject).to receive(:discussion_resolvable?).and_return(true)
       end
 
       context "when all notes are unresolvable" do
@@ -50,9 +50,9 @@ describe Discussion, model: true do
       end
     end
 
-    context "when not a diff discussion" do
+    context "when not potentially resolvable" do
       before do
-        allow(subject).to receive(:diff_discussion?).and_return(false)
+        allow(subject).to receive(:discussion_resolvable?).and_return(false)
       end
 
       it "returns false" do
@@ -522,9 +522,9 @@ describe Discussion, model: true do
   end
 
   describe "#collapsed?" do
-    context "when a diff discussion" do
+    context "when potentially resolvable" do
       before do
-        allow(subject).to receive(:diff_discussion?).and_return(true)
+        allow(subject).to receive(:discussion_resolvable?).and_return(true)
       end
 
       context "when resolvable" do
@@ -558,31 +558,43 @@ describe Discussion, model: true do
           allow(subject).to receive(:resolvable?).and_return(false)
         end
 
-        context "when active" do
+        context "when a diff discussion" do
           before do
-            allow(subject).to receive(:active?).and_return(true)
+            allow(subject).to receive(:diff_discussion?).and_return(true)
           end
 
-          it "returns false" do
-            expect(subject.collapsed?).to be false
+          context "when active" do
+            before do
+              allow(subject).to receive(:active?).and_return(true)
+            end
+
+            it "returns false" do
+              expect(subject.collapsed?).to be false
+            end
           end
-        end
 
-        context "when outdated" do
-          before do
-            allow(subject).to receive(:active?).and_return(false)
+          context "when outdated" do
+            before do
+              allow(subject).to receive(:active?).and_return(false)
+            end
+
+            it "returns true" do
+              expect(subject.collapsed?).to be true
+            end
           end
+        end
 
-          it "returns true" do
-            expect(subject.collapsed?).to be true
+        context "when not a diff discussion" do
+          it "returns false" do
+            expect(subject.collapsed?).to be false
           end
         end
       end
     end
 
-    context "when not a diff discussion" do
+    context "when not potentially resolvable" do
       before do
-        allow(subject).to receive(:diff_discussion?).and_return(false)
+        allow(subject).to receive(:discussion_resolvable?).and_return(false)
       end
 
       it "returns false" do
@@ -609,4 +621,12 @@ describe Discussion, model: true do
       end
     end
   end
+
+  describe '#single_note?' do
+
+  end
+
+  describe "#reply_attributes" do
+
+  end
 end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 58ccd056328..e3cef6ac37a 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -202,7 +202,7 @@ describe MergeRequest, models: true do
     end
   end
 
-  describe "#mr_and_commit_notes" do
+  describe "#related_notes" do
     let!(:merge_request) { create(:merge_request) }
 
     before do
@@ -214,7 +214,7 @@ describe MergeRequest, models: true do
 
     it "includes notes for commits" do
       expect(merge_request.commits).not_to be_empty
-      expect(merge_request.mr_and_commit_notes.count).to eq(2)
+      expect(merge_request.related_notes.count).to eq(2)
     end
 
     it "includes notes for commits from target project as well" do
@@ -222,7 +222,7 @@ describe MergeRequest, models: true do
                               project: merge_request.target_project)
 
       expect(merge_request.commits).not_to be_empty
-      expect(merge_request.mr_and_commit_notes.count).to eq(3)
+      expect(merge_request.related_notes.count).to eq(3)
     end
   end
 
@@ -1119,12 +1119,12 @@ describe MergeRequest, models: true do
   end
 
   context "discussion status" do
-    let(:first_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) }
-    let(:second_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) }
-    let(:third_discussion) { Discussion.new([create(:diff_note_on_merge_request)]) }
+    let(:first_discussion) { Discussion.new([create(:discussion_note_on_merge_request)]) }
+    let(:second_discussion) { Discussion.new([create(:discussion_note_on_merge_request)]) }
+    let(:third_discussion) { Discussion.new([create(:discussion_note_on_merge_request)]) }
 
     before do
-      allow(subject).to receive(:diff_discussions).and_return([first_discussion, second_discussion, third_discussion])
+      allow(subject).to receive(:resolvable_discussions).and_return([first_discussion, second_discussion, third_discussion])
     end
 
     describe "#discussions_resolvable?" do
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 17a15b12dcb..eb6ab34cbbb 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -268,6 +268,14 @@ describe Note, models: true do
     end
   end
 
+  describe '.discussions' do
+    # TODO: Test
+  end
+
+  describe '.find_discussion' do
+    # TODO: Test
+  end
+
   describe ".grouped_diff_discussions" do
     let!(:merge_request) { create(:merge_request) }
     let(:project) { merge_request.project }
@@ -344,4 +352,43 @@ describe Note, models: true do
       end
     end
   end
+
+  describe '#to_discussion' do
+    subject { create(:discussion_note_on_merge_request) }
+    let!(:note2) { create(:discussion_note_on_merge_request, project: subject.project, noteable: subject.noteable, in_reply_to_discussion_id: subject.discussion_id) }
+
+    it "returns a discussion with just this note" do
+      discussion = subject.to_discussion
+
+      expect(discussion.id).to eq(subject.discussion_id)
+      expect(discussion.notes).to eq([subject])
+    end
+  end
+
+  describe "#discussion" do
+    let!(:note1) { create(:discussion_note_on_merge_request) }
+    let!(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) }
+
+    context 'when the note is part of a discussion' do
+      subject { create(:discussion_note_on_merge_request, project: note1.project, noteable: note1.noteable, in_reply_to_discussion_id: note1.discussion_id) }
+
+      it "returns the discussion this note is in" do
+        discussion = subject.discussion
+
+        expect(discussion.id).to eq(subject.discussion_id)
+        expect(discussion.notes).to eq([note1, subject])
+      end
+    end
+
+    context 'when the note is not part of a discussion' do
+      subject { create(:note) }
+
+      it "returns a discussion with just this note" do
+        discussion = subject.discussion
+
+        expect(discussion.id).to eq(subject.discussion_id)
+        expect(discussion.notes).to eq([subject])
+      end
+    end
+  end
 end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 25804696d2e..e8b72bc3e14 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -13,6 +13,9 @@ describe Notes::CreateService, services: true do
       project.team << [user, :master]
     end
 
+    # TODO: Test in_reply_to_discussion_id
+    # TODO: Test new_discussion
+
     context "valid params" do
       it 'returns a valid note' do
         note = Notes::CreateService.new(project, user, opts).execute