Add option to start a new discussion on an MR

32 files changed, 574 insertions, 496 deletions

diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb
index b223a22ae60..9545b980fbf 100644
--- a/spec/controllers/projects/commit_controller_spec.rb
+++ b/spec/controllers/projects/commit_controller_spec.rb
@@ -266,7 +266,7 @@ describe Projects::CommitController do
             diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path)
 
             expect(assigns(:diff_notes_disabled)).to be_falsey
-            expect(assigns(:comments_target)).to eq(noteable_type: 'Commit',
+            expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'Commit',
                                                     commit_id: commit2.id)
           end
 
diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb
index 79ab364a6f3..fe62898fa9b 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(:merge_request) { create(:merge_request) }
   let(:project) { merge_request.source_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/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 734966d50b2..731e511c270 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -508,7 +508,7 @@ describe Projects::IssuesController do
     end
 
     context 'resolving discussions in MergeRequest' do
-      let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+      let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
       let(:merge_request) { discussion.noteable }
       let(:project) { merge_request.source_project }
 
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 72f41f7209a..6f8755645a1 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -574,7 +574,7 @@ describe Projects::MergeRequestsController do
               diff_for_path(id: merge_request.iid, old_path: existing_path, new_path: existing_path)
 
               expect(assigns(:diff_notes_disabled)).to be_falsey
-              expect(assigns(:comments_target)).to eq(noteable_type: 'MergeRequest',
+              expect(assigns(:new_diff_note_attrs)).to eq(noteable_type: 'MergeRequest',
                                                       noteable_id: merge_request.id)
             end
 
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index d80780b1d90..87084806568 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -14,7 +14,15 @@ 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
+
     let(:merge_request) { create(:merge_request) }
     let(:project) { merge_request.source_project }
     let(:request_params) do
@@ -49,7 +57,8 @@ describe Projects::NotesController do
           note: 'some note',
           noteable_id: merge_request.id.to_s,
           noteable_type: 'MergeRequest',
-          merge_request_diff_head_sha: 'sha'
+          merge_request_diff_head_sha: 'sha',
+          in_reply_to_discussion_id: nil
         }
 
         expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true))
diff --git a/spec/factories/discussions.rb b/spec/factories/discussions.rb
new file mode 100644
index 00000000000..5e3a9b1e698
--- /dev/null
+++ b/spec/factories/discussions.rb
@@ -0,0 +1,5 @@
+FactoryGirl.define do
+  factory :discussion do
+    # TODO: Implement
+  end
+end
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index fe19a404e16..755c6d7e031 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -16,6 +16,15 @@ FactoryGirl.define do
     factory :note_on_personal_snippet,   traits: [:on_personal_snippet]
     factory :system_note,                traits: [:system]
 
+    factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do
+      association :project, :repository
+      
+      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 do
       association :project, :repository
     end
diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb
index 572bca3de21..58f897cba3e 100644
--- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb
+++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb
@@ -4,7 +4,7 @@ feature 'Resolving all open discussions in a merge request from an issue', featu
   let(:user) { create(:user) }
   let(:project) { create(:project) }
   let(:merge_request) { create(:merge_request, source_project: project) }
-  let!(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: merge_request, project: project)]).first }
+  let!(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
 
   describe 'as a user with access to the project' do
     before do
diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb
index fab2d532e06..783f2e93909 100644
--- a/spec/features/notes_on_merge_requests_spec.rb
+++ b/spec/features/notes_on_merge_requests_spec.rb
@@ -25,7 +25,7 @@ describe 'Comments', feature: true do
     describe 'the note form' do
       it 'is valid' do
         is_expected.to have_css('.js-main-target-form', visible: true, count: 1)
-        expect(find('.js-main-target-form input[type=submit]').value).
+        expect(find('.js-main-target-form .js-comment-button').value).
           to eq('Comment')
         page.within('.js-main-target-form') do
           expect(page).not_to have_link('Cancel')
diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb
index 77a04507be1..d2ec42ae70f 100644
--- a/spec/finders/notes_finder_spec.rb
+++ b/spec/finders/notes_finder_spec.rb
@@ -202,4 +202,8 @@ describe NotesFinder do
       end
     end
   end
+
+  describe '#target' do
+    # TODO: Test
+  end
 end
diff --git a/spec/models/commit_discussion_spec.rb b/spec/models/commit_discussion_spec.rb
new file mode 100644
index 00000000000..6a5042d8827
--- /dev/null
+++ b/spec/models/commit_discussion_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe CommitDiscussion, model: true do
+  # TODO: Test
+end
diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb
new file mode 100644
index 00000000000..51855fb2f0d
--- /dev/null
+++ b/spec/models/concerns/noteable_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe Noteable, model: true do
+  # TODO: Test
+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..3306b311e4a
--- /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 '#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 "#resolved?" do
+    # TODO: Test
+  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_discussion_spec.rb b/spec/models/diff_discussion_spec.rb
new file mode 100644
index 00000000000..8f3a4a8cc49
--- /dev/null
+++ b/spec/models/diff_discussion_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe DiffDiscussion, model: true do
+  # TODO: Test
+
+  describe "#truncated_diff_lines" do
+    let(:truncated_lines) { subject.truncated_diff_lines }
+
+    context "when diff is greater than allowed number of truncated diff lines " do
+      it "returns fewer lines"  do
+        expect(subject.diff_lines.count).to be > described_class::NUMBER_OF_TRUNCATED_DIFF_LINES
+
+        expect(truncated_lines.count).to be <= described_class::NUMBER_OF_TRUNCATED_DIFF_LINES
+      end
+    end
+
+    context "when some diff lines are meta" do
+      it "returns no meta lines"  do
+        expect(subject.diff_lines).to include(be_meta)
+        expect(truncated_lines).not_to include(be_meta)
+      end
+    end
+  end
+end
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index 9ea3a4b7020..92059936188 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
@@ -94,6 +57,10 @@ describe DiffNote, models: true do
     end
   end
 
+  describe "#original_position=" do
+    # TODO: Test
+  end
+
   describe "#diff_file" do
     it "returns the correct diff file" do
       diff_file = subject.diff_file
@@ -226,252 +193,6 @@ describe DiffNote, models: true do
     end
   end
 
-  describe "#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
-      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])
-      end
-    end
-  end
-
   describe "#discussion_id" do
     let(:note) { create(:diff_note_on_merge_request) }
 
diff --git a/spec/models/discussion_note_spec.rb b/spec/models/discussion_note_spec.rb
new file mode 100644
index 00000000000..d61ea05e318
--- /dev/null
+++ b/spec/models/discussion_note_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe DiscussionNote, models: true do
+  # TODO: Test
+end
diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb
index bc32fadd391..f48450589d0 100644
--- a/spec/models/discussion_spec.rb
+++ b/spec/models/discussion_spec.rb
@@ -3,14 +3,34 @@ 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 '.build' do
+    # TODO: Test
+  end
+
+  describe '.build_collection' do
+    # TODO: Test
+  end
+
+  describe '#id' do
+    # TODO: Test
+  end
+
+  describe '#original_id' do
+    # TODO: Test
+  end
+
+  describe '#potentially_resolvable?' do
+    # TODO: Test
+  end
 
   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 +70,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
@@ -530,10 +550,14 @@ describe Discussion, model: true do
     end
   end
 
+  describe "#last_resolved_note" do
+    # TODO: Test
+  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
@@ -567,31 +591,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
@@ -599,23 +635,4 @@ describe Discussion, model: true do
       end
     end
   end
-
-  describe "#truncated_diff_lines" do
-    let(:truncated_lines) { subject.truncated_diff_lines }
-
-    context "when diff is greater than allowed number of truncated diff lines " do
-      it "returns fewer lines"  do
-        expect(subject.diff_lines.count).to be > described_class::NUMBER_OF_TRUNCATED_DIFF_LINES
-
-        expect(truncated_lines.count).to be <= described_class::NUMBER_OF_TRUNCATED_DIFF_LINES
-      end
-    end
-
-    context "when some diff lines are meta" do
-      it "returns no meta lines"  do
-        expect(subject.diff_lines).to include(be_meta)
-        expect(truncated_lines).not_to include(be_meta)
-      end
-    end
-  end
 end
diff --git a/spec/models/individual_note_discussion_spec.rb b/spec/models/individual_note_discussion_spec.rb
new file mode 100644
index 00000000000..33de682aedc
--- /dev/null
+++ b/spec/models/individual_note_discussion_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe IndividualNoteDiscussion, models: true do
+  # TODO: Test
+end
diff --git a/spec/models/legacy_diff_discussion_spec.rb b/spec/models/legacy_diff_discussion_spec.rb
new file mode 100644
index 00000000000..72e33877b40
--- /dev/null
+++ b/spec/models/legacy_diff_discussion_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe LegacyDiffDiscussion, models: true do
+  # TODO: Test
+end
diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb
deleted file mode 100644
index 81517a18b74..00000000000
--- a/spec/models/legacy_diff_note_spec.rb
+++ /dev/null
@@ -1,101 +0,0 @@
-require 'spec_helper'
-
-describe LegacyDiffNote, models: true do
-  describe "Commit diff line notes" do
-    let!(:note) { create(:legacy_diff_note_on_commit, note: "+1 from me") }
-    let!(:commit) { note.noteable }
-
-    it "saves a valid note" do
-      expect(note.commit_id).to eq(commit.id)
-      expect(note.noteable.id).to eq(commit.id)
-    end
-
-    it "is recognized by #legacy_diff_note?" do
-      expect(note).to be_legacy_diff_note
-    end
-  end
-
-  describe '#active?' do
-    it 'is always true when the note has no associated diff line' do
-      note = build(:legacy_diff_note_on_merge_request)
-
-      expect(note).to receive(:diff_line).and_return(nil)
-
-      expect(note).to be_active
-    end
-
-    it 'is never true when the note has no noteable associated' do
-      note = build(:legacy_diff_note_on_merge_request)
-
-      expect(note).to receive(:diff_line).and_return(double)
-      expect(note).to receive(:noteable).and_return(nil)
-
-      expect(note).not_to be_active
-    end
-
-    it 'returns the memoized value if defined' do
-      note = build(:legacy_diff_note_on_merge_request)
-
-      note.instance_variable_set(:@active, 'foo')
-      expect(note).not_to receive(:find_noteable_diff)
-
-      expect(note.active?).to eq 'foo'
-    end
-
-    context 'for a merge request noteable' do
-      it 'is false when noteable has no matching diff' do
-        merge = build_stubbed(:merge_request, :simple)
-        note = build(:legacy_diff_note_on_merge_request, noteable: merge)
-
-        allow(note).to receive(:diff_line).and_return(double)
-        expect(note).to receive(:find_noteable_diff).and_return(nil)
-
-        expect(note).not_to be_active
-      end
-
-      it 'is true when noteable has a matching diff' do
-        merge = create(:merge_request, :simple)
-
-        # Generate a real line_code value so we know it will match. We use a
-        # random line from a random diff just for funsies.
-        diff = merge.raw_diffs.to_a.sample
-        line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample
-        code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
-
-        # We're persisting in order to trigger the set_diff callback
-        note = create(:legacy_diff_note_on_merge_request,  noteable: merge,
-                                                           line_code: code,
-                                                           project: merge.source_project)
-
-        # Make sure we don't get a false positive from a guard clause
-        expect(note).to receive(:find_noteable_diff).and_call_original
-        expect(note).to be_active
-      end
-    end
-  end
-
-  describe "#discussion_id" do
-    let(:note) { create(:note) }
-
-    context "when it is newly created" do
-      it "has a discussion id" do
-        expect(note.discussion_id).not_to be_nil
-        expect(note.discussion_id).to match(/\A\h{40}\z/)
-      end
-    end
-
-    context "when it didn't store a discussion id before" do
-      before do
-        note.update_column(:discussion_id, nil)
-      end
-
-      it "has a discussion id" do
-        # The discussion_id is set in `after_initialize`, so `reload` won't work
-        reloaded_note = Note.find(note.id)
-
-        expect(reloaded_note.discussion_id).not_to be_nil
-        expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/)
-      end
-    end
-  end
-end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 24e7c1b17d9..faa89ff0507 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1225,12 +1225,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 '#resolvable_discussions' do
@@ -1245,34 +1245,6 @@ describe MergeRequest, models: true do
       end
     end
 
-    describe '#discussions_can_be_resolved_by? user' do
-      let(:user) { build(:user) }
-
-      context 'all discussions can be resolved by the user' do
-        before do
-          allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true)
-          allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true)
-          allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true)
-        end
-
-        it 'allows a user to resolve the discussions' do
-          expect(subject.discussions_can_be_resolved_by?(user)).to be(true)
-        end
-      end
-
-      context 'one discussion cannot be resolved by the user' do
-        before do
-          allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true)
-          allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true)
-          allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false)
-        end
-
-        it 'allows a user to resolve the discussions' do
-          expect(subject.discussions_can_be_resolved_by?(user)).to be(false)
-        end
-      end
-    end
-
     describe "#discussions_resolvable?" do
       context "when all discussions are unresolvable" do
         before do
@@ -1398,6 +1370,38 @@ describe MergeRequest, models: true do
         end
       end
     end
+
+    describe "#discussions_to_be_resolved" do
+      # TODO: Test
+    end
+
+    describe '#discussions_can_be_resolved_by?' do
+      let(:user) { build(:user) }
+
+      context 'all discussions can be resolved by the user' do
+        before do
+          allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true)
+          allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true)
+          allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true)
+        end
+
+        it 'allows a user to resolve the discussions' do
+          expect(subject.discussions_can_be_resolved_by?(user)).to be(true)
+        end
+      end
+
+      context 'one discussion cannot be resolved by the user' do
+        before do
+          allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true)
+          allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true)
+          allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false)
+        end
+
+        it 'allows a user to resolve the discussions' do
+          expect(subject.discussions_can_be_resolved_by?(user)).to be(false)
+        end
+      end
+    end
   end
 
   describe '#conflicts_can_be_resolved_in_ui?' do
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 33536487c41..c5e4a639f06 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -245,6 +245,18 @@ describe Note, models: true do
     end
   end
 
+  describe '.discussions' do
+    # TODO: Test
+  end
+
+  describe '.find_original_discussion' 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 }
@@ -297,31 +309,6 @@ describe Note, models: true do
     end
   end
 
-  describe "#discussion_id" do
-    let(:note) { create(:note) }
-
-    context "when it is newly created" do
-      it "has a discussion id" do
-        expect(note.discussion_id).not_to be_nil
-        expect(note.discussion_id).to match(/\A\h{40}\z/)
-      end
-    end
-
-    context "when it didn't store a discussion id before" do
-      before do
-        note.update_column(:discussion_id, nil)
-      end
-
-      it "has a discussion id" do
-        # The discussion_id is set in `after_initialize`, so `reload` won't work
-        reloaded_note = Note.find(note.id)
-
-        expect(reloaded_note.discussion_id).not_to be_nil
-        expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/)
-      end
-    end
-  end
-
   describe '#for_personal_snippet?' do
     it 'returns false for a project snippet note' do
       expect(build(:note_on_project_snippet).for_personal_snippet?).to be_falsy
@@ -388,6 +375,86 @@ describe Note, models: true do
     end
   end
 
+  describe '#can_be_discussion_note?' do
+    # TODO: Test
+  end
+
+  describe '#discussion_class' do
+    # TODO: Test
+  end
+
+  describe "#discussion_id" do
+    let(:note) { create(:note) }
+
+    context "when it is newly created" do
+      it "has a discussion id" do
+        expect(note.discussion_id).not_to be_nil
+        expect(note.discussion_id).to match(/\A\h{40}\z/)
+      end
+    end
+
+    context "when it didn't store a discussion id before" do
+      before do
+        note.update_column(:discussion_id, nil)
+      end
+
+      it "has a discussion id" do
+        # The discussion_id is set in `after_initialize`, so `reload` won't work
+        reloaded_note = Note.find(note.id)
+
+        expect(reloaded_note.discussion_id).not_to be_nil
+        expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/)
+      end
+    end
+  end
+
+  describe "#original_discussion_id" do
+    # TODO: Test
+  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
+
+  describe "#part_of_discussion?" do
+    # TODO: Test
+  end
+
   describe 'expiring ETag cache' do
     let(:note) { build(:note_on_issue) }
 
diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb
new file mode 100644
index 00000000000..2fc6cce471f
--- /dev/null
+++ b/spec/models/sent_notification_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe SentNotification, model: true do
+  # TODO: Test
+end
diff --git a/spec/models/simple_discussion_spec.rb b/spec/models/simple_discussion_spec.rb
new file mode 100644
index 00000000000..1cb149aa270
--- /dev/null
+++ b/spec/models/simple_discussion_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe SimpleDiscussion, model: true do
+  # TODO: Test
+end
diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb
index b1b398a897e..91d9057075f 100644
--- a/spec/requests/api/v3/issues_spec.rb
+++ b/spec/requests/api/v3/issues_spec.rb
@@ -824,7 +824,7 @@ describe API::V3::Issues, api: true  do
     end
 
     context 'resolving issues in a merge request' do
-      let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+      let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
       let(:merge_request) { discussion.noteable }
       let(:project) { merge_request.source_project }
       before do
diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb
index 12c3cdf28c6..ab8df7b74cd 100644
--- a/spec/services/discussions/resolve_service_spec.rb
+++ b/spec/services/discussions/resolve_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Discussions::ResolveService do
   describe '#execute' do
-    let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+    let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
     let(:project) { merge_request.project }
     let(:merge_request) { discussion.noteable }
     let(:user) { create(:user) }
@@ -41,7 +41,7 @@ describe Discussions::ResolveService do
     end
 
     it 'can resolve multiple discussions at once' do
-      other_discussion = Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: discussion.noteable, project: discussion.noteable.source_project)]).first
+      other_discussion = create(:diff_note_on_merge_request, noteable: discussion.noteable, project: discussion.noteable.source_project).to_discussion
 
       service.execute([discussion, other_discussion])
 
diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb
index 17990f41b3b..d166b9783ad 100644
--- a/spec/services/issues/build_service_spec.rb
+++ b/spec/services/issues/build_service_spec.rb
@@ -96,13 +96,13 @@ describe Issues::BuildService, services: true do
         end
 
         it 'mentions all the authors in the description' do
-          authors = merge_request.diff_discussions.map(&:author)
+          authors = merge_request.resolvable_discussions.map(&:author)
 
           expect(issue.description).to include(*authors.map(&:to_reference))
         end
 
         it 'has a link for each unresolved discussion in the description' do
-          notes = merge_request.diff_discussions.map(&:first_note)
+          notes = merge_request.resolvable_discussions.map(&:first_note)
           links = notes.map { |note| Gitlab::UrlBuilder.build(note) }
 
           expect(issue.description).to include(*links)
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 776cbc4296b..80bfb731550 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -141,7 +141,7 @@ describe Issues::CreateService, services: true do
     it_behaves_like 'new issuable record that supports slash commands'
 
     context 'resolving discussions' do
-      let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+      let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
       let(:merge_request) { discussion.noteable }
       let(:project) { merge_request.source_project }
 
diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb
new file mode 100644
index 00000000000..065f6eeeacb
--- /dev/null
+++ b/spec/services/notes/build_service_spec.rb
@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe Notes::CreateService, services: true do
+  # TODO: Test
+end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 152c6d20daa..b406afeb34d 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 = described_class.new(project, user, opts).execute
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index e3146a56495..c4e00fcf080 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -439,7 +439,7 @@ describe NotificationService, services: true do
 
           notification.new_note(note)
 
-          expect(SentNotification.last.position).to eq(note.position)
+          expect(SentNotification.last.in_reply_to_discussion_id).to eq(note.original_discussion_id)
         end
       end
     end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 5ec1ed8237b..42d63a9f9ba 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -796,7 +796,7 @@ describe SystemNoteService, services: true do
   end
 
   describe '.discussion_continued_in_issue' do
-    let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+    let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
     let(:merge_request) { discussion.noteable }
     let(:project) { merge_request.source_project }
     let(:issue) { create(:issue, project: project) }